Aug 5 - Update code to follow up ST - Fix pending items

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

app/routes/inspection_schedules.py

@@ -340,18 +340,47 @@ def index():
     if current_user.role == 'customer':
         abort(403)
 
-    q = InspectionSchedule.query
+    # Two tabs (phase51): Pending = schedules still producing occurrences
-    if current_user.role == 'inspector':
+    # (active); Completed = closed ones — fulfilled one-times, recurring
-        q = q.filter(InspectionSchedule.inspector_id == current_user.id)
+    # 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():

app/templates/inspection_schedules/index.html

@@ -23,8 +23,27 @@
   {% endif %}
 </p>
 
+{# Pending / Completed tabs (phase51). The partition is on `active`, so it is
+   exhaustive — no schedule can fall between the two tabs. #}
+<ul class="nav nav-tabs mb-0">
+  <li class="nav-item">
+    <a class="nav-link {{ 'active' if tab == 'pending' }}"
+       href="{{ url_for('inspection_schedules.index', tab='pending') }}">
+      <i class="bi bi-hourglass-split"></i> Pending
+      <span class="badge rounded-pill bg-{{ 'primary' if tab == 'pending' else 'secondary' }} ms-1">{{ pending_count }}</span>
+    </a>
+  </li>
+  <li class="nav-item">
+    <a class="nav-link {{ 'active' if tab == 'completed' }}"
+       href="{{ url_for('inspection_schedules.index', tab='completed') }}">
+      <i class="bi bi-check2-circle"></i> Completed
+      <span class="badge rounded-pill bg-{{ 'primary' if tab == 'completed' else 'secondary' }} ms-1">{{ completed_count }}</span>
+    </a>
+  </li>
+</ul>
+
 {% if schedules %}
-<div class="card shadow-sm">
+<div class="card shadow-sm border-top-0 rounded-top-0">
   <div class="card-body p-0">
     <div class="table-responsive">
       <table class="table table-hover mb-0 align-middle">
@@ -155,13 +174,29 @@
   </div>
 </div>
 {% else %}
-<div class="card shadow-sm">
+<div class="card shadow-sm border-top-0 rounded-top-0">
   <div class="card-body text-center py-5 text-muted">
+    {% if tab == 'completed' %}
+    <i class="bi bi-check2-circle fs-1 d-block mb-3 opacity-25"></i>
+    <p class="mb-0">No completed schedules yet.</p>
+    {% 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. #}
+    <i class="bi bi-calendar-check fs-1 d-block mb-3 opacity-25"></i>
+    <p class="mb-3">Nothing pending — all schedules are complete.</p>
+    <a href="{{ url_for('inspection_schedules.index', tab='completed') }}"
+       class="btn btn-outline-secondary">
+      <i class="bi bi-check2-circle"></i> View Completed ({{ completed_count }})
+    </a>
+    {% else %}
     <i class="bi bi-calendar-x fs-1 d-block mb-3 opacity-25"></i>
     <p class="mb-3">No inspection schedules configured yet.</p>
+    {% if current_user.role != 'inspector' %}
     <a href="{{ url_for('inspection_schedules.create') }}" class="btn btn-primary">
       <i class="bi bi-plus-circle"></i> Create First Schedule
     </a>
+    {% endif %}
+    {% endif %}
   </div>
 </div>
 {% endif %}

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/<id> 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