refactor(app): extract view-model builders into scripts/views.py
All checks were successful
Deploy to K8s / deploy (push) Successful in 8s
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>
This commit is contained in:
185
docs/plans/2026-05-07-1431-m5-json-api-parity.md
Normal file
185
docs/plans/2026-05-07-1431-m5-json-api-parity.md
Normal file
@@ -0,0 +1,185 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user