Skip to content

Commit 79db344

Browse files
authored
Merge pull request #739 from Nickatak/docs/add-docstrings
docs: Add docstring conventions and apply across backend + frontend
2 parents f189749 + 24e08c6 commit 79db344

77 files changed

Lines changed: 2085 additions & 171 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CONTRIBUTING.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,18 @@ The Python codebase has `mypy` running in **gradual mode** — `disallow_untyped
3737
- **Existing code you touch**: if you're already editing a function for another reason, adding annotations is welcome but not required.
3838
- **Don't annotate just to silence mypy.** If a type is genuinely ambiguous (`Any` is fine, `# type: ignore[code]` with the specific code is fine when you know why), say so explicitly rather than papering over it.
3939
- **Django models**: `django-stubs` and `djangorestframework-stubs` are installed; mypy can check ORM call sites without per-call annotation.
40+
- **Don't annotate Django model fields.** `name = models.CharField(...)` should stay un-annotated. `django-stubs` reads the field declaration (including `null=True`, `choices=...`, etc.) and synthesizes the correct instance-attribute type via mypy's plugin protocol. Manual annotations duplicate that work and drift from the field-level config (e.g. flipping `null=True` without updating an `Optional[str]` annotation). Annotate methods, helper functions, and custom managers as normal.
4041

4142
The goal is that contributors can land work without fighting the type checker, while the typed surface grows incrementally where it pays for itself.
4243

44+
### Docstrings (backend)
45+
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.
47+
48+
### Docstrings (frontend)
49+
50+
Frontend code uses JSDoc with a different shape from the backend: **heavy on the module level, light at the per-export level**. The file header is the primary place for context (what the file does, why, design decisions); per-export comments are one-liners unless behavior is non-obvious. See [docs/developer/frontend-docstring-style.md](docs/developer/frontend-docstring-style.md) for the per-kind templates and examples.
51+
4352
If an issue takes much longer than its size suggested, post an update on the issue with an honest read on whether you can finish; releasing it back to the backlog is fine.
4453

4554
### Frontend vs backend issues

backend/backend/asgi.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
1-
"""
2-
ASGI config for backend project.
1+
"""ASGI config for the backend project (the entry point used by `daphne`).
2+
3+
In CTJ this module is the production entry point - `entrypoint.sh`
4+
runs `daphne -b 0.0.0.0 -p 8000 backend.asgi:application` to start
5+
the server. The dev server (`python manage.py runserver`) also
6+
routes through ASGI here because `daphne` is first in
7+
`INSTALLED_APPS`, which makes Django's `runserver` use daphne's
8+
ASGI handler instead of the default WSGI one.
39
4-
It exposes the ASGI callable as a module-level variable named ``application``.
10+
Exposes the ASGI callable as a module-level variable named
11+
`application`.
512
6-
For more information on this file, see
7-
https://docs.djangoproject.com/en/5.0/howto/deployment/asgi/
13+
For more on ASGI deployment:
14+
https://docs.djangoproject.com/en/6.0/howto/deployment/asgi/
815
"""
916

1017
import os

backend/backend/settings.py

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,34 @@
1-
"""
2-
Django settings for backend project.
3-
4-
Generated by 'django-admin startproject' using Django 5.0.6.
5-
6-
For more information on this file, see
7-
https://docs.djangoproject.com/en/5.0/topics/settings/
8-
9-
For the full list of settings and their values, see
10-
https://docs.djangoproject.com/en/5.0/ref/settings/
1+
"""Django settings for the CTJ backend project.
2+
3+
Settings are read from environment variables via `python-decouple`'s
4+
`config()` function. Required vars (no default - server fails to
5+
start if missing): `SECRET_KEY`, `SQL_ENGINE`, `SQL_DATABASE`,
6+
`SQL_USER`, `SQL_PASSWORD`, `SQL_HOST`, `SQL_PORT`. Optional vars
7+
with defaults are inline below. Per
8+
`project_envvars_owned_by_devops`, CTJ engineering declares the
9+
vars; devops populates the values per environment (incubator
10+
repo).
11+
12+
Notable choices:
13+
- `AUTH_USER_MODEL = "ctj_api.CustomUser"` (subclasses
14+
`AbstractUser` rather than extending the default User).
15+
- `daphne` is first in `INSTALLED_APPS` so Django's `runserver` and
16+
the production server both route through ASGI - see
17+
`backend.asgi`.
18+
- `whitenoise` serves static files; the `immutable_file_test`
19+
function below identifies hashed asset filenames so they can be
20+
served with long-cache headers.
21+
- HSTS controls are env-driven; defaults are off so dev/stage stay
22+
unaffected. Production is expected to set `HSTS_ENABLED=True` and
23+
the three `*_SECURE`/`SECURE_*` flags below.
24+
25+
A few config remnants here are dead post-rewrite (template dir
26+
references to a no-longer-existent `frontend_dist`, plus a
27+
whitenoise storage workaround). See `scratch/bugs.md` (BUG-007 and
28+
BUG-010) for the full bug list.
29+
30+
For Django settings reference:
31+
https://docs.djangoproject.com/en/6.0/ref/settings/
1132
"""
1233

1334
import re
@@ -167,7 +188,19 @@
167188
# Whitenoise settings
168189
# http://whitenoise.evans.io/en/stable/django.html#WHITENOISE_IMMUTABLE_FILE_TEST
169190
def immutable_file_test(path, url):
170-
# Match vite (rollup)-generated hashes, à la, `some_file-CSliV9zW.js`
191+
"""Identify static files with hashed names for whitenoise's immutable-cache headers.
192+
193+
Matches filenames where the segment immediately before the
194+
extension looks like a content hash (8-12 alphanumeric chars,
195+
separator preceded by `.` or `-`). Examples that match:
196+
`app-CSliV9zW.js`, `chunk.abc12345.css`.
197+
198+
Note: the original regex was tuned for Vite/rollup-style hashes
199+
(the dropped pre-rewrite frontend). Whether it correctly matches
200+
Next.js/webpack hashes depends on the chunk-naming output of the
201+
current build - and post-rewrite the backend doesn't serve the
202+
frontend's static files anyway. See BUG-008 in scratch/bugs.md.
203+
"""
171204
return re.match(r"^.+[.-][0-9a-zA-Z_-]{8,12}\..+$", url)
172205

173206

backend/backend/urls.py

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
1-
"""
2-
URL configuration for backend project.
1+
"""Top-level URL routing for the backend Django project.
2+
3+
Three top-level mounts:
4+
- `/admin/` for the Django admin site.
5+
- `/api/` delegates to `ctj_api.urls` for all CTJ API routes.
6+
- Everything else falls through to the SPA catchall in
7+
`backend.views`, which serves an HTML template.
8+
9+
Order matters: the SPA catchall is last so the explicit `/admin/`
10+
and `/api/` mounts win first.
311
4-
The `urlpatterns` list routes URLs to views. For more information please see:
5-
https://docs.djangoproject.com/en/5.0/topics/http/urls/
6-
Examples:
7-
Function views
8-
1. Add an import: from my_app import views
9-
2. Add a URL to urlpatterns: path('', views.home, name='home')
10-
Class-based views
11-
1. Add an import: from other_app.views import Home
12-
2. Add a URL to urlpatterns: path('', Home.as_view(), name='home')
13-
Including another URLconf
14-
1. Import the include() function: from django.urls import include, path
15-
2. Add a URL to urlpatterns: path('blog/', include('blog.urls'))
12+
Architectural note: the SPA catchall is a remnant from the
13+
pre-rewrite monolithic setup, when Django served both the API and
14+
the rendered SPA shell from one process. The post-rewrite
15+
architecture runs the frontend as a separate Next.js container that
16+
handles all non-API traffic; anyone hitting the backend service
17+
directly at a non-`/admin/`, non-`/api/` path is probably misrouted.
18+
The catchall is preserved for now but is a candidate for removal -
19+
deferred out of this docs-only PR.
1620
"""
1721

1822
from django.contrib import admin

backend/backend/views.py

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,32 @@
1-
from django.conf import settings
2-
from django.views.generic import TemplateView
3-
4-
catchall_dev = TemplateView.as_view(template_name="dev-mode.html")
5-
6-
catchall_prod = TemplateView.as_view(template_name="index.html")
7-
8-
catchall = catchall_dev if settings.DEBUG else catchall_prod
1+
"""Top-level Django views for the backend project.
2+
3+
Currently only the SPA catchall lives here, used by `backend.urls`'s
4+
final catch-all `re_path` to serve an HTML template for non-API
5+
requests. See the architectural note in `backend.urls` - this entire
6+
file is a candidate for removal post-rewrite.
7+
8+
Two flags worth knowing about:
9+
10+
1. The branch between `catchall_dev` and `catchall_prod` is evaluated
11+
at module-import time (the `if settings.DEBUG` runs once when this
12+
module first loads). Toggling `DEBUG` at runtime will NOT swap the
13+
template; the server must be restarted.
14+
15+
2. `index.html` (the production template) does not exist in
16+
`backend/templates/backend/`. In `DEBUG=False`, `catchall_prod`
17+
would resolve to that missing template and raise
18+
`TemplateDoesNotExist` on any matching request. The dev template
19+
(`dev-mode.html`) does exist. This is a real bug, not just a docs
20+
note - probably wants the file removed entirely once the SPA
21+
catchall is dropped, or a real `index.html` placeholder added.
22+
Deferred out of this docs-only PR.
23+
"""
24+
25+
from django.conf import settings
26+
from django.views.generic import TemplateView
27+
28+
catchall_dev = TemplateView.as_view(template_name="dev-mode.html")
29+
30+
catchall_prod = TemplateView.as_view(template_name="index.html")
31+
32+
catchall = catchall_dev if settings.DEBUG else catchall_prod

backend/backend/wsgi.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
1-
"""
2-
WSGI config for backend project.
1+
"""WSGI config for the backend project (currently unused).
2+
3+
CTJ runs `daphne` (an ASGI server) in dev, stage, and prod, so
4+
this WSGI entry point is unreferenced today. Django's
5+
`startproject` generates this file by default; it's preserved
6+
here in case CTJ ever switches to a WSGI-only server (gunicorn,
7+
uwsgi). If the ASGI choice is permanent, this file can be
8+
removed; deferred out of this docs-only PR.
39
4-
It exposes the WSGI callable as a module-level variable named ``application``.
10+
Exposes the WSGI callable as a module-level variable named
11+
`application`.
512
6-
For more information on this file, see
7-
https://docs.djangoproject.com/en/5.0/howto/deployment/wsgi/
13+
For more on WSGI deployment:
14+
https://docs.djangoproject.com/en/6.0/howto/deployment/wsgi/
815
"""
916

1017
import os

backend/ctj_api/admin.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
"""Django admin registrations for CTJ's domain models.
2+
3+
All seven models are registered with the default `ModelAdmin` (no
4+
customization). Effect: admins see every field on every row, with
5+
no list filters, search fields, or read-only protections. This is
6+
enough for Stage 1 curation of the admin-managed reference tables
7+
(`Skill`, `Role`, `Project`, `CommunityOfPractice`) plus emergency
8+
edits to user records and opportunities.
9+
10+
If a model's admin needs filters, list display, or search later,
11+
register it with a dedicated `ModelAdmin` subclass instead of the
12+
default.
13+
14+
`SkillMatrix` is registered here even though its model docstring
15+
classifies it as `internal` (no direct API endpoint). Admin
16+
exposure lets staff view and edit the underlying JSON blob
17+
directly, which is useful for debugging matching-algorithm inputs.
18+
"""
19+
120
from django.contrib import admin
221

322
from .models import (
@@ -10,7 +29,6 @@
1029
SkillMatrix,
1130
)
1231

13-
# Register your models here.
1432
admin.site.register(CommunityOfPractice)
1533
admin.site.register(Role)
1634
admin.site.register(Skill)

backend/ctj_api/apps.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,14 @@
22

33

44
class CtjApiConfig(AppConfig):
5+
"""Django app config for `ctj_api`, the project's only domain app.
6+
7+
`default_auto_field` is set to `BigAutoField` to silence Django's
8+
startup warning about implicit auto field selection, but in
9+
practice it's unused: every CTJ model declares its own
10+
`UUIDField` primary key, so no model relies on this default.
11+
Carried for parity with Django's scaffold; harmless to leave.
12+
"""
13+
514
default_auto_field = "django.db.models.BigAutoField"
615
name = "ctj_api"

0 commit comments

Comments
 (0)