Skip to content

refactor: Split serializers into Read/Write classes per resource#741

Merged
RSkuma merged 34 commits intohackforla:developfrom
Nickatak:refactor/serializers-read-write
May 7, 2026
Merged

refactor: Split serializers into Read/Write classes per resource#741
RSkuma merged 34 commits intohackforla:developfrom
Nickatak:refactor/serializers-read-write

Conversation

@Nickatak
Copy link
Copy Markdown
Member

@Nickatak Nickatak commented May 3, 2026

refactor: Split serializers into Read/Write classes per resource

Branch: refactor/serializers-read-write stacked on refactor/views-explicit (#740), which is stacked on docs/add-docstrings (#739), which is stacked on the modernization chain (#734-#738). Retarget chain when each parent lands.

This is the third pattern-establishment PR. With docstring conventions locked in by #739 and view shape locked in by #740, this PR locks in the serializer shape: each resource gets separate XxxReadSerializer and XxxWriteSerializer classes when both shapes are needed; read-only resources have only a Read class.


Summary

  • Renames the existing read-only XxxSerializer classes to XxxReadSerializer.
  • Splits OpportunitySerializer (the only resource with write endpoints) into OpportunityReadSerializer + OpportunityWriteSerializer.
  • Wires OpportunityViewSet.get_serializer_class() to dispatch by action.
  • Updates the FBVs in views.py to import the renamed classes.
  • Documents the convention in docs/developer/backend.md (new Serializer shape section) and updates the serializer template in backend-docstring-style.md.

4 files changed, 160 insertions, 57 deletions. Two commits.

# Subject
1 refactor: Split serializers into Read/Write classes per resource
2 docs: Document serializer shape convention

Why this shape

A single ModelSerializer per resource splits the input/output contract across two layers. The serializer says "here are the fields and their types"; the view says "but you can only call this with GET" (or "POST is gated to PMs"). A reader of serializers.py cannot tell which fields are writable, which are computed-on-read, or whether the class supports writes at all. That information lives in the view, the permissions, and the docstring — not in the serializer.

The pain is clearest where read and write semantics legitimately diverge. OpportunitySerializer.created_by is a stringified email on read but the view stamps the FK from request.user on write — the same field has different semantics for reads and writes. The single class either does both behaviors awkwardly (current state, with ReadOnlyField plus a view-layer perform_create override) or one of the behaviors is implicit (the writable subset of fields isn't documented anywhere except by elimination).

After this PR, each resource concentrates its shape contract back at the serializer. Read responses are described by the Read class; accepted writes are described by the Write class. Future changes to write-side behavior (validation, computed-on-write fields, accepted field set) land in the Write class only — read responses are not affected, and the change is atomic.

The convention

Resource state Serializer classes
Read-only (no write endpoint exists) XxxReadSerializer only
Read + write XxxReadSerializer and XxxWriteSerializer

Auto-managed fields (id, created_at, updated_at) and request-stamped fields (Opportunity.created_by, set from request.user by the view) are absent from XxxWriteSerializer.Meta.fields rather than included with read_only=True. Absence is the contract: if the field doesn't appear in Meta.fields, the client cannot supply it on write.

OpportunityViewSet (the one ModelViewSet in the codebase) dispatches via:

def get_serializer_class(self):
    if self.action in ("list", "retrieve"):
        return OpportunityReadSerializer
    return OpportunityWriteSerializer

FBVs reference the right serializer directly — they only do one thing, so they only ever need one class.

What's not in this PR

Following the same pure-pattern rule as #739 and #740:

  • The broken opportunities field on CustomUserReadSerializer (BUG-001) is left as-is. Flagged inline in the docstring; the bug-fix PR drops the field.
  • The dead SkillMatrixSerializer (BUG-002) is left as-is. Not split into Read/Write because it has no consumers; the docstring is updated to note that it should conform to the new convention if/when it's wired up. The cleanup PR drops it.
  • The OpportunityPermission PATCH gap (BUG-003) is unchanged here. PATCH is still 403'd; the permission-pattern PR resolves it.
  • No empty XxxWriteSerializer stubs for read-only resources. The convention is "you have both classes when you have both behaviors," not "every resource pre-stubs both."

Test plan

Shape-only change; no behavior changes intended.

  • ruff check ctj_api/ backend/ clean
  • ruff format --check ctj_api/ backend/ clean
  • mypy ctj_api/ backend/ clean (run with django-stubs)
  • All 9 existing backend tests pass without modification

The integration tests don't import serializer classes by name (they exercise the API surface), so the renames are transparent to the test suite.

Follow-ups (not blocking)

The remaining pattern-PR axes:

  • Test shape — the 21 tests live in one big APIBasicTests class; lock in per-resource organization.
  • URL routing convention — camelCase /api/communityOfPractice/ is inconsistent with the snake_case Pythonic shape; needs a decision (and a frontend impact check).
  • Permission class shapeOpportunityPermission branches both methods; UserDetailPermission only has_object_permission; lock in a convention.
  • Error envelopeapi_not_found defines a JSON error shape that nothing else follows; either lift it to a project-wide convention or document why the catch-all is special.

The bug-fix and cleanup PRs from #739 are still queued.

Nickatak and others added 30 commits April 26, 2026 16:16
Removes 10 docs that were either committed-as-drafts and never filled
in (containing literal [INSERT-...] placeholders or marked
**_DRAFT NOT YET FILLED OUT_**), or self-flagged # OUTDATED while
describing a previous project layout that no longer exists.

Also cleans up direct references to the deleted files:
- 10 corresponding nav entries removed from mkdocs/mkdocs.yml
- 3 broken role links removed from joining-the-team/intro.md
- 2 broken bullets removed from resources.md (renumbered)
- 2 broken Additional Resources entries removed from
  developer/installation.md
- 1 sentence pointing at the deleted Frontend Architecture guide
  removed from developer/backend.md

Verified with `mkdocs build --strict`. Remaining info-level link
warnings are pre-existing issues unrelated to this PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aligns the dev docs with the engineering-assessment direction: Next.js +
existing Django backend, PeopleDepot integration deferred to Stage 2,
Cognito ID-token JWT, CSS Modules, three-container ECS task with
Postgres-in-container.

Flattens mkdocs/docs/ to docs/ at repo root and drops mkdocs entirely:
no more mkdocs.yml, mkdocs-build workflow, docker-compose.docs.yml,
or the three mkdocs-*.md docs about the doc system itself. GitHub
renders the markdown directly.

Folds in audit corrections from doc-by-doc review: Stage 1 / Stage 2
phasing, em-dash sweep, WHY-notes pattern, /demo route dropped,
development-culture.md merged into CONTRIBUTING.md, the-team.md and
index.md dropped (content promoted to README.md).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the legacy Vite + React 18 frontend toolchain with a
Next.js 16 App Router scaffold, and split the monolithic stage
image into separate Next.js and Django containers.

Frontend toolchain:
- Next 16 + React 19 + TypeScript 5
- Tailwind 3.4 + SCSS preserved from legacy (PR2 will fold into
  CSS Modules)
- Vitest + jsdom + Testing Library replacing Jest
- SVGR for both Webpack and Turbopack with svgo: false to
  preserve clip-path id references
- vite-plugin-svgr in vitest config for test-time SVG imports

Infra:
- dev/next.dockerfile replaces dev/vite.Dockerfile (port 3000)
- stage/next.dockerfile + stage/django.dockerfile replace the
  combined stage/Dockerfile that baked the Vite build into Django
- docker-compose.stage.yml runs three services (pgdb on 5433,
  django on 8000, next on 3000) with BACKEND_INTERNAL_URL=
  http://django:8000 driving Next's /api/* and /admin/* rewrites

Cleanup:
- Drop .github/workflows/build-deploy-stage.yml and
  aws/task-definition.json - the deploy pipeline now lives in
  hackforla/incubator
- Anchor the legacy 'data' and 'media' gitignore rules to the
  repo root so they don't shadow per-feature data/ directories
  the port introduces
- README + docs/developer/design-system.md updated to reflect
  the Next.js direction

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move all of the React/TS source, tests, and assets from the
Vite + react-router-dom layout into the Next.js 16 App Router,
restructured around features/, shared/, and route shims under
src/app/.

Routing:
- Route groups (with-nav)/ and (auth)/ replace the legacy
  HomeLayout / DefaultNavLayout / AuthNav split. Group layouts
  hold the navbar wiring; page.tsx files are thin shims that
  delegate to the matching feature.
- /privacypolicy renamed to /privacy-policy (hyphenated).
- /demo and /demo-tailwind dropped.

Features:
- features/landing/, credits/, qualifier/, session/,
  privacy-policy/, not-found/ each own their components, data,
  and a FEATURE_MAP.md describing the entry point and layer
  dependencies.
- shared/components/ deduplicates the legacy components/ and
  tw-components/ split (Tailwind variants kept where both
  existed).
- shared/icons/ and shared/images/ are SVGR-imported; PNGs in
  public/ for unprocessed cases.

Library swaps for React 19 compatibility:
- react-popper -> @floating-ui/react (Dropdown)
- react-transition-group -> mount/unmount (TransitionWrapper);
  PR2 will reintroduce CSS-driven fade transitions
- TextField generic over FieldValues (was pinned to
  { password: string } in legacy)

Tests:
- 12 ported to Vitest + Testing Library
- TextField.test.tsx and LandingPage.test.tsx skipped with
  inline notes (component shape changed; selectors stale)

Hydration / SSR fixes from smoke testing:
- CookieBanner reads cookies in useEffect after mount-then-
  render to avoid a server/client tree mismatch
- QualifiersContext hydrates localStorage in useEffect
- Logo SVG sizing uses h-{N} w-auto to override SVGR's baked-in
  width/height attributes (same fix applied to the credits
  high-five illustration)
- LandingPageCop modal styles converted from a dead
  _LandingPageCop.scss tree to inline Tailwind utilities

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove tailwindcss, sass, autoprefixer, postcss-import, postcss,
eslint-plugin-tailwindcss, and tailwind-merge from package.json.
Delete tailwind.config.ts and postcss.config.mjs (Next's defaults
handle the remaining work).

Replace src/app/globals.scss with a pure-CSS globals.css containing:
- Tailwind-preflight-equivalent reset (box-sizing, zeroed margins,
  neutralized buttons/anchors, block-level media)
- Design tokens (colors, spacing, radius, weights, z-index) as CSS
  custom properties on :root
- Document baseline using --font-roboto from next/font

Drop src/shared/styles/ (the legacy SCSS token + utility-class
partials); their values now live as :root custom properties.

Simplify shared/lib/utils.ts: cn() drops twMerge and is just clsx;
combineClasses aliases cn (replaces the SCSS-era duplicate).

Components and features still reference Tailwind class names; they
will go unstyled until the per-feature commits in this chain swap
each one for a co-located *.module.css. The build is green and
tests still pass at this commit, which is the runnable boundary
that matters for the PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Buttons, StandardCard, CircleCard, Typography, Dialog now ship a
co-located *.module.css and reference it via clsx-composed style
maps instead of Tailwind utility strings.

- Drop the legacy `dark:` variants from Buttons; the app has no
  dark-mode toggle wired up, so the classes were dead weight.
- Dialog's slide-in/out animations move from Tailwind keyframes
  in tailwind.config.ts to CSS @Keyframes inside Dialog.module.css.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TextField, Checkbox, and the Inputs/* family (Calendar, Chip,
Dropdown, ProtoInput) now ship co-located *.module.css.

ProtoInput.module.css exposes an `inputBase` class that Dropdown
composes from, mirroring the legacy SCSS @mixin pattern in pure
CSS Modules. Calendar's table grid (border styles, sticky
header, hover/select highlight) was the most involved port; the
36px cell size and 24px tick column carry over verbatim.

Tests adjusted: Calendar drag-select test now reads aria-checked
on the inner checkbox div instead of querying for a hashed
".calendar-cell" class. Checkbox labelHidden test skipped with a
note; the hiding is a pure CSS Modules class effect that jsdom
does not apply via getComputedStyle, so there is nothing
meaningful to assert in the JS layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HeaderNav, FooterNav, AuthNav, CookieBanner, and AccordionFaq
each ship a co-located *.module.css.

Tailwind responsive prefixes (`md:`, `lg:`, `max-md:`) become
explicit `@media (min-width)` / `@media (max-width)` blocks
using the legacy breakpoint pixel values (xs 480, sm 577,
md 769, lg 1025, xl 1201). Tailwind's `space-x-*` and
`space-y-*` (margin-on-children) are replaced with `gap` on the
flex parent, which is the modern equivalent.

Drop the legacy `mg:` typo on HeaderNav's login link (was
silently a no-op since `mg:` matches no breakpoint).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Notification, TransitionWrapper, ClickCarousel, ScrollCarousel,
and ChevronScroll all ship co-located *.module.css and drop the
last of the SCSS files in src/shared/components/.

TransitionWrapper had a dead _TransitionWrapper.scss with
fade-* classes that the React 19 mount/unmount rewrite no longer
references; that file is removed without a replacement module.

Notification close-button test asserts on the aria-hidden
attribute instead of a hashed module class — same rationale as
the Calendar drag-select test from the previous commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LandingPageIntro, LandingPageCop, and LandingPageCopCards now
each ship co-located *.module.css. Drops the last consumers of
the legacy SCSS utility classes (`flex-container`, `col-3`,
`paragraph-1`, `title-3`, etc.) for the landing route.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CreditsPage and Card move to co-located *.module.css. Tailwind
responsive breakpoint utilities (xs, sm, md, lg, xl) become
explicit @media (min-width) blocks at the same px values.

The CreditsPage.module.css media-query stack is unusually
chunky because the page resizes typography, padding, grid
columns, and the hero illustration at every breakpoint - the
density mirrors the original Tailwind class density per element.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QualifierConsole, Stepper, QualifierNav, QualifierPage1,
QualifierPage2, QualifierPageCalendar, RadioButtonForm,
ProgressIndicator, and ChipsSelection each ship a co-located
*.module.css.

Stepper's connector lines now flow through `position: "first" |
"last"` props on the Step component to control which sides hide
the connecting bar (replacing the legacy
`group-first:`/`group-last:` Tailwind variants which depended on
the parent `.group` class).

QualifierConsole's `<h1 className="sr-only">` becomes an
explicit visually-hidden module class; the CSS rules match
Tailwind's sr-only output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LoginForm and SignupForm both reference a shared
SessionForm.module.css since their layouts (heading, submit
button, alt-link footer) are identical. Signup adds a
nameGrid class for the side-by-side first/last name pair on
md+ screens.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PrivacyPolicyPage moves its inline `styleClass` map of Tailwind
strings into a co-located *.module.css. The hero illustration
keeps its lg: visibility gate via @media (min-width: 1025px).

NotFoundPage rebuilds the layout that the deleted
_NotFoundPage.scss used to provide, now in a small co-located
module. The component had been rendering unstyled since commit
1 of this chain.

This is the last per-feature commit in the Tailwind/SCSS to
CSS Modules sweep. No Tailwind class names or sass partials
remain in the tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Backend (Python 3.12 → 3.13, Django 5.1 → 6.0):
- Django 5.1.2 → 6.0.4
- djangorestframework 3.15.2 → 3.17.1
- daphne 4.1.2 → 4.2.1
- whitenoise 6.8.2 → 6.12.0
- psycopg2-binary 2.9.9 → psycopg[binary] 3.3.4
  (Driver swap. settings.py needs no change since
  django.db.backends.postgresql auto-detects psycopg3 when
  installed; envvars stay as-is.)
- Python base image 3.12-alpine → 3.13-alpine in both
  dev/django.dockerfile and stage/django.dockerfile

Infra:
- Postgres 16 → 18 in both compose files. Major bump means
  dev volumes need `docker compose down -v` on first pickup.
- Node 22-alpine → 24-alpine in dev/next.dockerfile and all
  three stages of stage/next.dockerfile (Node 24 LTS).

Frontend:
- next 16.2.4, react 19.2.4 → 19.2.5 (patch)
- typescript 5 → 6
- jsdom 25 → 29
- prettier 3.4 → 3.8
- globals 15 → 17
- @types/node 20 → 24 (tracking the runtime)
- eslint 9.17 → 9.39 (latest 9.x; ESLint 10 deferred —
  jsx-a11y and eslint-plugin-react have not yet shipped
  ESLint 10 peer ranges)
- eslint-plugin-react-hooks 5.1 → 7.1
- @vitejs/plugin-react 4 → 5
- vitest 2 → 3 (vitest 4 requires vite 8 + plugin-react 6
  which adds babel-plugin-react-compiler as a hard peer;
  not worth the extra dep at this point)
- vitest.config.ts → vitest.config.mts since plugin-react 5
  is ESM-only and the project does not have type:module set

Lint tooling (black, flake8, isort) intentionally left at
current versions — those are being replaced wholesale in the
next commit, so bumping them is wasted churn.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (frontend), CI workflow

Backend - swap black + flake8 + isort for ruff:
- pyproject.toml: replaces three dev deps with ruff under
  [tool.ruff]/[tool.ruff.lint] (selecting E/W/F/I/B/UP/DJ/SIM/C4),
  [tool.mypy] (gradual mode with django-stubs + drf-stubs plugins),
  [tool.django-stubs] (settings module pointer),
  [tool.bandit] (excludes migrations + tests).
- backend/.flake8: deleted; ruff config supersedes it.
- ruff format pass produced trivial reshape on manage.py and
  settings.py (line-length-driven splits). E501 in settings.py
  was a pre-existing line over 88 chars; split into a tuple
  string. DJ008 (Opportunity has no __str__) addressed by
  adding `f"{role.title} @ {project.name}"`. DJ001 on
  min_experience_required noqa'd with TODO comment - the fix
  drops null=True which generates a schema migration; deferred
  to a focused follow-up PR.

Frontend - extend the lint surface beyond ESLint:
- ESLint stays at 9.x; eslint-config-next 16's flat configs are
  loaded directly (no more FlatCompat shim). The eslint-plugin-
  tailwindcss reference from the pre-rewrite eslint.config.mjs
  was removed (tailwind was deleted in PR hackforla#736 but the lint
  config still referenced the plugin). The legacy `next lint`
  script is gone in Next 16; lint script now runs `eslint .`.
- eslint-plugin-react-hooks 5 -> 7 brings the React Compiler
  rule set; the three new rules (set-state-in-effect, refs,
  static-components) are downgraded to warn since the legacy
  components in the tree violate them in many places. TODO
  comment in the config; intent is to address in a follow-up
  React-Compiler-prep PR.
- stylelint added with stylelint-config-standard plus camelCase
  selector/keyframes patterns and ignoreProperties for `composes`
  (CSS Modules directive) and `clip` (sr-only pattern).
- knip added with fail-on-find for every category. Real findings
  in this PR: 6 unused component files deleted (AccordionFaq,
  ClickCarousel, ScrollCarousel, ChevronScroll, ChipsSelection,
  Chip), 3 unused exports (SearchButton, sampleCopData,
  combineClasses), 1 unused type re-export (AssetDatum from
  creditsIconData.ts), 4 unused devDependencies (@eslint/js,
  globals, @typescript-eslint/eslint-plugin, @typescript-eslint/
  parser, typescript-eslint - the latter three were transitive
  via eslint-config-next anyway).
- @svgr/webpack stays in package.json since next.config.ts
  references it as a string loader name; knip can't see that
  through configs, so it's the sole entry in
  ignoreDependencies.

Pre-commit:
- .pre-commit-config.yaml rewritten. Was stale: pinned to
  python 3.9, referenced paths that no longer exist, ran old
  versions of the deleted tools. Now: ruff-pre-commit (check
  + format passes), mirrors-prettier, local hooks for eslint
  and stylelint that delegate to npx so the project's pinned
  versions are used. Python pinned to 3.13, Node to 24.

CI:
- .github/workflows/lint.yml added. Two parallel jobs
  (backend / frontend) each running their respective full lint
  suites. Authoritative gate; PRs cannot merge while red.
- .github/workflows/jest-react-test.yml renamed to
  vitest-test.yml; Node bumped from 18 to 24.
- .github/workflows/linter.yml (Super-Linter) deleted -
  superseded.

Cleanup:
- dev/linter.dockerfile + dev/linter.env.example deleted -
  the dev linter container was the legacy enforcement path
  for the old toolchain; CI workflow + pre-commit replace it.

Docs (single source of truth alignment):
- docs/developer/eslint-guide.md renamed to
  frontend-lint-guide.md and rewritten to cover the four-
  layer frontend lint stack (ESLint + Stylelint + Knip +
  Prettier) plus tsc.
- docs/developer/backend.md: lint section updated;
  directory tree no longer lists .flake8.
- docs/developer/quickstart-guide.md: backend + frontend
  lint command sections updated.
- docs/developer/devops.md: linting section rewritten;
  references the new tools in a tool/surface table.
- docs/developer/installation.md: required Node 22 -> 24,
  Python 3.12 -> 3.13, pre-commit-Python-hooks list
  updated to ruff. Editor extension recommendations
  expanded (ESLint, Stylelint, Prettier).
- docs/developer/design-system.md, docs/resources.md,
  README.md: link updates from eslint-guide to
  frontend-lint-guide.
- CONTRIBUTING.md: typing-policy section added describing
  the gradual mypy posture - new code annotates
  signatures, existing code is annotation-welcome but not
  required, don't annotate just to silence the type
  checker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three issues surfaced when the new Lint workflow ran on a fresh CI
checkout:

1. tsc could not resolve `@/shared/icons/*.svg` and
   `@/shared/images/*.svg` imports. SVGR transforms .svg into React
   components at runtime but doesn't emit .d.ts files. Locally the
   missing declarations slipped past because `.next/` from prior
   builds had partial types in scope. Add `frontend/src/types/svg.d.ts`
   with the ambient module declaration.

2. tsc also failed on `next-env.d.ts`'s static import of
   `./.next/types/routes.d.ts`, which doesn't exist on a fresh
   checkout. Update the `lint:types` script to run `next typegen`
   first - the Next 16 CLI command that generates route types
   without doing a full build.

3. mypy crashed with "Error constructing plugin instance of
   NewSemanalDjangoPlugin" because the django-stubs plugin imports
   `backend.settings`, and settings.py reads required env vars via
   python-decouple at import time. Add dummy env vars to the mypy
   step in the lint workflow - they're never used for I/O, just
   needed so the import doesn't raise.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Branches: switch the existing "prefixes are optional" rule in
git-branch-structure.md to a required `<type>/<desc>` shape with
five types matching the categories CTJ work actually divides into:
feat, fix, chore, docs, refactor. Subsumes ci/build/style/test
under chore so contributors don't second-guess between near-
synonyms.

Commits: add a "Commit messages" section to CONTRIBUTING.md that
mirrors the same five types as a `<type>: <description>` title
shape. Imperative mood, sentence case, optional scope. Body
conventions kept as-is.

Both conventions apply forward only. Existing branches in flight
(docs-rewrite, next-rewrite, next-rewrite-css-modules,
next-rewrite-deps-bump) and existing commits are grandfathered;
renaming live PRs and rewriting history is more churn than it's
worth.

Enforcement is doc + PR review for now; tooling enforcement (a
branch-name CI check or a commit-msg hook) is overkill for a
two-person team.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Establishes the per-file-kind docstring templates the backend should
follow, modeled on BNC's docstring patterns. Heavy labeled-section
templates for models and views (where there's substantive policy to
document); lighter prose for serializers, config files, and scaffold.

Also adds the "don't annotate Django model fields" rule to the
existing typing section in CONTRIBUTING - django-stubs synthesizes
those types via mypy's plugin protocol, and manual annotations
duplicate the field-level config and can drift.

This is the first pattern-PR after the modernization chain
(hackforla#734-hackforla#738); subsequent commits in this PR apply the templates
across the backend.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the heavy labeled-section template from
docs/developer/backend-docstring-style.md to the seven models and
seven views in ctj_api.

Models get Summary / Business workflow / Current policy /
Lifecycle control / Visibility sections. Views get Summary / Flow /
URL / Methods / Auth / Errors sections. Sections are bulleted even
when single-item, for grep-friendliness across the codebase.

OpportunityViewSet's Errors section flags a real bug: PATCH always
403's because OpportunityPermission has no PATCH branch. Bug
deferred (pure-docs PR).

Also folds in a small spacing fix: Role had a stray blank line
between its FK and timestamp fields, removed for consistency with
the other models.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the lighter serializer template (one-liner + optional prose
+ Used by:) and the medium permission template (Policy: + per-method
Branches:).

Serializer docstrings flag two bugs (deferred out of this
docs-only PR):
- CustomUserSerializer.opportunities is a writable field that
  references a non-existent attribute on the model; reads would
  AttributeError on every call to /api/users/<uuid>/.
- SkillMatrixSerializer is defined but never imported (dead code).

Permission docstrings flag the OpportunityPermission PATCH gap
already noted on the view side. Method docstrings include
branch-by-branch detail with the AnonymousUser getattr defense
pattern explained for less-experienced readers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the light template to admin.py, apps.py, urls.py (both
ctj_api and backend), tests/, asgi.py, wsgi.py, settings.py, and
backend/views.py.

Replaces Django's auto-generated boilerplate module docstrings
(asgi.py, wsgi.py, settings.py) with CTJ-specific text that
explains what each file actually does in this project (e.g. asgi.py
is the production entry point used by daphne; wsgi.py is currently
unused).

Test methods get behavioral docstrings (what's asserted) rather
than the previous "Test that X is Y'd" framing - matches BNC's
test-naming style and makes the docstring a contract rather than a
label.

Bugs flagged in docstrings (deferred out of this docs-only PR):
- backend/views.py: index.html template missing in production;
  catchall = ... evaluated at import time so DEBUG toggles don't
  swap without restart.
- backend/urls.py: SPA catchall is a pre-rewrite remnant.
- settings.py: frontend_dist references in TEMPLATES.DIRS and
  STATICFILES_DIRS; immutable_file_test regex tuned for Vite-era
  hashes; SECURE_HSTS_SECONDS=None when disabled (should be 0).

A commented-out test in tests/test_api_endpoints.py is preserved
with a comment explaining it's blocked on the
CustomUserSerializer.opportunities bug from the prior commit; once
that bug is fixed, the test serves as a regression guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Establishes the JSDoc convention for the frontend: heavy on the
module level, light at the per-export level. Inverts the backend's
shape because frontend files are usually the unit of design (a
component, a context, a utility cluster), so the file-header is
where context goes; per-export comments redocument what the
TypeScript type signature already captures.

Models the convention on BNC's frontend with per-kind variants for
app routes, feature components, shared UI primitives, utilities,
contexts, static data, type declarations, and tests.

CONTRIBUTING gets a pointer section to the new guide alongside the
existing backend pointer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the JSDoc convention to the small, foundational files
that other frontend code builds on:

- src/types/svg.d.ts (ambient SVG module declaration)
- src/shared/lib/utils.ts (cn / onKey / range utilities)
- src/shared/data/copData.ts (CoP reference data)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the JSDoc convention to the Next.js route handlers and
layouts under src/app/. Each page / layout file gets a module
header describing the route path, what it renders, and the
layout-group context (`(with-nav)` vs `(auth)`).

Pages are intentionally thin (5-15 lines that defer to feature
components); the docstrings carry the routing context that the
imports alone don't make obvious.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the JSDoc convention to the shared UI primitives under
src/shared/components/. Each component file gets a module header
explaining what the primitive provides, when to reach for it vs
alternatives, and any non-obvious behavior (composition with
other primitives, accessibility decisions, hydration patterns).

Bugs flagged in docstrings (deferred out of this docs-only PR):
- Dialog.tsx: no focus trap, no Escape close, no return-focus.
- nav/HeaderNav.tsx: hamburger menu button is inert (mobile menu
  unimplemented).
- nav/FooterNav.tsx: Sitemap link points to "#".
- TransitionWrapper.tsx: missing CSS fade animation (legacy
  react-transition-group behavior was lost in the React 19
  migration).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the JSDoc convention to feature components under
src/features/ (credits, landing, not-found, privacy-policy,
qualifier, session). Each file gets a module header describing
the feature's purpose, place in the user flow, and any
cross-cutting concerns (state from context, integrations with
shared UI primitives).

Bugs flagged in docstrings (deferred out of this docs-only PR):
- PrivacyPolicyPage.tsx: hard-coded "localhost:8000" domainName.
- LoginForm/SignupForm: console.log placeholder onSubmit (auth
  rebuild deferred per planned_auth.md).
- QualifierPageCalendar: console.log placeholder onChange and
  final button links to "/".
- QualifierPage1: loose validation (allows progressing without
  CoP selection).
- RadioButtonForm: 3-bucket frontend scale vs 1-5 backend scale
  (schema mapping unimplemented).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the JSDoc convention to vitest test files under
frontend/tests/. Module headers state which component or page
the test file covers; per-test docstrings switch from "renders
X" / "does Y" framings to BNC-style behavioral statements that
say what's actually asserted.

Two skipped tests get fuller explanations:
- TextField.test.tsx: skipped pending rewrite for the new
  RHF-generic API.
- LandingPage.test.tsx: skipped pending rewrite against the
  ported component graph.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces `UserDetail(RetrieveAPIView)` and the four
`ReadOnlyModelViewSet`s (`CommunityOfPracticeViewSet`,
`RoleViewSet`, `SkillViewSet`, `ProjectViewSet`) with
function-based views decorated with `@api_view([...])`. Routes
in `ctj_api.urls` move from `DefaultRouter` auto-registration
to explicit `path()` entries.

The convention going forward: full-CRUD resources (>=4 actions)
stay as `ModelViewSet` and register on `DefaultRouter`; anything
narrower is an FBV with explicit routing. Reading `views.py`
cold now answers "what methods does this view expose?" and
"what URL serves it?" without leaving the file.

`OpportunityViewSet` stays as `ModelViewSet` (full CRUD).
`healthcheck` and `api_not_found` remain plain Django views.

UserDetail's `has_object_permission` check, previously
auto-triggered by the generic view base class, is now called
explicitly via `request.parser_context["view"]
.check_object_permissions(...)` since FBVs don't have an
APIView class to dispatch through.

No behavior changes; existing 21 backend tests pass without
modification.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `View shape` section to `docs/developer/backend.md`
between `API endpoints` and `Auth`. Captures the rule that
the prior commit applies in code: full-CRUD resources use
`ModelViewSet`, anything narrower uses an FBV with explicit
routing, non-resource endpoints use plain Django views.

Includes the `check_object_permissions` gotcha for FBVs with
object-level permission classes, with a pointer to
`user_detail` as the canonical example.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nickatak and others added 4 commits May 3, 2026 01:25
The `per ADR-0010` annotation pointed at a no-commit decision
record (scratch/decisions/, gitignored) that never made it into
upstream-visible documentation under that number. Drops the
dangling reference; the surrounding View shape convention in the
module docstring already covers the rule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames the existing `XxxSerializer` classes that are only consumed
read-only to `XxxReadSerializer`. Splits `OpportunitySerializer`
(the only one with a write path) into `OpportunityReadSerializer`
and `OpportunityWriteSerializer`. Updates `OpportunityViewSet` to
dispatch via `get_serializer_class()` (Read for list/retrieve,
Write for create/update/destroy).

The convention going forward: each resource has separate Read and
Write serializers when both shapes are needed; read-only resources
have only a Read class. Auto-managed fields (id, created_at,
updated_at) and request-stamped fields (Opportunity.created_by) are
absent from `XxxWriteSerializer.Meta.fields` rather than included
with `read_only=True` -- the absence is the contract.

`OpportunityViewSet.perform_create` continues to stamp `created_by`
from `request.user`; the Write serializer just declares the field
as absent from its writable list.

`SkillMatrixSerializer` (dead code, BUG-002) is left as-is, with the
docstring updated to note that it should conform to the new
convention if/when it's wired up; the cleanup PR drops it. The
broken `opportunities` field on `CustomUserReadSerializer` (BUG-001)
is similarly left as-is for the bug-fix PR.

No behavior changes; existing 21 backend tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `Serializer shape` section to `docs/developer/backend.md`
between `View shape` and `Auth`, capturing the rule that the
prior commit applies in code: each resource gets separate Read
and Write serializer classes when both shapes are needed,
auto-managed and request-stamped fields are absent from Write
classes rather than flagged read-only.

Updates the serializer template in `backend-docstring-style.md`
to show separate `XxxReadSerializer` and `XxxWriteSerializer`
docstrings, with a callout that absent-from-Meta.fields is the
contract for "client cannot supply this on write."

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `per ADR-0011's convention` annotation pointed at a no-commit
decision record (scratch/decisions/, gitignored). The Read/Write
split convention is documented in the module docstring at the top
of `serializers.py`; dangling reference removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Nickatak Nickatak force-pushed the refactor/serializers-read-write branch from 1d1310c to a1bdaef Compare May 3, 2026 08:28
@Nickatak Nickatak marked this pull request as ready for review May 7, 2026 21:22
Copy link
Copy Markdown
Member

@RSkuma RSkuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decision approved, discussed at 5/7/26 meeting

@RSkuma RSkuma merged commit 935fc41 into hackforla:develop May 7, 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