From a4a8d848011b668b41e877a302b99d246d4c62bf Mon Sep 17 00:00:00 2001 From: NguyenND Date: Wed, 19 Aug 2026 16:26:32 -0400 Subject: [PATCH] Aug 19 - Re-evaluation --- app/routes/inspection_schedules.py | 53 +++++++++++++++++--- app/templates/inspection_schedules/form.html | 14 +++--- tests/test_external_inspector.py | 4 +- tests/test_ui_theme.py | 2 +- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/app/routes/inspection_schedules.py b/app/routes/inspection_schedules.py index 178bc91..32ebeb6 100644 --- a/app/routes/inspection_schedules.py +++ b/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/') @@ -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 diff --git a/app/templates/inspection_schedules/form.html b/app/templates/inspection_schedules/form.html index 49a676a..cea7b75 100644 --- a/app/templates/inspection_schedules/form.html +++ b/app/templates/inspection_schedules/form.html @@ -116,10 +116,11 @@ {% endfor %}
- 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.
@@ -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 = - ''; + ''; inspectorSel.disabled = false; return; } diff --git a/tests/test_external_inspector.py b/tests/test_external_inspector.py index bff3c11..91be4e3 100644 --- a/tests/test_external_inspector.py +++ b/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' diff --git a/tests/test_ui_theme.py b/tests/test_ui_theme.py index 3cbf382..c550d20 100644 --- a/tests/test_ui_theme.py +++ b/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