Jul 30 - Allow customer to flag follow-up a inspection
This commit is contained in:
@@ -477,6 +477,8 @@ Management (`/scheduled-inspections/new|edit|delete`) is `@project_manager_requi
|
||||
| Contracts | ✅ | ✅ | ✅ | read | scoped |
|
||||
| Templates | ✅ | ✅ | ❌ | ❌ | ❌ |
|
||||
| Inspections (execute) | ✅ | ✅ | ✅ | ✅ | read |
|
||||
| Inspection follow-up (request) | ✅ | ✅ | ❌ | ❌ | ✅ own facilities |
|
||||
| Inspection follow-up (clear) | ✅ | ✅ | ❌ | ❌ | ❌ |
|
||||
| Issues (create/assign) | ✅ | ✅ | ✅ | ✅ | ✅ create own |
|
||||
| Issues (quick-assign) | ✅ | ✅ | ❌ | ❌ | ❌ |
|
||||
| Issue verification | ✅ | ✅ | ❌ | ❌ | ❌ |
|
||||
@@ -751,12 +753,27 @@ EVENT_CUSTOMER_INSPECTION_DONE = 'customer_inspection_completed'
|
||||
EVENT_CUSTOMER_ISSUE_UPDATED = 'customer_issue_updated'
|
||||
EVENT_SCORE_ALERT = 'score_alert' ← Phase 27
|
||||
EVENT_SCHEDULED_INSPECTION = 'scheduled_inspection' ← Phase 36
|
||||
EVENT_FOLLOWUP_REQUESTED = 'followup_requested' ← Phase 46
|
||||
```
|
||||
|
||||
### Inspector role scoping for `inspection_completed`
|
||||
|
||||
`notify_by_matrix()` special-cases the **inspector** role for the `inspection_completed` event: instead of notifying every active inspector, it notifies **only the inspection's own inspector** (`Inspection.inspector_id`, resolved from the passed `inspection_id`). So enabling the "Inspector" column for "Inspection completed" in the matrix alerts just the inspector who submitted that inspection — not the whole inspector pool. All three dispatch sites (web `routes/inspections.py`, both mobile-API `api/inspections.py`) pass `inspection_id`, so the scoping applies uniformly; if `inspection_id` is ever omitted for this event, the inspector role notifies no one (fail-closed). Other roles/events are unaffected.
|
||||
|
||||
### Customer-requested follow-up (Phase 46)
|
||||
|
||||
`inspections.flag_followup` is no longer `@supervisor_required`. It gates in the body instead: **admin/director** as before, **plus customers for their own facilities** — a client unhappy with a result asks for a re-inspection directly instead of going through support. Inspector / PM / auditor stay refused (403).
|
||||
|
||||
Customers can only *request*. `clear_followup` remains admin/director, `reinspect()` still refuses customers, and the "Start Re-inspection" button inside the follow-up alert is hidden from them (it 403'd on click before). Three extra customer-only guards in the route:
|
||||
|
||||
- facility must be in `get_customer_scope()` — else 403 (a crafted POST must not reach another client's inspection);
|
||||
- inspection must be `completed` — nothing to follow up on otherwise;
|
||||
- if `follow_up_required` is already set the request is a **no-op**, so a repeat submission can't overwrite the pending note/attribution.
|
||||
|
||||
**Attribution** (`follow_up_requested_by` / `follow_up_requested_at`, phase46) records who asked and when; `clear_followup` nulls both. `inspections/view.html` renders a "Requested by customer" / "Requested by staff" badge from `inspection.follow_up_requester.role`, so staff can see at a glance that a client is waiting.
|
||||
|
||||
**Dispatch** goes through `notify_by_matrix(EVENT_FOLLOWUP_REQUESTED, ...)` — the new `followup_requested` matrix event (admin/director/PM on by default). The inspection's own inspector is notified directly by the route and passed in `exclude_user_ids` so they aren't double-notified; the requester is excluded too. Routing via the matrix (rather than hardcoding managers) is what makes per-contract recipients fire — rule 73. Without it a customer request would reach only the inspector and nobody would own scheduling the re-inspection.
|
||||
|
||||
### Per-Contract Additional Recipients (Phase 33)
|
||||
|
||||
`notify_by_matrix()` is the single dispatch point for all broadcast events. After routing to the global matrix roles + global custom emails, it calls `_notify_contract_recipients()`, which:
|
||||
@@ -863,7 +880,24 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif
|
||||
→ phase40_auditor_role
|
||||
→ phase41_internal_handler
|
||||
→ phase42_internal_contact
|
||||
→ phase43_sched_recurrence ← HEAD
|
||||
→ phase43_sched_recurrence
|
||||
→ phase44_sched_end_date
|
||||
→ phase45_sched_parent_insp
|
||||
→ phase46_followup_req_by ← HEAD
|
||||
```
|
||||
|
||||
#### phase46 — follow-up request attribution
|
||||
|
||||
Revision id `phase46_followup_req_by` (file `phase46_followup_requested_by.py`, down_revision `phase45_sched_parent_insp`). Adds `inspections.follow_up_requested_by INT NULL` (FK → `users.id` ON DELETE SET NULL) and `follow_up_requested_at DATETIME NULL`. Backs **customer-requested follow-ups** — see §11 "Customer-requested follow-up". **No backfill**: legacy rows keep NULL and render as an unattributed follow-up exactly as before.
|
||||
|
||||
**Breaking detail:** this is the *second* FK from `inspections` to `users`, which made `User.inspections` ambiguous at mapper-configure time (`AmbiguousForeignKeysError` on the first ORM use, not at import). `User.inspections` now declares `foreign_keys='Inspection.inspector_id'` — it means "inspections I performed". Any future FK from `inspections` to `users` needs the same treatment.
|
||||
|
||||
`INFORMATION_SCHEMA` column + constraint checks — safe to re-run.
|
||||
|
||||
**Deploy order:**
|
||||
```bash
|
||||
flask db upgrade
|
||||
sudo systemctl restart gunicorn
|
||||
```
|
||||
|
||||
#### phase44 — scheduled inspection end date
|
||||
@@ -1403,6 +1437,7 @@ timeout = 30
|
||||
| 78 | **`PATCH /api/v1/issues/<id>/handler` allows the inspector on purpose — do NOT align it to the web form's admin/director/PM restriction** | The iPad lets the assigned inspector set "Handled By" from the field, scoped via `get_inspector_scope()` (403 if the issue's facility isn't contracted). This is a deliberate divergence from the web form. `_issue_payload()` must keep returning all handler fields (`handler_type`, `handler_label`, `facility_handler_*`, `vendor_*`, `internal_handler_name`, `internal_handler_contact`) or the iPad's "Handled By" panel silently blanks — same failure mode as rule 40. |
|
||||
| 79 | **`auditor` = `project_manager` access + issue management, minus delete — keep the two decorators distinct** | Auditor is added to `@project_manager_required` (PM baseline) and to every `project_manager` role check in routes/templates. Its *extra* issue powers (verify/bulk-verify/verification-queue) go through the separate `@issue_manager_required` (admin/director/auditor). Issue **delete** stays `@supervisor_required` — never add auditor there. When adding a new PM-level gate, include `auditor`; when adding a director-only or delete-level gate, do not. The three issue **delete** template gates (spaced `['admin', 'director']` in `issues/list.html` + `issues/view.html`) are deliberately left without auditor. Auditor is also in the `_ALLOWED_ROLES` set of every `app/api/*` module — a **new** API blueprint's `_ALLOWED_ROLES` must include `auditor` for PM parity. |
|
||||
| 80 | **Assignee dropdowns are `director`/`inspector`/`auditor` (admin removed, auditor added)** | The issue/inspection assignee `<select>`s query `User.role.in_([...])` — admin was removed and auditor added (the inspection flag-issue list also keeps `project_manager`). These lists control who can be *assigned*, distinct from who can *edit*. The issue-update route (`issues.view`) defensively appends any current `assigned_to` who is not in the set (e.g. a legacy admin assignment) to `form.assigned_to.choices` so saving the form never silently unassigns them. Do not remove that guard. |
|
||||
| 86 | **A second FK from a table to `users` breaks any relationship that didn't pin `foreign_keys`** | Adding `inspections.follow_up_requested_by` (phase46) made `User.inspections` ambiguous — `AmbiguousForeignKeysError`, raised at first ORM *use*, not at import, so the app starts fine and then every request 500s. `User.inspections` now pins `foreign_keys='Inspection.inspector_id'`. Check existing relationships before adding another FK to `users` from a table that already has one. |
|
||||
| 83 | **A bad `scheduled_inspection_id` must NEVER fail the inspection submission** | `_resolve_schedule()` in `app/api/inspections.py` drops an unknown or foreign link and logs a warning instead of returning 404/403. The app is offline-first: a completed inspection can sit in the outbox for days, during which the schedule may be deleted, reassigned, or rolled forward. Erroring would burn the 5 sync retries and permanently strand that inspection **and its photos** on the device. A missed fulfil is fixable from the web; a stranded submission is not. The ownership check still refuses to *link* a foreign schedule (one inspector must not fulfil another's) — it just accepts the inspection anyway. |
|
||||
| 82 | **A schedule's recurrence columns must be CLEARED when they don't apply to the chosen frequency** | `_apply_recurrence()` in `routes/scheduled_inspections.py` is the single write path for `frequency` + `weekdays`/`month_mode`/`day_of_month`/`nth_week`/`nth_weekday`, and it NULLs the blocks that don't apply. Setting `sched.frequency` directly (as create/edit used to) leaves stale settings behind — a weekly→monthly switch would keep `weekdays` and `recurrence_label` would lie. The hidden form blocks still POST their values, so client-side hiding is not enough. |
|
||||
| 84 | **"Instructions" is a LABEL over `notes` — never rename the field, attribute, column or API key** | `ScheduledInspectionForm.notes` renders as "Instructions" and both the web execute page and the iPad say "Instructions". The wire key stays `notes` (`api/scheduled.py::_scheduled_payload`), which is what `APIScheduledInspection.notes` decodes into `LocalScheduledInspection.notes`; the iPad exposes it through a computed `instructions` accessor that also trims blank text. Renaming any of the storage identifiers would silently break the iPad decode — the field is `try?`-decoded, so it would fail to nil rather than throwing. |
|
||||
|
||||
Reference in New Issue
Block a user