Aug 17 - Update forms will be assigned per contract, edit template page fix
This commit is contained in:
@@ -335,7 +335,17 @@ template_contracts: id, template_id (FK→inspection_templates CASCADE, indexed)
|
||||
|
||||
`InspectionTemplate` helpers: `contract_ids`, `is_shared`, `available_for_project(project_id)`, `set_contracts([ids])` (does **not** commit), and the static **`available_query(project_id)`** — the single definition of "which forms may this contract use", used by every picker, by the POST validation behind it, and by the mobile API, so they cannot disagree. A facility with **no** contract can only use shared forms (fail-closed).
|
||||
|
||||
Managed on the template create/edit pages via an "Available on contracts" multi-select (admin/director); the template list shows a **Shared** badge or one badge per contract.
|
||||
Managed by admin/director in **three** places, because the template screens have three separate edit paths — all must keep the picker or a form silently stays shared:
|
||||
|
||||
| Where | Route | Notes |
|
||||
|---|---|---|
|
||||
| **Edit Template modal** on the template list | `POST /templates/<id>/rename` | The one most people actually use. Posts a hidden `contracts_present=1` marker so an empty selection means "make it shared"; a POST **without** the marker (an older client, or another caller of this route) leaves the existing restrictions untouched rather than wiping them. Ids are validated against active contracts. |
|
||||
| Create Template | `POST /templates/new` | |
|
||||
| Full form editor | `POST /templates/<id>/edit` | `obj=` cannot read association rows, so the multi-select is seeded from `contract_ids` on GET. |
|
||||
|
||||
`duplicate_template()` copies the restrictions across — duplicating a customer's bespoke form must not yield a copy shared with everyone.
|
||||
|
||||
The template list shows a **Shared** badge or one badge per contract.
|
||||
|
||||
### Notification / NotificationPreference
|
||||
|
||||
@@ -1662,6 +1672,7 @@ timeout = 30
|
||||
| 85 | **`next_due_date` is mutable state, `end_date` is a fixed boundary — never conflate them** | `fulfill()` rewrites `next_due_date` after every completed inspection; `end_date` is set by the manager and never touched by the app. The old single label "Start / Due Date" said both at once, which is what users reported as confusing. The label now follows context — `form.next_due_date.label.text` is set to "Start Date" in `create()` and "Next Due Date" in `edit()`. Do not rename the `next_due_date` column to match a label: it is indexed, it is the API payload key the iPad decodes, and the reminder cron filters on it. |
|
||||
| 87 | **Never write `role == 'inspector'` — use `user.is_inspector` (`User.INSPECTOR_ROLES`)** | phase49 added `external_inspector`, which must behave as an inspector everywhere. An equality check silently drops it into the *privileged* branch of every `if inspector: scope … else: org-wide` block — i.e. a third-party inspector would see **every contract in the system**. This is a fail-OPEN mistake: nothing errors, the data just leaks. The sweep converted ~44 Python sites and 7 template sites; the only surviving `== 'inspector'` literals are the matrix docstring, the `MATRIX_DEFAULTS` mirror comprehension, and the default-checked box in `admin/broadcast.html`. Query-level checks use `User.role.in_(User.INSPECTOR_ROLES)` (never `filter_by(role='inspector')`). A **new** `app/api/*` blueprint's `_ALLOWED_ROLES` must include `external_inspector`, same as rule 79 requires for `auditor`. |
|
||||
| 88 | **`app/enrollment/` writes no DB row and has exactly ONE read — keep the vertical slice sealed** | The enrollment form describes accounts that do NOT exist yet (no contract, facility or user to key a row against), so it stores flat JSON in `ENROLLMENT_DIR` and owns its own templates. The single permitted model access is `mailer._admin_recipients()` reading active `admin` users to address the new-enrollment alert — function-local, read-only, and guarded so a DB failure cannot break a submission. Adding a model/migration for enrollment, or letting the public POST **create** Users, would couple an unauthenticated endpoint to the account system — the exact thing the separation buys. If enrollment must ever provision accounts, do it as a separate admin-triggered action that reads a stored submission. Submission ids are filesystem paths: validate against `_ID_RE` before every open (path traversal). See §24. |
|
||||
| 97 | **The template list's Edit modal (`/rename`) is a THIRD edit path — keep it in sync with create and the form editor** | The modal on the template list posts to `rename_template`, not `edit_template`, so a field added only to the two WTForms pages is invisible to the people who edit templates from the list. It carries a hidden `contracts_present=1` marker: an empty selection with the marker means "make this shared", while a POST without it leaves restrictions untouched — otherwise any other caller of that route would silently share a restricted form with every customer. |
|
||||
| 95 | **A template with NO `template_contracts` rows is SHARED, not hidden** | The empty set means "available on every contract" — that is what makes phase52 additive and why it needed no backfill. Reading it the other way would hide every pre-phase52 form from every contract at once. The convention lives in exactly one place, `InspectionTemplate.available_query()`; every picker, the POST validation behind it, and the mobile API call it rather than writing their own filter. A facility with no contract gets shared forms only (fail-closed). |
|
||||
| 96 | **An explicit `?project_id=` / `?facility_id=` filter must still be intersected with the caller's own scope** | Accepting a caller-supplied contract filter *instead of* their scope is a leak, not a filter: a Customer Inspector could pass another customer's facility id and get that customer's form names back. `_visible_templates()` returns `[]` for an out-of-scope contract — empty rather than an error, so the endpoint does not confirm the contract exists either. Applies to any future endpoint that takes a scope-shaped query parameter. |
|
||||
| 93 | **The flag-issue assignee list is contract-scoped, and BOTH call sites must use `_assignable_staff_for()`** | `execute()` renders the dropdown, `flag_issue()` builds the choices that validate the POST — the choices are the security boundary. Two separate queries had already drifted (offcanvas offered project_manager/auditor, choices rejected them), which silently discarded issues. Contract scoping applies to the two inspector roles for EVERY actor, not just customer ones: an org-wide list let anyone assign another client's Customer Inspector, who was then emailed that facility's name and issue description. Never widen this back to an unscoped `User.query.filter(role.in_(...))`. |
|
||||
|
||||
Reference in New Issue
Block a user