diff --git a/app/models/inspection.py b/app/models/inspection.py index d22d98a..5cb4fd4 100644 --- a/app/models/inspection.py +++ b/app/models/inspection.py @@ -81,11 +81,33 @@ class Inspection(db.Model): ) follow_up_required = db.Column(db.Boolean, nullable=False, default=False) follow_up_note = db.Column(db.Text, nullable=True) + # phase49 — WHO asked for the follow-up and when. `follow_up_required` alone + # cannot distinguish a client request from an internal one, and staff need to + # know who is waiting. Set by flag_followup(), nulled by clear_followup(). + # NULL on every pre-phase49 row, which the UI renders as an unattributed + # follow-up exactly as before. + follow_up_requested_by = db.Column( + db.Integer, + db.ForeignKey('users.id', ondelete='SET NULL', + name='fk_inspections_follow_up_requested_by'), + nullable=True, + ) + follow_up_requested_at = db.Column(db.DateTime, nullable=True) results = db.relationship('InspectionResult', backref='inspection', lazy='dynamic', cascade='all, delete-orphan') issues = db.relationship('Issue', backref='inspection', lazy='dynamic', cascade='all, delete-orphan') follow_ups = db.relationship('Inspection', backref=db.backref('parent', remote_side='Inspection.id'), lazy='dynamic', foreign_keys='Inspection.parent_inspection_id') + # phase49. Explicit foreign_keys is required: inspector_id also points at + # users.id, so SQLAlchemy cannot infer which column this relationship uses. + follow_up_requester = db.relationship('User', + foreign_keys=[follow_up_requested_by]) + # The schedule this inspection was started from / materialised by, so the + # detail view can show the cadence and who set it up. Explicit foreign_keys + # again: inspection_schedules.parent_inspection_id points back here (phase48), + # so neither side's join is inferable. + inspection_schedule = db.relationship( + 'InspectionSchedule', foreign_keys=[inspection_schedule_id]) def __repr__(self): return f'' diff --git a/app/models/notification.py b/app/models/notification.py index d3b57c5..ff70728 100644 --- a/app/models/notification.py +++ b/app/models/notification.py @@ -37,6 +37,12 @@ EVENT_INSPECTION_SCHEDULED = 'inspection_scheduled' # order via the tokenized public link (phase36). EVENT_WORK_ORDER = 'work_order_update' +# Fired when a follow-up re-inspection is requested — by a manager, or (phase49) +# by a customer against their own facility. Routed through notify_by_matrix so +# recipients stay admin-configurable; the inspection's own inspector is notified +# directly by the route rather than through the matrix. +EVENT_FOLLOWUP_REQUESTED = 'followup_requested' + ALL_EVENT_TYPES = { EVENT_ISSUE_ASSIGNED: 'Issue assigned to me', EVENT_ISSUE_STATUS: 'Issue status changed', @@ -48,6 +54,7 @@ ALL_EVENT_TYPES = { EVENT_ADMIN_BROADCAST: 'Admin broadcast (system announcements)', EVENT_INSPECTION_SCHEDULED: 'Scheduled inspection due (assigned to me)', EVENT_WORK_ORDER: 'Contractor updated a work order', + EVENT_FOLLOWUP_REQUESTED: 'Follow-up re-inspection requested', # Customer-facing — only relevant for customer role accounts EVENT_CUSTOMER_INSPECTION_DONE: 'Inspection completed at my facility (portal)', EVENT_CUSTOMER_ISSUE_UPDATED: 'Issue created or updated at my facility (portal)', diff --git a/app/models/notification_matrix.py b/app/models/notification_matrix.py index 80e7a7b..40cb5e5 100644 --- a/app/models/notification_matrix.py +++ b/app/models/notification_matrix.py @@ -27,6 +27,7 @@ issue_flagged : admin ✓ director ✓ inspector ✗ pm ✗ cust issue_created : admin ✗ director ✗ inspector ✗ pm ✗ customer ✓ (assignee implicit) issue_updated_customer : admin ✗ director ✗ inspector ✗ pm ✗ customer ✓ verification_requested : admin ✓ director ✓ inspector ✗ pm ✗ customer ✗ +followup_requested : admin ✓ director ✓ inspector ✗ pm ✓ customer ✗ (inspection's own inspector implicit) sla_alert : admin ✓ director ✗ inspector ✗ pm ✗ customer ✗ (assignee + followers implicit) score_alert : admin ✓ director ✓ inspector ✗ pm ✗ customer ✗ (facility score drop cron) """ @@ -59,6 +60,7 @@ MATRIX_EVENTS = { 'issue_created': 'Issue created (standalone)', 'issue_updated_customer': 'Issue updated (customer)', 'verification_requested': 'Verification requested', + 'followup_requested': 'Follow-up requested (incl. by customer)', 'sla_alert': 'SLA at-risk / breached', 'score_alert': 'Facility score trend alert (significant drop)', } @@ -143,6 +145,16 @@ MATRIX_DEFAULTS = { ('verification_requested', 'project_manager'): False, ('verification_requested', 'customer'): False, ('verification_requested', 'custom'): False, + # followup_requested (phase49) — a customer (or manager) asks for a + # re-inspection. On for the roles who action it; the inspection's own + # inspector is notified directly by the route, so the inspector column stays + # off to avoid alerting the whole inspector pool. + ('followup_requested', 'admin'): True, + ('followup_requested', 'director'): True, + ('followup_requested', 'inspector'): False, + ('followup_requested', 'project_manager'): True, + ('followup_requested', 'customer'): False, + ('followup_requested', 'custom'): False, # sla_alert (assignee + followers always notified implicitly) ('sla_alert', 'admin'): True, ('sla_alert', 'director'): False, diff --git a/app/models/user.py b/app/models/user.py index b930e82..a145edf 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -43,7 +43,13 @@ class User(UserMixin, db.Model): mfa_recovery_codes = db.Column(db.JSON, nullable=True) # Relationships - inspections = db.relationship('Inspection', backref='inspector', lazy='dynamic') + # phase49: inspections now has TWO foreign keys to users.id — inspector_id + # and follow_up_requested_by — so the join is otherwise ambiguous and every + # mapper configuration fails with AmbiguousForeignKeysError. This + # relationship means "inspections I performed": inspector_id only. + inspections = db.relationship('Inspection', backref='inspector', + lazy='dynamic', + foreign_keys='Inspection.inspector_id') # ── Flask-Login integration ──────────────────────────────────────────── # Override UserMixin.is_active so that disabled accounts are rejected diff --git a/app/routes/inspections.py b/app/routes/inspections.py index b095a96..9f50176 100644 --- a/app/routes/inspections.py +++ b/app/routes/inspections.py @@ -21,6 +21,7 @@ from app.utils.notifications import notify, notify_customers_for_facility, notif from app.models.notification import ( EVENT_INSPECTION_DONE, EVENT_ISSUE_ASSIGNED, EVENT_CUSTOMER_INSPECTION_DONE, EVENT_CUSTOMER_ISSUE_UPDATED, + EVENT_FOLLOWUP_REQUESTED, ) from app.utils.audit import log_action, ACTION_CREATE, ACTION_UPDATE, ACTION_DELETE, ACTION_EXPORT from app.tenancy.gates import quota_soft_check @@ -1216,29 +1217,62 @@ def export_pdf(inspection_id): @bp.route('//flag-followup', methods=['POST']) @login_required -@supervisor_required def flag_followup(inspection_id): - """Mark an inspection as requiring a follow-up re-inspection.""" + """Mark an inspection as requiring a follow-up re-inspection. + + phase49: no longer @supervisor_required. Open to admin/director AND to + customers for their own facilities — a client unhappy with a result can ask + for a re-inspection directly rather than going through support. Every other + role is refused, so inspectors and auditors are no worse off than before. + + Customers may only *request*: they cannot clear the flag (clear_followup is + still @supervisor_required) nor run the re-inspection itself. + """ inspection = db.session.get(Inspection, inspection_id) if inspection is None: abort(404) + + is_customer = current_user.role == 'customer' + if is_customer: + # Same facility scope as view() — a customer must not be able to reach + # another client's inspection with a crafted POST. + if inspection.facility_id not in (get_customer_scope(current_user) or []): + abort(403) + # Nothing to follow up on until the inspection has been submitted. + if inspection.status != 'completed': + flash('You can only request a follow-up on a completed inspection.', 'warning') + return redirect(url_for('inspections.view', inspection_id=inspection_id)) + # Don't let a repeat request overwrite the note/attribution of a pending + # one — the flag is already raised and staff are already on it. + if inspection.follow_up_required: + flash('A follow-up has already been requested for this inspection.', 'info') + return redirect(url_for('inspections.view', inspection_id=inspection_id)) + elif current_user.role not in ('admin', 'director'): + abort(403) + note = request.form.get('follow_up_note', '').strip() or None - inspection.follow_up_required = True - inspection.follow_up_note = note + inspection.follow_up_required = True + inspection.follow_up_note = note + inspection.follow_up_requested_by = current_user.id + inspection.follow_up_requested_at = now_eastern() db.session.commit() - # Notify the original inspector so they see it on the iPad + note_suffix = f' Note: {note}' if note else '' + who = (f'The customer ({current_user.display_name})' if is_customer + else current_user.display_name) + body = ( + f'{who} has requested a follow-up re-inspection ' + f'of "{inspection.template.name}" at {inspection.facility.name}.{note_suffix}' + ) + + # Notify the original inspector so they see it on the iPad. inspector = db.session.get(User, inspection.inspector_id) if inspector and inspector.id != current_user.id: - note_suffix = f' Note: {note}' if note else '' notify( recipient = inspector, title = f'Follow-Up Required: Inspection #{inspection_id}', - body = ( - f'{current_user.username} has requested a follow-up re-inspection ' - f'of "{inspection.template.name}" at {inspection.facility.name}.{note_suffix}' - ), + body = body, link = url_for('inspections.view', inspection_id=inspection_id), inspection_id = inspection_id, event_type = EVENT_INSPECTION_DONE, @@ -1246,14 +1280,34 @@ def flag_followup(inspection_id): ) db.session.commit() + # Route to the staff who action follow-ups. Going through notify_by_matrix + # rather than notifying managers directly keeps recipients admin-configurable + # and lets per-contract recipients fire too. This matters most for a customer + # request: without it only the inspector would hear about it and nobody would + # be accountable for scheduling the re-inspection. + notify_by_matrix( + event_type = EVENT_FOLLOWUP_REQUESTED, + title = f'Follow-Up Requested: Inspection #{inspection_id}', + body = body, + link = url_for('inspections.view', inspection_id=inspection_id), + inspection_id = inspection_id, + facility_id = inspection.facility_id, + exclude_user_ids = {current_user.id, + inspector.id if inspector else None} - {None}, + ) + db.session.commit() + current_app.logger.info( - 'INSPECTION FOLLOW-UP FLAGGED | id=%s | by=%s | note=%r', - inspection_id, current_user.username, note, + 'INSPECTION FOLLOW-UP FLAGGED | id=%s | by=%s (%s) | note=%r', + inspection_id, current_user.username, current_user.role, note, ) log_action(ACTION_UPDATE, 'Inspection', inspection_id, f'{inspection.template.name} @ {inspection.facility.name}', - f'follow_up_required=True; note={note!r}') - flash('Follow-up inspection required flag set.', 'warning') + f'follow_up_required=True; by_role={current_user.role}; note={note!r}') + if is_customer: + flash('Follow-up re-inspection requested. The team has been notified.', 'success') + else: + flash('Follow-up inspection required flag set.', 'warning') return redirect(url_for('inspections.view', inspection_id=inspection_id)) @@ -1267,6 +1321,11 @@ def clear_followup(inspection_id): abort(404) inspection.follow_up_required = False inspection.follow_up_note = None + # phase49 — clear the attribution with the flag. Leaving it behind would + # make the next unattributed follow-up appear to have been requested by + # whoever raised the previous one. + inspection.follow_up_requested_by = None + inspection.follow_up_requested_at = None db.session.commit() log_action(ACTION_UPDATE, 'Inspection', inspection_id, f'{inspection.template.name} @ {inspection.facility.name}', diff --git a/app/templates/inspections/view.html b/app/templates/inspections/view.html index ae407f5..052de20 100644 --- a/app/templates/inspections/view.html +++ b/app/templates/inspections/view.html @@ -356,6 +356,16 @@ Re-inspect {% endif %} + {# phase49 — customers may REQUEST a follow-up on their own completed + inspections; only admin/director can clear one. #} + {% if current_user.role == 'customer' and inspection.status == 'completed' + and not inspection.follow_up_required %} + + {% endif %} {% if current_user.role in ['admin','director'] %} {% if not inspection.follow_up_required %}
- + {% if is_cust %} +

+ Ask the team to re-inspect this facility. Your request is sent to the + inspector and management right away. +

+ {% endif %} +
diff --git a/migrations/versions/phase49_followup_requested_by.py b/migrations/versions/phase49_followup_requested_by.py new file mode 100644 index 0000000..e8bb955 --- /dev/null +++ b/migrations/versions/phase49_followup_requested_by.py @@ -0,0 +1,97 @@ +"""phase49 — follow-up request attribution (customer-raised follow-ups) + +Ports single-tenant phase46 onto the multi-tenant chain. Adds to `inspections`: + + follow_up_requested_by INT NULL FK → users(id) ON DELETE SET NULL + follow_up_requested_at DATETIME NULL + +Customers can now request a follow-up re-inspection of a completed inspection at +their own facilities (previously admin/director only), so `follow_up_required` +alone is no longer enough — staff need to see WHO is waiting on the +re-inspection, and a client request must be visibly distinct from an internal +one. `flag_followup()` sets both columns; `clear_followup()` nulls them. + +No backfill: legacy rows keep NULL, which the UI renders as an unattributed +follow-up exactly as it did before. FK is SET NULL so deleting a user never +deletes inspection history. + +Revision id note +---------------- +`alembic_version.version_num` is VARCHAR(32); the id below is 23 characters. +The filename stays descriptive — Alembic keys on the `revision` string. + +Uses INFORMATION_SCHEMA checks — safe to re-run on every tenant DB. Additive +only: nothing is renamed, retyped or dropped. +""" + +revision = 'phase49_followup_req_by' +down_revision = 'phase48_sched_parent_insp' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +_TABLE = 'inspections' +_FK_NAME = 'fk_inspections_follow_up_requested_by' + + +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 _fk_exists(conn, table, name): + return conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS " + "WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = :t " + "AND CONSTRAINT_NAME = :n AND CONSTRAINT_TYPE = 'FOREIGN KEY'" + ), {"t": table, "n": name}).scalar() > 0 + + +def upgrade(): + bind = op.get_bind() + if not _table_exists(bind, _TABLE): + return + + if not _column_exists(bind, _TABLE, 'follow_up_requested_by'): + op.execute(sa.text( + f"ALTER TABLE {_TABLE} " + f"ADD COLUMN follow_up_requested_by INT NULL AFTER follow_up_note" + )) + if not _column_exists(bind, _TABLE, 'follow_up_requested_at'): + op.execute(sa.text( + f"ALTER TABLE {_TABLE} " + f"ADD COLUMN follow_up_requested_at DATETIME NULL " + f"AFTER follow_up_requested_by" + )) + if not _fk_exists(bind, _TABLE, _FK_NAME): + op.execute(sa.text( + f"ALTER TABLE {_TABLE} ADD CONSTRAINT {_FK_NAME} " + f"FOREIGN KEY (follow_up_requested_by) REFERENCES users(id) " + f"ON DELETE SET NULL" + )) + + +def downgrade(): + bind = op.get_bind() + if not _table_exists(bind, _TABLE): + return + + # FK first — MySQL refuses to drop a column still referenced by one. + if _fk_exists(bind, _TABLE, _FK_NAME): + op.execute(sa.text(f"ALTER TABLE {_TABLE} DROP FOREIGN KEY {_FK_NAME}")) + for col in ('follow_up_requested_at', 'follow_up_requested_by'): + if _column_exists(bind, _TABLE, col): + op.execute(sa.text(f"ALTER TABLE {_TABLE} DROP COLUMN {col}")) diff --git a/tests/conftest.py b/tests/conftest.py index 46a7a3c..e41adf5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -36,7 +36,10 @@ import sys # ── Import-time env (must be set BEFORE `import app`) ──────────────────────── # Assigned, NOT setdefault — see the module docstring. -os.environ['SECRET_KEY'] = 'test-secret-key' +# SECRET_KEY doubles as the JWT signing key (app/api/jwt_utils.py), and PyJWT +# warns below 32 bytes for HMAC-SHA256 (RFC 7518 §3.2). Throwaway, but sized so +# the API tests do not emit InsecureKeyLengthWarning on every token. +os.environ['SECRET_KEY'] = 'test-secret-key-not-for-production-use-0123456789' os.environ['DATABASE_URL'] = 'sqlite:///:memory:' os.environ['MULTI_TENANT_ENABLED'] = 'false' os.environ['BILLING_ENABLED'] = 'false' @@ -65,6 +68,23 @@ import pytest # noqa: E402 @pytest.fixture(scope='session') def app(): """A minimal single-tenant app on in-memory SQLite (multi-tenancy inert).""" + # Flask-Limiter uses in-memory storage keyed on the remote address, and this + # fixture is session-scoped — so every login across the WHOLE suite shares + # one counter against /auth/login's '20 per minute'. Past that the login + # returns 429, the test client stays anonymous, and whatever the test does + # next is redirected to the login page. The failure surfaces as an unrelated + # assertion ("the edit did not apply"), only in full runs, and only once + # enough tests have logged in — so it moves around as tests are added or + # reordered. + # + # This MUST happen before create_app(). Limiter.init_app() does + # `self.enabled = config.setdefault('RATELIMIT_ENABLED', self.enabled)` and + # returns early when false, registering no request hooks — and `enabled` is + # never consulted again at request time (flask-limiter 4.x). Setting it + # afterwards is silently a no-op. Production limits are untouched. + from app import limiter + limiter.enabled = False + from app import create_app application = create_app('default') application.config.update(TESTING=True, WTF_CSRF_ENABLED=False, SQLALCHEMY_ECHO=False) diff --git a/tests/test_followup_requests.py b/tests/test_followup_requests.py new file mode 100644 index 0000000..3f55d65 --- /dev/null +++ b/tests/test_followup_requests.py @@ -0,0 +1,336 @@ +""" +tests/test_followup_requests.py +-------------------------------- +Behaviour tests for phase49 — follow-up request attribution and +customer-raised follow-ups. + +Runs on the in-memory SQLite app fixture (multi-tenancy inert). Covers: + + * a customer can request a follow-up on a completed inspection at THEIR + facility, and both attribution columns are set + * a customer cannot reach another client's inspection with a crafted POST, + cannot request on a draft, and cannot overwrite a pending request + * a customer cannot CLEAR a follow-up — request-only + * admin/director keep their existing flag behaviour, now attributed + * inspector and auditor are refused (they were before too, via + @supervisor_required; phase49 must not widen access to them) + * clear_followup() nulls the attribution along with the flag + * the request routes through notify_by_matrix as EVENT_FOLLOWUP_REQUESTED, + reaching the managers who action it while excluding the actor and the + inspection's own inspector (who is notified directly instead) +""" + +import pytest + + +@pytest.fixture +def client(app): + """Fresh schema + test client for each test (isolated in-memory DB).""" + with app.app_context(): + from app import db + # get_inspector_scope() imports this model lazily, so the mapper is not + # registered at create_all() time and the table is missing when a + # logged-in inspector hits the dashboard. Import it up front. + from app.models import inspector_assignment # noqa: F401 + db.drop_all() + db.create_all() + yield app.test_client() + db.session.remove() + + +def _user(username, role, **kw): + 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, **kw) + u.set_password('pw-correct1') + db.session.add(u) + db.session.commit() + return u + + +def _seed(): + """Two facilities, an inspector, a manager pool, and two customers.""" + from app import db + from app.models.facility import Facility + from app.models.inspection import InspectionTemplate + from app.models.project import Project, CustomerAssignment + + # MT scopes customers through a Project; customer_assignments.project_id is + # NOT NULL, so each facility needs one even for a facility-level assignment. + proj_a = Project(name='Contract A', active=True) + proj_b = Project(name='Contract B', active=True) + tmpl = InspectionTemplate(name='Restroom Check', active=True, + form_schema=[{'id': 'f1', 'type': 'rating_5', 'label': 'Clean', + 'row': 0, 'col': 0, + 'rowSpan': 1, 'colSpan': 1}]) + db.session.add_all([proj_a, proj_b, tmpl]) + db.session.commit() + + fac_a = Facility(name='Client A Site', active=True, project_id=proj_a.id) + fac_b = Facility(name='Client B Site', active=True, project_id=proj_b.id) + db.session.add_all([fac_a, fac_b]) + db.session.commit() + + inspector = _user('ivy', 'inspector') + admin = _user('ada', 'admin') + director = _user('dan', 'director') + pm = _user('pat', 'project_manager') + cust_a = _user('cara', 'customer') + cust_b = _user('carl', 'customer') + + db.session.add_all([ + CustomerAssignment(user_id=cust_a.id, project_id=proj_a.id, + facility_id=fac_a.id), + CustomerAssignment(user_id=cust_b.id, project_id=proj_b.id, + facility_id=fac_b.id), + ]) + db.session.commit() + + return dict(tmpl=tmpl, fac_a=fac_a, fac_b=fac_b, inspector=inspector, + admin=admin, director=director, pm=pm, + cust_a=cust_a, cust_b=cust_b) + + +def _inspection(env, facility, status='completed'): + from app import db + from app.models.inspection import Inspection + from app.utils.time_utils import now_eastern + + insp = Inspection(template_id=env['tmpl'].id, facility_id=facility.id, + inspector_id=env['inspector'].id, + inspection_date=now_eastern(), status=status, + completed_at=now_eastern() if status == 'completed' else None, + overall_score=71.0) + db.session.add(insp) + db.session.commit() + return insp + + +def _login(client, user): + return client.post('/auth/login', + data={'username': user.username, 'password': 'pw-correct1'}, + follow_redirects=True) + + +def _flag(client, inspection_id, note=None): + data = {'follow_up_note': note} if note else {} + return client.post(f'/inspections/{inspection_id}/flag-followup', + data=data, follow_redirects=False) + + +# ── Customer requests ──────────────────────────────────────────────────────── + +def test_customer_can_request_follow_up_at_their_own_facility(client): + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env['cust_a']) + + resp = _flag(client, insp.id, note='Stalls still dirty') + assert resp.status_code == 302 + + db.session.expire_all() + insp = db.session.get(Inspection, insp.id) + assert insp.follow_up_required is True + assert insp.follow_up_note == 'Stalls still dirty' + assert insp.follow_up_requested_by == env['cust_a'].id + assert insp.follow_up_requested_at is not None + # The relationship is what the template renders the badge from. + assert insp.follow_up_requester.role == 'customer' + + +def test_customer_cannot_reach_another_clients_inspection(client): + """A crafted POST must not cross the facility scope.""" + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_b']) # Client B's facility + _login(client, env['cust_a']) # Client A's customer + + assert _flag(client, insp.id).status_code == 403 + db.session.expire_all() + assert db.session.get(Inspection, insp.id).follow_up_required is False + + +def test_customer_cannot_request_on_a_draft(client): + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a'], status='in_progress') + _login(client, env['cust_a']) + + _flag(client, insp.id) + db.session.expire_all() + assert db.session.get(Inspection, insp.id).follow_up_required is False + + +def test_repeat_customer_request_does_not_overwrite_the_pending_one(client): + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env['cust_a']) + + _flag(client, insp.id, note='First note') + db.session.expire_all() + first_at = db.session.get(Inspection, insp.id).follow_up_requested_at + + _flag(client, insp.id, note='Second note') + db.session.expire_all() + reloaded = db.session.get(Inspection, insp.id) + assert reloaded.follow_up_note == 'First note' + assert reloaded.follow_up_requested_at == first_at + + +def test_customer_cannot_clear_a_follow_up(client): + """Customers may REQUEST only — clearing stays @supervisor_required.""" + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env['cust_a']) + _flag(client, insp.id) + + resp = client.post(f'/inspections/{insp.id}/clear-followup', + follow_redirects=False) + assert resp.status_code in (302, 403) + + db.session.expire_all() + # Whether it redirected or 403'd, the flag must still be up. + assert db.session.get(Inspection, insp.id).follow_up_required is True + + +# ── Staff behaviour is preserved ───────────────────────────────────────────── + +def test_admin_flag_still_works_and_is_now_attributed(client): + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env['admin']) + + _flag(client, insp.id, note='Rework required') + db.session.expire_all() + insp = db.session.get(Inspection, insp.id) + assert insp.follow_up_required is True + assert insp.follow_up_requested_by == env['admin'].id + assert insp.follow_up_requester.role == 'admin' + + +@pytest.mark.parametrize('role_key', ['inspector', 'pm']) +def test_roles_without_permission_are_refused(client, role_key): + """phase49 removed @supervisor_required from this route. It must still + refuse everyone who could not flag before.""" + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env[role_key]) + + assert _flag(client, insp.id).status_code == 403 + db.session.expire_all() + assert db.session.get(Inspection, insp.id).follow_up_required is False + + +def test_auditor_is_refused(client): + from app import db + from app.models.inspection import Inspection + + env = _seed() + auditor = _user('aud', 'auditor') + insp = _inspection(env, env['fac_a']) + _login(client, auditor) + + assert _flag(client, insp.id).status_code == 403 + db.session.expire_all() + assert db.session.get(Inspection, insp.id).follow_up_required is False + + +def test_clear_followup_nulls_the_attribution(client): + """Leaving it behind would make the NEXT unattributed follow-up appear to + have been requested by whoever raised the previous one.""" + from app import db + from app.models.inspection import Inspection + + env = _seed() + insp = _inspection(env, env['fac_a']) + + _login(client, env['cust_a']) + _flag(client, insp.id, note='Please recheck') + client.get('/auth/logout', follow_redirects=True) + + _login(client, env['admin']) + client.post(f'/inspections/{insp.id}/clear-followup', follow_redirects=True) + + db.session.expire_all() + insp = db.session.get(Inspection, insp.id) + assert insp.follow_up_required is False + assert insp.follow_up_note is None + assert insp.follow_up_requested_by is None + assert insp.follow_up_requested_at is None + assert insp.follow_up_requester is None + + +# ── Notification routing ───────────────────────────────────────────────────── + +def test_request_notifies_managers_via_the_matrix(client): + from app import db + from app.models.notification import Notification, EVENT_FOLLOWUP_REQUESTED + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env['cust_a']) + _flag(client, insp.id, note='Still dirty') + + rows = Notification.query.filter_by(event_type=EVENT_FOLLOWUP_REQUESTED).all() + recipients = {n.user_id for n in rows} + + # Defaults: admin ✓ director ✓ project_manager ✓ + assert env['admin'].id in recipients + assert env['director'].id in recipients + assert env['pm'].id in recipients + # The actor never notifies themselves. + assert env['cust_a'].id not in recipients + # The inspection's own inspector is notified DIRECTLY instead, so the matrix + # must exclude them rather than double-notifying. + assert env['inspector'].id not in recipients + # ...and that direct notification did happen. + direct = Notification.query.filter_by(user_id=env['inspector'].id).all() + assert any('Follow-Up Required' in n.title for n in direct) + + +def test_matrix_defaults_include_followup_requested(client): + from app.models.notification_matrix import MATRIX_EVENTS, MATRIX_DEFAULTS + + assert 'followup_requested' in MATRIX_EVENTS + assert MATRIX_DEFAULTS[('followup_requested', 'admin')] is True + assert MATRIX_DEFAULTS[('followup_requested', 'director')] is True + assert MATRIX_DEFAULTS[('followup_requested', 'project_manager')] is True + # Off, or every request would alert the entire inspector pool. + assert MATRIX_DEFAULTS[('followup_requested', 'inspector')] is False + assert MATRIX_DEFAULTS[('followup_requested', 'customer')] is False + + +def test_notification_body_marks_a_customer_request_as_such(client): + """Staff must be able to tell a client request from an internal one at a + glance, not just from the badge on the detail page.""" + from app.models.notification import Notification, EVENT_FOLLOWUP_REQUESTED + + env = _seed() + insp = _inspection(env, env['fac_a']) + _login(client, env['cust_a']) + _flag(client, insp.id) + + row = Notification.query.filter_by( + event_type=EVENT_FOLLOWUP_REQUESTED, user_id=env['admin'].id).first() + assert row is not None + assert 'customer' in row.body.lower()