Aug 25 - Implement new function allow Director (internal & customer) to assign and inspection to another inspector

CLAUDE.md

@@ -575,7 +575,7 @@ Management of the underlying routes is otherwise unchanged; **Start** is the **a
 | Contracts | ✅ | ✅ | ✅ | read | scoped |
 | Templates | ✅ | ✅ | ❌ | ❌ | ❌ |
 | Inspections (execute) | ✅ | ✅ | ✅ | ✅ | read |
-| Inspection follow-up (request) | ✅ | ✅ | ❌ | ❌ | ✅ own facilities |
+| Inspection follow-up (request + assign) | ✅ | ✅ | ❌ | ❌ | ✅ own facilities |
 | Scheduled inspections (plan) | ✅ | ✅ | ✅ | ❌ | ✅ own contracts |
 | Inspection follow-up (clear) | ✅ | ✅ | ❌ | ❌ | ❌ |
 | Issues (create/assign) | ✅ | ✅ | ✅ | ✅ | ✅ create own |
@@ -886,6 +886,28 @@ Customers can only *request*. `clear_followup` remains admin/director, `reinspec
 
 **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.
 
+### Assigning a follow-up to another inspector (Phase 53)
+
+A follow-up used to belong implicitly to whoever performed the original inspection: they were the one notified, and `GET /api/v1/inspections?follow_up_required=true` filtered on `inspector_id == caller`, so nobody else could even see it. `inspections.follow_up_assigned_to` (FK → users, SET NULL) lets a director — or a **Customer Director**, for their own facilities — hand the re-inspection to someone else.
+
+**NULL means what it always meant**: the follow-up belongs to the inspection's own inspector. No backfill, no behaviour change for existing rows. `Inspection.follow_up_owner` (assignee *or* inspector) is the single definition of ownership, so the web display, the notification and the API filter cannot disagree.
+
+**The assignee takes over.** Only the owner is notified, and only the owner sees it — the original inspector's list no longer shows a follow-up that was handed to someone else. In the API that means the two arms must be mutually exclusive:
+
+```python
+db.or_(
+    Inspection.follow_up_assigned_to == user.id,
+    db.and_(Inspection.follow_up_assigned_to.is_(None),
+            Inspection.inspector_id == user.id),
+)
+```
+
+Without the `is_(None)` on the second arm the original inspector keeps seeing it and two people turn up to do the same re-inspection.
+
+**The generic "inspectors see only their own inspections" filter has to be deferred** when `follow_up_required=true` is requested — an assigned follow-up lives on an inspection somebody *else* performed, so applying authorship first hides exactly the rows the assignee needs.
+
+**The picker is contract-scoped** (`_followup_assignees_for()`), for the same reason the flag-issue list is (rule 93): a Customer Director must never see, or assign work to, another client's inspector. Only the two INSPECTOR roles are offered — directors/PMs/auditors hold no `InspectorAssignment`, so they could not open the re-inspection anyway. The POST re-validates against that list, and a facility with no contract offers nobody (fail-closed, follow-up stays with the original inspector). `clear_followup` (single and bulk) clears the assignment too.
+
 ### Per-Account Overrides for Customer Roles (Phase 51)
 
 `notify_by_matrix()` consults `UserNotificationMatrix` (§5) for the two customer-side role columns. One query per dispatch (`overrides_for_event`), then `users = [u for u in users if overrides.get(u.id, enabled)]` — an account with no row falls back to the global column, which is what makes both directions work.
@@ -1015,7 +1037,24 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif
                                                         → phase49_external_inspector
                                                             → phase50_default_modern
                                                                 → phase51_user_notif_matrix
-                                                                    → phase52_template_contracts   ← HEAD
+                                                                    → phase52_template_contracts
+                                                                        → phase53_followup_assignee   ← HEAD
+
+#### phase53 — assign a follow-up to another inspector
+
+Revision id `phase53_followup_assignee`. Adds `inspections.follow_up_assigned_to` (FK → `users.id`, ON DELETE SET NULL) — see §11 "Assigning a follow-up to another inspector".
+
+**No backfill.** NULL means the follow-up belongs to the inspection's own inspector, which is exactly what every existing row already means, so this cannot change who owns anything on deploy.
+
+**This is the THIRD FK from `inspections` to `users`** (rule 86). `Inspection.follow_up_assignee` pins `foreign_keys` explicitly; `User.inspections` was already pinned in phase46. Get this wrong and the mapper is ambiguous — and it raises on first ORM *use*, not at import, so the app starts cleanly and then every request 500s.
+
+`INFORMATION_SCHEMA` column + constraint checks — safe to re-run. `downgrade()` drops the FK then the column, returning every follow-up to its original inspector.
+
+**Deploy order:**
+```bash
+flask db upgrade
+sudo systemctl restart gunicorn
+```
 
 #### phase52 — restrict forms to specific contracts
 

app/api/inspections.py

@@ -210,6 +210,11 @@ def _inspection_payload(inspection):
         # ── Follow-up / re-inspection fields ──────────────────────────────
         'follow_up_required':    inspection.follow_up_required,
         'follow_up_note':        inspection.follow_up_note,
+        # phase53 — who is to perform the follow-up. NULL means the
+        # inspection's own inspector, which is what it always meant.
+        'follow_up_assigned_to':      inspection.follow_up_assigned_to,
+        'follow_up_assigned_to_name': (inspection.follow_up_assignee.display_name
+                                       if inspection.follow_up_assignee else None),
         '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
@@ -267,8 +272,15 @@ def list_inspections():
 
     query = Inspection.query
 
-    # Inspectors only see their own inspections
+    # Inspectors only see their own inspections.
-    if user.is_inspector:
+    #
+    # EXCEPT when asking for follow-up requests: a follow-up can now be handed
+    # to a different inspector (phase53), and that request lives on an
+    # inspection somebody ELSE performed. Applying this filter first would hide
+    # exactly the rows the assignee needs, so it is deferred to the follow-up
+    # block below, which applies ownership instead of authorship.
+    wants_follow_ups = request.args.get('follow_up_required', '').lower() in ('true', '1')
+    if user.is_inspector and not wants_follow_ups:
         query = query.filter(Inspection.inspector_id == user.id)
 
     # Optional filters
@@ -280,7 +292,7 @@ def list_inspections():
     if status:
         query = query.filter(Inspection.status == status)
 
-    if request.args.get('follow_up_required', '').lower() in ('true', '1'):
+    if wants_follow_ups:
         # Must mean exactly what "Follow-up" means everywhere on the web
         # (inspections.list / reports status_filter == 'follow_up'): flagged,
         # completed, and not yet answered by a linked re-inspection.
@@ -298,6 +310,22 @@ def list_inspections():
             Inspection.status == 'completed',
         ).filter(~Inspection.follow_ups.any())
 
+        # Ownership, not authorship (phase53). Mirrors
+        # Inspection.follow_up_owner: an assigned follow-up belongs to the
+        # assignee ALONE, an unassigned one to the inspection's own inspector.
+        #
+        # The two arms are mutually exclusive on purpose. Without the second
+        # arm's `is_(None)` an inspector would keep seeing a follow-up that had
+        # been handed to someone else, and two people would turn up to do it.
+        if user.is_inspector:
+            query = query.filter(db.or_(
+                Inspection.follow_up_assigned_to == user.id,
+                db.and_(
+                    Inspection.follow_up_assigned_to.is_(None),
+                    Inspection.inspector_id == user.id,
+                ),
+            ))
+
     from_date_str = request.args.get('from_date')
     if from_date_str:
         try:

app/models/inspection.py

@@ -202,6 +202,21 @@ class Inspection(db.Model):
     )
     follow_up_requested_at = db.Column(db.DateTime, nullable=True)
 
+    # phase53 — who is to PERFORM the follow-up re-inspection.
+    #
+    # NULL keeps the original behaviour: the follow-up belongs to the
+    # inspection's own inspector. When set, that person owns it instead — they
+    # are the one notified, and the one it appears for on the iPad. Lets a
+    # director (or a Customer Director) hand a re-inspection to someone other
+    # than whoever did the original.
+    #
+    # This is the THIRD FK from inspections to users (rule 86): every
+    # relationship spanning the two must pin foreign_keys explicitly, or the
+    # mapper is ambiguous and blows up on first ORM USE rather than at import.
+    follow_up_assigned_to = db.Column(
+        db.Integer, db.ForeignKey('users.id', ondelete='SET NULL'), nullable=True
+    )
+
     results     = db.relationship('InspectionResult', backref='inspection', lazy='dynamic', cascade='all, delete-orphan')
     issues      = db.relationship('Issue', backref='inspection', lazy='dynamic', cascade='all, delete-orphan')
     # The ScheduledInspection this inspection was started from (phase36), if any.
@@ -212,6 +227,19 @@ class Inspection(db.Model):
     # Explicit foreign_keys: `inspector_id` also points at users.
     follow_up_requester = db.relationship('User',
                                           foreign_keys=[follow_up_requested_by])
+    follow_up_assignee  = db.relationship('User',
+                                          foreign_keys=[follow_up_assigned_to])
+
+    @property
+    def follow_up_owner(self):
+        """Who is expected to carry out the follow-up.
+
+        The explicit assignee when one is set, otherwise the inspection's own
+        inspector — the single definition of ownership, so the web display, the
+        notification and the mobile API filter cannot disagree about who owns a
+        follow-up.
+        """
+        return self.follow_up_assignee or self.inspector
     follow_ups  = db.relationship('Inspection', backref=db.backref('parent', remote_side='Inspection.id'),
                                   lazy='dynamic', foreign_keys='Inspection.parent_inspection_id')
 

app/routes/inspections.py

@@ -969,7 +969,9 @@ def view(inspection_id):
             'unchanged': sum(1 for r in rows if r['delta'] == 0),
         }
 
+    followup_assignees = _followup_assignees_for(inspection, current_user)
     return render_template('inspections/view.html',
+                           followup_assignees=followup_assignees,
                            inspection=inspection,
                            form_fields=form_fields,
                            form_data=form_data,
@@ -1394,6 +1396,46 @@ def _view_url(inspection_id):
     return url_for('inspections.view', inspection_id=inspection_id)
 
 
+def _followup_assignees_for(inspection, actor):
+    """Inspectors who may be handed this inspection's follow-up.
+
+    Contract-scoped, for the same reason the flag-issue list is (rule 93): a
+    Customer Director must never see — let alone assign work to — another
+    client's inspector, and one of our own directors picking the wrong name
+    would leak this facility to an outsider.
+
+    Only the two INSPECTOR roles are offered: a follow-up is an inspection, and
+    directors/PMs/auditors hold no InspectorAssignment, so they cannot be
+    scoped to a contract and could not open the re-inspection anyway.
+
+    A facility with no contract yields nobody — fail-closed, leaving the
+    follow-up with the original inspector.
+    """
+    from app.models.inspector_assignment import InspectorAssignment
+
+    project_id = inspection.facility.project_id if inspection.facility else None
+    if not project_id:
+        return []
+
+    users = (
+        User.query
+        .join(InspectorAssignment, InspectorAssignment.user_id == User.id)
+        .filter(
+            InspectorAssignment.project_id == project_id,
+            User.role.in_(User.INSPECTOR_ROLES),
+            User.active == True,
+        )
+        .order_by(User.full_name, User.username)
+        .all()
+    )
+    seen, out = set(), []
+    for u in users:                      # the join repeats across assignments
+        if u.id not in seen:
+            seen.add(u.id)
+            out.append(u)
+    return out
+
+
 def _collect_inspection_photos(inspection):
     """Relative storage keys owned by an inspection, for cleanup after delete.
 
@@ -1597,6 +1639,7 @@ def bulk_action():
             insp.follow_up_note         = None
             insp.follow_up_requested_by = None
             insp.follow_up_requested_at = None
+            insp.follow_up_assigned_to  = None
             cleared.append(insp)
             changed += 1
         db.session.commit()
@@ -1661,22 +1704,48 @@ def flag_followup(inspection_id):
 
     note = request.form.get('follow_up_note', '').strip() or None
 
+    # ── Assignee (phase53) ────────────────────────────────────────────────
+    # Optional. Blank keeps the original behaviour: the follow-up belongs to
+    # the inspection's own inspector. Validated against the contract-scoped
+    # list rather than trusted, so a crafted id cannot hand work to another
+    # customer's inspector (and tell them this facility's name in the email).
+    assignee_id = request.form.get('follow_up_assigned_to', type=int) or None
+    if assignee_id:
+        allowed = {u.id for u in _followup_assignees_for(inspection, current_user)}
+        if assignee_id not in allowed:
+            current_app.logger.warning(
+                'FOLLOW-UP | out-of-contract assignee blocked | inspection=%s | '
+                'assignee=%s | by=%s',
+                inspection_id, assignee_id, current_user.username)
+            flash('That inspector is not assigned to this facility\'s contract.',
+                  'danger')
+            return redirect(_view_url(inspection_id))
+
     inspection.follow_up_required     = True
     inspection.follow_up_note         = note
     inspection.follow_up_requested_by = current_user.id
     inspection.follow_up_requested_at = now_eastern()
+    inspection.follow_up_assigned_to  = assignee_id
     db.session.commit()
 
     note_suffix = f' Note: {note}' if note else ''
     who = (f'The customer ({current_user.display_name})' if is_customer
            else current_user.display_name)
+    assigned_suffix = ''
+    if inspection.follow_up_assignee:
+        assigned_suffix = (f' It has been assigned to '
+                           f'{inspection.follow_up_assignee.display_name}.')
     body = (
         f'{who} has requested a follow-up re-inspection '
-        f'of "{inspection.template.name}" at {inspection.facility.name}.{note_suffix}'
+        f'of "{inspection.template.name}" at {inspection.facility.name}.'
+        f'{assigned_suffix}{note_suffix}'
     )
 
-    # Notify the original inspector so they see it on the iPad.
+    # Notify whoever now OWNS the follow-up — the assignee when one was named,
-    inspector = db.session.get(User, inspection.inspector_id)
+    # otherwise the original inspector (Inspection.follow_up_owner). Notifying
+    # the original inspector for work that has been handed to someone else is
+    # noise, and worse, it implies they are expected to do it.
+    inspector = inspection.follow_up_owner
     if inspector and inspector.id != current_user.id:
         notify(
             recipient     = inspector,
@@ -1732,6 +1801,7 @@ def clear_followup(inspection_id):
     inspection.follow_up_note         = None
     inspection.follow_up_requested_by = None
     inspection.follow_up_requested_at = None
+    inspection.follow_up_assigned_to  = None
     db.session.commit()
     log_action(ACTION_UPDATE, 'Inspection', inspection_id,
                f'{inspection.template.name} @ {inspection.facility.name}',

app/templates/inspections/view.html

@@ -413,6 +413,19 @@
       {% if inspection.follow_up_requested_at %}
       <span class="small text-muted ms-1">{{ inspection.follow_up_requested_at.strftime('%b %d, %Y %I:%M %p') }}</span>
       {% endif %}
+      {# Who is expected to DO it — the assignee when one was named, otherwise
+         the original inspector (Inspection.follow_up_owner). #}
+      {% if inspection.follow_up_owner %}
+      <div class="small mt-1">
+        <i class="bi bi-person-check me-1"></i>Assigned to
+        <strong>{{ inspection.follow_up_owner.display_name }}</strong>
+        {% if not inspection.follow_up_assignee %}
+        <span class="text-muted">(original inspector)</span>
+        {% elif inspection.follow_up_owner.id == current_user.id %}
+        <span class="badge bg-warning text-dark ms-1">You</span>
+        {% endif %}
+      </div>
+      {% endif %}
       {% if inspection.follow_up_note %}<br><span class="small">{{ inspection.follow_up_note }}</span>{% endif %}
       {# Re-inspection is staff work — reinspect() already refuses customers. #}
       {% if current_user.role != 'customer' %}
@@ -947,6 +960,38 @@ document.addEventListener('keydown', e => { if (e.key === 'Escape') closeMedia()
           </label>
           <textarea name="follow_up_note" class="form-control" rows="3"
                     placeholder="Describe what needs to be addressed in the follow-up inspection…"></textarea>
+
+          {# ── Assign it (phase53) ────────────────────────────────────────
+             Optional. Left blank, the follow-up stays with whoever performed
+             the original inspection — the behaviour before this existed. The
+             list is contract-scoped in _followup_assignees_for(), so a
+             Customer Director only ever sees inspectors on their own
+             contracts. #}
+          {% if followup_assignees %}
+          <div class="mt-3">
+            <label class="form-label fw-semibold">
+              Assign to
+              <span class="text-muted small">(optional)</span>
+            </label>
+            <select name="follow_up_assigned_to" class="form-select">
+              <option value="">
+                — {{ inspection.inspector.display_name }} (original inspector) —
+              </option>
+              {% for u in followup_assignees %}
+              {% if u.id != inspection.inspector_id %}
+              <option value="{{ u.id }}">
+                {{ u.display_name }}{{ ' (Customer)' if u.is_external_inspector }}
+              </option>
+              {% endif %}
+              {% endfor %}
+            </select>
+            <div class="form-text">
+              Choose someone else to carry out the re-inspection. They are
+              notified and it appears in their list on the web and the iPad;
+              the original inspector is not asked to do it.
+            </div>
+          </div>
+          {% endif %}
         </div>
         <div class="modal-footer">
           <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>

migrations/versions/phase53_followup_assignee.py

@@ -0,0 +1,68 @@
+"""phase53 — assign a follow-up re-inspection to a specific inspector
+
+Adds `inspections.follow_up_assigned_to` (FK -> users.id, ON DELETE SET NULL).
+
+Until now a follow-up implicitly belonged to whoever performed the original
+inspection: they were the one notified, and the mobile API only ever showed
+follow-ups where `inspector_id == the caller`. A director could not hand the
+re-inspection to somebody else.
+
+NULL means exactly what it meant before — the follow-up belongs to the
+inspection's own inspector — so every existing row keeps its current behaviour
+and no backfill is needed. `Inspection.follow_up_owner` is the one place that
+resolves assignee-or-inspector.
+
+**This is the THIRD FK from inspections to users** (inspector_id,
+follow_up_requested_by, and now this). Rule 86: any relationship between the two
+tables must pin `foreign_keys` explicitly or the mapper is ambiguous — and it
+raises on first ORM *use*, not at import, so the app starts fine and then every
+request 500s. `Inspection.follow_up_assignee` pins it; `User.inspections` was
+already pinned in phase46.
+
+INFORMATION_SCHEMA checks — safe to re-run.
+"""
+
+revision      = 'phase53_followup_assignee'
+down_revision = 'phase52_template_contracts'
+branch_labels = None
+depends_on    = None
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def _has_column(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 _has_constraint(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"
+    ), {'t': table, 'n': name}).scalar() > 0
+
+
+def upgrade():
+    conn = op.get_bind()
+    if not _has_column(conn, 'inspections', 'follow_up_assigned_to'):
+        op.add_column('inspections',
+                      sa.Column('follow_up_assigned_to', sa.Integer, nullable=True))
+    if not _has_constraint(conn, 'inspections', 'fk_inspections_followup_assignee'):
+        op.create_foreign_key(
+            'fk_inspections_followup_assignee', 'inspections', 'users',
+            ['follow_up_assigned_to'], ['id'], ondelete='SET NULL',
+        )
+
+
+def downgrade():
+    conn = op.get_bind()
+    if _has_constraint(conn, 'inspections', 'fk_inspections_followup_assignee'):
+        op.drop_constraint('fk_inspections_followup_assignee', 'inspections',
+                           type_='foreignkey')
+    if _has_column(conn, 'inspections', 'follow_up_assigned_to'):
+        # Assignments are discarded; every follow-up reverts to belonging to the
+        # inspection's own inspector, which is where it started.
+        op.drop_column('inspections', 'follow_up_assigned_to')