diff --git a/app/routes/inspection_schedules.py b/app/routes/inspection_schedules.py index 65184e8..178bc91 100644 --- a/app/routes/inspection_schedules.py +++ b/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', - current_user.username, inspector_id) - errors.append('That inspector does not work on your contracts.') + 'SCHED INSP | out-of-contract inspector blocked | user=%s | ' + 'inspector_id=%s | facility_id=%s', + 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(): - """Inspectors the current user may assign a schedule to. +def _inspectors_for_project(project_id): + """Inspectors assignable to a schedule on *project_id*. - A Customer Director sees only inspectors holding an InspectorAssignment on - their own contracts — their own people and ours, never another client's - Customer Inspector (rule 93). Staff see the whole active pool. + Two rules, both deliberate: + + * **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 _is_customer_director(current_user): - from app.models.inspector_assignment import InspectorAssignment - pids = _customer_project_ids() - q = (q.join(InspectorAssignment, InspectorAssignment.user_id == User.id) - .filter(InspectorAssignment.project_id.in_(pids)) - if pids else q.filter(False)) + if not project_id: + return [] + if _is_customer_director(current_user) and project_id not in _customer_project_ids(): + logger.warning( + 'SCHED INSP | out-of-scope inspector list request | user=%s | project_id=%s', + current_user.username, project_id) + 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/') +@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: diff --git a/app/templates/inspection_schedules/form.html b/app/templates/inspection_schedules/form.html index 36aacdc..49a676a 100644 --- a/app/templates/inspection_schedules/form.html +++ b/app/templates/inspection_schedules/form.html @@ -108,13 +108,19 @@
+
+ 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. +
@@ -241,7 +247,7 @@
@@ -298,21 +304,60 @@ // an empty list rather than that customer's building names. (function () { 'use strict'; - var contractSel = document.getElementById('contract_select'); - var facilitySel = document.getElementById('facility_id'); - var areaSel = document.getElementById('area_id'); + var contractSel = document.getElementById('contract_select'); + var facilitySel = document.getElementById('facility_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 = ''; facilitySel.disabled = true; loadAreas('', false); + if (inspectorSel) { + inspectorSel.innerHTML = ''; + 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 = ''; + 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 = + ''; + inspectorSel.disabled = false; + return; + } + inspectorSel.innerHTML = ''; + 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 = ''; + }); } function loadAreas(facilityId, keepSelection) { @@ -356,8 +401,12 @@ } contractSel.addEventListener('change', function () { - if (this.value) { loadFacilities(this.value, null); } - else { setPlaceholder(); } + if (this.value) { + 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(); }