Aug 19 - Re-evaluation

app/routes/inspection_schedules.py

@@ -423,7 +423,7 @@ def _scope_errors(template_id, facility_id, inspector_id):
     # 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(
+        allowed = {u.id for u in _assignable_inspectors(
             facility.project_id if facility else None)}
         if inspector_id not in allowed:
             logger.warning(
@@ -435,17 +435,15 @@ def _scope_errors(template_id, facility_id, inspector_id):
             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:
+                # Only reachable for a Customer Director (staff get the
+                # fallback pool above), whose scope IS the contract.
                 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.')
+                    'That inspector does not work on your contracts.')
 
     template = db.session.get(InspectionTemplate, template_id) if template_id else None
     if template is not None and facility is not None:
@@ -505,6 +503,45 @@ def _inspectors_for_project(project_id):
     return uniq
 
 
+def _all_active_inspectors():
+    """Every active inspector, either role. Never managers (see below)."""
+    return (User.query
+            .filter(User.role.in_(User.INSPECTOR_ROLES), User.active == True)
+            .order_by(User.full_name, User.username)
+            .all())
+
+
+def _assignable_inspectors(project_id):
+    """Who may be assigned a schedule at a facility on *project_id*.
+
+    Contract-scoped, with ONE fallback: when the contract has no inspectors
+    assigned — or the facility is on no contract at all — our own staff get the
+    full active inspector pool instead of an empty list.
+
+    The fallback exists because the strict rule alone makes scheduling
+    impossible until someone wires up InspectorAssignment rows, and it fails
+    with an error about a screen the person may not have thought about. A
+    facility legitimately has no contract (`facilities.project_id` is nullable),
+    and that must not become "no inspections can be planned here".
+
+    It is deliberately NOT offered to a Customer Director: for them the contract
+    boundary is a confidentiality boundary, and widening it on a setup gap would
+    hand one customer another customer's inspectors — the exact leak this
+    scoping closes (rule 93). They get the empty list and an actionable message.
+
+    Managers (admin / director / project_manager) are never in either list. A
+    schedule names whoever must go and do the work; a manager who intends to do
+    it holds a contract assignment like anyone else.
+    """
+    scoped = _inspectors_for_project(project_id)
+    if scoped or _is_customer_director(current_user):
+        return scoped
+    logger.info(
+        'SCHED INSP | no inspectors assigned to contract %s — falling back to '
+        'the full inspector pool for %s', project_id, current_user.username)
+    return _all_active_inspectors()
+
+
 # ── AJAX: inspectors working on a contract ──────────────────────────────────
 
 @bp.route('/inspectors-for-contract/<int:project_id>')
@@ -520,7 +557,7 @@ def inspectors_for_contract(project_id):
     return jsonify([
         {'id': u.id,
          'name': u.display_name + (' (Customer)' if u.is_external_inspector else '')}
-        for u in _inspectors_for_project(project_id)
+        for u in _assignable_inspectors(project_id)
     ])
 
 
@@ -650,7 +687,7 @@ def _form_choices(project_id=None):
     # 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)
+    inspectors = _assignable_inspectors(project_id)
     return templates, facilities, inspectors
 
 

app/templates/inspection_schedules/form.html

@@ -116,10 +116,11 @@
               {% 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
+              Inspectors assigned to this contract — the person named here has
+              to be able to open the inspection. Managers are never listed: a
               manager who will do the work holds a contract assignment like
-              anyone else.
+              anyone else. If the contract has nobody assigned yet, every
+              inspector is offered so the schedule is not blocked.
             </div>
           </div>
 
@@ -338,10 +339,11 @@
       .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.
+          // Only reachable for a Customer Director: our own staff fall back to
+          // the full inspector pool server-side. Say what is wrong rather than
+          // leaving an empty dropdown.
           inspectorSel.innerHTML =
-            '<option value="">— No inspectors assigned to this contract —</option>';
+            '<option value="">— No inspectors on this contract —</option>';
           inspectorSel.disabled = false;
           return;
         }

tests/test_external_inspector.py

@@ -116,7 +116,9 @@ def test_is_external_inspector_distinguishes_the_two(client):
 
 def test_role_label_renders_display_name(client):
     env = _seed()
-    assert env['external'].role_label == 'External Inspector'
+    # phase51 renamed the LABEL only — the role value is still
+    # 'external_inspector' (see User.ROLE_LABELS).
+    assert env['external'].role_label == 'Customer Inspector'
     assert env['internal'].role_label == 'Inspector'
 
 

tests/test_ui_theme.py

@@ -257,4 +257,4 @@ def test_theme_votes_renders_for_admin(client):
     assert resp.status_code == 200
     body = resp.get_data(as_text=True)
     # MT-15's ROLE_LABELS must be used for the raw group_by role strings.
-    assert 'External Inspector' in body
+    assert 'Customer Inspector' in body