Aug 19 - Update New Schedule assignment scoped to contract inspector only

app/routes/inspection_schedules.py

@@ -188,15 +188,6 @@ def _end_date_errors(sched, form, frequency):
     return errors
 
 
-def _active_inspectors():
-    """Users who can be assigned inspections (inspector-capable roles)."""
-    return User.query.filter(
-        User.active.is_(True),
-        User.role.in_(['inspector', 'external_inspector', 'project_manager',
-                       'director', 'admin']),
-    ).order_by(User.full_name, User.username).all()
-
-
 # ── Email "Confirm receipt" one-click token (phase50) ─────────────────────────
 # A signed, STATELESS token — no DB column, nothing to clean up — lets the
 # assigned inspector confirm receipt straight from the assignment email without
@@ -427,11 +418,34 @@ def _scope_errors(template_id, facility_id, inspector_id):
                 current_user.username, facility_id)
             errors.append('That facility is not one of yours. '
                           'Choose a facility from your contracts.')
-        if inspector_id and inspector_id not in {u.id for u in _schedulable_inspectors()}:
+    # The assignee must hold an InspectorAssignment on the chosen facility's
+    # contract — the same rule that built the dropdown. Enforced for EVERY
+    # role: the dropdown is a UI hint, this is the boundary, and a stale page
+    # (or a crafted POST) must not slip an out-of-contract assignee through.
+    if inspector_id:
+        allowed = {u.id for u in _inspectors_for_project(
+            facility.project_id if facility else None)}
+        if inspector_id not in allowed:
             logger.warning(
-                'SCHED INSP | out-of-scope inspector blocked | user=%s | user_id=%s',
+                'SCHED INSP | out-of-contract inspector blocked | user=%s | '
-                current_user.username, inspector_id)
+                'inspector_id=%s | facility_id=%s',
-            errors.append('That inspector does not work on your contracts.')
+                current_user.username, inspector_id, facility_id)
+            who = db.session.get(User, inspector_id)
+            name = who.display_name if who else 'That person'
+            contract = (facility.project.name
+                        if facility is not None and facility.project else None)
+            if contract:
+                # Name the fix: an empty list here usually means the contract
+                # simply has no inspectors assigned yet, which is a setup step,
+                # not a mistake in this form.
+                errors.append(
+                    f'{name} is not assigned to {contract}. Choose an inspector '
+                    f'who works on that contract, or assign them to it first '
+                    f'(Admin \u2192 Users \u2192 Assign Contracts).')
+            else:
+                errors.append(
+                    'That facility is not on a contract, so no inspector can be '
+                    'assigned to work there. Put the facility on a contract first.')
 
     template = db.session.get(InspectionTemplate, template_id) if template_id else None
     if template is not None and facility is not None:
@@ -442,22 +456,48 @@ def _scope_errors(template_id, facility_id, inspector_id):
     return errors
 
 
-def _schedulable_inspectors():
+def _inspectors_for_project(project_id):
-    """Inspectors the current user may assign a schedule to.
+    """Inspectors assignable to a schedule on *project_id*.
 
-    A Customer Director sees only inspectors holding an InspectorAssignment on
+    Two rules, both deliberate:
-    their own contracts — their own people and ours, never another client's
+
-    Customer Inspector (rule 93). Staff see the whole active pool.
+    * **Inspector roles only.** admin / director / project_manager are NOT
+      offered even though they can open any inspection. A schedule names the
+      person who must go and do the work, and a manager who intends to do it
+      themselves holds an InspectorAssignment like anyone else. Offering the
+      whole staff list made the dropdown a roster of the company and invited
+      assigning work to someone who never inspects.
+    * **Scoped to the contract**, via the same InspectorAssignment rows
+      get_inspector_scope() reads — so whoever is offered can actually open
+      what they are given, and one customer's inspector can never be handed
+      another customer's building (rule 93, the flag-issue leak, in the
+      scheduling form).
+
+    Returns [] for a facility with no contract: fail-closed, and the caller
+    turns that into an actionable message rather than an empty dropdown.
+
+    A Customer Director asking for a contract that is not theirs also gets [] —
+    the contract selector cannot name one, but the AJAX endpoint takes an id
+    from the client.
     """
-    q = User.query.filter(User.role.in_(User.INSPECTOR_ROLES), User.active == True)
+    if not project_id:
-    if _is_customer_director(current_user):
+        return []
-        from app.models.inspector_assignment import InspectorAssignment
+    if _is_customer_director(current_user) and project_id not in _customer_project_ids():
-        pids = _customer_project_ids()
+        logger.warning(
-        q = (q.join(InspectorAssignment, InspectorAssignment.user_id == User.id)
+            'SCHED INSP | out-of-scope inspector list request | user=%s | project_id=%s',
-              .filter(InspectorAssignment.project_id.in_(pids))
+            current_user.username, project_id)
-             if pids else q.filter(False))
+        return []
+
+    from app.models.inspector_assignment import InspectorAssignment
+    rows = (User.query
+            .join(InspectorAssignment, InspectorAssignment.user_id == User.id)
+            .filter(User.role.in_(User.INSPECTOR_ROLES),
+                    User.active == True,
+                    InspectorAssignment.project_id == project_id)
+            .order_by(User.full_name, User.username)
+            .all())
     seen, uniq = set(), []
-    for u in q.order_by(User.username).all():
+    for u in rows:
         # The join can repeat a user across assignment rows.
         if u.id not in seen:
             seen.add(u.id)
@@ -465,6 +505,25 @@ def _schedulable_inspectors():
     return uniq
 
 
+# ── AJAX: inspectors working on a contract ──────────────────────────────────
+
+@bp.route('/inspectors-for-contract/<int:project_id>')
+@login_required
+@schedule_manager_required
+def inspectors_for_contract(project_id):
+    """Assignable inspectors for one contract — powers the form's cascade.
+
+    Same list the POST validation uses, so the two cannot drift. Returns an
+    empty list rather than 403 for a contract the caller may not see, so it
+    does not confirm whether that contract exists.
+    """
+    return jsonify([
+        {'id': u.id,
+         'name': u.display_name + (' (Customer)' if u.is_external_inspector else '')}
+        for u in _inspectors_for_project(project_id)
+    ])
+
+
 # ── CRUD ──────────────────────────────────────────────────────────────────────
 
 @bp.route('/')
@@ -556,7 +615,7 @@ def _project_for_facility(facility_id):
     return fac.project_id if fac else None
 
 
-def _form_choices():
+def _form_choices(project_id=None):
     """Lists offered on the schedule form, narrowed to the actor's scope.
 
     For a Customer Director every list is limited to their own contracts —
@@ -586,7 +645,12 @@ def _form_choices():
         templates = (InspectionTemplate.query.filter_by(active=True)
                      .order_by(InspectionTemplate.name).all())
 
-    inspectors = _schedulable_inspectors() if customer_scoped else _active_inspectors()
+    # Inspectors are scoped to the CONTRACT, so the list is empty until one is
+    # chosen; the form's JS refills it from inspectors_for_contract on every
+    # contract change. On edit and on a re-render after a validation error the
+    # contract is known here, so the saved assignee is present in the markup
+    # before that call returns.
+    inspectors = _inspectors_for_project(project_id)
     return templates, facilities, inspectors
 
 
@@ -594,9 +658,12 @@ def _form_choices():
 @login_required
 @schedule_manager_required
 def create():
-    templates, facilities, inspectors = _form_choices()
     projects            = _active_contracts()
-    selected_project_id = None
+    # On a POST the chosen facility names the contract, so the inspector list
+    # can be rebuilt for the re-render; on a fresh GET there is none yet.
+    selected_project_id = _project_for_facility(
+        request.form.get('facility_id', type=int)) if request.method == 'POST' else None
+    templates, facilities, inspectors = _form_choices(selected_project_id)
 
     if request.method == 'POST':
         name         = request.form.get('name', '').strip()
@@ -604,8 +671,6 @@ def create():
         facility_id  = request.form.get('facility_id', type=int)
         area_id      = request.form.get('area_id', type=int) or None
         inspector_id = request.form.get('inspector_id', type=int)
-        # Re-seeds the contract selector when this POST comes back invalid.
-        selected_project_id = _project_for_facility(facility_id)
         frequency    = request.form.get('frequency', 'weekly')
         mode         = request.form.get('mode', 'auto')
         notes        = request.form.get('notes', '').strip() or None
@@ -703,9 +768,13 @@ def edit(schedule_id):
         abort(404)
     if not _schedule_in_scope(schedule):
         abort(403)
-    templates, facilities, inspectors = _form_choices()
     projects            = _active_contracts()
-    selected_project_id = _project_for_facility(schedule.facility_id)
+    # A POST may be moving the schedule to another contract; the re-render must
+    # show that contract's inspectors, not the saved one's.
+    selected_project_id = (_project_for_facility(request.form.get('facility_id', type=int))
+                           if request.method == 'POST'
+                           else None) or _project_for_facility(schedule.facility_id)
+    templates, facilities, inspectors = _form_choices(selected_project_id)
 
     if request.method == 'POST':
         old_inspector_id      = schedule.inspector_id
@@ -713,8 +782,6 @@ def edit(schedule_id):
         template_id           = request.form.get('template_id', type=int)
         facility_id           = request.form.get('facility_id', type=int)
         inspector_id          = request.form.get('inspector_id', type=int)
-        selected_project_id   = (_project_for_facility(facility_id)
-                                 or selected_project_id)
         frequency             = request.form.get('frequency', schedule.frequency)
 
         if frequency not in _FREQUENCIES:

app/templates/inspection_schedules/form.html

@@ -108,13 +108,19 @@
           <div class="mb-3">
             <label class="form-label fw-semibold" for="inspector_id">Assign to inspector</label>
             <select name="inspector_id" id="inspector_id" class="form-select" required>
-              <option value="">— Choose an inspector —</option>
+              <option value="">— Select a Contract first —</option>
               {% for u in inspectors %}
               <option value="{{ u.id }}"
                 {{ 'selected' if v_inspector == u.id }}>
                 {{ u.display_name }}{{ ' (Customer)' if u.is_external_inspector }}</option>
               {% endfor %}
             </select>
+            <div class="form-text">
+              Only inspectors assigned to this contract — the person named here
+              has to be able to open the inspection. Managers are not listed: a
+              manager who will do the work holds a contract assignment like
+              anyone else.
+            </div>
           </div>
 
           <div class="row">
@@ -241,7 +247,7 @@
 
           <div class="mb-3">
             <label class="form-label fw-semibold" for="notes">
-              Notes for the inspector <span class="text-muted small fw-normal">(optional)</span>
+              Instructions for the inspector <span class="text-muted small fw-normal">(optional)</span>
             </label>
             <textarea name="notes" id="notes" class="form-control" rows="3"
                       placeholder="e.g. Front lobby carpet needs extra attention. Check loading dock after 3 PM — key is at the front desk.">{{ v_notes }}</textarea>
@@ -298,21 +304,60 @@
 // an empty list rather than that customer's building names.
 (function () {
   'use strict';
-  var contractSel = document.getElementById('contract_select');
+  var contractSel  = document.getElementById('contract_select');
-  var facilitySel = document.getElementById('facility_id');
+  var facilitySel  = document.getElementById('facility_id');
-  var areaSel     = document.getElementById('area_id');
+  var areaSel      = document.getElementById('area_id');
+  var inspectorSel = document.getElementById('inspector_id');
   if (!contractSel || !facilitySel) { return; }
 
   var FACILITIES_URL = '{{ url_for("inspections.facilities_for_project", project_id=0) }}'.replace('/0', '/');
+  var INSPECTORS_URL = '{{ url_for("inspection_schedules.inspectors_for_contract", project_id=0) }}'.replace('/0', '/');
   var AREAS_URL      = '{{ url_for("inspections.areas_for_facility", facility_id=0) }}'.replace('/0', '/');
   var preProjectId   = {{ selected_project_id | tojson }};
   var preFacilityId  = {{ v_facility | tojson }};
   var preAreaId      = {{ v_area | tojson }};
+  var preInspectorId = {{ v_inspector | tojson }};
 
   function setPlaceholder() {
     facilitySel.innerHTML = '<option value="">— Select a Contract first —</option>';
     facilitySel.disabled  = true;
     loadAreas('', false);
+    if (inspectorSel) {
+      inspectorSel.innerHTML = '<option value="">— Select a Contract first —</option>';
+      inspectorSel.disabled  = true;
+    }
+  }
+
+  // Inspectors come from the CONTRACT, not the facility: assignment rows are
+  // per contract. The server re-checks the chosen id against this same list.
+  function loadInspectors(projectId, restoreInspectorId) {
+    if (!inspectorSel) { return; }
+    inspectorSel.disabled  = true;
+    inspectorSel.innerHTML = '<option value="">Loading…</option>';
+    fetch(INSPECTORS_URL + projectId)
+      .then(function (r) { return r.json(); })
+      .then(function (data) {
+        if (!data.length) {
+          // Not an error state to hide: the contract has nobody assigned yet,
+          // and saving will fail until someone is. Say so here.
+          inspectorSel.innerHTML =
+            '<option value="">— No inspectors assigned to this contract —</option>';
+          inspectorSel.disabled = false;
+          return;
+        }
+        inspectorSel.innerHTML = '<option value="">— Choose an inspector —</option>';
+        data.forEach(function (u) {
+          var opt = document.createElement('option');
+          opt.value = u.id;
+          opt.textContent = u.name;
+          if (restoreInspectorId && u.id === restoreInspectorId) { opt.selected = true; }
+          inspectorSel.appendChild(opt);
+        });
+        inspectorSel.disabled = false;
+      })
+      .catch(function () {
+        inspectorSel.innerHTML = '<option value="">Could not load inspectors</option>';
+      });
   }
 
   function loadAreas(facilityId, keepSelection) {
@@ -356,8 +401,12 @@
   }
 
   contractSel.addEventListener('change', function () {
-    if (this.value) { loadFacilities(this.value, null); }
+    if (this.value) {
-    else { setPlaceholder(); }
+      loadFacilities(this.value, null);
+      loadInspectors(this.value, null);
+    } else {
+      setPlaceholder();
+    }
   });
 
   facilitySel.addEventListener('change', function () {
@@ -369,11 +418,13 @@
   if (preProjectId) {
     contractSel.value = String(preProjectId);
     loadFacilities(preProjectId, preFacilityId);
+    loadInspectors(preProjectId, preInspectorId);
   } else if (preFacilityId) {
     // Facility on no contract (or one the selector cannot name): keep the
     // server-rendered options and the current choice rather than clearing it.
     facilitySel.disabled = false;
     loadAreas(String(preFacilityId), true);
+    if (inspectorSel) { inspectorSel.disabled = false; }
   } else {
     setPlaceholder();
   }