Jul 26 - Update scheduled inspection settings for iPad app

CLAUDE.md

@@ -645,11 +645,18 @@ The last eight styles (`SummaryTitle` through `TableCell`) were added for the fa
 
 | Endpoint | Auth | Description |
 |---|---|---|
-| `GET /api/v1/scheduled-inspections` | jwt_required | Active scheduled/recurring assignments (`app/api/scheduled.py`, new blueprint). **Inspector:** only rows where `inspector_id == self`. **admin/director/PM:** all active. Sorted by `next_due_date`. Returns per row: `id`, `facility_id`, `facility_name`, `template_id`, `template_name`, `inspector_id`, `frequency`, `frequency_label`, `next_due_date` (ISO date), `is_overdue`, `notes`, plus `total`/`limit`/`offset`. Powers the iPad "Scheduled" section on Dashboard + My Inspections. Read-only — the schedule lifecycle (fulfil/roll-forward) stays web-driven; the iPad "Start" just seeds the new-inspection flow. |
+| `GET /api/v1/scheduled-inspections` | jwt_required | Active scheduled/recurring assignments (`app/api/scheduled.py`, new blueprint). **Inspector:** only rows where `inspector_id == self`. **admin/director/PM:** all active. Sorted by `next_due_date`. Returns per row: `id`, `facility_id`, `facility_name`, `template_id`, `template_name`, `inspector_id`, `frequency`, `frequency_label`, `next_due_date` (ISO date), `is_overdue`, `notes`, plus `total`/`limit`/`offset`. Powers the iPad "Scheduled" section on Dashboard + My Inspections. Read-only *as a collection* — schedules are created/edited on the web only — but the iPad **does** fulfil them by submitting an inspection with `scheduled_inspection_id` (see below). |
 | `PATCH /api/v1/issues/<id>/handler` | jwt_required | Set "Handled By" from the iPad (`update_issue_handler`). Body: `{ "handler_type": "internal"\|"facility"\|"vendor", ...optional detail keys }`. Detail keys (`facility_handler_name/contact/notes`, `vendor_name/contact/notes`) are updated only when present; empty string clears a field. **`log_action()` after commit.** |
 
 **Handler permission divergence — deliberate (see rule 78).** The web issue form limits handler edits to admin/director/PM. This API endpoint additionally allows the assigned **inspector**, scoped by `get_inspector_scope()` (403 if the issue's facility isn't in their contracted set). The iPad is a field tool; inspectors set the handler from Issue Detail. Do not "align" the API back to the web restriction without explicit direction.
 
+**Schedule fulfilment from the iPad (July 2026 fix).** `POST /api/v1/inspections` and `PATCH /api/v1/inspections/<id>` both accept **`scheduled_inspection_id`**, and both call `_fulfill_schedule()` in the same atomic commit when the inspection reaches `completed` — mirroring the web execute route. Previously the iPad's Start passed only facility + template, so the inspection landed with `scheduled_inspection_id = NULL`: the schedule was never fulfilled (banner stayed on every dashboard, iPad "Scheduled" section never cleared) and the web inspection list showed no "Scheduled" badge. All three symptoms had this single cause.
+
+- **PATCH fulfils only on the `draft → completed` transition**, so re-PATCHing a completed inspection can't roll a recurring schedule forward twice. POST is guarded by the existing `mobile_local_id` idempotency check (a duplicate returns early, before fulfilment).
+- **`_resolve_schedule()` is deliberately NON-BLOCKING** — see rule 83.
+- `_inspection_payload()` returns `scheduled_inspection_id`.
+- No SyncManager change was needed: `pullScheduledInspections()` already runs after `processInspectionQueue()` in the same `triggerSync()` pass and deletes rows the server no longer returns, so the iPad banner clears on the same sync that submits the inspection.
+
 The new `scheduled` blueprint is registered in `app/api/__init__.py` and CSRF-exempted in `app/__init__.py` (`csrf.exempt(_api_scheduled_bp)` — parent-exempt does not cascade to child blueprints, per the CSRF pattern above).
 
 No migration was needed for either feature: the `scheduled_inspections` table (phase36) and the issue handler columns (phase35) already existed; both additions are pure serialization + one new route.
@@ -1388,6 +1395,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. |
+| 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. |
 | 81 | **Photo timestamp/geo overlay is burned at UPLOAD, never on `PATCH /issues/<id>/photos`** | That PATCH receives only path strings — the bytes are already in storage and the payload carries no capture metadata. Burning there would need a read-modify-write per key plus an overwrite-in-place primitive (`storage.save()` mints a NEW uuid key, and §22 requires key == DB path), and would risk a **double burn** since the endpoint is deliberately idempotent/retry-safe (rule 45). Stamp in `POST /photos/upload`, where the raw bytes + EXIF are in hand and each call writes exactly one already-stamped object. Stamping failures must always fall back to storing the ORIGINAL bytes — never lose a photo to a stamping bug. See §23. |
 

app/api/inspections.py

@@ -95,6 +95,58 @@ def _parse_datetime(value):
     return None
 
 
+def _resolve_schedule(schedule_id, user):
+    """Resolve a client-supplied scheduled_inspection_id, or None.
+
+    The iPad sends this when the inspector taps Start on a scheduled row;
+    without it the inspection lands unlinked and the schedule is never fulfilled
+    (no "Scheduled" badge, and the dashboard banner never clears).
+
+    NON-BLOCKING BY DESIGN. A bad link drops the link and logs a warning — it
+    never fails the submission. The app is offline-first, so a schedule can
+    legitimately be deleted or reassigned while a completed inspection sits in
+    the outbox for days; erroring here would retry-fail that inspection and
+    strand the inspector's work (and its photos) permanently. A missed fulfil is
+    recoverable from the web UI; a stranded submission is not.
+
+    The ownership check still matters: accepting a foreign link would let one
+    inspector fulfil another's schedule. So the link is refused — but the
+    inspection itself is still accepted.
+    """
+    from app.models.scheduled_inspection import ScheduledInspection
+
+    sched = db.session.get(ScheduledInspection, schedule_id)
+    if sched is None:
+        logger.warning('API INSPECTIONS | unknown scheduled_inspection_id=%s from user=%s '
+                       '— submitting unlinked', schedule_id, user.username)
+        return None
+    if user.role == 'inspector' and sched.inspector_id != user.id:
+        logger.warning('API INSPECTIONS | scheduled_inspection_id=%s not assigned to user=%s '
+                       '— submitting unlinked', schedule_id, user.username)
+        return None
+    return sched
+
+
+def _fulfill_schedule(inspection):
+    """Roll the originating schedule forward / deactivate it. Caller commits.
+
+    Mirrors the web execute route: one-time schedules deactivate (so the
+    dashboard banner, which filters on active, disappears), recurring ones
+    advance to their next occurrence and reset the reminder flags.
+    """
+    if not inspection.scheduled_inspection_id:
+        return
+    from app.models.scheduled_inspection import ScheduledInspection
+
+    sched = db.session.get(ScheduledInspection, inspection.scheduled_inspection_id)
+    if sched is None:
+        return
+    sched.fulfill()
+    logger.info('API INSPECTIONS | schedule fulfilled | schedule=%s | inspection=%s | next=%s',
+                sched.id, inspection.id,
+                sched.next_due_date if sched.active else 'deactivated')
+
+
 def _media(key):
     """Absolute display URL for a storage key (presigned on R2, absolute-static
     on local). '' for falsy keys. Used for iPad image rendering."""
@@ -157,6 +209,10 @@ def _inspection_payload(inspection):
         'follow_up_required':    inspection.follow_up_required,
         'follow_up_note':        inspection.follow_up_note,
         'parent_inspection_id':  inspection.parent_inspection_id,
+        # Set when this inspection was started from a ScheduledInspection —
+        # drives the "Scheduled" badge on the web list and lets the iPad show
+        # the same marker in history.
+        'scheduled_inspection_id': inspection.scheduled_inspection_id,
     }
 
 
@@ -333,6 +389,12 @@ def create_inspection():
         if parent is None:
             return api_error('Parent inspection not found', 404)
 
+    # ── Optional schedule link (started from a ScheduledInspection) ───────
+    scheduled_inspection_id = None
+    if data.get('scheduled_inspection_id'):
+        sched = _resolve_schedule(data['scheduled_inspection_id'], user)
+        scheduled_inspection_id = sched.id if sched else None
+
     # ── Score calculation ─────────────────────────────────────────────────
     overall_score = data.get('overall_score')
     if overall_score is None and status == 'completed':
@@ -383,11 +445,19 @@ def create_inspection():
         parent_inspection_id = parent_inspection_id,
         submit_latitude      = submit_latitude,
         submit_longitude     = submit_longitude,
+        scheduled_inspection_id = scheduled_inspection_id,
     )
 
     db.session.add(inspection)
     db.session.flush()
 
+    # ── Fulfil the originating schedule ───────────────────────────────────
+    # Staged into the same atomic commit as the inspection, mirroring the web
+    # execute route. Without this the schedule stays active: the dashboard
+    # banner and the iPad "Scheduled" section never clear.
+    if status == 'completed':
+        _fulfill_schedule(inspection)
+
     # ── Auto-clear follow-up flag on parent ───────────────────────────────
     # When a completed re-inspection arrives that links to a parent, clear
     # follow_up_required on the parent automatically. This mirrors the web
@@ -512,6 +582,13 @@ def update_inspection(inspection_id):
 
     prev_status = inspection.status
 
+    # Allow the link to be set/corrected on PATCH too — the iPad may create the
+    # inspection as a draft first and only attach the schedule on submit.
+    if data.get('scheduled_inspection_id'):
+        sched = _resolve_schedule(data['scheduled_inspection_id'], user)
+        if sched is not None:
+            inspection.scheduled_inspection_id = sched.id
+
     if 'status' in data:
         inspection.status = data['status']
 
@@ -527,12 +604,17 @@ def update_inspection(inspection_id):
     elif data.get('status') == 'completed' and not inspection.completed_at:
         inspection.completed_at = now_eastern()
 
-    db.session.commit()
-
-    # Notify when a draft transitions to completed — mirrors the POST handler.
     transitioning_to_complete = (
         data.get('status') == 'completed' and prev_status != 'completed'
     )
+    # Fulfil the schedule on the draft → completed transition only, so a later
+    # PATCH on an already-completed inspection can't roll it forward twice.
+    if transitioning_to_complete:
+        _fulfill_schedule(inspection)
+
+    db.session.commit()
+
+    # Notify when a draft transitions to completed — mirrors the POST handler.
     if transitioning_to_complete:
         score_val = inspection.overall_score
         score_display = f'{score_val:.1f}%' if score_val is not None else 'N/A'