Skip to content

Fix bugs and refactor: AuthContext, ProtectedRoute, DetectorDetail endpoint, settings hardening#32

Closed
Copilot wants to merge 3 commits intomasterfrom
copilot/review-project-structure
Closed

Fix bugs and refactor: AuthContext, ProtectedRoute, DetectorDetail endpoint, settings hardening#32
Copilot wants to merge 3 commits intomasterfrom
copilot/review-project-structure

Conversation

Copy link
Copy Markdown

Copilot AI commented Feb 20, 2026

Code review of the Django + React codebase identified several bugs and structural issues that would compound as the project grows. This PR addresses the most impactful ones.

Backend Bugs

  • False exception logging: logger.exception("Error creating organization") was called in the success path of Organizations view — fixed to logger.info
  • Silent 200 on invalid data: MeasurementsPost had no return when serializer.is_valid() failed, returning None with HTTP 200 — fixed to return HTTP 400 with errors
  • Typo: "Detektor not found.""Detector not found."

Backend Architecture

  • Deduplicated check_org_admin_permission: was copy-pasted in both detectors.py and organizations.py — extracted to api/permissions.py
  • Added GET /detector/<id>/ endpoint: frontend was fetching all detectors and doing client-side .find() by ID — inefficient and wrong at scale
  • Settings hardening: DEBUG = True hardcoded; ALLOWED_HOSTS included "*" — both now read from env vars with safe defaults

Frontend Bug

  • Stray literal text in JSX (LogbookEntryPage.tsx line 187): <div className="panel">theme.colors.danger, padding: theme.spacing['3xl'] was rendered as visible text

Frontend Architecture

Eliminated prop-drillingapiBase, isAuthed, getAuthHeader were passed individually to every page component. Replaced with AuthContext:

// Before: every route passed 3 auth props
<LogbooksPage apiBase={API_BASE} isAuthed={isAuthed} getAuthHeader={getAuthHeader} />

// After: pages consume context directly
export const LogbooksPage = () => {
  const { API_BASE, getAuthHeader } = useAuthContext()
  // ...
}
  • ProtectedRoute component: replaced duplicated if (!isAuthed) return <LoginRequired /> in every page with a single wrapper
  • isLoading guard in App.tsx: prevents flash of unauthenticated content while token is read from localStorage
  • Memoized getAuthHeader with useCallback([token]) so it's stable in useEffect deps
  • Shared Detector / DetectorType / DetectorManufacturer types in types.ts: same interface Detector was defined separately in 3+ files
  • package.json name typo: dosportal-rectdosportal-react
Original prompt

Jsi senioeni fullstack vývojář se zaměřením na react, Django.
Podivej se na tento projekt. Je ve vývoji. Mělo by vše být nastavené tak, aby se zpětně nic nemuselo prekopavat od základů. Aktuálně se mimo jiné migrují django views na react.
Vidíš nějaké nedostatky? ať už jde o strukturu, best practices, nebo něco co by se prostě mělo dělat lépe aby se projekt dále dobře vyvíjel.

Záměr se především na strukturu a kod frontend a django rest api, a lehce muzes i na testy.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits February 20, 2026 22:02
…ndpoint

Co-authored-by: cisar2218 <69775422+cisar2218@users.noreply.github.com>
…n package name, clean up empty lines

Co-authored-by: cisar2218 <69775422+cisar2218@users.noreply.github.com>
Copilot AI changed the title [WIP] Review project structure for Django and React migration Fix bugs and refactor: AuthContext, ProtectedRoute, DetectorDetail endpoint, settings hardening Feb 20, 2026
Copilot AI requested a review from cisar2218 February 20, 2026 22:09
@cisar2218
Copy link
Copy Markdown
Contributor

This agent run was triggered by mistake. Initial prompt was meant to be done to discuss how development direction in this project could be improved.

Most of findings were addressed outside this PR.

@cisar2218 cisar2218 closed this Mar 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants