diff --git a/CLAUDE.md b/CLAUDE.md index 445271b..4a359ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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//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 `
+ {% set is_cust = current_user.role == 'customer' %}
-
Flag Follow-up Required
+
+ {{ 'Request a Follow-up Inspection' if is_cust else 'Flag Follow-up Required' }} +
- + {% if is_cust %} +

+ Ask the team to re-inspect this facility. Your request is sent to the + inspector and management right away. +

+ {% endif %} +
diff --git a/migrations/versions/phase46_followup_requested_by.py b/migrations/versions/phase46_followup_requested_by.py new file mode 100644 index 0000000..6957afb --- /dev/null +++ b/migrations/versions/phase46_followup_requested_by.py @@ -0,0 +1,74 @@ +"""phase46 — follow-up request attribution (customer-raised follow-ups) + +Adds to `inspections`: + + follow_up_requested_by INT NULL FK → users(id) ON DELETE SET NULL + follow_up_requested_at DATETIME NULL + +Customers can now request a follow-up re-inspection of a completed inspection at +their own facilities (previously admin/director only), so `follow_up_required` +alone is no longer enough — staff need to see WHO is waiting on the +re-inspection, and a client request must be visibly distinct from an internal +one. `flag_followup()` sets both columns; `clear_followup()` nulls them. + +No backfill: legacy rows keep NULL, which the UI renders as an unattributed +follow-up exactly as it did before. FK is SET NULL so deleting a user never +deletes inspection history. + +Uses INFORMATION_SCHEMA checks — safe to re-run. +""" + +revision = 'phase46_followup_req_by' +down_revision = 'phase45_sched_parent_insp' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +_FK_NAME = 'fk_inspections_follow_up_requested_by' + + +def _column_exists(conn, table, column): + return conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS " + "WHERE TABLE_SCHEMA = DATABASE() " + "AND TABLE_NAME = :t AND COLUMN_NAME = :c" + ), {"t": table, "c": column}).scalar() > 0 + + +def _fk_exists(conn, table, name): + return conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS " + "WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = :t " + "AND CONSTRAINT_NAME = :n AND CONSTRAINT_TYPE = 'FOREIGN KEY'" + ), {"t": table, "n": name}).scalar() > 0 + + +def upgrade(): + bind = op.get_bind() + + if not _column_exists(bind, 'inspections', 'follow_up_requested_by'): + op.execute(sa.text( + "ALTER TABLE inspections ADD COLUMN follow_up_requested_by INT NULL" + )) + if not _column_exists(bind, 'inspections', 'follow_up_requested_at'): + op.execute(sa.text( + "ALTER TABLE inspections ADD COLUMN follow_up_requested_at DATETIME NULL" + )) + if not _fk_exists(bind, 'inspections', _FK_NAME): + op.execute(sa.text( + f"ALTER TABLE inspections ADD CONSTRAINT {_FK_NAME} " + "FOREIGN KEY (follow_up_requested_by) REFERENCES users(id) " + "ON DELETE SET NULL" + )) + + +def downgrade(): + bind = op.get_bind() + if _fk_exists(bind, 'inspections', _FK_NAME): + op.execute(sa.text(f"ALTER TABLE inspections DROP FOREIGN KEY {_FK_NAME}")) + for col in ('follow_up_requested_at', 'follow_up_requested_by'): + if _column_exists(bind, 'inspections', col): + op.execute(sa.text(f"ALTER TABLE inspections DROP COLUMN {col}"))