Aug 17 - Update forms will be assigned per contract
This commit is contained in:
@@ -321,6 +321,22 @@ Triage of `handler_type` + facility/vendor detail fields on the **update** panel
|
||||
|
||||
**`reported_by`:** Added in phase18. Set at creation time to the user who filed the issue. Nullable for backward compatibility. Used by `GET /api/v1/issues` to return issues the inspector created but hasn't been assigned yet.
|
||||
|
||||
### TemplateContract (Phase 52)
|
||||
|
||||
```
|
||||
template_contracts: id, template_id (FK→inspection_templates CASCADE, indexed),
|
||||
project_id (FK→projects CASCADE, indexed), created_at
|
||||
UniqueConstraint(template_id, project_id)
|
||||
```
|
||||
|
||||
**Restricts a form to specific contracts** — a customer's bespoke form must not be visible to, or startable against, another customer's facilities.
|
||||
|
||||
**No rows means the form is SHARED** (available on every contract), not "available nowhere". That convention is the whole migration story: every template that existed before phase52 has no rows, so nothing changed on deploy, and a form becomes customer-specific only when an admin attaches it to at least one contract. Inverting the default would silently hide every shared form from every contract.
|
||||
|
||||
`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.
|
||||
|
||||
### Notification / NotificationPreference
|
||||
|
||||
```
|
||||
@@ -663,8 +679,8 @@ The last eight styles (`SummaryTitle` through `TableCell`) were added for the fa
|
||||
|---|---|---|
|
||||
| `GET /api/v1/facilities` | jwt_required | All active facilities scoped to user |
|
||||
| `GET /api/v1/facilities/<id>/areas` | jwt_required | Areas for a facility |
|
||||
| `GET /api/v1/templates` | jwt_required | Template list (summary, no form_schema) |
|
||||
| `GET /api/v1/templates/<id>` | jwt_required | Full template with form_schema |
|
||||
| `GET /api/v1/templates` | jwt_required | Template list (summary, no form_schema). **Contract-scoped (phase52):** an inspector gets shared forms plus those attached to their assigned contracts. Optional `?project_id=` / `?facility_id=` narrows to one contract — **and is intersected with the caller's own scope**, so passing another customer's facility id returns `[]` rather than listing their form names. |
|
||||
| `GET /api/v1/templates/<id>` | jwt_required | Full template with form_schema. **404** (not 403) when the form is restricted to a contract the caller cannot reach — whether another customer's form exists is not their business. |
|
||||
|
||||
### Phase B Endpoints
|
||||
|
||||
@@ -970,7 +986,20 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif
|
||||
→ phase48_user_ui_theme
|
||||
→ phase49_external_inspector
|
||||
→ phase50_default_modern
|
||||
→ phase51_user_notif_matrix ← HEAD
|
||||
→ phase51_user_notif_matrix
|
||||
→ phase52_template_contracts ← HEAD
|
||||
|
||||
#### phase52 — restrict forms to specific contracts
|
||||
|
||||
Revision id `phase52_template_contracts`. Creates `template_contracts` — see §5 `TemplateContract`.
|
||||
|
||||
**No backfill, and it cannot change behaviour on deploy.** Every existing template has no rows, and no rows means *shared with every contract*, which is exactly what they do today. Table-existence check — safe to re-run. `downgrade()` drops the table, returning every form to shared: no form becomes unusable, they just stop being restricted.
|
||||
|
||||
**Deploy order:**
|
||||
```bash
|
||||
flask db upgrade
|
||||
sudo systemctl restart gunicorn
|
||||
```
|
||||
|
||||
#### phase51 — per-account notification overrides
|
||||
|
||||
@@ -1633,6 +1662,8 @@ 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. |
|
||||
| 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_(...))`. |
|
||||
| 94 | **A failed flag-issue POST must return a non-2xx** | The offcanvas JS branches on `res.ok`, so a 200 re-render of the invalid form reads as success: the panel closes, the page reloads, and no issue exists — with nothing in the logs and no message to the user. `flag_issue()` returns 400 on a failed POST for exactly this reason. Any future AJAX-submitted form needs the same treatment (rule 60 is the same failure seen from the other end). |
|
||||
| 91 | **A bulk-action form must live OUTSIDE the table; row checkboxes join it via the HTML5 `form=` attribute** | Wrapping the table in the bulk form nests the per-row delete/unfollow forms inside it, and browsers **silently discard** nested forms (rule 9) — the row buttons would post nothing, with no console error and no server log. `<form id="issuesBulkForm">` sits above the table and each checkbox carries `form="issuesBulkForm"`. Same for `inspectionsBulkForm`. Applies to all four list templates (classic + modern). |
|
||||
|
||||
Reference in New Issue
Block a user