From 97c1dec54d3f6666623b260d890fa1f06cd709ad Mon Sep 17 00:00:00 2001 From: NguyenND Date: Wed, 5 Aug 2026 12:40:31 -0400 Subject: [PATCH] Aug 5 - Update code to follow up ST - Fix pending items --- app/api/inspections.py | 49 +++ app/routes/inspection_schedules.py | 47 ++- app/templates/inspection_schedules/index.html | 39 +- tests/test_open_items.py | 350 ++++++++++++++++++ 4 files changed, 474 insertions(+), 11 deletions(-) create mode 100644 tests/test_open_items.py diff --git a/app/api/inspections.py b/app/api/inspections.py index c8c74a8..00bf6ac 100644 --- a/app/api/inspections.py +++ b/app/api/inspections.py @@ -644,6 +644,22 @@ def update_inspection(inspection_id): _sched = _resolve_schedule(_sched_id, user) if _sched is not None: inspection.inspection_schedule_id = _sched.id + # phase48 parity with the POST path: a schedule created by + # "Schedule Follow-up" knows which inspection it answers, so a draft + # that only gets its schedule attached here still becomes a properly + # linked re-inspection. Never overrides an explicit parent. + if not inspection.parent_inspection_id and _sched.parent_inspection_id: + inspection.parent_inspection_id = _sched.parent_inspection_id + logger.info('API INSPECTIONS | parent inherited from schedule on ' + 'PATCH | schedule=%s | parent=%s | user=%s', + _sched.id, _sched.parent_inspection_id, user.username) + + # An explicitly supplied parent still wins, and can be set on the draft + # before submit — mirrors the POST handler's field list. + if 'parent_inspection_id' in data: + _pid = data.get('parent_inspection_id') + if isinstance(_pid, int) and db.session.get(Inspection, _pid) is not None: + inspection.parent_inspection_id = _pid if 'status' in data: inspection.status = data['status'] @@ -670,8 +686,41 @@ def update_inspection(inspection_id): if transitioning_to_complete: _fulfill_schedule(inspection) + # ── Auto-clear follow-up flag on parent ─────────────────────────────── + # Mirrors the POST handler. This was previously MISSING here, so an iPad + # that created a follow-up as a draft and submitted it via PATCH left the + # parent flagged forever — the re-inspection happened, but the parent still + # showed "Follow-up Inspection Required" and stayed in every manager's + # outstanding list. phase48 made that a normal path, since a schedule-started + # follow-up is a draft first. + # + # Same commit-ordering rule as the POST handler: log_action() must fire AFTER + # db.session.commit(), because audit.py commits internally and would + # otherwise persist the parent's flag change before this inspection's own + # changes are committed — a partial state if the main commit then failed. + _parent_log_args = None + if transitioning_to_complete and inspection.parent_inspection_id: + parent_insp = db.session.get(Inspection, inspection.parent_inspection_id) + if parent_insp and parent_insp.follow_up_required: + parent_insp.follow_up_required = False + logger.info( + 'API INSPECTIONS | follow_up cleared on PATCH | parent_id=%s | ' + 'by_inspection_id=%s | user=%s', + parent_insp.id, inspection.id, user.username, + ) + # Snapshot label strings now — ORM objects may be expired after commit. + _parent_log_args = ( + parent_insp.id, + f'{parent_insp.template.name} @ {parent_insp.facility.name}', + f'follow_up_required=False (cleared by re-inspection ' + f'#{inspection.id} via mobile API)', + ) + db.session.commit() + if _parent_log_args: + log_action(ACTION_UPDATE, 'Inspection', *_parent_log_args) + # Notify when a draft transitions to completed — mirrors the POST handler. if transitioning_to_complete: score_val = inspection.overall_score diff --git a/app/routes/inspection_schedules.py b/app/routes/inspection_schedules.py index b054722..602bd82 100644 --- a/app/routes/inspection_schedules.py +++ b/app/routes/inspection_schedules.py @@ -340,18 +340,47 @@ def index(): if current_user.role == 'customer': abort(403) - q = InspectionSchedule.query - if current_user.role == 'inspector': - q = q.filter(InspectionSchedule.inspector_id == current_user.id) + # Two tabs (phase51): Pending = schedules still producing occurrences + # (active); Completed = closed ones — fulfilled one-times, recurring + # schedules past their end date, and manually paused ones. The partition is + # exhaustive and non-overlapping on `active`, so every schedule appears in + # exactly one tab and none can be lost; the in-row Status badge + # (Active / Ended / Paused) disambiguates the closed ones. + # + # Before this the list was a single table sorted active-first, which meant a + # tenant with years of one-time follow-ups buried the handful of live + # schedules an inspector actually needed to act on. + tab = request.args.get('tab', 'pending') + if tab not in ('pending', 'completed'): + tab = 'pending' + + base = InspectionSchedule.query + # Inspectors see only their own assignments; managers see everything. + if current_user.role == 'inspector': + base = base.filter(InspectionSchedule.inspector_id == current_user.id) + + # Counts are computed on the same scoped query, so the badges match what the + # viewer can actually open. + pending_count = base.filter(InspectionSchedule.active.is_(True)).count() + completed_count = base.filter(InspectionSchedule.active.is_(False)).count() + + if tab == 'pending': + schedules = (base.filter(InspectionSchedule.active.is_(True)) + .order_by(InspectionSchedule.next_run_at.asc(), + InspectionSchedule.name).all()) + else: + # Most recently completed first. A schedule switched off before it ever + # ran has a NULL last_completed_at and sorts last under DESC. + schedules = (base.filter(InspectionSchedule.active.is_(False)) + .order_by(InspectionSchedule.last_completed_at.desc(), + InspectionSchedule.next_run_at.desc(), + InspectionSchedule.name).all()) - schedules = q.order_by( - InspectionSchedule.active.desc(), - InspectionSchedule.next_run_at.asc(), - InspectionSchedule.name, - ).all() now = now_eastern() return render_template('inspection_schedules/index.html', - schedules=schedules, now=now, today=now.date()) + schedules=schedules, now=now, today=now.date(), + tab=tab, pending_count=pending_count, + completed_count=completed_count) def _form_choices(): diff --git a/app/templates/inspection_schedules/index.html b/app/templates/inspection_schedules/index.html index 4a8b7f3..c476059 100644 --- a/app/templates/inspection_schedules/index.html +++ b/app/templates/inspection_schedules/index.html @@ -23,8 +23,27 @@ {% endif %}

+{# Pending / Completed tabs (phase51). The partition is on `active`, so it is + exhaustive — no schedule can fall between the two tabs. #} + + {% if schedules %} -
+
@@ -155,13 +174,29 @@ {% else %} -
+
+ {% if tab == 'completed' %} + +

No completed schedules yet.

+ {% elif completed_count %} + {# Nothing pending but there IS history — offer the other tab rather than + inviting them to create a duplicate of something already closed. #} + +

Nothing pending — all schedules are complete.

+ + View Completed ({{ completed_count }}) + + {% else %}

No inspection schedules configured yet.

+ {% if current_user.role != 'inspector' %} Create First Schedule + {% endif %} + {% endif %}
{% endif %} diff --git a/tests/test_open_items.py b/tests/test_open_items.py new file mode 100644 index 0000000..0ae92e6 --- /dev/null +++ b/tests/test_open_items.py @@ -0,0 +1,350 @@ +""" +tests/test_open_items.py +------------------------- +Tests for the two open items closed in this pass. + + 1. PATCH /api/v1/inspections/ now clears the parent's + follow_up_required on the draft → completed transition, and inherits + parent_inspection_id from a schedule attached at PATCH time. Previously + only the POST path did either, so an iPad that created a follow-up as a + draft and submitted it via PATCH left the parent flagged forever. + + 2. The schedule list is split into Pending / Completed tabs, partitioned on + `active` so the split is exhaustive and nothing can be lost between them. +""" + +from datetime import timedelta + +import pytest + + +@pytest.fixture +def client(app): + app.config['DIGEST_SECRET'] = 'test-digest' + with app.app_context(): + from app import db + from app.models import inspector_assignment # noqa: F401 + db.drop_all() + db.create_all() + yield app.test_client() + db.session.remove() + + +def _today(): + from app.utils.time_utils import now_eastern + return now_eastern().date() + + +def _user(username, role): + from app import db + from app.models.user import User + u = User(username=username, full_name=username.title(), role=role, + email=f'{username}@example.com', active=True) + u.set_password('pw-correct1') + db.session.add(u) + db.session.commit() + return u + + +def _seed(): + from app import db + from app.models.facility import Facility + from app.models.inspection import InspectionTemplate + tmpl = InspectionTemplate(name='Restroom Check', active=True, + form_schema=[{'id': 'f1', 'type': 'rating_5', + 'label': 'Clean', 'row': 0, 'col': 0, + 'rowSpan': 1, 'colSpan': 1}]) + fac = Facility(name='Main Office', active=True) + db.session.add_all([tmpl, fac]) + db.session.commit() + return tmpl, fac + + +def _auth(user): + from app.api.jwt_utils import generate_access_token + return {'Authorization': f'Bearer {generate_access_token(user)}'} + + +def _completed(tmpl, fac, user, follow_up=True): + from app import db + from app.models.inspection import Inspection + from app.utils.time_utils import now_eastern + i = Inspection(template_id=tmpl.id, facility_id=fac.id, inspector_id=user.id, + inspection_date=now_eastern(), status='completed', + completed_at=now_eastern(), overall_score=60.0, + follow_up_required=follow_up) + db.session.add(i) + db.session.commit() + return i + + +def _draft(tmpl, fac, user, **kw): + from app import db + from app.models.inspection import Inspection + from app.utils.time_utils import now_eastern + i = Inspection(template_id=tmpl.id, facility_id=fac.id, inspector_id=user.id, + inspection_date=now_eastern(), status='in_progress', **kw) + db.session.add(i) + db.session.commit() + return i + + +# ── Item 1: PATCH clears the parent's follow-up flag ───────────────────────── + +def test_patch_to_completed_clears_the_parents_follow_up(client): + from app import db + from app.models.inspection import Inspection + + tmpl, fac = _seed() + user = _user('ivy', 'admin') + parent = _completed(tmpl, fac, user) + draft = _draft(tmpl, fac, user, parent_inspection_id=parent.id) + + resp = client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'status': 'completed', 'form_data': {'f1': 5}}) + assert resp.status_code == 200 + + db.session.expire_all() + assert db.session.get(Inspection, parent.id).follow_up_required is False + + +def test_patch_inherits_the_parent_from_a_schedule_attached_at_patch_time(client): + """A draft created before the schedule was known, then linked on submit.""" + from app import db + from app.models.inspection import Inspection + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + user = _user('ivy', 'admin') + parent = _completed(tmpl, fac, user) + + sched = InspectionSchedule(name='Follow-up', template_id=tmpl.id, + facility_id=fac.id, inspector_id=user.id, + frequency='once', mode='plan', active=True, + parent_inspection_id=parent.id) + db.session.add(sched) + db.session.commit() + + draft = _draft(tmpl, fac, user) # no parent yet + resp = client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'status': 'completed', 'form_data': {'f1': 5}, + 'scheduled_inspection_id': sched.id}) + assert resp.status_code == 200 + + db.session.expire_all() + assert db.session.get(Inspection, draft.id).parent_inspection_id == parent.id + assert db.session.get(Inspection, parent.id).follow_up_required is False + + +def test_patch_explicit_parent_wins_over_the_schedule(client): + from app import db + from app.models.inspection import Inspection + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + user = _user('ivy', 'admin') + parent_a = _completed(tmpl, fac, user) + parent_b = _completed(tmpl, fac, user) + + sched = InspectionSchedule(name='Follow-up', template_id=tmpl.id, + facility_id=fac.id, inspector_id=user.id, + frequency='once', mode='plan', active=True, + parent_inspection_id=parent_a.id) + db.session.add(sched) + db.session.commit() + + draft = _draft(tmpl, fac, user) + client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'status': 'completed', 'form_data': {'f1': 5}, + 'scheduled_inspection_id': sched.id, + 'parent_inspection_id': parent_b.id}) + + db.session.expire_all() + assert db.session.get(Inspection, draft.id).parent_inspection_id == parent_b.id + assert db.session.get(Inspection, parent_b.id).follow_up_required is False + # The schedule's own parent must NOT have been touched. + assert db.session.get(Inspection, parent_a.id).follow_up_required is True + + +def test_patch_that_does_not_complete_leaves_the_flag_alone(client): + """Only the draft → completed transition clears it.""" + from app import db + from app.models.inspection import Inspection + + tmpl, fac = _seed() + user = _user('ivy', 'admin') + parent = _completed(tmpl, fac, user) + draft = _draft(tmpl, fac, user, parent_inspection_id=parent.id) + + client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'notes': 'still working on it'}) + + db.session.expire_all() + assert db.session.get(Inspection, parent.id).follow_up_required is True + + +def test_patch_on_an_already_completed_inspection_is_not_a_transition(client): + """A second PATCH must not re-fire the side effects.""" + from app import db + from app.models.inspection import Inspection + + tmpl, fac = _seed() + user = _user('ivy', 'admin') + parent = _completed(tmpl, fac, user) + draft = _draft(tmpl, fac, user, parent_inspection_id=parent.id) + + client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'status': 'completed', 'form_data': {'f1': 5}}) + db.session.expire_all() + + # Re-raise the parent's flag, then PATCH the already-completed child again. + p = db.session.get(Inspection, parent.id) + p.follow_up_required = True + db.session.commit() + + client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'status': 'completed'}) + db.session.expire_all() + assert db.session.get(Inspection, parent.id).follow_up_required is True + + +def test_patch_without_a_parent_is_unaffected(client): + """The ordinary case: no parent link, nothing to clear, no error.""" + from app import db + from app.models.inspection import Inspection + + tmpl, fac = _seed() + user = _user('ivy', 'admin') + draft = _draft(tmpl, fac, user) + + resp = client.patch(f'/api/v1/inspections/{draft.id}', headers=_auth(user), + json={'status': 'completed', 'form_data': {'f1': 5}}) + assert resp.status_code == 200 + db.session.expire_all() + assert db.session.get(Inspection, draft.id).status == 'completed' + + +# ── Item 2: Pending / Completed tabs ───────────────────────────────────────── + +def _schedule(tmpl, fac, user, name, active=True, **kw): + from app import db + from app.models.inspection_schedule import InspectionSchedule + from app.utils.time_utils import now_eastern + s = InspectionSchedule(name=name, template_id=tmpl.id, facility_id=fac.id, + inspector_id=user.id, frequency='weekly', + mode='plan', active=active, + next_run_at=now_eastern() + timedelta(days=2), **kw) + db.session.add(s) + db.session.commit() + return s + + +def _login(client, user): + return client.post('/auth/login', + data={'username': user.username, 'password': 'pw-correct1'}, + follow_redirects=True) + + +def test_tabs_partition_schedules_exhaustively(client): + tmpl, fac = _seed() + mgr = _user('mona', 'admin') + _schedule(tmpl, fac, mgr, 'Live one', active=True) + _schedule(tmpl, fac, mgr, 'Closed one', active=False) + _login(client, mgr) + + pending = client.get('/inspection-schedules/?tab=pending').get_data(as_text=True) + assert 'Live one' in pending + assert 'Closed one' not in pending + + done = client.get('/inspection-schedules/?tab=completed').get_data(as_text=True) + assert 'Closed one' in done + assert 'Live one' not in done + + +def test_pending_is_the_default_tab(client): + tmpl, fac = _seed() + mgr = _user('mona', 'admin') + _schedule(tmpl, fac, mgr, 'Live one', active=True) + _schedule(tmpl, fac, mgr, 'Closed one', active=False) + _login(client, mgr) + + body = client.get('/inspection-schedules/').get_data(as_text=True) + assert 'Live one' in body + assert 'Closed one' not in body + + +def test_an_unknown_tab_falls_back_to_pending(client): + tmpl, fac = _seed() + mgr = _user('mona', 'admin') + _schedule(tmpl, fac, mgr, 'Live one', active=True) + _login(client, mgr) + + resp = client.get('/inspection-schedules/?tab=nonsense') + assert resp.status_code == 200 + assert 'Live one' in resp.get_data(as_text=True) + + +def test_counts_are_scoped_to_what_the_viewer_can_see(client): + """An inspector's badges must count their own schedules, not everyone's.""" + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + mgr = _user('mona', 'admin') + mine = _user('ivy', 'inspector') + other = _user('otto', 'inspector') + + _schedule(tmpl, fac, mine, 'Mine live', active=True) + _schedule(tmpl, fac, mine, 'Mine closed', active=False) + _schedule(tmpl, fac, other, 'Theirs live', active=True) + + _login(client, mine) + body = client.get('/inspection-schedules/?tab=pending').get_data(as_text=True) + assert 'Mine live' in body + assert 'Theirs live' not in body + + # Scoped counts: one pending, one completed — not the tenant-wide 2 and 1. + from app.models.user import User + q = InspectionSchedule.query.filter(InspectionSchedule.inspector_id == mine.id) + assert q.filter(InspectionSchedule.active.is_(True)).count() == 1 + assert q.filter(InspectionSchedule.active.is_(False)).count() == 1 + + +def test_empty_pending_with_history_points_at_the_completed_tab(client): + """Inviting someone to 'Create First Schedule' when they have history would + prompt a duplicate of something already closed.""" + tmpl, fac = _seed() + mgr = _user('mona', 'admin') + _schedule(tmpl, fac, mgr, 'Closed one', active=False) + _login(client, mgr) + + body = client.get('/inspection-schedules/?tab=pending').get_data(as_text=True) + assert 'all schedules are complete' in body.lower() + assert 'Create First Schedule' not in body + + +def test_truly_empty_state_still_offers_creation(client): + tmpl, fac = _seed() + mgr = _user('mona', 'admin') + _login(client, mgr) + + body = client.get('/inspection-schedules/').get_data(as_text=True) + assert 'Create First Schedule' in body + + +def test_inspector_is_not_offered_schedule_creation(client): + """Inspectors cannot create schedules — the route is + @project_manager_required, so the empty state must not dangle the button.""" + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + _login(client, insp) + + body = client.get('/inspection-schedules/').get_data(as_text=True) + assert 'Create First Schedule' not in body + + +def test_customers_are_still_barred(client): + tmpl, fac = _seed() + cust = _user('cara', 'customer') + _login(client, cust) + assert client.get('/inspection-schedules/?tab=completed').status_code == 403