Aug 19 - Update code to catch up with ST

app/routes/inspections.py

@@ -15,7 +15,7 @@ from app.models.project import Project
 from app.models.issue import Issue
 from app.models.user import User
 from app.utils.forms import StartInspectionForm, IssueForm
-from app.utils.decorators import supervisor_required
+from app.utils.decorators import supervisor_required, return_url
 from app.utils.pdf_export import generate_inspection_pdf, generate_inspections_list_pdf
 from app.utils.notifications import notify, notify_customers_for_facility, notify_by_matrix
 from app.models.notification import (
@@ -350,7 +350,6 @@ def index():
 def start():
     form = StartInspectionForm()
 
-    templates = InspectionTemplate.query.filter_by(active=True).order_by(InspectionTemplate.name).all()
     projects  = Project.query.filter_by(active=True).order_by(Project.name).all()
 
     # Scope projects to inspector's assigned contracts
@@ -362,7 +361,6 @@ def start():
         }
         projects = [p for p in projects if p.id in assigned_pids]
 
-    form.template_id.choices = [(t.id, t.name) for t in templates]
     form.project_id.choices  = [(p.id, p.name) for p in projects]
 
     # Seed facility choices: use submitted project_id, session value, or first project
@@ -376,6 +374,13 @@ def start():
     else:
         selected_project_id = projects[0].id if projects else None
 
+    # phase52 — forms are offered per CONTRACT: shared forms plus any attached
+    # to the selected contract. This is also the POST validation (SelectField
+    # validates against its choices), so a crafted template_id for another
+    # customer's form is rejected here, not merely hidden in the UI.
+    templates = InspectionTemplate.available_query(selected_project_id).all()
+    form.template_id.choices = [(t.id, t.name) for t in templates]
+
     if selected_project_id:
         facilities = Facility.query.filter_by(active=True, project_id=selected_project_id).order_by(Facility.name).all()
     else:
@@ -408,6 +413,19 @@ def start():
         if template is None:
             abort(404)
 
+        # Belt-and-braces: the choices above already reject a form that is not
+        # available on this contract, but that guard lives in how the list was
+        # built. Re-assert it against the FACILITY actually chosen, so a future
+        # change to the choice-building cannot quietly open a cross-customer
+        # hole here.
+        _fac = db.session.get(Facility, form.facility_id.data)
+        if not template.available_for_project(_fac.project_id if _fac else None):
+            logger_msg = ('INSPECTION START BLOCKED | template=%s not available for '
+                          'facility=%s | user=%s')
+            current_app.logger.warning(logger_msg, template.id,
+                                       form.facility_id.data, current_user.username)
+            abort(403)
+
         # Inspector facility scope check — prevent crafted POST from selecting
         # a facility outside their assigned contracts.
         if current_user.is_inspector:
@@ -465,6 +483,45 @@ def facilities_for_project(project_id):
     return jsonify([{'id': f.id, 'name': f.name} for f in facilities])
 
 
+# ── AJAX: forms available on a given contract (phase52) ──────────────────────
+
+@bp.route('/templates_for_project/<int:project_id>')
+@login_required
+def templates_for_project(project_id):
+    """Forms usable on this contract — shared ones plus any attached to it.
+
+    Powers the Contract -> Form cascade on the start-inspection page, the same
+    way facilities_for_project powers Contract -> Facility.
+
+    **Scoped to the caller's own contracts.** The POST validation in start() is
+    what stops a form being *used* across contracts, but this endpoint would
+    otherwise happily list one customer's bespoke form NAMES to another
+    customer's inspector who simply asked for a contract id — the same leak
+    that rule 96 covers on the mobile API. Empty list rather than 403, so it
+    does not confirm whether the contract exists either.
+    """
+    if current_user.is_inspector:
+        fids = get_inspector_scope(current_user) or []
+        allowed = {
+            f.project_id
+            for f in Facility.query.filter(Facility.id.in_(fids)).all()
+        } if fids else set()
+        if project_id not in allowed:
+            logger_msg = ('TEMPLATES_FOR_PROJECT | out-of-scope request | '
+                          'user=%s | project_id=%s')
+            current_app.logger.warning(logger_msg, current_user.username, project_id)
+            return jsonify([])
+    elif current_user.role == 'customer':
+        # Customers never start inspections; nothing here is theirs to see.
+        return jsonify([])
+
+    templates = InspectionTemplate.available_query(project_id).all()
+    return jsonify([
+        {'id': t.id, 'name': t.name, 'shared': t.is_shared}
+        for t in templates
+    ])
+
+
 # ── Execute ───────────────────────────────────────────────────────────────────
 
 @bp.route('/<int:inspection_id>/execute', methods=['GET', 'POST'])
@@ -479,7 +536,7 @@ def execute(inspection_id):
         return redirect(url_for('inspections.index'))
 
     if inspection.status == 'completed':
-        return redirect(url_for('inspections.view', inspection_id=inspection_id))
+        return redirect(_view_url(inspection_id))
 
     template    = inspection.template
     form_fields = template.get_form_schema()
@@ -601,7 +658,7 @@ def execute(inspection_id):
                        f'status=completed; score={score}')
 
             flash('Inspection submitted successfully!', 'success')
-            return redirect(url_for('inspections.view', inspection_id=inspection_id))
+            return redirect(_view_url(inspection_id))
 
         else:
             _save_draft(inspection, responses)
@@ -609,11 +666,10 @@ def execute(inspection_id):
             flash('Draft saved. You can continue filling in the form later.', 'success')
             return redirect(url_for('inspections.execute', inspection_id=inspection_id))
 
-    staff_for_flag_issue = User.query.filter(
-        User.role.in_(['director', 'inspector', 'external_inspector',
-                       'project_manager', 'auditor']),
-        User.active == True,
-    ).order_by(User.full_name, User.username).all()
+    # Scoped to this inspection's contract — see _assignable_staff_for().
+    # Must match flag_issue()'s choices exactly or the offcanvas silently
+    # fails to save (rule 60).
+    staff_for_flag_issue = _assignable_staff_for(inspection, current_user)
 
     return render_template('inspections/execute.html',
                            inspection=inspection,
@@ -924,6 +980,95 @@ def view(inspection_id):
 
 # ── Flag issue during inspection ──────────────────────────────────────────────
 
+#: Internal roles that are NOT contract-scoped — they work across the whole
+#: organisation, so they are offered regardless of which contract the
+#: inspection belongs to. Only ever shown to our own people.
+_ORG_WIDE_ASSIGNEE_ROLES = ('director', 'project_manager', 'auditor')
+
+
+def _assignable_staff_for(inspection, actor):
+    """Users `actor` may assign an issue to, for THIS inspection.
+
+    The candidate list is scoped by the inspection's CONTRACT, not taken
+    org-wide. Two distinct problems this fixes:
+
+    1. **Cross-customer leak.** A Customer Inspector could assign an issue to
+       anyone in the system — including another client's Customer Inspector.
+       The assignee is notified by email and in-app with the facility name and
+       issue description, so this handed one customer's data to another. It is
+       a leak whoever flags the issue, so the contract scope is applied to the
+       two inspector roles for EVERY actor, not just customer ones.
+
+    2. An external account should not see our internal org chart at all. For a
+       customer-side actor the list is their co-workers on shared contracts —
+       inspectors assigned to this inspection's contract — and nothing else.
+
+    Rules applied:
+      * inspector / external_inspector  -> only those holding an
+        InspectorAssignment on this inspection's contract (the same rows
+        get_inspector_scope() reads, so the list can never disagree with what
+        the assignee can actually open).
+      * director / project_manager / auditor -> org-wide, but offered ONLY to
+        our own staff. These roles carry no InspectorAssignment rows, so
+        contract-scoping them would remove them entirely and break the normal
+        "escalate to the contract manager" flow.
+      * inactive accounts are never offered.
+
+    A facility with no contract yields no contract-scoped candidates; that is
+    fail-closed and correct — an external actor then gets an empty list and can
+    only leave the issue unassigned.
+
+    Used by BOTH the offcanvas dropdown in execute() and the choices that
+    validate the POST in flag_issue(). They MUST stay identical: a value the UI
+    offers but the choices reject fails `validate_on_submit()`, and the
+    offcanvas JS treats the resulting 200 as success — the issue is silently
+    never saved (rule 60's failure mode, which is exactly what the two
+    hand-maintained lists were already doing to project_manager and auditor).
+    """
+    from app.models.inspector_assignment import InspectorAssignment
+
+    # `is_customer_account` is used here to WITHHOLD internal staff from an
+    # external account — the narrowing direction, which rule 89 permits. It
+    # must never be used to grant a customer-side account anything.
+    actor_is_external = bool(actor) and actor.is_customer_account
+
+    project_id = inspection.facility.project_id if inspection.facility else None
+
+    candidates = []
+    if project_id:
+        candidates = (
+            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()
+        )
+
+    if not actor_is_external:
+        candidates += (
+            User.query
+            .filter(
+                User.role.in_(_ORG_WIDE_ASSIGNEE_ROLES),
+                User.active == True,
+            )
+            .order_by(User.full_name, User.username)
+            .all()
+        )
+
+    # The join can repeat a user across assignment rows; dedupe by id, keeping
+    # a stable display order.
+    seen, out = set(), []
+    for u in candidates:
+        if u.id not in seen:
+            seen.add(u.id)
+            out.append(u)
+    out.sort(key=lambda u: (u.display_name or '').lower())
+    return out
+
 @bp.route('/<int:inspection_id>/flag-issue', methods=['GET', 'POST'])
 @login_required
 def flag_issue(inspection_id):
@@ -936,15 +1081,16 @@ def flag_issue(inspection_id):
         return redirect(url_for('inspections.index'))
 
     form  = IssueForm()
-    staff = User.query.filter(
-        User.role.in_(['director', 'inspector', 'external_inspector'])
-    ).order_by(User.username).all()
+    # SAME list the offcanvas rendered — this is what actually validates the
+    # POST, so it is also the security boundary: a crafted assigned_to for
+    # someone outside this contract fails validation rather than being stored.
+    staff = _assignable_staff_for(inspection, current_user)
 
     form.facility_id.choices = [(inspection.facility_id, inspection.facility.name)]
-    # MT-15: suffix external (customer / third-party) inspectors so whoever is
-    # triaging can see the work is going outside the company. Display only.
+    # Suffix customer-employed inspectors so whoever is triaging can see the
+    # work is going outside the company. Display only.
     form.assigned_to.choices = [(0, '— Unassigned —')] + [
-        (u.id, u.username + (' (External)' if u.is_external_inspector else ''))
+        (u.id, u.display_name + (' (Customer)' if u.is_external_inspector else ''))
         for u in staff
     ]
 
@@ -1011,6 +1157,21 @@ def flag_issue(inspection_id):
         flash('Issue logged successfully.', 'success')
         return redirect(url_for('inspections.execute', inspection_id=inspection_id))
 
+    # A failed POST must NOT come back 200. The flag-issue offcanvas treats
+    # `res.ok` as success and reloads the page, so a 200 here means the issue
+    # is silently discarded with the user believing it was logged — the exact
+    # failure rule 60 describes. Returning 400 routes it to the JS error branch
+    # so the reason is shown and the form stays open with its input intact.
+    if request.method == 'POST':
+        if form.assigned_to.errors:
+            # Most likely an assignee outside this inspection's contract:
+            # either a stale page rendered before the assignment changed, or a
+            # crafted id. Say something actionable rather than "invalid choice".
+            flash('That person cannot be assigned to an issue on this contract. '
+                  'Reopen the panel to refresh the list.', 'danger')
+        return render_template('inspections/flag_issue.html',
+                               form=form, inspection=inspection), 400
+
     return render_template('inspections/flag_issue.html',
                            form=form, inspection=inspection)
 
@@ -1223,6 +1384,250 @@ def export_pdf(inspection_id):
 
 # ── Flag / clear follow-up required ──────────────────────────────────────────
 
+def _view_url(inspection_id):
+    """inspections.view URL that carries the list `next` through.
+
+    Actions posted from the detail page redirect back to that same page;
+    re-attaching `next` is what keeps its Back button (and the next action)
+    pointed at the filtered list the user arrived from.
+    """
+    nxt = request.form.get('next') or request.args.get('next')
+    if nxt:
+        return url_for('inspections.view', inspection_id=inspection_id, next=nxt)
+    return url_for('inspections.view', inspection_id=inspection_id)
+
+
+def _collect_inspection_photos(inspection):
+    """Relative storage keys owned by an inspection, for cleanup after delete.
+
+    Two sources: image field values inside the submitted form data (stored as
+    `uploads/...` strings in the notes JSON), and the primary photo of each
+    issue flagged during the inspection. Shared by the single and bulk delete
+    paths so they cannot drift — a miss here leaves orphaned files in storage
+    forever, and it is invisible.
+    """
+    paths = []
+    if inspection.notes:
+        try:
+            notes_data = json.loads(inspection.notes)
+            form_data  = notes_data.get('_form_data', {}) if isinstance(notes_data, dict) else {}
+            for val in form_data.values():
+                if isinstance(val, str) and val.startswith('uploads/'):
+                    paths.append(val)
+        except (json.JSONDecodeError, TypeError):
+            pass
+
+    for issue in inspection.issues.all():
+        if issue.photo_path:
+            paths.append(issue.photo_path)
+    return paths
+
+
+# ── Bulk actions from the inspections list ───────────────────────────────────
+
+@bp.route('/bulk', methods=['POST'])
+@login_required
+def bulk_action():
+    """Apply one action to every ticked inspection on the list page.
+
+    Partial-failure policy: act on every eligible row, skip the rest, and
+    report exact counts. Permission is checked per ACTION (all are
+    admin/director level except the PDF export, which anyone who can see the
+    list may run); `skipped` therefore means "this row was not in a state the
+    action applies to".
+    """
+    back   = return_url(url_for('inspections.index'))
+    action = request.form.get('action', '')
+    ids    = request.form.getlist('inspection_ids', type=int)
+
+    if not ids:
+        flash('No inspections selected.', 'warning')
+        return redirect(back)
+
+    supervisor = current_user.role in ('admin', 'director')
+    allowed = {
+        'export':          True,           # read-only, already scoped below
+        'delete':          supervisor,
+        'flag_followup':   supervisor,
+        'clear_followup':  supervisor,
+    }
+    if action not in allowed:
+        flash('Unknown bulk action.', 'danger')
+        return redirect(back)
+    if not allowed[action]:
+        flash('You do not have permission for that bulk action.', 'danger')
+        return redirect(back)
+
+    q = Inspection.query.options(
+        joinedload(Inspection.facility),
+        joinedload(Inspection.template),
+        joinedload(Inspection.inspector),
+    ).filter(Inspection.id.in_(ids))
+
+    # Re-apply the viewer's facility scope to the SELECTED ids. The list page
+    # only ever shows in-scope rows, but the id list arrives in the POST body
+    # and must not be trusted — a crafted request could otherwise name any
+    # inspection in the system.
+    if current_user.is_inspector:
+        fids = get_inspector_scope(current_user) or []
+        q = q.filter(Inspection.facility_id.in_(fids)) if fids else q.filter(False)
+    elif current_user.role == 'customer':
+        fids = get_customer_scope(current_user) or []
+        q = q.filter(Inspection.facility_id.in_(fids)) if fids else q.filter(False)
+
+    inspections = q.order_by(Inspection.inspection_date.desc()).all()
+    out_of_scope = len(ids) - len(inspections)
+    changed = 0
+    skipped = out_of_scope
+
+    # ── Export selected to PDF ───────────────────────────────────────────
+    if action == 'export':
+        if not inspections:
+            flash('None of the selected inspections are available to you.', 'warning')
+            return redirect(back)
+        from flask import Response
+        pdf = generate_inspections_list_pdf(
+            inspections,
+            f'Selected inspections ({len(inspections)})',
+        )
+        log_action(ACTION_EXPORT, 'Inspection', None, 'bulk PDF export',
+                   f'ids={[i.id for i in inspections]}')
+        return Response(
+            pdf,
+            mimetype='application/pdf',
+            headers={'Content-Disposition':
+                     'attachment; filename="selected_inspections.pdf"'},
+        )
+
+    # ── Delete ───────────────────────────────────────────────────────────
+    if action == 'delete':
+        from app.utils import storage
+        photo_paths = []
+        # Snapshot (id, label) BEFORE deleting: the objects are expired after
+        # the commit, and the audit pass must run after it. log_action()
+        # commits internally (rule 41), so auditing inside this loop would
+        # commit the deletes one at a time — and a mid-loop failure would
+        # leave rows gone with the photo cleanup below never reached.
+        deleted = []
+        for insp in inspections:
+            photo_paths.extend(_collect_inspection_photos(insp))
+            deleted.append((
+                insp.id,
+                f'{insp.template.name if insp.template else "—"} @ '
+                f'{insp.facility.name if insp.facility else "—"}',
+            ))
+            db.session.delete(insp)
+            changed += 1
+        db.session.commit()
+        for insp_id, label in deleted:
+            log_action(ACTION_DELETE, 'Inspection', insp_id, label,
+                       f'bulk deleted by {current_user.username}')
+        # Files only after the rows are gone — an orphaned file is recoverable,
+        # a deleted file belonging to a surviving row is not.
+        for rel_path in photo_paths:
+            storage.delete(rel_path)
+        _flash_bulk(changed, skipped, 'permanently deleted')
+
+    # ── Request follow-up ────────────────────────────────────────────────
+    elif action == 'flag_followup':
+        note = request.form.get('follow_up_note', '').strip() or None
+        # Only the rows this run actually flagged. Re-deriving it afterwards
+        # from `follow_up_requested_by == current_user.id` would also match
+        # inspections this same user flagged on an EARLIER run and that were
+        # skipped here as already-flagged — re-notifying their inspectors.
+        flagged = []
+        for insp in inspections:
+            # Same two guards as the single-inspection route: nothing to follow
+            # up on before submission, and a repeat request must not overwrite
+            # the pending one's note or attribution.
+            if insp.status != 'completed' or insp.follow_up_required:
+                skipped += 1
+                continue
+            insp.follow_up_required     = True
+            insp.follow_up_note         = note
+            insp.follow_up_requested_by = current_user.id
+            insp.follow_up_requested_at = now_eastern()
+            flagged.append(insp)
+            changed += 1
+        db.session.commit()
+
+        for insp in flagged:
+            body = (f'{current_user.display_name} has requested a follow-up '
+                    f're-inspection of "{insp.template.name if insp.template else "—"}" '
+                    f'at {insp.facility.name if insp.facility else "—"}.'
+                    + (f' Note: {note}' if note else ''))
+            inspector = db.session.get(User, insp.inspector_id)
+            if inspector and inspector.id != current_user.id:
+                notify(
+                    recipient     = inspector,
+                    title         = f'Follow-Up Required: Inspection #{insp.id}',
+                    body          = body,
+                    link          = url_for('inspections.view', inspection_id=insp.id),
+                    inspection_id = insp.id,
+                    event_type    = EVENT_INSPECTION_DONE,
+                    send_email    = True,
+                )
+            # Through the matrix, not straight to managers — rule 73, so
+            # per-contract recipients fire here exactly as they do for a
+            # single request.
+            notify_by_matrix(
+                event_type       = EVENT_FOLLOWUP_REQUESTED,
+                title            = f'Follow-Up Requested: Inspection #{insp.id}',
+                body             = body,
+                link             = url_for('inspections.view', inspection_id=insp.id),
+                inspection_id    = insp.id,
+                facility_id      = insp.facility_id,
+                exclude_user_ids = {current_user.id,
+                                    inspector.id if inspector else None} - {None},
+            )
+        db.session.commit()   # notify() does not commit — rule 70
+        # Audited after the commit (rule 41) — log_action commits internally.
+        for insp in flagged:
+            log_action(ACTION_UPDATE, 'Inspection', insp.id,
+                       f'{insp.template.name if insp.template else "—"}',
+                       f'bulk follow_up_required=True by {current_user.username}')
+        _flash_bulk(changed, skipped, 'flagged for follow-up',
+                    skip_reason='not submitted, or already flagged')
+
+    # ── Clear follow-up ──────────────────────────────────────────────────
+    elif action == 'clear_followup':
+        cleared = []
+        for insp in inspections:
+            if not insp.follow_up_required:
+                skipped += 1
+                continue
+            insp.follow_up_required     = False
+            insp.follow_up_note         = None
+            insp.follow_up_requested_by = None
+            insp.follow_up_requested_at = None
+            cleared.append(insp)
+            changed += 1
+        db.session.commit()
+        for insp in cleared:            # after the commit — rule 41
+            log_action(ACTION_UPDATE, 'Inspection', insp.id,
+                       f'{insp.template.name if insp.template else "—"}',
+                       f'bulk follow_up cleared by {current_user.username}')
+        _flash_bulk(changed, skipped, 'cleared of the follow-up flag',
+                    skip_reason='not flagged')
+
+    current_app.logger.info(
+        'INSPECTIONS | bulk | action=%s user=%s selected=%s changed=%s skipped=%s',
+        action, current_user.username, len(ids), changed, skipped,
+    )
+    return redirect(back)
+
+
+def _flash_bulk(changed, skipped, verb, skip_reason='no change needed'):
+    """One consistent result message for every bulk action."""
+    if not changed and not skipped:
+        flash('Nothing to do.', 'info')
+        return
+    parts = [f'{changed} inspection{"s" if changed != 1 else ""} {verb}']
+    if skipped:
+        parts.append(f'{skipped} skipped ({skip_reason})')
+    flash('. '.join(parts) + '.', 'success' if changed else 'warning')
+
+
 @bp.route('/<int:inspection_id>/flag-followup', methods=['POST'])
 @login_required
 def flag_followup(inspection_id):
@@ -1249,12 +1654,12 @@ def flag_followup(inspection_id):
         # Nothing to follow up on until the inspection has been submitted.
         if inspection.status != 'completed':
             flash('You can only request a follow-up on a completed inspection.', 'warning')
-            return redirect(url_for('inspections.view', inspection_id=inspection_id))
+            return redirect(_view_url(inspection_id))
         # Don't let a repeat request overwrite the note/attribution of a pending
         # one — the flag is already raised and staff are already on it.
         if inspection.follow_up_required:
             flash('A follow-up has already been requested for this inspection.', 'info')
-            return redirect(url_for('inspections.view', inspection_id=inspection_id))
+            return redirect(_view_url(inspection_id))
     elif current_user.role not in ('admin', 'director'):
         abort(403)
 
@@ -1316,7 +1721,7 @@ def flag_followup(inspection_id):
         flash('Follow-up re-inspection requested. The team has been notified.', 'success')
     else:
         flash('Follow-up inspection required flag set.', 'warning')
-    return redirect(url_for('inspections.view', inspection_id=inspection_id))
+    return redirect(_view_url(inspection_id))
 
 
 @bp.route('/<int:inspection_id>/clear-followup', methods=['POST'])
@@ -1339,7 +1744,7 @@ def clear_followup(inspection_id):
                f'{inspection.template.name} @ {inspection.facility.name}',
                'follow_up_required=False (cleared)')
     flash('Follow-up flag cleared.', 'success')
-    return redirect(url_for('inspections.view', inspection_id=inspection_id))
+    return redirect(_view_url(inspection_id))
 
 
 # ── Start a re-inspection (linked to parent) ──────────────────────────────────
@@ -1385,20 +1790,7 @@ def delete(inspection_id):
     template_name  = inspection.template.name
     inspector_name = inspection.inspector.username
 
-    photo_paths = []
-    if inspection.notes:
-        try:
-            notes_data = json.loads(inspection.notes)
-            form_data  = notes_data.get('_form_data', {}) if isinstance(notes_data, dict) else {}
-            for val in form_data.values():
-                if isinstance(val, str) and val.startswith('uploads/'):
-                    photo_paths.append(val)
-        except (json.JSONDecodeError, TypeError):
-            pass
-
-    for issue in inspection.issues.all():
-        if issue.photo_path:
-            photo_paths.append(issue.photo_path)
+    photo_paths = _collect_inspection_photos(inspection)
 
     db.session.delete(inspection)
     db.session.commit()
@@ -1425,4 +1817,4 @@ def delete(inspection_id):
         f'has been permanently deleted.',
         'success'
     )
-    return redirect(url_for('inspections.index'))
+    return redirect(return_url(url_for('inspections.index')))