All checks were successful
Deploy to K8s / deploy (push) Successful in 8s
Pull 350+ lines of inline per-row computation out of adults_view, juniors_view, and payments into three pure builder functions with no Flask globals or IO dependencies. Route handlers now contain only cache/IO calls and a single render_template. No behaviour change — all 27 tests pass. Also moves get_month_labels, group_payments_by_person, and adapt_junior_members out of app.py. Prep for /api/* shadow endpoints (M5 Go parity). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
186 lines
7.6 KiB
Markdown
186 lines
7.6 KiB
Markdown
# Python view-model cleanup (M5 prep — Python side only)
|
||
|
||
Scoped-down precursor to M5 of the Go rewrite. See:
|
||
- [2026-05-03-2349-go-backend-rewrite.md](2026-05-03-2349-go-backend-rewrite.md)
|
||
- [2026-05-03-2349-go-backend-rewrite-progress.md](2026-05-03-2349-go-backend-rewrite-progress.md)
|
||
|
||
## Context
|
||
|
||
The Python app is still production. Before adding `/api/X` shadow
|
||
routes, JSON DTOs, parity tooling, and Go handlers, get the Python
|
||
side into a clean shape. Today the `/adults` and `/juniors` routes
|
||
each carry 150–200 lines of inline view-model construction inside
|
||
the Flask handler:
|
||
|
||
- [adults_view](app.py#L179) — 167 LOC, computes per-row
|
||
status/cell_text/balance/credits/debts inline.
|
||
- [juniors_view](app.py#L347) — 205 LOC, same plus `"?"` sentinel
|
||
branching and `:NJ,MA` breakdown.
|
||
- [payments](app.py#L553) — 35 LOC, lighter but still mixes IO and
|
||
grouping.
|
||
|
||
Pulling that into pure builder functions:
|
||
- shrinks `app.py` and makes each route handler do only what a route
|
||
handler should (IO + cache + step timing + render);
|
||
- gives us **already-tested** pure functions that future M5 work can
|
||
call from a `/api/X` shadow endpoint with one line of `jsonify(...)`;
|
||
- has zero behavioural change (existing tests in
|
||
[test_app.py](tests/test_app.py) act as the regression guard).
|
||
|
||
This is a Python-only change. No Go work, no JSON contract, no shadow
|
||
routes — those come after.
|
||
|
||
## Approach
|
||
|
||
Three pure builder functions in a new `scripts/views.py` module. Each
|
||
takes already-loaded, deserialized inputs (no Flask globals, no IO,
|
||
no cache calls) and returns the exact dict that today's route passes
|
||
to `render_template`.
|
||
|
||
```python
|
||
def build_adults_view_model(
|
||
members, sorted_months, transactions, exceptions, current_month,
|
||
*, attendance_url, payments_url, bank_account,
|
||
) -> dict: ...
|
||
|
||
def build_juniors_view_model(
|
||
junior_members, sorted_months, transactions, exceptions, current_month,
|
||
*, attendance_url, payments_url, bank_account,
|
||
) -> dict: ...
|
||
|
||
def build_payments_view_model(
|
||
transactions, member_names,
|
||
*, attendance_url, payments_url,
|
||
) -> dict: ...
|
||
```
|
||
|
||
The route handlers shrink to: load data (with `get_cached_data` and
|
||
`record_step` calls staying in `app.py`), call the builder, render.
|
||
|
||
```python
|
||
@app.route("/adults")
|
||
def adults_view():
|
||
members, sorted_months = ... # cached loads + record_step calls
|
||
transactions = ...
|
||
exceptions = ...
|
||
result_meta = ...
|
||
record_step("process_data")
|
||
vm = build_adults_view_model(
|
||
members, sorted_months, transactions, exceptions,
|
||
datetime.now().strftime("%Y-%m"),
|
||
attendance_url=..., payments_url=..., bank_account=BANK_ACCOUNT,
|
||
)
|
||
return render_template("adults.html", **vm)
|
||
```
|
||
|
||
## Decisions
|
||
|
||
1. **Pure builders, no Flask state.** No `record_step`, no `g.*`, no
|
||
`get_cached_data` inside builders. They take plain args, return
|
||
plain dicts. This is what makes them trivially unit-testable and,
|
||
later, trivially reusable from `/api/X`.
|
||
2. **Preserve byte-equal behaviour.** The dicts returned must match
|
||
today's `render_template(...)` kwargs key-for-key, value-for-value
|
||
— including the existing `json.dumps(member_data)` /
|
||
`json.dumps(month_labels_json)` / `json.dumps(raw_payments_json)`
|
||
wrappers. Those wrappers exist for inline JS in templates; the
|
||
refactor doesn't touch them. (When `/api/X` lands later, that route
|
||
will produce a sibling dict with the wrappers stripped, but that's
|
||
future work.)
|
||
3. **`reconcile()` stays inside the route, not the builder.** It
|
||
crosses the IO/cache boundary in spirit (its inputs come from
|
||
cache); but it's also pure-domain and called by both adults and
|
||
juniors. Keep the call site in the route so `record_step("reconcile")`
|
||
timing isn't lost. Builder takes `result` as an argument.
|
||
4. **Shared helpers stay in `app.py`** for now —
|
||
[get_month_labels](app.py#L41) and
|
||
[group_payments_by_person](app.py#L60) are already module-level
|
||
pure functions, used by routes and now by builders. Either leave
|
||
them in `app.py` and import into `scripts/views.py`, or move them
|
||
into `scripts/views.py`. **Choose: move into `scripts/views.py`**
|
||
— they're view-model concerns, not Flask concerns, and `app.py`
|
||
should keep shrinking.
|
||
5. **No new test file needed.** Existing
|
||
[tests/test_app.py](tests/test_app.py) tests
|
||
`test_adults_route`, `test_juniors_route`, `test_payments_route`
|
||
exercise the rendered HTML end-to-end. If they pass after the
|
||
refactor, behaviour is preserved. Adding builder-level unit tests
|
||
is a *nice-to-have* but not required for this iteration.
|
||
|
||
## Tasks
|
||
|
||
### 1. Create `scripts/views.py` with the three builders + shared helpers
|
||
|
||
- Move `get_month_labels` and `group_payments_by_person` from
|
||
[app.py:41–77](app.py#L41-L77) into `scripts/views.py`. Update the
|
||
import in `app.py`.
|
||
- Implement `build_adults_view_model` by extracting
|
||
[app.py:200–344](app.py#L200-L344) (everything between `result =
|
||
reconcile(...)` and `return render_template(...)`). Take `result`
|
||
as a parameter; emit the same dict that's currently passed as
|
||
`**kwargs` to `render_template`.
|
||
- Implement `build_juniors_view_model` by extracting
|
||
[app.py:370–550](app.py#L370-L550). Same shape — including the
|
||
`adapted_members` adapter loop, the junior `?`-sentinel branches,
|
||
and the `:NJ,MA` breakdown.
|
||
- Implement `build_payments_view_model` by extracting
|
||
[app.py:570–586](app.py#L570-L586) (the `group_payments_by_person`
|
||
call + `Unmatched / Unknown` bucket + sort).
|
||
|
||
### 2. Slim down the route handlers in `app.py`
|
||
|
||
Each handler keeps:
|
||
- `attendance_url`/`payments_url` URL building
|
||
- `get_cached_data` calls (with `record_step` between)
|
||
- `reconcile(...)` call (adults/juniors) with `record_step("reconcile")`
|
||
- `record_step("process_data")` after the builder call
|
||
- `return render_template("X.html", **view_model)`
|
||
|
||
Each handler **drops** all the per-row computation, totals
|
||
formatting, credits/debts sorting, `raw_payments_by_person` building,
|
||
and `import json` lines.
|
||
|
||
Target post-refactor LOC for each route handler: ~25–30 lines
|
||
(currently 167 / 205 / 35).
|
||
|
||
### 3. Run the existing test suite + manual smoke
|
||
|
||
```
|
||
make test # all tests green
|
||
make web-py # browse /adults /juniors /payments visually
|
||
```
|
||
|
||
## Critical files
|
||
|
||
- [app.py](app.py) — routes shrink dramatically; `get_month_labels`
|
||
and `group_payments_by_person` get moved out.
|
||
- New: `scripts/views.py` — three builders + the two helpers.
|
||
- [tests/test_app.py](tests/test_app.py) — unchanged; serves as the
|
||
regression guard.
|
||
|
||
## Verification
|
||
|
||
1. `make test` — all four `TestWebApp` tests pass unchanged.
|
||
2. `make web-py` and visit `/adults`, `/juniors`, `/payments` — each
|
||
renders identically to before (same table contents, same totals,
|
||
same credits/debts, same `?` rendering on juniors).
|
||
3. `git diff app.py` shows substantial deletions (route bodies
|
||
shrink) and only thin glue calling the new builders.
|
||
4. Optional sanity check: temporarily add `print(repr(view_model))`
|
||
in the route before `render_template` on `main` and on the branch,
|
||
diff for one fixture run — should be byte-identical dicts.
|
||
|
||
## Out of scope (future iterations, not this plan)
|
||
|
||
- `/api/<route>` shadow endpoints in Flask
|
||
- Go `internal/web/api/` DTOs and JSON Schemas
|
||
- Go `internal/services/membership/views.go` aggregators
|
||
- Go HTTP handlers for `/api/X`
|
||
- `cmd/parity/main.go` and `make parity` target
|
||
- Junior breakdown sidecar work in
|
||
`go/internal/services/membership/sources.go`
|
||
|
||
These are the original M5 tasks (M5.1–M5.4 in the progress tracker).
|
||
They become much easier once today's refactor lands, because the
|
||
shadow `/api/X` routes will be one-liners over the new builders.
|