feat: Drop How to Join / Projects from the header nav#753
Open
Nickatak wants to merge 14 commits into
Open
Conversation
First post-modernization run of `pre-commit run --all-files` surfaced cruft that accumulated before the lint stack landed (hackforla#737): trailing newlines missing on `.dockerignore` / `.gitignore`, CRLF line endings on `docker-compose.yml`, mixed line endings in `backend/poetry.lock`, prettier-formatted JSON in `frontend/knip.json` and `frontend/tsconfig.json`, etc. Pure auto-fixes from the configured hooks: - `end-of-file-fixer`: missing trailing newlines. - `mixed-line-ending --fix=lf`: CRLF -> LF normalization. - `trailing-whitespace`: stripped where present. - `prettier`: collapsed short JSON arrays to single-line per its default printWidth. No semantic changes; `make lint` and `make local-test-backend` both green pre- and post-fix. Splitting these out before the `accounts` app PR keeps the structural diff readable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… app Creates a Django `accounts` app and moves identity / auth concerns out of `ctj_api`. `ctj_api` now hosts only the recruitment-catalog and taxonomy domain (Opportunity, Project, Role, Skill, SkillMatrix, CommunityOfPractice). The new app earns its boundary by giving the upcoming Stage 1 auth flow (signup, login, logout, me, csrf - lands in a follow-up commit) a natural home and by isolating the eventual Stage 2 work (PeopleDepot integration, Cognito ID-token mode) to a single app surface. Done now while there's no real DB data to migrate. What moved (ctj_api -> accounts): - `CustomUser` model. - `user_detail` view + `UserDetailPermission` + `CustomUserReadSerializer`. - `users/<uuid>/` URL route (still served at `/api/users/<uuid>/`; `accounts.urls` mounts at the same `/api/` prefix from `backend.urls`, ordered before `ctj_api.urls` so its catch-all doesn't shadow). - `make_pm_user` / `make_regular_user` test factories. - `test_users.py`. Cross-app boundaries: - `Opportunity.created_by` FK now uses `settings.AUTH_USER_MODEL` instead of importing `CustomUser` directly. Avoids circular imports (accounts.models needs `CommunityOfPractice` and `SkillMatrix` from `ctj_api`) and follows the Django-recommended pattern - the FK shape stays the same when Stage 2 swaps `AUTH_USER_MODEL`. - `accounts.tests.common` imports its own factories; `ctj_api.tests.common` keeps domain factories; `test_opportunities.py` imports user factories from `accounts.tests.common`. Settings: - `INSTALLED_APPS` adds `accounts.apps.AccountsConfig` (before `ctj_api` for natural dependency order). - `AUTH_USER_MODEL` flips from `"ctj_api.CustomUser"` to `"accounts.CustomUser"`. Migration strategy: - Nuked `ctj_api/migrations/0001_initial.py` and `0002_*.py`. - Regenerated: `accounts/migrations/0001_initial.py` (CustomUser), `accounts/migrations/0002_initial.py` (FKs to ctj_api models), `ctj_api/migrations/0001_initial.py` (six domain models). - Anyone with a local dev DB needs to nuke it (`make db-reset-hard`); no stage data exists. Lint config: - `.github/workflows/lint.yml`: add `accounts` to mypy and bandit invocations. - `backend/pyproject.toml`: add `accounts/tests` to bandit `exclude_dirs`. - `Makefile`: drop hardcoded `ctj_api.tests` from `local-test-backend` so test discovery picks up both apps. Tests pass on a fresh test DB (10 tests across both apps). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Path B (no-CORS) infra was already wired for compose dev and local stage in hackforla#747 (BACKEND_INTERNAL_URL + next.config.ts rewrites), but host-based dev (`make local-run-frontend`) skipped the env entirely - the dev frontend at :3000 had no proxy and relative `/api/*` calls would land on the Next dev server with no listener. Wiring the host path now matters because forms get hooked to the backend in a follow-up commit. Changes: - `Makefile`: add `FRONTEND_RUN` macro that sources `dev/dev.env` and overrides `BACKEND_INTERNAL_URL=http://localhost:8000` (host Next can't reach the docker DNS name `django`). Parallel to how `BACKEND_RUN` overrides `SQL_HOST=localhost` for the same reason. `local-run-frontend` uses it. - `frontend/next.config.ts`: rewrite the rewrites() comment - the prior comment claimed dev runs cross-origin via `NEXT_PUBLIC_API_URL` (no longer true post-hackforla#747; that var is unused, removable in a separate sweep). Replaced with the per- environment table for `BACKEND_INTERNAL_URL`. - `dev/dev.env.example`: note the compose vs host value distinction inline next to the var. Same-origin in dev / compose / stage / prod -> SPA uses relative URLs end-to-end -> no CORS surface anywhere. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Backend half of the Stage 1 auth flow. Endpoints under `/api/auth/`
implement Django session + cookie auth via DRF
`SessionAuthentication`. Frontend wiring lands in a follow-up
commit.
Endpoints (all in `accounts/views.py` as FBVs):
- `GET /api/auth/csrf/` - `@ensure_csrf_cookie` -> sets the
`csrftoken` cookie on response. The SPA
calls this once on app load.
- `POST /api/auth/signup/` - validates via `RegisterSerializer`,
creates `CustomUser` (`username = email`,
`people_depot_user_id = local:<uuid>`
placeholder), auto-logs-in, returns 201
with the user record.
- `POST /api/auth/login/` - validates via `LoginSerializer`
(which calls `authenticate()`), creates
session, returns 200.
- `POST /api/auth/logout/` - calls `logout(request)`, returns 204.
Idempotent for anonymous callers.
- `GET /api/auth/me/` - returns the current authenticated user.
403 envelope when anonymous.
Settings:
- `REST_FRAMEWORK.DEFAULT_AUTHENTICATION_CLASSES` pinned to
`[SessionAuthentication]`. DRF's default also enables
`BasicAuthentication`; nothing here uses it (Stage 1 is cookie/
session SPA; Stage 2 is Cognito ID-token), so pinning explicitly
keeps the auth surface tight.
Serializer cleanup:
- Dropped the `opportunities` field from `CustomUserReadSerializer`.
It was a writable PK-related-field referencing a non-existent
attribute on `CustomUser` - documented as broken since the shape
PR; would have crashed `auth_me` and `user_detail` on first call.
- Uncommented `test_authenticated_user_can_view_own_record` in
`test_users.py` (was gated on the broken field being dropped) and
expanded it to assert response body shape.
Tests (`accounts/tests/test_auth.py`):
- 14 new tests across the five endpoints: happy paths, auth
failures, CSRF cookie set, duplicate-email / weak-password /
missing-field rejections, anonymous-403 on `me`,
logout-clears-session, logout-when-anonymous-is-noop.
CSRF caveat:
- DRF's `@api_view` marks the resulting view csrf_exempt at the
Django middleware level; CSRF is enforced only by
`SessionAuthentication`'s own check, which fires for
authenticated requests. Effect: signup and login do *not* enforce
CSRF (they're anonymous when called); logout, me, and any
authenticated mutation *do*. The frontend client still sends
`X-CSRFToken` on every mutation - it's free once the cookie is
set. Hardening signup/login CSRF (via `@csrf_protect` over
`@api_view`, or a custom `EnforceCsrfMixin`) is a follow-up;
flagged here so review knows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Frontend half of the auth wiring. Adds a fetch wrapper, typed
endpoint wrappers, and a React context that exposes auth state +
actions to the rest of the app. Forms get hooked to these in a
follow-up commit.
Files:
- `shared/lib/api/client.ts` - `apiFetch<T>(path, options)` wrapper.
Sets `credentials: "include"` so the session cookie rides along,
reads `csrftoken` from `document.cookie` and sends it as
`X-CSRFToken` on mutating methods, parses non-2xx responses
through the `civic_exception_handler` envelope shape, and throws
a typed `ApiError` (with `status`, `code`, `message`, `details`)
so callers can discriminate without re-parsing JSON.
- `shared/lib/api/auth.ts` - typed wrappers for the five
`/api/auth/*` endpoints. `User` mirrors `CustomUserReadSerializer`.
- `shared/contexts/AuthContext.tsx` - `AuthProvider` + `useAuth`
hook. On mount: seeds CSRF cookie, then hits `/auth/me/` to
hydrate from any persisted session (403 -> stay anonymous).
Exposes `{ user, loading, login, signup, logout }`. Mounted in
the root layout (`src/app/layout.tsx`) so every route - logged
in or logged out - can read auth state.
- `useAuth` throws if used outside `AuthProvider`, matching the
existing `useQualifiersContext` defensive pattern.
Tests:
- `tests/lib/api/client.test.ts` - 8 tests covering happy path
(200, 204), credential / CSRF header behavior, body
serialization, error envelope unwrapping, non-JSON-error fallback.
- `tests/contexts/AuthContext.test.tsx` - 8 tests covering mount
hydration (authenticated and anonymous), `login()` / `signup()` /
`logout()` state transitions, error propagation,
outside-provider throw, and child render.
Auth context mount-point judgment call (from the spec): root
layout. Reason: even logged-out state is a real state nav
consumers care about (Login button vs avatar), and mounting at
the root means any page can read it without route-group
duplication.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the `console.log` placeholder onSubmits with real calls
through `useAuth()`. Both forms now hit the Stage 1 backend and
redirect on success.
LoginForm:
- Calls `login({ email, password })` from the auth context.
- On success: `router.push("/")` (landing).
- On `ApiError`: surfaces `err.message` in a `role="alert"` banner
at the top of the form. Field-level details (`err.fields`) are
not displayed inline; login errors are typically a single
"Invalid email or password" string, not field-shaped.
SignupForm:
- Combines `firstName + lastName` -> `name` so the existing two-
field UX maps cleanly onto the backend's single-`name` shape.
- Calls `signup({ email, password, name })`. Backend auto-logs-in,
so the context picks up the new `user` directly and the form
pushes to `/`.
- Same alert banner pattern for `ApiError`.
Both forms:
- Disable the submit button while the request is in flight; button
text swaps to "Logging in..." / "Signing up...".
- New CSS: `.submit:disabled` (opacity / cursor) and `.serverError`
(colored alert banner). Variables fall back to literal hex if the
CSS custom property isn't set.
- `ApiError` is the only specifically-handled error type; any other
error renders a generic "Something went wrong" message so silent
failure modes don't slip through.
Tests:
- `tests/components/LoginForm.test.tsx` - 4 tests: success ->
redirect, server error -> banner, button disabled while
in flight, RHF blocks empty submission.
- `tests/components/SignupForm.test.tsx` - 3 tests: name combine +
redirect, server error -> banner, button disabled while in flight.
- All mock `next/navigation` and `useAuth` to keep the unit tests
hermetic.
Auth flow is now end-to-end. Manual smoke test path: visit
/signup, register, get redirected to /, navigate to /login,
log out via... pending - no logout UI yet (an avatar / nav
hookup is its own follow-up).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aligns the developer docs with the new `accounts` app and the five `/api/auth/*` endpoints: - `backend.md` project layout - add the `accounts/` tree; reframe `ctj_api/` as the domain app (CustomUser is no longer there). `auth.py` placeholder moves from `ctj_api/` to `accounts/` to match the Stage 2 description below. - `backend.md` API table - add the five auth endpoints with their methods, auth requirements, and notes. - `backend.md` auth section - expand with the SPA cookie + CSRF flow narrative (4-step bootstrap, same-origin via Next rewrites, CSRF caveat for anonymous endpoints). Update Stage 2 reference from `ctj_api.auth.py` -> `accounts.auth.py`. - `backend.md` test shape - tests live in `accounts/tests/` and `ctj_api/tests/` per app; user factories moved to `accounts.tests.common`, domain factories stay in `ctj_api.tests.common`. - `backend.md` permissions section - `UserDetailPermission` now lives in `accounts.permissions`. - `backend.md` lint commands - mypy and bandit invocations get `accounts` added to the args. - `installation.md` local-dev auth - mention the new SPA signup path (`/signup` -> `POST /api/auth/signup/`); add the `isProjectManager` toggle hint for PM elevation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Next's default `trailingSlash: false` strips the trailing slash off incoming requests (308 redirect) before `rewrites()` runs, so the SPA's trailing-slash API calls (`/api/auth/csrf/`, `/api/auth/signup/`, ...) were proxied to Django as `/api/auth/csrf`, missing every `path()` entry and falling through to the `api_not_found` catch-all. Append the slash on the rewrite destination and pair slash/no-slash `source` entries so the post-308 form and any slashless caller both resolve. The browser auth flow through `:3000` was non-functional before this. The unit tests didn't catch it because vitest mocks `fetch` and never exercises the Next rewrite layer; deployed stage was unaffected because the ALB path-routes `/api/*` straight to Django without this proxy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`quickstart-guide.md` pointed at `/api/healthcheck` (no trailing slash), which 404s under the ADR-0011 trailing-slash routing convention; the route is `/api/healthcheck/`. `installation.md` step 3 still said `docker compose up --watch`, predating ADR-0014 (Make is the canonical task runner); point it at `make docker-up` / `make docker-watch`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`HeaderNav` becomes a client component reading `useAuth()`: when a session is active the right-hand slot is a "Log out" button that calls the (previously unused) `logout` action; otherwise it's the existing "Log In" link. `loading` is treated as signed-out, so a logged-in user sees a brief "Log In" -> "Log out" flip on first paint. Logout only clears context state -- no redirect, since nothing is behind auth yet. Deliberately minimal: there is no Figma frame for the signed-in nav (the original app had no auth UI), so this is an interim placeholder in the same slot the "Log In" link occupies -- no avatar, no account menu, no name. The hamburger menu stays inert. 3 new tests (`HeaderNav.test.tsx`) cover the signed-out / loading / signed-in branches against a mocked `useAuth`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The SPA runs on :3000 and is proxied to Django via Next `rewrites()`,
which rewrite `Host` to the backend's address -- so an authenticated
POST reaches Django carrying `Origin: http://localhost:3000` against a
request host that isn't that origin, and Django's CSRF check rejects it
("Origin checking failed -- ... does not match any trusted origins").
The first request that hits this is logout (the first authenticated
POST; signup/login are CSRF-exempt -- anonymous when called -- and `me`
is a GET).
Add an env-driven `CSRF_TRUSTED_ORIGINS` with a dev default covering the
host-dev and compose-dev SPA origins; declare it in the env examples and
document it in backend.md. Deployed stage/prod route `/api/*` to Django
on the same hostname (ALB path routing), so the request host already
matches there; devops sets the var per environment. 2 new tests
exercise the real session + CSRF-token path with an `Origin` header --
the curl-based smoke missed this because curl doesn't send `Origin`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vitest 3 narrowed the default `MockInstance` returned by a bare, unparameterized `vi.spyOn` reference. `ReturnType<typeof vi.spyOn>` now resolves to `MockInstance<(this: unknown, ...args: unknown[]) => unknown>`, which the spy actually produced by `vi.spyOn(authApi, "login")` etc. can't assign into - function parameters are contravariant, and `LoginPayload` isn't assignable to `unknown` in the contained call signature. vitest 2 was looser, so this only surfaced after the runtime/dep bump in hackforla#737. Type the spy variables directly as `MockInstance<typeof target.method>`, sidestepping the `vi.spyOn` generic constraints entirely. Three spies affected (login, signup, fetch); the no-arg spies (csrf, me, logout) had no contravariance problem and are left unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ApiErrorBody` is used only inside `client.ts` (typing the parsed error envelope before constructing `ApiError`) and was never imported anywhere else, so the `export` keyword was dead. Knip caught it on CI. Dropping the export is the right fix - consumers should read `ApiError.code` / `.message` / `.fields` rather than the raw body shape, which is exactly what the rest of the codebase already does. Added an internal-only comment explaining why the type isn't exported, to head off the next "should I export this?" question. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the two external-link entries from `HeaderNav`'s
`menuItems` array, keeping only "Hack for LA". Updates the
docstring ("three external links" -> "a single external link")
and adds a WHY comment recording the 2026-05-14 nav decision and
why the array structure stays (the follow-up that adds
"View Opportunities" re-adds an entry).
The decision's other half - adding "View Opportunities" - is
deliberately out of scope: it's blocked on its destination
(internal CTJ route vs the HfLA opportunities section).
The inert mobile hamburger menu is unchanged and remains
explicitly out of scope; the mobile-nav behavior is an undecided
design question, not just an unimplemented one.
Tests: extends `HeaderNav.test.tsx` with a structure block (logo
links home) and an external-links block (Hack for LA present;
How to Join and Projects absent). 3 new tests; frontend suite
40 pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nickatak
force-pushed
the
feat/navbar-trim-org-links
branch
from
46910ca to
9e7f062
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on
feat/auth(#752). Both PRs editHeaderNav.tsx;basing this off
feat/authavoids a same-file conflict and lets thelink tests live in the auth-aware
HeaderNav.test.tsx. Diff is cleanagainst
feat/auth(2 files); review/merge it after #752 lands.Summary
Implements the removal half of the 2026-05-14 nav decision
(
scratch/planning/issue-navbar-opportunities-link.md): drop the"How to Join" (
hackforla.org/getting-started) and "Projects"(
hackforla.org/projects/) external links from the header nav,keeping only "Hack for LA" (
hackforla.org/).The decision's other half - adding a "View Opportunities" link - is
deliberately out of scope: it's blocked on its destination
(internal CTJ route vs the HfLA opportunities section). The
menuItemsarray and itsMenuObjectshape are kept (single entry)so the follow-up re-adds an entry rather than reintroducing the
structure.
Commits
One commit; the test change folds into it.
feat: Drop How to Join / Projects from the header nav(2 files;menuItemstrimmed to one entry + WHY comment, docstring updated"three external links" -> "a single external link", 3 new tests:
logo-links-home, Hack-for-LA-present, dropped-links-absent).
Decisions baked in
destination decision before it can land; shipping the unblocked
removal now keeps the PR small and unblocked.
menuItemsstays an array, not inlined to a single<a>. Thefollow-up re-adds an entry; collapsing the structure now just to
re-expand it later is churn.
HeaderNav.test.tsx(added onfeat/auth) with a seconddescribeblock rather than a newHeaderNav.links.test.tsx. One component, one test file.feat. A user-visible curation of nav contents - noneof the five prefixes fit cleanly (it's a removal, not new
functionality, a bugfix, or internal);
featis least-wrong perADR-0006's acknowledged 10% bikeshed case.
remains inert (no menu opens;
aria-expandedhard-coded"false"). This PR does not touch that and does not regress it.It's an undecided design question, not an oversight: there's no
agreed behavior for the mobile menu yet (what it contains, how it
opens), so wiring it would be guessing at a spec. Explicitly out
of scope until that's decided - see follow-ups.
Test plan
npx vitest run-> 40 pass (37 fromfeat/auth+ 3new tests; the 2 file-level + 3 test-level skips are
pre-existing).
make lintclean (ruff / ruff-format / prettier / eslint /stylelint pass).
npm run lint:typesclean. The previously-inherited failurefrom
feat/authwas fixed in feat: Stage 1 authentication #752's commit 12 and this branchstacks on the updated
feat/auth.Follow-ups (out of scope for this PR)
(internal route vs
hackforla.org) and its nav position ischosen. See
scratch/planning/issue-navbar-opportunities-link.md.open/close behavior), then wire it. Undecided, not just
unimplemented - needs a design call first. If "View Opportunities"
lands before this, it must be wired into the mobile menu too.