Skip to content

Commit 2dd5b09

Browse files
authored
Merge pull request #746 from Nickatak/docs/public-adrs
docs: Add public ADR series in docs/decisions/
2 parents ab86114 + dac4034 commit 2dd5b09

23 files changed

Lines changed: 987 additions & 17 deletions

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ The goal is that contributors can land work without fighting the type checker, w
4343

4444
### Docstrings (backend)
4545

46-
Backend code follows a labeled-section docstring convention modeled on the project's sister codebase (BNC). The shape is per-file-kind: heavy labeled templates for models and views (where there's substantive policy to document), lighter prose for serializers and config files. See [docs/developer/backend-docstring-style.md](docs/developer/backend-docstring-style.md) for the full templates, vocabulary, and rationale.
46+
Backend code follows a labeled-section docstring convention. The shape is per-file-kind: heavy labeled templates for models and views (where there's substantive policy to document), lighter prose for serializers and config files. See [docs/developer/backend-docstring-style.md](docs/developer/backend-docstring-style.md) for the full templates, vocabulary, and rationale.
4747

4848
### Docstrings (frontend)
4949

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ The current process involves several manual steps for both sides. CivicTechJobs
2626

2727
## Stack
2828

29-
- **Frontend**: Next.js 16 (App Router) + React 19 + TypeScript 5, styled with CSS Modules.
30-
- **Backend**: Django 5.1 + Django REST Framework 3.15, served via Daphne (ASGI).
31-
- **Database**: PostgreSQL 16.
29+
- **Frontend**: Next.js 16 (App Router) + React 19 + TypeScript 6, styled with CSS Modules.
30+
- **Backend**: Django 6.0 + Django REST Framework 3.17, served via Daphne (ASGI).
31+
- **Database**: PostgreSQL 18.
3232
- **Deployment**: AWS ECS (three containers in one task) on Hack for LA's Incubator account.
3333

3434
See [docs/developer/](docs/developer/) for the full architecture writeup.
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
# ADR-0001: Tooling and dependency baseline
2+
3+
**Status:** Accepted
4+
**Date:** 2026-05-02
5+
6+
## Context
7+
8+
The CTJ rewrite re-evaluated the project's tooling stack from scratch. Pre-rewrite, the codebase used a Python tooling chain centered on black + flake8 + isort + pylint, a frontend chain centered on Jest + Tailwind + SCSS + PostCSS, and a `.pre-commit-config.yaml` that had drifted into stale-territory (pinned to Python 3.9, referenced paths that no longer exist post-rewrite).
9+
10+
This ADR consolidates the version pins and tool choices made across the modernization PRs. Each sub-decision has its own short rationale; the deeper architectural choices (test-framework swap, styling-system swap, container shape) live in their own ADRs.
11+
12+
## Decisions
13+
14+
### Backend lint: ruff replaces black + flake8 + isort
15+
16+
`ruff check` and `ruff format` cover formatting, import sorting, lint-rule coverage equivalent to flake8, and a fast Rust-based runner. The combined tool replaces three separate tools with overlapping responsibilities; `pyproject.toml` carries one config block instead of three.
17+
18+
Pylint (which the legacy config tied to `config.settings`, the old project's settings module) is dropped. Ruff's lint coverage is sufficient for a small volunteer codebase; pylint's deeper analysis is not load-bearing for the kinds of bugs CTJ surfaces.
19+
20+
### Backend types: mypy in gradual mode + django-stubs + drf-stubs
21+
22+
`mypy --strict` is too aggressive for a partially-typed codebase (every untyped function would need annotation before mypy passes). Gradual mode (`--check-untyped-defs` off, `--ignore-missing-imports` on for un-stubbed deps) lets typed code be checked while leaving older code as-is until contributors get to it.
23+
24+
`django-stubs` and `djangorestframework-stubs` provide model-field synthesis (model attribute types come from field declarations via mypy's plugin protocol) and DRF type stubs.
25+
26+
### Backend security: bandit
27+
28+
`bandit` runs as part of the lint pipeline. Catches the obvious classes of security issue (hardcoded secrets, weak crypto choices, shell injection patterns). Cheap to run; finds enough to earn its place.
29+
30+
### Backend Python: 3.13
31+
32+
Latest stable major. Type-system improvements over 3.12 (PEP 695 generic syntax, narrower generics) make typed code cleaner. Aligned with Django 6's targeted Python versions.
33+
34+
### Backend framework: Django 6.0 (skip 5.2 LTS)
35+
36+
Django 6.0 is the latest major; 5.2 is the most recent LTS. The choice is between LTS-stability (5.2) and feature-currency (6.0).
37+
38+
LTS is appealing for projects with long maintenance horizons and limited capacity to upgrade. CTJ's situation is the opposite: the rewrite is the upgrade, and there will be no separate "upgrade Django" PR for a while. Starting on 6.0 means the project is on a current major right out of the rewrite gate, and the next upgrade conversation is years away. The async ORM improvements and async-view refinements in 6.0 are also nice-to-haves for the eventual matching engine.
39+
40+
If a feature CTJ needs lands first in 5.x's patch series and not in 6.0, the choice could be revisited. Today there's no such pull.
41+
42+
### Database driver: psycopg3 (replaces psycopg2)
43+
44+
psycopg3 is the actively-maintained successor; psycopg2 is in deep maintenance mode only. psycopg3's typing is better, async support is better, and the connection-pool story is cleaner. The migration cost is low - most psycopg2 calls work unchanged on psycopg3 with the binary build (`psycopg[binary]`).
45+
46+
### Database: PostgreSQL 18
47+
48+
Latest stable Postgres. No version-specific features are load-bearing yet; choice is "current at the time of the rewrite" rather than "we need feature X."
49+
50+
### Frontend Node: 24
51+
52+
Latest LTS-track. ESM support is mature; Vite, Vitest, and Next.js 16 all align with Node 24+.
53+
54+
### Frontend framework: Next.js 16 (with React 19, TypeScript 6)
55+
56+
The rewrite assessment originally targeted Next.js 15; by the time the actual port landed, 16 had GA'd with React 19 GA support. The migration cost is paid on the rewrite branch (not on a future maintenance branch), and `eslint-config-next`, the React 19 type definitions, and the Turbo CLI all align in 16's direction. App Router is the architecture either way.
57+
58+
### Frontend lint: ESLint 9 (defer 10), Stylelint 17, Knip 6, Prettier 3.8
59+
60+
- **ESLint 9 pin (defer 10)**: ESLint 10 dropped support for some plugin shapes that `eslint-plugin-jsx-a11y` and `eslint-plugin-react` haven't yet shipped 10-compatible peers for. Pin to 9 until the plugin ecosystem catches up; revisit when peers ship.
61+
- **Stylelint 17**: standard CSS lint tier; configured with `stylelint-config-standard` plus camelCase patterns to match CSS Modules' identifier conventions.
62+
- **Knip 6 (fail-on-find)**: dead-code/unused-export finder. Configured to fail CI on any finding rather than warning - dead code is cheap to delete, and "warn but don't fail" tends to drift to "warn forever."
63+
- **Prettier 3.8**: standard formatter; runs via pre-commit and lint workflow.
64+
65+
### Frontend test runner: Vitest 3 (defer 4)
66+
67+
The framework swap (Vitest replaces Jest) is documented in [ADR-0005](0005-vitest-replaces-jest.md). The version pin is here:
68+
69+
Vitest 4 pulls `babel-plugin-react-compiler` as a peer dependency of `@vitejs/plugin-react`. The React Compiler isn't enabled in this codebase yet (the eslint hooks-rules for the compiler are downgraded to warnings; see below). Pinning to vitest 3 avoids the peer pull until the React Compiler work is actually scoped.
70+
71+
### React Compiler hooks rules: downgraded from `error` to `warn`
72+
73+
`react-hooks` 7.x adds rules that surface React Compiler concerns: `set-state-in-effect`, `refs`, `static-components`. At the modernization tip, these rules surface 28 warnings across the existing codebase. Fixing them all at once would mean a sweeping refactor that conflates "modernize tooling" with "rewrite hooks logic."
74+
75+
The rules are kept enabled but downgraded to `warn`, so contributors see them on every PR and a follow-up cleanup can address them in batches once the pattern PRs land.
76+
77+
### Pre-commit: keep the pre-commit framework, rewrite the config
78+
79+
The pre-rewrite `.pre-commit-config.yaml` was unrunnable (Python 3.9 pin, references to nonexistent paths, deprecated version of `mirrors-prettier`).
80+
81+
Two paths: delete the config and rely entirely on CI, or rewrite it against the new tools. Delete-and-rely-on-CI is simpler but has higher feedback latency (push → workflow run → status). Rewriting keeps fast local feedback at commit time; CI is the authoritative gate. Both reference the same pinned tool versions, so drift between local and CI is bounded.
82+
83+
The new config covers file-shape hygiene (trailing whitespace, EOF fixer, merge-conflict markers, YAML/TOML/JSON syntax), backend lint (`ruff-pre-commit`), frontend formatting (`mirrors-prettier`), and frontend lint (local hooks shelling out to `npx eslint` + `npx stylelint` so the project's pinned versions are used, not a frozen mirror's).
84+
85+
## Why these choices, in aggregate
86+
87+
The pattern across all of these is "modernize, but pin versions, defer the in-flight majors." Specifically:
88+
89+
- Pinned versions: ESLint 9 (not 10), Vitest 3 (not 4) - both deferred because their plugin ecosystem isn't ready.
90+
- Picked currents: Django 6 (not 5.2 LTS), Next.js 16 (not 15), Python 3.13, Node 24, PostgreSQL 18, psycopg3 - chosen because the rewrite is the upgrade window.
91+
- Single-tool consolidations: ruff replaces three Python tools; CSS Modules replaces the Tailwind+SCSS+PostCSS chain (see [ADR-0004](0004-css-modules-without-preprocessor.md)); Vitest replaces Jest.
92+
93+
The bias is toward fewer tools doing more, currents over LTS where the upgrade horizon is short, and explicit deferrals when the ecosystem hasn't caught up.
94+
95+
## Consequences
96+
97+
**Accepted:**
98+
99+
- Two version pins (ESLint 9, Vitest 3) carry follow-up watch tasks: revisit when their respective plugin ecosystems ship 10/4-compatible peers.
100+
- The 28 react-hooks warnings are visible noise on every PR until a cleanup PR addresses them. Documented; not blocking.
101+
- New contributors install pre-commit (`pip install pre-commit && pre-commit install`); documented in the installation guide.
102+
- The Stage 1 → Stage 2 migration (PeopleDepot integration; see [ADR-0002](0002-phase-1-skip-peopledepot.md)) is a separate concern from this baseline.
103+
104+
**Won:**
105+
106+
- Single source of truth for each tool's version: pinned in `pyproject.toml` (backend) or `package.json` (frontend), referenced by both pre-commit hooks and the CI lint workflow.
107+
- Lint runs are fast (ruff's Rust runner; Vitest's parallel workers).
108+
- The rewrite ships on currents; the next "modernize" PR is years away.
109+
- Configuration is concentrated: `pyproject.toml` for backend, `package.json` + `eslint.config.mjs` + `stylelint.config.mjs` + `vitest.config.mts` + `knip.json` for frontend. No scattered `.<tool>rc` files.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# ADR-0002: Stage 1 skips PeopleDepot integration
2+
3+
**Status:** Accepted
4+
**Date:** 2026-05-02
5+
6+
## Context
7+
8+
PeopleDepot (PD) is HfLA's centralized reference-data service. It owns the canonical records for users, projects, practice areas, and (eventually) skills across all HfLA volunteer projects. The CTJ rewrite's eventual end state (Stage 2) depends on PD: user identity comes from PD via Cognito ID-token, projects/roles/practice-areas are sourced from PD at request time, and the skill catalog is synced from PD into a local cache.
9+
10+
PD's prod-deployment posture is unstable upstream ([PeopleDepot Issue #218](https://github.com/hackforla/peopledepot/issues/218)). The OAuth/Cognito JWT-validation flow CTJ would consume is not finalized; PD's API surface is in flux; the data shapes for users/projects/roles aren't locked.
11+
12+
Building CTJ against PD now would mean either pinning to an in-flight PD shape (high churn, integration tax compounds with every PD change) or shimming PD with mocks (carrying the cost of building a fake PD that doesn't validate against the real one).
13+
14+
## Decision
15+
16+
**Stage 1 of the rewrite ships without any PeopleDepot integration. CTJ owns its own User table (via Django's `AbstractUser`-based `CustomUser`), its own Skills catalog, its own Project and Role tables, and uses Django session authentication. PeopleDepot integration is Stage 2, gated on PD's upstream posture stabilizing.**
17+
18+
Concretely, Stage 1:
19+
20+
- `CustomUser` extends `AbstractUser` and is the auth user model. Identity fields (name, email) are locally editable.
21+
- `Skill`, `Role`, `Project`, `CommunityOfPractice` tables are locally curated through Django admin.
22+
- `people_depot_user_id` and `people_depot_project_id` columns exist on `CustomUser` and `Project` as forward-compatibility hooks; in Stage 1 they hold local placeholder strings. In Stage 2 they become real PD UUIDs and the local fields become PD-synced.
23+
- Auth is Django's session + cookie machinery; no Cognito JWT validation.
24+
25+
Stage 2 (deferred) will add a custom DRF authentication backend at `ctj_api/auth.py` that validates Cognito JWTs, a PD client at `ctj_api/clients/peopledepot.py`, a periodic sync job that updates the local Skill catalog from PD, and backfill logic to populate `people_depot_*_id` columns from PD lookups.
26+
27+
## Alternatives considered
28+
29+
- **Integrate PD immediately.** Build CTJ against the in-flight PD API. Discarded because PD's prod posture isn't ready; CTJ would either pin to a snapshot (high integration tax) or chase PD's surface as it changes. Either path also blocks CTJ progress on PD's release schedule.
30+
31+
- **Shim PD with mocks.** Build a fake PD service that CTJ talks to in Stage 1; replace with the real PD in Stage 2. Discarded because the shim carries its own maintenance cost (someone has to keep the fake aligned with the real PD's evolving shape) without producing forward progress. The shim's behavior at integration time would not have been validated against real PD anyway.
32+
33+
- **Defer auth + PD work entirely; ship a PM-only / admin-curated phase.** Effectively what was chosen, except the auth surface is included in Stage 1's eventual scope. The Stage 1 user model (`CustomUser` with `isProjectManager` flag) is real-user-facing, not admin-only.
34+
35+
- **Adopt PD-shaped types but ignore PD-shaped runtime.** Build CTJ's models with PD's UUID types and naming as if PD existed, just not connected. Partially adopted (`people_depot_user_id` and `people_depot_project_id` columns) as forward-compat hooks; the rest of the schema is shaped for CTJ's local needs.
36+
37+
## Consequences
38+
39+
**Accepted:**
40+
41+
- CTJ's Stage 1 User and reference tables drift from PD's eventual shape. The Stage 2 migration has to reconcile (e.g. backfill `people_depot_user_id` for users who signed up locally; deduplicate users who exist in both stores). Mitigated by treating Stage 1 as a finite-lifetime data layer.
42+
43+
- Stage 2 work is real, not "flip a flag." The Cognito validation backend, PD client, sync job, and backfill all need to be built when PD is ready.
44+
45+
- The `people_depot_*_id` columns exist on Stage 1 rows with placeholder values that will be rewritten in Stage 2. Anyone joining on those fields needs to know they're not authoritative yet.
46+
47+
- Documentation makes the staging explicit: every model, every endpoint, every auth surface either says "Stage 1 only" or describes what changes in Stage 2.
48+
49+
**Won:**
50+
51+
- CTJ can ship Stage 1 without waiting for PD. The schedule is decoupled.
52+
53+
- Stage 1's data model is shaped for CTJ's actual needs (recruitment matching, qualifier flow), not for PD's reference-data shape.
54+
55+
- New volunteers can sign up, complete the qualifier flow, and use the matching engine without any PD-side dependency. This is the MVP.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# ADR-0003: Separate frontend and backend containers, both publicly exposed
2+
3+
**Status:** Accepted
4+
**Date:** 2026-05-02
5+
6+
## Context
7+
8+
Pre-rewrite, CTJ was a single Django container that served both the API (`/api/*`) and the frontend (built React assets via whitenoise + a SPA-catchall view rendering `index.html`). One image, one process, one origin.
9+
10+
The rewrite separates the frontend (Next.js, App Router, server components) from the backend (Django + DRF). Once Next.js is running its own server (for server components, route handlers, image optimization, dev rewrites, etc.), the question is how the two services compose:
11+
12+
- Single combined image with one runtime embedded inside the other.
13+
- One container fronts the other (e.g. Next reverse-proxies the API to Django).
14+
- Two containers, both publicly exposed.
15+
16+
## Decision
17+
18+
**Build two independent Docker images - one for Next.js, one for Django - and expose both publicly. In stage and prod, an ALB-shaped routing layer sits in front and dispatches `/api/*` and `/admin/*` to Django, everything else to Next. Locally, dev and stage use Next's `rewrites` config to mimic the single-origin shape from the browser's POV.**
19+
20+
Concretely:
21+
22+
- `dev/django.dockerfile` and `dev/next.dockerfile` build the dev images. Both run on local ports (Django 8000, Next 3000).
23+
- `stage/django.dockerfile` builds Django for daphne (ASGI). `stage/next.dockerfile` is a three-stage build (deps → build → runner) producing the Next.js standalone output.
24+
- `docker-compose.yml` and `docker-compose.stage.yml` expose both services. Postgres is a third container.
25+
- `BACKEND_INTERNAL_URL` is the inter-container DNS name Next uses to reach Django (e.g. `http://django:8000` in compose). Next's `rewrites` config in `next.config.ts` proxies `/api/*` and `/admin/*` to that URL during local dev and stage so the browser never sees a CORS-relevant cross-origin request.
26+
- Production deployment is owned externally (the `incubator` repo's Terraform + ALB configuration). CTJ's responsibility ends at "the images build and run independently."
27+
28+
## Alternatives considered
29+
30+
- **Keep Django serving both (status quo).** Simplest deployment, single image, single origin. Discarded because Next.js's server-side rendering, server components, and route handlers can't run inside a Django container - Next needs its own Node runtime.
31+
32+
- **Next reverse-proxies the API to Django.** Browser only sees Next's origin; Django sits behind Next. Discarded because (a) Next isn't a load-balancer; using it as one mixes responsibilities; (b) Django admin (`/admin/*`) is a Django concern that shouldn't route through Next; (c) the upstream-Next-as-edge pattern adds latency on every request to a service already handling rendering.
33+
34+
- **Single combined image with Next embedded as a subprocess.** Discarded because the two services have different runtime needs (Python 3.13 vs Node 24), different scaling characteristics, and different deploy cadences. Combining them forces a lockstep that benefits neither.
35+
36+
- **Backend hidden behind Next (not publicly exposed).** Even with Next reverse-proxying to Django, Django could be unpublished. Discarded because Django admin (`/admin/*`) is a real human-facing surface that shouldn't be reachable only through Next.
37+
38+
## Consequences
39+
40+
**Accepted:**
41+
42+
- Two images to build, two health checks, two scaling policies. Mitigated by Docker Compose locally and the deploy machinery in stage/prod.
43+
44+
- The pre-rewrite SPA-catchall + `frontend_dist` + whitenoise machinery is dead weight. A follow-up architectural-cleanup PR drops it.
45+
46+
- Inter-container DNS (`BACKEND_INTERNAL_URL`) is a real environment variable per service. Mistyping it breaks dev rewrites silently; documented in the devops and installation guides.
47+
48+
- Cross-origin work is split: in dev/stage, Next's `rewrites` make it a same-origin problem from the browser's POV. In prod, the ALB does the same. Pure-frontend cross-origin work (CORS headers, preflight) is killed.
49+
50+
**Won:**
51+
52+
- Each service scales, deploys, and is debugged independently. A rebuild of the frontend doesn't restart Django and vice versa.
53+
54+
- Next.js's full feature set (server components, route handlers, edge rendering) is available because Next runs in its own Node process.
55+
56+
- Django admin is a Django concern, served by Django, not piped through Next. The PM-facing CMS path stays simple.
57+
58+
- The deployment story is the standard "ALB with two target groups" shape that HfLA infra already supports.

0 commit comments

Comments
 (0)