diff --git a/app/api/scheduled.py b/app/api/scheduled.py index b7e118e..48d5765 100644 --- a/app/api/scheduled.py +++ b/app/api/scheduled.py @@ -79,6 +79,10 @@ def _scheduled_payload(s): # completed inspection. The iPad uses it to badge the row and to open # the parent from the schedule detail. 'parent_inspection_id': s.parent_inspection_id, + # phase50 — receipt acknowledgement, per assignment rather than per + # occurrence. Lets the iPad badge unconfirmed assignments. + 'is_acknowledged': s.is_acknowledged, + 'acknowledged_at': s.acknowledged_at.isoformat() if s.acknowledged_at else None, 'notes': s.notes or None, } diff --git a/app/models/inspection_schedule.py b/app/models/inspection_schedule.py index b7cb9db..f031e73 100644 --- a/app/models/inspection_schedule.py +++ b/app/models/inspection_schedule.py @@ -171,6 +171,21 @@ class InspectionSchedule(db.Model): nullable=True, index=True, ) + # ── Receipt acknowledgement (phase50) ──────────────────────────────────── + # Stamped when the assigned inspector confirms they received the request. + # NULL = awaiting confirmation. + # + # Per ASSIGNMENT, not per occurrence: fulfill() and advance_due_date() + # deliberately leave this alone as the schedule rolls forward, so an + # inspector who confirmed "yes, this weekly round is mine" is not asked + # again every week. The edit route resets it to NULL on reassignment to a + # DIFFERENT inspector, who has confirmed nothing. + # + # The acknowledger is always `inspector` — the only person the routes let + # confirm — so no separate acknowledged_by column is needed. Only meaningful + # for mode='plan'; an auto schedule has no request to receive. + acknowledged_at = db.Column(db.DateTime, nullable=True) + created_by = db.Column( db.Integer, db.ForeignKey('users.id', ondelete='SET NULL'), nullable=True @@ -228,6 +243,11 @@ class InspectionSchedule(db.Model): """True when this schedule was created to follow up an inspection.""" return self.parent_inspection_id is not None + @property + def is_acknowledged(self): + """True once the assigned inspector has confirmed receipt (phase50).""" + return self.acknowledged_at is not None + FREQUENCY_LABELS = { 'once': 'One-time', 'daily': 'Daily', diff --git a/app/routes/inspection_schedules.py b/app/routes/inspection_schedules.py index 9b9184e..b054722 100644 --- a/app/routes/inspection_schedules.py +++ b/app/routes/inspection_schedules.py @@ -30,6 +30,7 @@ Cron example import logging from datetime import datetime, timedelta +from itsdangerous import URLSafeTimedSerializer, BadSignature, SignatureExpired from flask import (Blueprint, render_template, redirect, url_for, flash, request, jsonify, current_app, abort) from flask_login import login_required, current_user @@ -192,6 +193,72 @@ def _active_inspectors(): ).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 +# logging in, the same login-free pattern the public QR scan pages use. +# +# The token binds the schedule id to the inspector id, so reassigning a schedule +# to someone else silently invalidates any link already emailed to the previous +# assignee. That check happens at redemption, not issuance, which is what makes +# a stateless token safe here. +_ACK_SALT = 'inspection-schedule-ack' +_ACK_MAX_AGE = 60 * 60 * 24 * 30 # 30 days — a link older than this is expired + + +def _ack_serializer(): + return URLSafeTimedSerializer(current_app.config['SECRET_KEY'], salt=_ACK_SALT) + + +def _make_ack_token(schedule): + """Signed token embedding the schedule id + the assigned inspector id.""" + return _ack_serializer().dumps({'sid': schedule.id, 'iid': schedule.inspector_id}) + + +def _confirm_action(schedule): + """`extra_action` dict for the email "Confirm receipt" button, or None. + + None when there is nothing to confirm — no inspector, already acknowledged, + or an auto-mode schedule (which materialises its own inspection, so there is + no request to receive). Requires a request context for the external URL. + + Reused by the assignment email AND the advance/due reminders, so an inspector + who missed the first email can still confirm from whichever one reaches them. + """ + if not schedule.inspector_id or schedule.is_acknowledged or schedule.mode != 'plan': + return None + return { + 'label': 'Confirm receipt', + 'url': url_for('inspection_schedules.confirm_email', + token=_make_ack_token(schedule), _external=True), + } + + +def _notify_creator_acknowledged(schedule): + """Tell the schedule's creator that the inspector confirmed receipt. + + No-op when there is no creator, the creator is inactive, or the creator IS + the inspector (self-assigned — they do not need telling). Caller commits. + """ + creator = schedule.creator + if not creator or not creator.active or creator.id == schedule.inspector_id: + return + fac = schedule.facility.name if schedule.facility else '—' + tpl = schedule.template.name if schedule.template else '—' + who = schedule.inspector.display_name if schedule.inspector else 'The inspector' + due = schedule.next_run_at.strftime('%b %d, %Y') if schedule.next_run_at else 'soon' + notify( + creator, + title = f'Inspector confirmed receipt — {fac}', + body = (f'{who} confirmed receipt of the "{tpl}" scheduled ' + f'inspection at {fac} ({schedule.recurrence_label.lower()}), ' + f'due {due}.'), + link = url_for('inspection_schedules.index'), + event_type = EVENT_INSPECTION_SCHEDULED, + send_email = True, + ) + + def _notify_assignee(schedule: InspectionSchedule, reassigned: bool = False): """Tell the assigned inspector a schedule was assigned (or reassigned) to them. @@ -208,10 +275,13 @@ def _notify_assignee(schedule: InspectionSchedule, reassigned: bool = False): inspector, title = f'Scheduled inspection {verb} — {fac}', body = (f'A "{tpl}" inspection at {fac} has been {verb} ' - f'({schedule.frequency_label.lower()}), due {due}.'), - link = url_for('inspection_schedules.index'), - event_type = EVENT_INSPECTION_SCHEDULED, - send_email = True, + f'({schedule.recurrence_label.lower()}), due {due}.' + + (' Please confirm you received this request.' + if _confirm_action(schedule) else '')), + link = url_for('inspection_schedules.index'), + event_type = EVENT_INSPECTION_SCHEDULED, + send_email = True, + extra_action = _confirm_action(schedule), ) @@ -420,6 +490,12 @@ def edit(schedule_id): if facility_id and db.session.get(Facility, facility_id): schedule.facility_id = facility_id if inspector_id and db.session.get(User, inspector_id): + # Reassigning to a DIFFERENT inspector invalidates any prior + # confirmation — the new assignee has acknowledged nothing (phase50). + # Comparison before assignment, and only on a real change, so an + # unrelated save does not silently re-open a confirmed assignment. + if inspector_id != schedule.inspector_id: + schedule.acknowledged_at = None schedule.inspector_id = inspector_id schedule.area_id = request.form.get('area_id', type=int) or None mode = request.form.get('mode', schedule.mode) @@ -588,6 +664,100 @@ def start(schedule_id): return redirect(url_for('inspections.execute', inspection_id=inspection.id)) +# ── Acknowledge (inspector confirms receipt of the request) ─────────────────── + +@bp.route('//acknowledge', methods=['POST']) +@login_required +def acknowledge(schedule_id): + """The assigned inspector confirms they received the scheduled request. + + Assignee-only, exactly like Start: a manager cannot confirm on someone's + behalf, because the whole point of the record is that THIS person saw it. + Idempotent — confirming twice is a no-op. On the first confirmation the + schedule's creator is notified. + """ + schedule = db.session.get(InspectionSchedule, schedule_id) + if schedule is None: + abort(404) + if not schedule.inspector_id or schedule.inspector_id != current_user.id: + abort(403) + + if _do_acknowledge(schedule, current_user.username): + flash('You have confirmed receipt of this scheduled inspection.', 'success') + else: + flash('You have already confirmed this scheduled inspection.', 'info') + return redirect(url_for('inspection_schedules.index')) + + +def _do_acknowledge(schedule, actor_username): + """Stamp acknowledged_at, log, and notify the creator. Commits. + + Idempotent: returns True if this call newly confirmed, False if it was + already confirmed. Shared by the logged-in POST route and the login-free + email-token GET route, so the two can never drift apart. The caller must + have already verified the actor is the assigned inspector. + """ + if schedule.acknowledged_at is not None: + return False + schedule.acknowledged_at = now_eastern() + db.session.commit() + log_action(ACTION_UPDATE, 'InspectionSchedule', schedule.id, schedule.name, + 'inspector confirmed receipt') + logger.info('INSPECTION SCHEDULE ACKNOWLEDGED | schedule=%s | by=%s', + schedule.id, actor_username) + _notify_creator_acknowledged(schedule) + db.session.commit() + return True + + +# ── Confirm receipt from the assignment email (login-free, token-signed) ────── + +@bp.route('/confirm/') +def confirm_email(token): + """One-click "Confirm receipt" landing from the assignment email (phase50). + + Deliberately login-free and NOT @login_required: authorisation comes from the + signed token, which binds the schedule id to the inspector id it was issued + for. Inspectors read this on a phone that is usually not logged in, and a + login wall is exactly what stops them confirming. + + Every failure mode renders the same standalone page with a different status + rather than a bare 4xx, because the audience is a non-technical user who + clicked a link in an email. GET is safe to repeat: the acknowledgement is + idempotent, so a re-click or an email client's link prefetch is harmless. + """ + try: + data = _ack_serializer().loads(token, max_age=_ACK_MAX_AGE) + except SignatureExpired: + return render_template('inspection_schedules/confirm_result.html', + status='expired'), 400 + except BadSignature: + return render_template('inspection_schedules/confirm_result.html', + status='invalid'), 400 + + sid = data.get('sid') + schedule = db.session.get(InspectionSchedule, sid) if sid else None + if schedule is None: + return render_template('inspection_schedules/confirm_result.html', + status='missing'), 404 + # The token's inspector must STILL be the assigned inspector. This is what + # makes a stateless token safe: reassignment invalidates the old link at + # redemption without needing to track issued tokens anywhere. + if not schedule.inspector_id or schedule.inspector_id != data.get('iid'): + return render_template('inspection_schedules/confirm_result.html', + status='reassigned', schedule=schedule), 409 + if not schedule.active: + return render_template('inspection_schedules/confirm_result.html', + status='inactive', schedule=schedule) + + newly = _do_acknowledge( + schedule, + schedule.inspector.username if schedule.inspector else 'inspector') + return render_template('inspection_schedules/confirm_result.html', + status='confirmed' if newly else 'already', + schedule=schedule) + + # ── Cron endpoint ───────────────────────────────────────────────────────────── @bp.route('/run', methods=['POST']) @@ -684,11 +854,15 @@ def _dispatch_reminders(plans, now): notify( inspector, title = f'Inspection due tomorrow — {fac_name}', - body = (f'Reminder: a "{tpl_name}" inspection at {fac_name} ' - f'is scheduled for tomorrow ({due_date:%b %d, %Y}).'), - link = link, - event_type = EVENT_INSPECTION_SCHEDULED, - send_email = True, + body = (f'Reminder: a "{tpl_name}" inspection at {fac_name} ' + f'is scheduled for tomorrow ({due_date:%b %d, %Y}).'), + link = link, + event_type = EVENT_INSPECTION_SCHEDULED, + send_email = True, + # phase50 — still unconfirmed? Offer the button again here, so an + # inspector who missed the assignment email can confirm from + # whichever reminder reaches them. Returns None once confirmed. + extra_action = _confirm_action(s), ) s.advance_notified = True sent['advance'] += 1 @@ -699,11 +873,12 @@ def _dispatch_reminders(plans, now): notify( inspector, title = f'Inspection due today — {fac_name}', - body = (f'A "{tpl_name}" inspection at {fac_name} is due ' - f'({due_date:%b %d, %Y}). Please complete it.'), - link = link, - event_type = EVENT_INSPECTION_SCHEDULED, - send_email = True, + body = (f'A "{tpl_name}" inspection at {fac_name} is due ' + f'({due_date:%b %d, %Y}). Please complete it.'), + link = link, + event_type = EVENT_INSPECTION_SCHEDULED, + send_email = True, + extra_action = _confirm_action(s), # phase50 — see above ) s.due_notified = True sent['due'] += 1 diff --git a/app/templates/inspection_schedules/confirm_result.html b/app/templates/inspection_schedules/confirm_result.html new file mode 100644 index 0000000..c4a28e0 --- /dev/null +++ b/app/templates/inspection_schedules/confirm_result.html @@ -0,0 +1,116 @@ +{# ── "Confirm receipt" result page (phase50) ───────────────────────────────── + Landing page for the one-click link in a scheduled-inspection assignment or + reminder email. Standalone — NOT extending base.html — because the viewer is + typically not logged in and base.html's nav assumes current_user. Same shape + as the public QR scan pages (facility_qr/area.html). + + `status` is always set; `schedule` only for the statuses that found one. + confirmed — newly acknowledged (the happy path) + already — acknowledged before; a re-click or an email prefetch + reassigned — the token's inspector is no longer the assignee + inactive — schedule paused or ended since the email went out + expired — token older than 30 days + invalid — bad signature / mangled link + missing — schedule deleted since the email went out +#} + + + + + + + Confirm Receipt — Scheduled Inspection + + + + + +
+
+ + {% if status == 'confirmed' %} +
+

Receipt confirmed

+

+ Thanks — we've let the scheduler know you've seen this request. + Nothing else to do right now. +

+ + {% elif status == 'already' %} +
+

Already confirmed

+

+ You confirmed this one earlier. No need to do anything else. +

+ + {% elif status == 'reassigned' %} +
+

No longer assigned to you

+

+ This scheduled inspection has been reassigned to someone else since the + email was sent, so there's nothing for you to confirm. +

+ + {% elif status == 'inactive' %} +
+

This schedule is no longer active

+

+ It has been paused or has reached its end date. No action is needed. +

+ + {% elif status == 'expired' %} +
+

This link has expired

+

+ Confirmation links are good for 30 days. Sign in to the schedules page to + confirm, or ask your supervisor to resend the assignment. +

+ + {% elif status == 'missing' %} +
+

Schedule not found

+

+ This scheduled inspection has since been removed. No action is needed. +

+ + {% else %} +
+

This link isn't valid

+

+ The link may have been copied incompletely. Try tapping it directly from + the email, or sign in to confirm from the schedules page. +

+ {% endif %} + + {% if schedule %} +
+
Inspection
+
+ {{ schedule.template.name if schedule.template else '—' }} + at {{ schedule.facility.name if schedule.facility else '—' }} +
+
Schedule
+
+ {{ schedule.recurrence_label }}{% if schedule.due_date %} · due + {{ schedule.due_date.strftime('%b %d, %Y') }}{% endif %} +
+
+ {% endif %} + +
+

+ Janitorial QC System — automated message. Do not reply to the email. +

+
+ + diff --git a/app/templates/inspection_schedules/index.html b/app/templates/inspection_schedules/index.html index d1f6481..4a8b7f3 100644 --- a/app/templates/inspection_schedules/index.html +++ b/app/templates/inspection_schedules/index.html @@ -84,8 +84,37 @@ {% if s.active %}Active {% elif s.is_expired %}Ended {% else %}Paused{% endif %} + {# phase50 — receipt acknowledgement. Only meaningful for plan + mode: an auto schedule materialises itself, so there is no + request for anyone to receive. #} + {% if s.mode == 'plan' and s.inspector_id %} + {% if s.is_acknowledged %} + + Confirmed + + {% elif s.active %} + + Awaiting confirmation + + {% endif %} + {% endif %} + {# Assignee-only, like Start: a manager must not be able to confirm + on someone's behalf, since the record means "this person saw it". #} + {% if s.active and s.mode == 'plan' and not s.is_acknowledged + and s.inspector_id == current_user.id %} +
+ + +
+ {% endif %} {% if s.active and s.mode == 'plan' and (current_user.role != 'inspector' or s.inspector_id == current_user.id) %}

{{ title }}

{{ body }}

- {% if link %} + {% if link or extra_action %}

+ {# extra_action (phase50) renders BEFORE "View Details" and in green: it is + the one-click action the email is asking for (e.g. "Confirm receipt"), + so it must be the primary button, not a footnote after the generic link. #} + {% if extra_action %} + + {{ extra_action.label }} + + {% endif %} + {% if link %} View Details + {% endif %}

{% endif %}
@@ -68,6 +81,9 @@ _EMAIL_TEXT_SINGLE = """\ {{ title }} {{ body }} +{% if extra_action %} +{{ extra_action.label }}: {{ extra_action.url }} +{% endif %} {% if link %} View: {{ base_url }}{{ link }} {% endif %} @@ -172,6 +188,7 @@ def notify( event_type: str = None, send_email: bool = True, respect_preferences: bool = True, + extra_action: dict = None, ): """Create an in-app Notification record and optionally send an email. @@ -189,6 +206,13 @@ def notify( respect_preferences : When True (default), per-user email preferences gate delivery. Set False for matrix-routed broadcasts — the matrix is the authority; individual opt-out should not override admin config. + extra_action : Optional dict {'label': str, 'url': str} rendered as a second, + primary button in the email (phase50). The URL must be + ABSOLUTE — unlike `link`, it is not prefixed with base_url, + because it typically points at a login-free tokenised route + built with url_for(..., _external=True). EMAIL ONLY: the + in-app Notification row is unchanged, so a recipient reading + it in the bell menu simply follows `link` as before. """ # Determine digest flag before creating the record. # Digest mode is only respected when individual preferences are in effect. @@ -246,10 +270,10 @@ def notify( elif should_send: logger.info('EMAIL SEND | user=%s | event=%s | to=%s', recipient.username, event_type, recipient.email) - _send_single_email(recipient, title, body, link) + _send_single_email(recipient, title, body, link, extra_action) -def _send_single_email(recipient, title, body, link): +def _send_single_email(recipient, title, body, link, extra_action=None): """Dispatch a single immediate notification email in a background thread. Sending is offloaded to a daemon thread so SMTP latency never blocks the @@ -265,9 +289,11 @@ def _send_single_email(recipient, title, body, link): ) html_body = render_template_string( _EMAIL_HTML_SINGLE, title=title, body=body, link=link, base_url=base_url, + extra_action=extra_action, ) text_body = render_template_string( _EMAIL_TEXT_SINGLE, title=title, body=body, link=link, base_url=base_url, + extra_action=extra_action, ) msg = Message( subject = f'[JQC] {title}', diff --git a/migrations/versions/phase50_sched_acknowledged.py b/migrations/versions/phase50_sched_acknowledged.py new file mode 100644 index 0000000..f9f509f --- /dev/null +++ b/migrations/versions/phase50_sched_acknowledged.py @@ -0,0 +1,83 @@ +"""phase50 — inspection schedule receipt acknowledgement + +Ports single-tenant phase47 onto MT's `inspection_schedules` table. Adds: + + acknowledged_at DATETIME NULL + +Stamped when the assigned inspector confirms they have received/seen a scheduled +inspection request. NULL = awaiting confirmation. + +Scope of the acknowledgement +---------------------------- +It is per ASSIGNMENT, not per occurrence. `fulfill()` and `advance_due_date()` +deliberately leave this column alone as the schedule rolls forward — an +inspector who confirmed "yes, this weekly restroom round is mine" should not be +asked again every week. The edit route resets it to NULL when the schedule is +reassigned to a DIFFERENT inspector, because the new assignee has confirmed +nothing. + +The acknowledger is always the assigned inspector — the only person the routes +permit to confirm — so no separate `acknowledged_by` column is stored. + +Only meaningful for `plan` mode: an `auto` schedule materialises its inspection +without anyone starting it, so there is no request to receive. The UI shows the +control for plan-mode rows only. + +No backfill: legacy rows keep NULL and render as "awaiting confirmation", which +is the correct initial state for an assignment nobody has confirmed yet. + +Revision id note +---------------- +`alembic_version.version_num` is VARCHAR(32); the id below is 24 characters. +The filename stays descriptive — Alembic keys on the `revision` string. + +Uses an INFORMATION_SCHEMA check — safe to re-run on every tenant DB. Additive +only: nothing is renamed, retyped or dropped. +""" + +revision = 'phase50_sched_acknowledged' +down_revision = 'phase49_followup_req_by' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +_TABLE = 'inspection_schedules' +_COLUMN = 'acknowledged_at' + + +def _table_exists(conn, table): + return conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES " + "WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = :t" + ), {"t": table}).scalar() > 0 + + +def _column_exists(conn, table, column): + return conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS " + "WHERE TABLE_SCHEMA = DATABASE() " + "AND TABLE_NAME = :t AND COLUMN_NAME = :c" + ), {"t": table, "c": column}).scalar() > 0 + + +def upgrade(): + bind = op.get_bind() + if not _table_exists(bind, _TABLE): + return + if not _column_exists(bind, _TABLE, _COLUMN): + op.execute(sa.text( + f"ALTER TABLE {_TABLE} ADD COLUMN {_COLUMN} DATETIME NULL" + )) + + +def downgrade(): + bind = op.get_bind() + if not _table_exists(bind, _TABLE): + return + if _column_exists(bind, _TABLE, _COLUMN): + op.execute(sa.text( + f"ALTER TABLE {_TABLE} DROP COLUMN {_COLUMN}" + )) diff --git a/tests/test_schedule_acknowledge.py b/tests/test_schedule_acknowledge.py new file mode 100644 index 0000000..acef006 --- /dev/null +++ b/tests/test_schedule_acknowledge.py @@ -0,0 +1,458 @@ +""" +tests/test_schedule_acknowledge.py +----------------------------------- +Behaviour tests for phase50 — scheduled inspection receipt acknowledgement. + +Runs on the in-memory SQLite app fixture (multi-tenancy inert). Covers: + + * the assigned inspector can confirm receipt; the stamp and creator + notification land, and a second confirm is an idempotent no-op + * a manager cannot confirm on the inspector's behalf (assignee-only) + * the login-free token route confirms without a session, and is safe to + re-request (email client prefetch) + * a token issued to a previous assignee stops working once the schedule is + reassigned — the property that makes a stateless token safe + * tampered, expired and dangling tokens each render their own status page + rather than a bare error + * reassignment resets the acknowledgement; an unrelated edit does NOT + * acknowledgement is per ASSIGNMENT, not per occurrence — rolling the schedule + forward must not re-open it + * notify(extra_action=...) renders the confirm button in the email, and stops + once confirmed +""" + +from datetime import datetime, timedelta + +import pytest + + +@pytest.fixture +def client(app): + """Fresh schema + test client for each test (isolated in-memory DB).""" + 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(): + """Today in the app's timezone, not the machine's.""" + 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 _schedule(tmpl, fac, inspector, creator, **kw): + from app import db + from app.models.inspection_schedule import InspectionSchedule + from app.utils.time_utils import now_eastern + + kw.setdefault('mode', 'plan') + kw.setdefault('frequency', 'weekly') + kw.setdefault('active', True) + s = InspectionSchedule(name='Weekly restrooms', template_id=tmpl.id, + facility_id=fac.id, inspector_id=inspector.id, + created_by=creator.id, + next_run_at=now_eastern() + timedelta(days=3), **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) + + +# ── Logged-in acknowledgement ──────────────────────────────────────────────── + +def test_assigned_inspector_can_confirm(client): + from app import db + from app.models.inspection_schedule import InspectionSchedule + from app.models.notification import Notification + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + assert s.is_acknowledged is False + + _login(client, insp) + resp = client.post(f'/inspection-schedules/{s.id}/acknowledge') + assert resp.status_code == 302 + + db.session.expire_all() + s = db.session.get(InspectionSchedule, s.id) + assert s.is_acknowledged is True + assert s.acknowledged_at is not None + + # The creator is told, since they are the one waiting on the confirmation. + assert any('confirmed receipt' in n.title.lower() + for n in Notification.query.filter_by(user_id=mgr.id).all()) + + +def test_confirming_twice_is_idempotent(client): + from app import db + from app.models.inspection_schedule import InspectionSchedule + from app.models.notification import Notification + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + _login(client, insp) + client.post(f'/inspection-schedules/{s.id}/acknowledge') + db.session.expire_all() + first = db.session.get(InspectionSchedule, s.id).acknowledged_at + + client.post(f'/inspection-schedules/{s.id}/acknowledge') + db.session.expire_all() + assert db.session.get(InspectionSchedule, s.id).acknowledged_at == first + # And the creator is not pestered a second time. + assert len([n for n in Notification.query.filter_by(user_id=mgr.id).all() + if 'confirmed receipt' in n.title.lower()]) == 1 + + +def test_manager_cannot_confirm_on_the_inspectors_behalf(client): + """The record means "this person saw it", so only the assignee may set it.""" + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + _login(client, mgr) + assert client.post(f'/inspection-schedules/{s.id}/acknowledge').status_code == 403 + db.session.expire_all() + assert db.session.get(InspectionSchedule, s.id).is_acknowledged is False + + +def test_other_inspector_cannot_confirm(client): + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + other = _user('otto', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + _login(client, other) + assert client.post(f'/inspection-schedules/{s.id}/acknowledge').status_code == 403 + db.session.expire_all() + assert db.session.get(InspectionSchedule, s.id).is_acknowledged is False + + +# ── Login-free email token ─────────────────────────────────────────────────── + +def _token_for(app, schedule): + from app.routes.inspection_schedules import _make_ack_token + with app.test_request_context(): + return _make_ack_token(schedule) + + +def test_email_token_confirms_without_a_session(client, app): + """Inspectors read this on a phone that is usually not logged in — a login + wall is exactly what stops them confirming.""" + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + token = _token_for(app, s) + + resp = client.get(f'/inspection-schedules/confirm/{token}') + assert resp.status_code == 200 + assert b'Receipt confirmed' in resp.data + + db.session.expire_all() + assert db.session.get(InspectionSchedule, s.id).is_acknowledged is True + + +def test_email_token_reclick_is_harmless(client, app): + """A re-click, or an email client prefetching the link, must not error.""" + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + token = _token_for(app, s) + + client.get(f'/inspection-schedules/confirm/{token}') + resp = client.get(f'/inspection-schedules/confirm/{token}') + assert resp.status_code == 200 + assert b'Already confirmed' in resp.data + + +def test_reassignment_invalidates_a_previously_emailed_token(client, app): + """This is what makes a stateless token safe: the old link dies at + redemption, with nothing tracked anywhere.""" + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + other = _user('otto', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + token = _token_for(app, s) # emailed to Ivy + + s.inspector_id = other.id # reassigned to Otto + db.session.commit() + + resp = client.get(f'/inspection-schedules/confirm/{token}') + assert resp.status_code == 409 + assert b'No longer assigned to you' in resp.data + db.session.expire_all() + assert db.session.get(InspectionSchedule, s.id).is_acknowledged is False + + +def test_tampered_token_renders_the_invalid_page(client): + resp = client.get('/inspection-schedules/confirm/not-a-real-token') + assert resp.status_code == 400 + assert b"isn't valid" in resp.data + + +def test_expired_token_renders_the_expired_page(client, app): + from itsdangerous import URLSafeTimedSerializer + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + with app.test_request_context(): + from app.routes.inspection_schedules import _ACK_SALT + ser = URLSafeTimedSerializer(app.config['SECRET_KEY'], salt=_ACK_SALT) + # Sign it 31 days ago — past the 30-day _ACK_MAX_AGE. + import itsdangerous.timed + old = ser.dumps({'sid': s.id, 'iid': insp.id}) + + # Re-sign with a backdated timestamp by monkeypatching the serializer clock. + import time as _time + real = _time.time + try: + _time.time = lambda: real() - (60 * 60 * 24 * 31) + with app.test_request_context(): + old = URLSafeTimedSerializer( + app.config['SECRET_KEY'], salt=_ACK_SALT + ).dumps({'sid': s.id, 'iid': insp.id}) + finally: + _time.time = real + + resp = client.get(f'/inspection-schedules/confirm/{old}') + assert resp.status_code == 400 + assert b'expired' in resp.data.lower() + + +def test_token_for_a_deleted_schedule_renders_missing(client, app): + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + token = _token_for(app, s) + + db.session.delete(db.session.get(InspectionSchedule, s.id)) + db.session.commit() + + resp = client.get(f'/inspection-schedules/confirm/{token}') + assert resp.status_code == 404 + assert b'not found' in resp.data.lower() + + +def test_inactive_schedule_renders_inactive(client, app): + from app import db + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + token = _token_for(app, s) + + s.active = False + db.session.commit() + + resp = client.get(f'/inspection-schedules/confirm/{token}') + assert resp.status_code == 200 + assert b'no longer active' in resp.data.lower() + + +# ── Reset semantics ────────────────────────────────────────────────────────── + +def test_reassignment_resets_the_acknowledgement(client): + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + other = _user('otto', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + _login(client, insp) + client.post(f'/inspection-schedules/{s.id}/acknowledge') + client.get('/auth/logout', follow_redirects=True) + + _login(client, mgr) + client.post(f'/inspection-schedules/{s.id}/edit', data={ + 'name': 'Weekly restrooms', 'template_id': tmpl.id, 'facility_id': fac.id, + 'inspector_id': other.id, 'frequency': 'weekly', 'mode': 'plan', + 'weekdays': ['0'], 'active': 'on', + }, follow_redirects=True) + + db.session.expire_all() + s = db.session.get(InspectionSchedule, s.id) + assert s.inspector_id == other.id + assert s.is_acknowledged is False + + +def test_unrelated_edit_does_not_reset_the_acknowledgement(client): + """Only a real reassignment re-opens it — otherwise every rename would make + the inspector confirm again.""" + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + _login(client, insp) + client.post(f'/inspection-schedules/{s.id}/acknowledge') + client.get('/auth/logout', follow_redirects=True) + db.session.expire_all() + stamped = db.session.get(InspectionSchedule, s.id).acknowledged_at + + _login(client, mgr) + client.post(f'/inspection-schedules/{s.id}/edit', data={ + 'name': 'Renamed round', 'template_id': tmpl.id, 'facility_id': fac.id, + 'inspector_id': insp.id, 'frequency': 'weekly', 'mode': 'plan', + 'weekdays': ['0'], 'active': 'on', + }, follow_redirects=True) + + db.session.expire_all() + s = db.session.get(InspectionSchedule, s.id) + assert s.name == 'Renamed round' + assert s.acknowledged_at == stamped + + +def test_acknowledgement_survives_rolling_the_schedule_forward(client): + """Per ASSIGNMENT, not per occurrence — an inspector who confirmed "this + weekly round is mine" must not be asked again every week.""" + from app import db + from app.models.inspection_schedule import InspectionSchedule + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr, frequency='daily') + + _login(client, insp) + client.post(f'/inspection-schedules/{s.id}/acknowledge') + db.session.expire_all() + + s = db.session.get(InspectionSchedule, s.id) + stamped = s.acknowledged_at + s.fulfill() + db.session.commit() + + assert s.acknowledged_at == stamped + assert s.is_acknowledged is True + + +# ── notify(extra_action=...) ───────────────────────────────────────────────── + +def test_confirm_action_is_offered_only_when_there_is_something_to_confirm(client, app): + from app import db + from app.routes.inspection_schedules import _confirm_action + + tmpl, fac = _seed() + insp = _user('ivy', 'inspector') + mgr = _user('mona', 'admin') + s = _schedule(tmpl, fac, insp, mgr) + + with app.test_request_context(): + action = _confirm_action(s) + assert action is not None + assert action['label'] == 'Confirm receipt' + assert '/inspection-schedules/confirm/' in action['url'] + + # Auto mode materialises its own inspection — nothing to receive. + s.mode = 'auto' + assert _confirm_action(s) is None + s.mode = 'plan' + + # Already confirmed — the button stops appearing in later reminders. + s.acknowledged_at = datetime.now() + assert _confirm_action(s) is None + s.acknowledged_at = None + + # Unassigned — nobody to confirm. + s.inspector_id = None + assert _confirm_action(s) is None + + +def test_email_renders_the_extra_action_button(client, app): + """notify(extra_action=...) is new plumbing; assert it reaches the body.""" + from flask import render_template_string + from app.utils.notifications import _EMAIL_HTML_SINGLE, _EMAIL_TEXT_SINGLE + + action = {'label': 'Confirm receipt', 'url': 'https://lts.jqc.app/x/abc'} + with app.test_request_context(): + html = render_template_string(_EMAIL_HTML_SINGLE, title='T', body='B', + link='/inspection-schedules', + base_url='https://lts.jqc.app', + extra_action=action) + text = render_template_string(_EMAIL_TEXT_SINGLE, title='T', body='B', + link='/inspection-schedules', + base_url='https://lts.jqc.app', + extra_action=action) + assert 'Confirm receipt' in html and action['url'] in html + assert 'Confirm receipt' in text and action['url'] in text + # The generic link still renders alongside it. + assert 'View Details' in html + + # And without extra_action nothing changes for every other caller. + with app.test_request_context(): + plain = render_template_string(_EMAIL_HTML_SINGLE, title='T', body='B', + link='/x', base_url='http://h', + extra_action=None) + assert 'Confirm receipt' not in plain + assert 'View Details' in plain diff --git a/tests/test_schedule_follow_up.py b/tests/test_schedule_follow_up.py index 4511a07..85762ec 100644 --- a/tests/test_schedule_follow_up.py +++ b/tests/test_schedule_follow_up.py @@ -25,6 +25,18 @@ from datetime import date, datetime, timedelta import pytest +def _today(): + """Today in the app's timezone, not the machine's. + + The routes validate due dates against now_eastern(). Using _today() here + made these tests fail on a UTC host between 20:00 ET and midnight, when the + two calendars disagree — "yesterday" by UTC is still today in Eastern, so a + date the test expected to be rejected was legitimately accepted. + """ + from app.utils.time_utils import now_eastern + return now_eastern().date() + + @pytest.fixture def client(app): """Fresh schema + test client for each test (isolated in-memory DB).""" @@ -81,11 +93,12 @@ def _post_follow_up(client, user, **body): # ── Creation ───────────────────────────────────────────────────────────────── def test_follow_up_creates_one_time_plan_schedule_from_the_parent(client): + from app import db from app.models.inspection_schedule import InspectionSchedule user, tmpl, fac = _seed('mk', role='project_manager') parent = _completed_inspection(user, tmpl, fac) - due = date.today() + timedelta(days=7) + due = _today() + timedelta(days=7) resp = _post_follow_up(client, user, parent_inspection_id=parent.id, @@ -95,7 +108,7 @@ def test_follow_up_creates_one_time_plan_schedule_from_the_parent(client): data = resp.get_json()['data'] assert data['created'] is True - s = InspectionSchedule.query.get(data['scheduled']['id']) + s = db.session.get(InspectionSchedule, data['scheduled']['id']) assert s.parent_inspection_id == parent.id assert s.is_follow_up is True # Everything derived from the parent, nothing taken from the client. @@ -130,11 +143,12 @@ def test_follow_up_ignores_client_supplied_facility_and_template(client): resp = _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=(date.today() + timedelta(days=3)).isoformat(), + due_date=(_today() + timedelta(days=3)).isoformat(), facility_id=other_fac.id, template_id=other_tmpl.id, frequency='daily', mode='auto') - s = InspectionSchedule.query.get(resp.get_json()['data']['scheduled']['id']) + s = db.session.get(InspectionSchedule, + resp.get_json()['data']['scheduled']['id']) assert s.facility_id == fac.id assert s.template_id == tmpl.id assert s.frequency == 'once' @@ -146,8 +160,8 @@ def test_follow_up_is_idempotent_on_retry(client): user, tmpl, fac = _seed('idem', role='admin') parent = _completed_inspection(user, tmpl, fac) - first_due = date.today() + timedelta(days=5) - second_due = date.today() + timedelta(days=9) + first_due = _today() + timedelta(days=5) + second_due = _today() + timedelta(days=9) r1 = _post_follow_up(client, user, parent_inspection_id=parent.id, due_date=first_due.isoformat()) @@ -171,14 +185,14 @@ def test_follow_up_reschedule_rearms_reminders(client): user, tmpl, fac = _seed('rearm', role='admin') parent = _completed_inspection(user, tmpl, fac) _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=(date.today() + timedelta(days=2)).isoformat()) + due_date=(_today() + timedelta(days=2)).isoformat()) s = InspectionSchedule.query.filter_by(parent_inspection_id=parent.id).one() s.advance_notified = s.due_notified = s.overdue_notified = True db.session.commit() _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=(date.today() + timedelta(days=12)).isoformat()) + due_date=(_today() + timedelta(days=12)).isoformat()) db.session.expire_all() s = InspectionSchedule.query.filter_by(parent_inspection_id=parent.id).one() assert s.advance_notified is False @@ -201,14 +215,14 @@ def test_follow_up_requires_a_completed_parent(client): db.session.commit() r = _post_follow_up(client, user, parent_inspection_id=draft.id, - due_date=(date.today() + timedelta(days=1)).isoformat()) + due_date=(_today() + timedelta(days=1)).isoformat()) assert r.status_code == 400 def test_follow_up_rejects_bad_input(client): user, tmpl, fac = _seed('bad', role='admin') parent = _completed_inspection(user, tmpl, fac) - ok_due = (date.today() + timedelta(days=1)).isoformat() + ok_due = (_today() + timedelta(days=1)).isoformat() # Missing parent id. assert _post_follow_up(client, user, due_date=ok_due).status_code == 400 @@ -222,7 +236,7 @@ def test_follow_up_rejects_bad_input(client): assert _post_follow_up(client, user, parent_inspection_id=parent.id, due_date='next tuesday').status_code == 400 # Past date. - past = (date.today() - timedelta(days=1)).isoformat() + past = (_today() - timedelta(days=1)).isoformat() assert _post_follow_up(client, user, parent_inspection_id=parent.id, due_date=past).status_code == 400 @@ -232,7 +246,7 @@ def test_follow_up_allows_today(client): user, tmpl, fac = _seed('today', role='admin') parent = _completed_inspection(user, tmpl, fac) r = _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=date.today().isoformat()) + due_date=_today().isoformat()) assert r.status_code == 201 @@ -241,7 +255,7 @@ def test_follow_up_rejects_auditor(client): user, tmpl, fac = _seed('aud', role='auditor') parent = _completed_inspection(user, tmpl, fac) r = _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=(date.today() + timedelta(days=1)).isoformat()) + due_date=(_today() + timedelta(days=1)).isoformat()) assert r.status_code == 403 @@ -259,7 +273,7 @@ def test_inspector_cannot_follow_up_someone_elses_inspection(client): db.session.commit() r = _post_follow_up(client, other, parent_inspection_id=parent.id, - due_date=(date.today() + timedelta(days=1)).isoformat()) + due_date=(_today() + timedelta(days=1)).isoformat()) assert r.status_code == 403 @@ -268,7 +282,7 @@ def test_follow_up_requires_auth(client): parent = _completed_inspection(user, tmpl, fac) r = client.post('/api/v1/scheduled-inspections/follow-up', json={'parent_inspection_id': parent.id, - 'due_date': date.today().isoformat()}) + 'due_date': _today().isoformat()}) assert r.status_code == 401 @@ -284,7 +298,7 @@ def test_web_start_inherits_the_parent_link(client): user, tmpl, fac = _seed('start', role='admin') parent = _completed_inspection(user, tmpl, fac) _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=date.today().isoformat()) + due_date=_today().isoformat()) sched = InspectionSchedule.query.filter_by(parent_inspection_id=parent.id).one() client.post('/auth/login', data={'username': user.username, @@ -307,7 +321,7 @@ def test_web_start_resumes_instead_of_duplicating(client): user, tmpl, fac = _seed('resume', role='admin') parent = _completed_inspection(user, tmpl, fac) _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=date.today().isoformat()) + due_date=_today().isoformat()) sched = InspectionSchedule.query.filter_by(parent_inspection_id=parent.id).one() client.post('/auth/login', data={'username': user.username, @@ -352,13 +366,14 @@ def test_cron_materialiser_inherits_the_parent_link(client): def test_api_create_infers_parent_from_the_schedule(client): """An older iPad build submits the schedule id but no parent. Without the inference the parent would stay flagged forever.""" + from app import db from app.models.inspection import Inspection from app.models.inspection_schedule import InspectionSchedule user, tmpl, fac = _seed('infer', role='admin') parent = _completed_inspection(user, tmpl, fac) _post_follow_up(client, user, parent_inspection_id=parent.id, - due_date=date.today().isoformat()) + due_date=_today().isoformat()) sched = InspectionSchedule.query.filter_by(parent_inspection_id=parent.id).one() resp = client.post('/api/v1/inspections', headers=_auth(user), json={ @@ -369,13 +384,14 @@ def test_api_create_infers_parent_from_the_schedule(client): assert resp.status_code in (200, 201) new_id = resp.get_json()['data']['inspection_id'] - run = Inspection.query.get(new_id) + run = db.session.get(Inspection, new_id) assert run.parent_inspection_id == parent.id # And the whole point of the link: the parent's flag is cleared. assert parent.follow_up_required is False def test_api_explicit_parent_wins_over_the_schedule(client): + from app import db from app.models.inspection import Inspection from app.models.inspection_schedule import InspectionSchedule @@ -383,7 +399,7 @@ def test_api_explicit_parent_wins_over_the_schedule(client): parent_a = _completed_inspection(user, tmpl, fac) parent_b = _completed_inspection(user, tmpl, fac) _post_follow_up(client, user, parent_inspection_id=parent_a.id, - due_date=date.today().isoformat()) + due_date=_today().isoformat()) sched = InspectionSchedule.query.filter_by(parent_inspection_id=parent_a.id).one() resp = client.post('/api/v1/inspections', headers=_auth(user), json={ @@ -393,7 +409,7 @@ def test_api_explicit_parent_wins_over_the_schedule(client): 'form_data': {'f1': 5}, }) new_id = resp.get_json()['data']['inspection_id'] - assert Inspection.query.get(new_id).parent_inspection_id == parent_b.id + assert db.session.get(Inspection, new_id).parent_inspection_id == parent_b.id def test_ordinary_schedule_produces_no_parent_link(client):