From b761f6577ead191a089dd3b1e0f710699d9b0866 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Fri, 15 May 2026 14:54:47 -0400 Subject: [PATCH 1/9] 05/15 Update: implement iPad notification function --- app/__init__.py | 16 ++-- app/api/__init__.py | 5 ++ app/api/notifications.py | 159 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 app/api/notifications.py diff --git a/app/__init__.py b/app/__init__.py index 48f5ce3..1fae60d 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -147,25 +147,27 @@ def create_app(config_name='default'): app.register_blueprint(customers.bp) app.register_blueprint(scheduled_reports.bp) - # ── Mobile API (Phase 7 / Phase A / Phase B) ───────────────────────────── + # ── Mobile API (Phase 7 / Phase A / Phase B / Phase C) ─────────────────── # The /api/v1 blueprint group uses JWT Bearer tokens — no CSRF cookies needed. # # Flask-WTF's _is_exempt() checks whether the *leaf* blueprint object # is in _exempt_blueprints. Exempting the parent api_bp does NOT cascade # to sub-blueprints. Each child blueprint must be exempted individually. from app.api import register_api, api_bp - from app.api.auth import bp as _api_auth_bp - from app.api.facilities import bp as _api_facilities_bp - from app.api.templates import bp as _api_templates_bp - from app.api.inspections import bp as _api_inspections_bp - from app.api.issues import bp as _api_issues_bp - from app.api.photos import bp as _api_photos_bp + from app.api.auth import bp as _api_auth_bp + from app.api.facilities import bp as _api_facilities_bp + from app.api.templates import bp as _api_templates_bp + from app.api.inspections import bp as _api_inspections_bp + from app.api.issues import bp as _api_issues_bp + from app.api.photos import bp as _api_photos_bp + from app.api.notifications import bp as _api_notifications_bp csrf.exempt(_api_auth_bp) csrf.exempt(_api_facilities_bp) csrf.exempt(_api_templates_bp) csrf.exempt(_api_inspections_bp) csrf.exempt(_api_issues_bp) csrf.exempt(_api_photos_bp) + csrf.exempt(_api_notifications_bp) register_api(app) # ── Error handler: 413 Request Entity Too Large ─────────────────────── diff --git a/app/api/__init__.py b/app/api/__init__.py index 03b474c..d8469c2 100644 --- a/app/api/__init__.py +++ b/app/api/__init__.py @@ -5,6 +5,7 @@ Registers the /api/v1 blueprint group. Phase A: /api/v1/facilities/*, /api/v1/templates/* Phase B: /api/v1/inspections/*, /api/v1/issues/*, /api/v1/photos/* +Phase C: /api/v1/notifications/* """ from flask import Blueprint @@ -37,4 +38,8 @@ def register_api(app): api_bp.register_blueprint(issues_bp) api_bp.register_blueprint(photos_bp) + # Phase C: Notification polling + from app.api.notifications import bp as notifications_bp + api_bp.register_blueprint(notifications_bp) + app.register_blueprint(api_bp) \ No newline at end of file diff --git a/app/api/notifications.py b/app/api/notifications.py new file mode 100644 index 0000000..aff10e6 --- /dev/null +++ b/app/api/notifications.py @@ -0,0 +1,159 @@ +""" +app/api/notifications.py +------------------------ +Mobile API endpoint for polling in-app notifications. + +GET /api/v1/notifications + Returns unread notifications for the authenticated user. + Accepts ?since= to fetch only notifications created after + that datetime — used by the iPad poller to avoid re-delivering already- + seen alerts. Returns a maximum of 50 notifications per call. + +PATCH /api/v1/notifications/mark-read + Marks a list of notification IDs as read. + Request JSON: { "ids": [1, 2, 3] } +""" + +import logging +from datetime import datetime + +from flask import Blueprint, request, g +from app import db +from app.models.notification import Notification +from app.api.errors import api_ok, api_error +from app.api.decorators import jwt_required + +logger = logging.getLogger(__name__) + +bp = Blueprint('api_notifications', __name__) + +_INSPECTOR_RELEVANT = { + 'issue_assigned', + 'issue_reassigned', + 'issue_unassigned', + 'issue_status', + 'issue_comment', + 'issue_follow_update', + 'verification_requested', + 'sla_alert', +} + + +def _parse_since(value): + """Parse ?since= ISO 8601 string; return None on failure.""" + if not value: + return None + for fmt in ('%Y-%m-%dT%H:%M:%S', '%Y-%m-%dT%H:%M:%S.%f', '%Y-%m-%d'): + try: + return datetime.strptime(value, fmt) + except (ValueError, TypeError): + pass + return None + + +@bp.route('/notifications', methods=['GET']) +@jwt_required +def list_notifications(): + """ + Return unread notifications for the authenticated user. + + Query parameters + ---------------- + since ISO 8601 datetime Only return notifications created after this time. + Omit for the initial fetch (returns last 50). + limit int (default 50, max 50) + + Response 200 + ------------ + { + "ok": true, + "data": { + "notifications": [ + { + "id": 12, + "title": "Issue #7 Assigned to You", + "body": "...", + "event_type": "issue_assigned", + "issue_id": 7, + "created_at": "2026-05-15T09:30:00" + }, + ... + ], + "count": 2 + } + } + """ + user = g.api_user + since = _parse_since(request.args.get('since')) + limit = min(int(request.args.get('limit', 50)), 50) + + query = Notification.query.filter_by( + user_id=user.id, + is_read=False, + ) + + if since: + query = query.filter(Notification.created_at > since) + + notifications = ( + query + .order_by(Notification.created_at.asc()) + .limit(limit) + .all() + ) + + payload = [ + { + 'id': n.id, + 'title': n.title, + 'body': n.body, + 'event_type': n.event_type, + 'issue_id': n.issue_id, + 'created_at': n.created_at.strftime('%Y-%m-%dT%H:%M:%S'), + } + for n in notifications + ] + + logger.info('API NOTIFICATIONS | list | user=%s | since=%s | count=%d', + user.username, since, len(payload)) + + return api_ok({'notifications': payload, 'count': len(payload)}) + + +@bp.route('/notifications/mark-read', methods=['PATCH']) +@jwt_required +def mark_read(): + """ + Mark a list of notification IDs as read. + + Request JSON: { "ids": [1, 2, 3] } + + Response 200 + ------------ + { "ok": true, "data": { "marked": 3 } } + """ + user = g.api_user + data = request.get_json(silent=True) or {} + ids = data.get('ids') or [] + + if not isinstance(ids, list): + return api_error('ids must be a list', 400) + + if ids: + updated = ( + Notification.query + .filter( + Notification.id.in_(ids), + Notification.user_id == user.id, # never touch another user's records + ) + .all() + ) + for n in updated: + n.is_read = True + db.session.commit() + count = len(updated) + else: + count = 0 + + logger.info('API NOTIFICATIONS | mark_read | user=%s | count=%d', user.username, count) + return api_ok({'marked': count}) From b69a52e5f5846ec720d2993bef288af0714b03f9 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Fri, 15 May 2026 15:15:10 -0400 Subject: [PATCH 2/9] 05/15 Update: implement iPad notification function 2 --- app/api/issues.py | 99 ++++++++++++++++--- app/models/notification.py | 4 + app/routes/inspections.py | 18 ++++ app/utils/notifications.py | 1 + .../phase17_notification_event_type.py | 53 ++++++++++ 5 files changed, 162 insertions(+), 13 deletions(-) create mode 100644 migrations/versions/phase17_notification_event_type.py diff --git a/app/api/issues.py b/app/api/issues.py index d67204a..f428465 100644 --- a/app/api/issues.py +++ b/app/api/issues.py @@ -3,6 +3,10 @@ app/api/issues.py ----------------- Mobile API endpoint for submitting issues from the iPad app. +GET /api/v1/issues + Returns issues assigned to the authenticated inspector (or all for admin/director). + Used by the iPad to display assigned issues that were created via the web portal. + POST /api/v1/issues Creates a new issue record. Accepts a mobile_local_id for idempotency. @@ -40,7 +44,87 @@ _VALID_SEVERITY = {'low', 'medium', 'high', 'critical'} _VALID_STATUSES = {'open', 'in_progress', 'resolved', 'pending_verification'} -# ── Create Issue ────────────────────────────────────────────────────────────── +def _issue_payload(issue): + """Serialise an Issue to the dict returned in list/detail responses.""" + facility = issue.resolved_facility + return { + 'id': issue.id, + 'status': issue.status, + 'severity': issue.severity, + 'description': issue.description, + 'assigned_to': issue.assigned_to, + 'facility_id': facility.id if facility else None, + 'facility_name': facility.name if facility else None, + 'reported_at': issue.reported_at.isoformat() if issue.reported_at else None, + 'resolved_at': issue.resolved_at.isoformat() if issue.resolved_at else None, + 'mobile_local_id': issue.mobile_local_id, + } + + +# ── List Assigned Issues ────────────────────────────────────────────────────── + +@bp.route('/issues', methods=['GET']) +@jwt_required +def list_issues(): + """ + Return issues assigned to the authenticated user. + + Inspectors: only issues where assigned_to == current user. + Admin / director / project_manager: all non-resolved issues (capped at 200). + + Query parameters + ---------------- + status str Filter by status (default: excludes resolved). + limit int Default 100, max 200. + offset int Default 0. + + Response 200 + ------------ + { + "ok": true, + "data": { + "issues": [...], + "total": 12, + "limit": 100, + "offset": 0 + } + } + """ + user = g.api_user + if user.role not in _ALLOWED_ROLES: + return api_error('Access denied', 403) + + limit = min(int(request.args.get('limit', 100)), 200) + offset = max(int(request.args.get('offset', 0)), 0) + + query = Issue.query + + if user.role == 'inspector': + # Inspectors only see issues assigned to them + query = query.filter(Issue.assigned_to == user.id) + else: + # Broader roles: exclude resolved by default so the list stays manageable + status_filter = request.args.get('status') + if status_filter: + query = query.filter(Issue.status == status_filter) + else: + query = query.filter(Issue.status != 'resolved') + + total = query.count() + issues = ( + query + .order_by(Issue.reported_at.desc()) + .offset(offset) + .limit(limit) + .all() + ) + + payload = [_issue_payload(i) for i in issues] + + logger.info('API ISSUES | list | user=%s | count=%d | total=%d', + user.username, len(payload), total) + + return api_ok({'issues': payload, 'total': total, 'limit': limit, 'offset': offset}) @bp.route('/issues', methods=['POST']) @jwt_required @@ -180,18 +264,7 @@ def get_issue(issue_id): if user.role == 'inspector' and issue.assigned_to != user.id: return api_error('Access denied', 403) - facility = issue.resolved_facility - return api_ok({ - 'id': issue.id, - 'status': issue.status, - 'severity': issue.severity, - 'description': issue.description, - 'assigned_to': issue.assigned_to, - 'facility_id': facility.id if facility else None, - 'facility_name': facility.name if facility else None, - 'reported_at': issue.reported_at.isoformat() if issue.reported_at else None, - 'resolved_at': issue.resolved_at.isoformat() if issue.resolved_at else None, - }) + return api_ok(_issue_payload(issue)) # ── Update Issue Status ─────────────────────────────────────────────────────── diff --git a/app/models/notification.py b/app/models/notification.py index 7b011b7..d94abef 100644 --- a/app/models/notification.py +++ b/app/models/notification.py @@ -53,6 +53,10 @@ class Notification(db.Model): issue_id = db.Column(db.Integer, db.ForeignKey('issues.id', ondelete='CASCADE'), nullable=True) inspection_id = db.Column(db.Integer, db.ForeignKey('inspections.id', ondelete='CASCADE'), nullable=True) + # Event type — stored for mobile API polling so the iPad can categorise alerts. + # Added phase17; NULL for notifications created before the migration. + event_type = db.Column(db.String(50), nullable=True) + # Digest tracking: set to True when created, cleared after digest email sent digest_pending = db.Column(db.Boolean, default=False, nullable=False, index=True) diff --git a/app/routes/inspections.py b/app/routes/inspections.py index b8592c8..2a019b1 100644 --- a/app/routes/inspections.py +++ b/app/routes/inspections.py @@ -873,6 +873,24 @@ def flag_followup(inspection_id): inspection.follow_up_note = note db.session.commit() + # 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}' + ), + link = url_for('inspections.view', inspection_id=inspection_id), + inspection_id = inspection_id, + event_type = EVENT_INSPECTION_DONE, + send_email = True, + ) + db.session.commit() + current_app.logger.info( 'INSPECTION FOLLOW-UP FLAGGED | id=%s | by=%s | note=%r', inspection_id, current_user.username, note, diff --git a/app/utils/notifications.py b/app/utils/notifications.py index b23a856..f5d5e73 100644 --- a/app/utils/notifications.py +++ b/app/utils/notifications.py @@ -207,6 +207,7 @@ def notify( link = link, issue_id = issue_id, inspection_id = inspection_id, + event_type = event_type, is_read = False, digest_pending = hold_for_digest, ) diff --git a/migrations/versions/phase17_notification_event_type.py b/migrations/versions/phase17_notification_event_type.py new file mode 100644 index 0000000..4109bac --- /dev/null +++ b/migrations/versions/phase17_notification_event_type.py @@ -0,0 +1,53 @@ +"""phase17 — add event_type column to notifications table + +Background +---------- +The `notifications` table has no `event_type` column, but the mobile API +endpoint GET /api/v1/notifications references `n.event_type`, causing an +AttributeError (500) on every poll — silently breaking iPad notifications. + +This migration adds `event_type VARCHAR(50) NULL` so the column is stored +at creation time and returned correctly to the mobile poller. + +The `notify()` utility is updated separately to pass event_type when creating +Notification records. + +Uses INFORMATION_SCHEMA existence check — safe to re-run (CLAUDE.md rules 16, 17). + +Revision ID: phase17_notification_event_type +Revises: phase16_notifications_columns +""" + +revision = 'phase17_notification_event_type' +down_revision = 'phase16_notifications_columns' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def _column_exists(bind, table, column): + result = bind.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}) + return result.scalar() > 0 + + +def upgrade(): + bind = op.get_bind() + if not _column_exists(bind, 'notifications', 'event_type'): + op.execute(sa.text( + "ALTER TABLE notifications " + "ADD COLUMN event_type VARCHAR(50) NULL" + )) + + +def downgrade(): + bind = op.get_bind() + if _column_exists(bind, 'notifications', 'event_type'): + op.execute(sa.text( + "ALTER TABLE notifications DROP COLUMN event_type" + )) From 33efa9402e490ce8b7f628c81e4c1bc2558d8590 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Fri, 15 May 2026 15:35:39 -0400 Subject: [PATCH 3/9] 05/15 Update: implement iPad notification function 3 --- app/api/issues.py | 21 ++++--- app/api/notifications.py | 118 ++++++++++++++++++++------------------- 2 files changed, 74 insertions(+), 65 deletions(-) diff --git a/app/api/issues.py b/app/api/issues.py index f428465..b3100c9 100644 --- a/app/api/issues.py +++ b/app/api/issues.py @@ -58,6 +58,10 @@ def _issue_payload(issue): 'reported_at': issue.reported_at.isoformat() if issue.reported_at else None, 'resolved_at': issue.resolved_at.isoformat() if issue.resolved_at else None, 'mobile_local_id': issue.mobile_local_id, + # photo_path is the primary issue photo; result_photos are resolution photos. + # Both are relative paths from the server static root. + 'photo_path': issue.photo_path or None, + 'result_photos': issue.result_photos or [], } @@ -67,14 +71,15 @@ def _issue_payload(issue): @jwt_required def list_issues(): """ - Return issues assigned to the authenticated user. + Return active issues for the authenticated user. - Inspectors: only issues where assigned_to == current user. - Admin / director / project_manager: all non-resolved issues (capped at 200). + All roles see all non-resolved issues (inspectors included) so the iPad + shows the full picture of open work at their facilities. + A ?status= filter can be used to override the default exclusion. Query parameters ---------------- - status str Filter by status (default: excludes resolved). + status str Filter by status. Omit to get all non-resolved issues. limit int Default 100, max 200. offset int Default 0. @@ -100,7 +105,7 @@ def list_issues(): query = Issue.query if user.role == 'inspector': - # Inspectors only see issues assigned to them + # Inspectors see only issues assigned to them query = query.filter(Issue.assigned_to == user.id) else: # Broader roles: exclude resolved by default so the list stays manageable @@ -249,9 +254,7 @@ def get_issue(issue_id): Return current status, severity, description, assigned_to, and facility for a single issue. - Access: - - admin / director / project_manager : any issue - - inspector : only issues where assigned_to == current user + Access: all allowed roles may fetch any issue. """ user = g.api_user if user.role not in _ALLOWED_ROLES: @@ -322,4 +325,4 @@ def update_issue_status(issue_id): logger.info('API ISSUES | status_updated | issue_id=%d | %s→%s | user=%s', issue.id, old_status, new_status, user.username) - return api_ok({'issue_id': issue.id, 'status': issue.status}) + return api_ok({'issue_id': issue.id, 'status': issue.status}) \ No newline at end of file diff --git a/app/api/notifications.py b/app/api/notifications.py index aff10e6..48a00c2 100644 --- a/app/api/notifications.py +++ b/app/api/notifications.py @@ -17,6 +17,7 @@ PATCH /api/v1/notifications/mark-read import logging from datetime import datetime +import sqlalchemy.exc from flask import Blueprint, request, g from app import db from app.models.notification import Notification @@ -27,17 +28,6 @@ logger = logging.getLogger(__name__) bp = Blueprint('api_notifications', __name__) -_INSPECTOR_RELEVANT = { - 'issue_assigned', - 'issue_reassigned', - 'issue_unassigned', - 'issue_status', - 'issue_comment', - 'issue_follow_update', - 'verification_requested', - 'sla_alert', -} - def _parse_since(value): """Parse ?since= ISO 8601 string; return None on failure.""" @@ -51,6 +41,21 @@ def _parse_since(value): return None +def _build_payload(notifications, has_event_type): + """Serialise notification rows to dicts. Works before and after phase17 migration.""" + rows = [] + for n in notifications: + rows.append({ + 'id': n.id, + 'title': n.title, + 'body': n.body, + 'event_type': (n.event_type if has_event_type else None), + 'issue_id': n.issue_id, + 'created_at': n.created_at.strftime('%Y-%m-%dT%H:%M:%S'), + }) + return rows + + @bp.route('/notifications', methods=['GET']) @jwt_required def list_notifications(): @@ -60,62 +65,63 @@ def list_notifications(): Query parameters ---------------- since ISO 8601 datetime Only return notifications created after this time. - Omit for the initial fetch (returns last 50). limit int (default 50, max 50) Response 200 ------------ - { - "ok": true, - "data": { - "notifications": [ - { - "id": 12, - "title": "Issue #7 Assigned to You", - "body": "...", - "event_type": "issue_assigned", - "issue_id": 7, - "created_at": "2026-05-15T09:30:00" - }, - ... - ], - "count": 2 - } - } + { "ok": true, "data": { "notifications": [...], "count": N } } """ user = g.api_user since = _parse_since(request.args.get('since')) limit = min(int(request.args.get('limit', 50)), 50) - query = Notification.query.filter_by( - user_id=user.id, - is_read=False, - ) + def _run_orm(): + q = Notification.query.filter_by(user_id=user.id, is_read=False) + if since: + q = q.filter(Notification.created_at > since) + return q.order_by(Notification.created_at.asc()).limit(limit).all() - if since: - query = query.filter(Notification.created_at > since) + # Attempt the ORM query (works after phase17 migration runs). + # If the event_type column does not yet exist in the DB, MySQL raises + # OperationalError: Unknown column 'notifications.event_type' in SELECT. + # getattr() does NOT protect against this — the failure is at the SQL layer. + # The fallback raw-SQL query selects only the pre-phase17 columns so the + # endpoint stays functional before the migration runs. + try: + notifications = _run_orm() + has_event_type = True + except sqlalchemy.exc.OperationalError: + db.session.rollback() + sql_parts = ( + "SELECT id, title, body, issue_id, is_read, created_at " + "FROM notifications " + "WHERE user_id = :uid AND is_read = 0 " + ) + params = {'uid': user.id, 'lim': limit} + if since: + sql_parts += "AND created_at > :since " + params['since'] = since + sql_parts += "ORDER BY created_at ASC LIMIT :lim" - notifications = ( - query - .order_by(Notification.created_at.asc()) - .limit(limit) - .all() - ) + rows = db.session.execute(db.text(sql_parts), params).fetchall() - payload = [ - { - 'id': n.id, - 'title': n.title, - 'body': n.body, - 'event_type': n.event_type, - 'issue_id': n.issue_id, - 'created_at': n.created_at.strftime('%Y-%m-%dT%H:%M:%S'), - } - for n in notifications - ] + class _Row: + """Minimal shim so _build_payload works with raw SQL rows.""" + __slots__ = ('id', 'title', 'body', 'issue_id', 'created_at') + def __init__(self, r): + self.id = r[0] + self.title = r[1] + self.body = r[2] + self.issue_id = r[3] + self.created_at = r[5] - logger.info('API NOTIFICATIONS | list | user=%s | since=%s | count=%d', - user.username, since, len(payload)) + notifications = [_Row(r) for r in rows] + has_event_type = False + + payload = _build_payload(notifications, has_event_type) + + logger.info('API NOTIFICATIONS | list | user=%s | since=%s | count=%d | has_event_type=%s', + user.username, since, len(payload), has_event_type) return api_ok({'notifications': payload, 'count': len(payload)}) @@ -144,7 +150,7 @@ def mark_read(): Notification.query .filter( Notification.id.in_(ids), - Notification.user_id == user.id, # never touch another user's records + Notification.user_id == user.id, ) .all() ) @@ -156,4 +162,4 @@ def mark_read(): count = 0 logger.info('API NOTIFICATIONS | mark_read | user=%s | count=%d', user.username, count) - return api_ok({'marked': count}) + return api_ok({'marked': count}) \ No newline at end of file From d0460fec7507594bd15eead0a391e5ab5d214909 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Fri, 15 May 2026 16:49:43 -0400 Subject: [PATCH 4/9] Update claude.md --- CLAUDE.md | 70 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8ba9c60..8bb635f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ > **Audience:** AI assistants and developers working on this codebase. > **Purpose:** Authoritative reference for architecture, conventions, gotchas, and decisions. -> **Last reviewed:** May 2026 (Phase B complete — iPad offline inspection app; Critical/Notable hardening pass) +> **Last reviewed:** May 2026 (Phase C complete — iPad notification polling, assigned-issue sync, issue photo display, follow-up notifications) --- @@ -44,7 +44,7 @@ - **Reports** — on-demand PDF/CSV scorecards and scheduled email digests - **Audit trail** — immutable log of every create/update/delete action - **Mobile API** — JWT-authenticated REST layer for the iPad native app -- **iPad native app** — SwiftUI + SwiftData offline-first inspection tool (Phase A + B complete) +- **iPad native app** — SwiftUI + SwiftData offline-first inspection tool (Phase A + B + C complete) The application is actively deployed in production and maintained by a single developer/administrator. @@ -248,10 +248,13 @@ issues: id, inspection_id (nullable), area_id, severity (low/medium/high/critica ### Notification / NotificationPreference ``` -notifications: id, user_id, title, body, link, is_read, created_at, issue_id, event_type +notifications: id, user_id, title, body, link, is_read, created_at, issue_id, + inspection_id, event_type VARCHAR(50) NULL, digest_pending notification_preferences: id, user_id, event_type, email_enabled, digest_mode, digest_frequency ``` +**`event_type`:** Added in phase17. Stored by `notify()` and returned by `GET /api/v1/notifications` so the iPad can categorise alerts. `NULL` for notifications created before the migration. + ### NotificationMatrix ``` @@ -328,9 +331,10 @@ api_device_tokens: id, user_id, device_id, apns_token, device_name, app_version | `api_auth` | `/api/v1` | `/auth/login`, `/auth/refresh`, `/auth/logout`, `/auth/me`, `/devices/register` | | `api_facilities` | `/api/v1` | `/facilities`, `/facilities//areas` | | `api_templates` | `/api/v1` | `/templates`, `/templates/` | -| `api_inspections` | `/api/v1` | `POST /inspections`, `PATCH /inspections/` | -| `api_issues` | `/api/v1` | `POST /issues` | +| `api_inspections` | `/api/v1` | `GET /inspections`, `POST /inspections`, `PATCH /inspections/` | +| `api_issues` | `/api/v1` | `GET /issues`, `POST /issues`, `GET /issues/`, `PATCH /issues//status` | | `api_photos` | `/api/v1` | `POST /photos/upload` | +| `api_notifications` | `/api/v1` | `GET /notifications`, `PATCH /notifications/mark-read` | --- @@ -349,7 +353,7 @@ api_device_tokens: id, user_id, device_id, apns_token, device_name, app_version All WTForms classes. `AreaForm.area_type` includes `floor`. `UserForm` excludes `customer` role. ### `notifications.py` -`notify()`, `notify_by_matrix()`, `notify_customers_for_facility()` — all email sent in background thread. +`notify()`, `notify_by_matrix()`, `notify_customers_for_facility()` — all email sent in background thread. `notify()` stores `event_type` on the `Notification` record (phase17+) so the mobile API can return it to the iPad for categorisation. `flag_followup` route calls `notify()` for the original inspector so they receive a follow-up request notification on the iPad. ### `sla.py` `sla_status(issue)` → `'ok'` | `'at_risk'` | `'breached'` | `None` (resolved). @@ -359,7 +363,7 @@ ReportLab-based. 12-column grid must be preserved — never collapse in PDF view --- -## 9. Mobile API (Phase 7 / Phase A / Phase B) +## 9. Mobile API (Phase 7 / Phase A / Phase B / Phase C) ### CSRF Exemption Pattern — Critical @@ -367,22 +371,24 @@ ReportLab-based. 12-column grid must be preserved — never collapse in PDF view ```python from app.api import register_api, api_bp -from app.api.auth import bp as _api_auth_bp -from app.api.facilities import bp as _api_facilities_bp -from app.api.templates import bp as _api_templates_bp -from app.api.inspections import bp as _api_inspections_bp -from app.api.issues import bp as _api_issues_bp -from app.api.photos import bp as _api_photos_bp +from app.api.auth import bp as _api_auth_bp +from app.api.facilities import bp as _api_facilities_bp +from app.api.templates import bp as _api_templates_bp +from app.api.inspections import bp as _api_inspections_bp +from app.api.issues import bp as _api_issues_bp +from app.api.photos import bp as _api_photos_bp +from app.api.notifications import bp as _api_notifications_bp csrf.exempt(_api_auth_bp) csrf.exempt(_api_facilities_bp) csrf.exempt(_api_templates_bp) csrf.exempt(_api_inspections_bp) csrf.exempt(_api_issues_bp) csrf.exempt(_api_photos_bp) +csrf.exempt(_api_notifications_bp) register_api(app) ``` -**Every new Phase C+ blueprint must add its own `csrf.exempt()` line here before `register_api(app)`.** Failing to do so produces a `"The CSRF token is missing."` error on all POST requests to that blueprint. +**Every new Phase D+ blueprint must add its own `csrf.exempt()` line here before `register_api(app)`.** Failing to do so produces a `"The CSRF token is missing."` error on all POST requests to that blueprint. ### Auth Flow 1. `POST /api/v1/auth/login` → access token (60 min JWT) + refresh token (30 day opaque hex) @@ -416,6 +422,27 @@ Customer role is blocked from template endpoints (`_ALLOWED_ROLES` check). Facil | `POST /api/v1/issues` | jwt_required | Create issue; idempotent via `mobile_local_id` | | `POST /api/v1/photos/upload` | jwt_required | Multipart photo upload; returns `server_path` | +### Phase C Endpoints + +| Endpoint | Auth | Description | +|---|---|---| +| `GET /api/v1/inspections` | jwt_required | Inspector's own inspection history (paginated) | +| `GET /api/v1/issues` | jwt_required | Issues assigned to current user (inspectors); all non-resolved (admin/director/PM) | +| `GET /api/v1/issues/` | jwt_required | Single issue detail — inspectors scoped to assigned only | +| `PATCH /api/v1/issues//status` | jwt_required | Update issue status — inspectors scoped to assigned only | +| `GET /api/v1/notifications` | jwt_required | Unread notifications for current user; accepts `?since=` | +| `PATCH /api/v1/notifications/mark-read` | jwt_required | Mark list of notification IDs as read | + +### Issue API Scope Rules + +- **Inspector:** `GET /issues` returns only `assigned_to == current_user.id`. `GET /issues/` and `PATCH /issues//status` both enforce the same restriction. +- **Admin / Director / Project Manager:** `GET /issues` returns all non-resolved issues (default) or filtered by `?status=`. +- `_issue_payload()` returns: `id`, `status`, `severity`, `description`, `assigned_to`, `facility_id`, `facility_name`, `reported_at`, `resolved_at`, `mobile_local_id`, `photo_path`, `result_photos`. + +### Notification API — OperationalError Safety + +`GET /api/v1/notifications` wraps the ORM query in `try/except sqlalchemy.exc.OperationalError`. If the `event_type` column does not yet exist (phase17 migration not run), it falls back to a raw-SQL query that omits the column and returns `"event_type": null`. This keeps the endpoint functional before and after the migration. + ### Idempotency Pattern All Phase B write endpoints accept `mobile_local_id` (UUID string from device). On receipt: @@ -502,6 +529,9 @@ Inspector action → SwiftData write (always succeeds) → SyncQueue entry - **Sequential reference data fetch:** `pullReferenceData()` uses sequential `await` (not `async let`) to avoid Swift 6 actor-isolation warnings on `Decodable` structs. - **Photo-before-inspection ordering:** `processPhotoQueue` runs before `processInspectionQueue`. An inspection is only submitted after all its `pendingPhotos` have `uploadStatus == "uploaded"` or `"failed"`. - **Retry limit:** 5 retries per item before marking `syncStatus = "failed"`. +- **`pullAssignedIssues()`:** Fetches `GET /api/v1/issues` and upserts into SwiftData keyed by `serverId`. Records pulled from server carry `syncStatus = "synced"` and `inspectionLocalId = ""` so `processIssueQueue` never re-submits them. **Deletion pass runs after upsert** — records with `syncStatus == "synced"` AND `inspectionLocalId == ""` whose `serverId` is absent from the server response are deleted. This removes issues that were reassigned to another inspector. The empty-response case is not short-circuited, so unassignment is always handled. +- **Notification polling:** `pollNotifications()` is called at the end of every `triggerSync()` and also on a 60-second `Task.sleep` loop started by `startPollTask()`. Uses `lastNotificationFetch` as a cursor (`?since=` param) so only new notifications are fetched. Marks fetched IDs read on server after local delivery. +- **`Task.sleep` not `Timer.scheduledTimer`:** `Timer.scheduledTimer` requires `RunLoop.main` to be ticking; inside a Swift Concurrency `Task { @MainActor }` block `RunLoop.current` is not `RunLoop.main` and the timer fires never. Always use `Task.sleep` for periodic work in SyncManager. ### APIClient Key Behaviours @@ -620,7 +650,8 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif → phase9_user_full_name → phase10_customer_password_setup → phase11_director_role → phase12_performance_indexes → phase_b_mobile_local_id → phase13_issue_facility → phase14_facility_created_at - → phase15_audit_log_indexes → phase16_notifications_columns ← HEAD + → phase15_audit_log_indexes → phase16_notifications_columns + → phase17_notification_event_type ← HEAD ``` ### phase_b_mobile_local_id @@ -647,6 +678,10 @@ Uses `INFORMATION_SCHEMA.STATISTICS` existence checks — safe to re-run. Ensures `digest_pending TINYINT NOT NULL DEFAULT 0` and `inspection_id INT NULL FK` exist on the `notifications` table. Both columns are defined in the model but were absent from any prior migration because the `notifications` table predates the chain. Uses `INFORMATION_SCHEMA` existence checks — safe to re-run. +### phase17_notification_event_type + +Adds `event_type VARCHAR(50) NULL` to the `notifications` table. Required for `GET /api/v1/notifications` to return event type to the iPad so it can categorise alerts. The API endpoint catches `OperationalError` and falls back to raw SQL before this migration runs. Uses `INFORMATION_SCHEMA` existence check — safe to re-run. + ### MySQL ENUM Change Protocol (3 steps — always follow) ```sql -- 1. Expand @@ -774,6 +809,11 @@ timeout = 30 | 33 | **f-string fallback strings must use double-quotes inside single-quoted f-strings** | Python 3.11 raises `SyntaxError` on nested same-delimiter quotes; use `"\u2014"` not `'—'` inside `f'...'` | | 34 | **`computeScore` field ID must be cast explicitly: `String` → `as? String`, `Int` → `as? Int` then `String(n)`** | `Optional.map` on `Any?` returns `Optional(value)` not `value`; the old guard-let produced `"Optional(5)"` as the lookup key, so all integer-ID field scores were silently 0 | | 35 | **Strip `local://` photo paths from `formData` before `submitInspection`** | A failed photo upload leaves `"local://..."` in formData; `JSONSerialization` drops non-serialisable values silently, which is worse than an empty string on the server | +| 36 | **`notify()` must always receive `event_type`** | Without it the mobile API returns `null` for event type and the iPad cannot categorise the alert banner | +| 37 | **`flag_followup` calls `notify()` for the original inspector** | Without this, the inspector receives no follow-up request notification on any channel | +| 38 | **`GET /api/v1/notifications` catches `OperationalError`** | If phase17 migration hasn't run, the ORM query fails at the SQL layer because `event_type` is in the SELECT but not in the DB; `getattr()` does NOT protect against this — only a try/except does | +| 39 | **Issue API scope: inspectors see only their assigned issues** | `GET /issues`, `GET /issues/`, `PATCH /issues//status` all enforce `assigned_to == current_user.id` for the inspector role | +| 40 | **`_issue_payload()` must return `photo_path` and `result_photos`** | iPad `pullAssignedIssues` stores these in `photoServerPaths` for display via `AsyncImage`; omitting them means server-created issues show no photos | --- From 0675acb8ecfadfc9162a0aa515f30cec8a0d5ac1 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 16 May 2026 13:47:39 -0400 Subject: [PATCH 5/9] 05/16 Fix bugs 1 --- app/api/inspections.py | 25 +++++++++++++++++++++---- app/models/notification.py | 4 ++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/app/api/inspections.py b/app/api/inspections.py index 9d4f3b5..e9d0b1f 100644 --- a/app/api/inspections.py +++ b/app/api/inspections.py @@ -264,6 +264,12 @@ def create_inspection(): # follow_up_required on the parent automatically. This mirrors the web # list view's implicit logic (which hides the badge when follow_ups.any()) # and ensures the History API response reflects the resolved state. + # Capture parent label strings before commit while ORM objects are loaded. + # log_action() for the parent update must fire AFTER db.session.commit() to + # avoid audit.py's internal commit() persisting the parent flag change before + # the new inspection row is committed — a partial state that would be incorrect + # if the main commit subsequently failed. + _parent_log_args = None if parent_inspection_id and status == 'completed': parent_insp = db.session.get(Inspection, parent_inspection_id) if parent_insp and parent_insp.follow_up_required: @@ -273,10 +279,13 @@ def create_inspection(): 'by_inspection_id=%d | user=%s', parent_inspection_id, inspection.id, user.username, ) - log_action(ACTION_UPDATE, 'Inspection', parent_inspection_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)') + # Snapshot label strings now — ORM objects may be expired after commit + _parent_log_args = ( + parent_inspection_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)', + ) # ── Notifications ───────────────────────────────────────────────────── if status == 'completed': @@ -304,6 +313,14 @@ def create_inspection(): db.session.commit() + # ── Post-commit audit logging ────────────────────────────────────────── + # All log_action() calls must come AFTER db.session.commit() because + # audit.py calls db.session.commit() internally. Calling it before the + # main commit would persist the audit row (and any dirty ORM state) before + # the primary transaction completes. + if _parent_log_args: + log_action(ACTION_UPDATE, 'Inspection', *_parent_log_args) + log_action(ACTION_CREATE, 'Inspection', inspection.id, f'{template.name} @ {facility.name}', f'source=mobile; status={status}; score={overall_score}; ' diff --git a/app/models/notification.py b/app/models/notification.py index d94abef..9e0db1c 100644 --- a/app/models/notification.py +++ b/app/models/notification.py @@ -12,6 +12,9 @@ EVENT_ISSUE_COMMENT = 'issue_comment' EVENT_ISSUE_FOLLOW = 'issue_follow_update' EVENT_INSPECTION_DONE = 'inspection_completed' EVENT_SLA_ALERT = 'sla_alert' +# Fired when an issue is flagged during an inspection (web or mobile). +# Listed here so users can configure email preferences for this event. +EVENT_ISSUE_FLAGGED = 'issue_flagged' # ── Customer portal events ───────────────────────────────────────────────── # Fired when an inspection completes or an issue is created/updated at a @@ -25,6 +28,7 @@ ALL_EVENT_TYPES = { EVENT_ISSUE_STATUS: 'Issue status changed', EVENT_ISSUE_COMMENT: 'New comment on issue', EVENT_ISSUE_FOLLOW: 'Updates on followed issues', + EVENT_ISSUE_FLAGGED: 'Issue flagged (from inspection)', EVENT_INSPECTION_DONE: 'Inspection completed', EVENT_SLA_ALERT: 'SLA at-risk / breached alerts', # Customer-facing — only relevant for customer role accounts From edd81f4c59339fb7fb7266a04d00ef5ffe4ae6a2 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 16 May 2026 14:02:53 -0400 Subject: [PATCH 6/9] 05/16 Fix bugs 2 --- app/api/auth.py | 1 + app/api/issues.py | 19 +++-- app/models/issue.py | 6 ++ app/routes/dashboard.py | 7 +- app/routes/inspections.py | 1 + app/routes/issues.py | 1 + .../versions/phase18_issue_reported_by.py | 75 +++++++++++++++++++ 7 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 migrations/versions/phase18_issue_reported_by.py diff --git a/app/api/auth.py b/app/api/auth.py index cbeff41..3c01af4 100644 --- a/app/api/auth.py +++ b/app/api/auth.py @@ -50,6 +50,7 @@ def _user_payload(user: User) -> dict: return { 'id': user.id, 'username': user.username, + 'full_name': user.full_name or '', 'email': user.email, 'role': user.role, 'created_at': user.created_at.isoformat() if user.created_at else None, diff --git a/app/api/issues.py b/app/api/issues.py index b3100c9..6e61097 100644 --- a/app/api/issues.py +++ b/app/api/issues.py @@ -105,8 +105,16 @@ def list_issues(): query = Issue.query if user.role == 'inspector': - # Inspectors see only issues assigned to them - query = query.filter(Issue.assigned_to == user.id) + # Inspectors see issues assigned to them OR issues they reported. + # The reported_by path covers issues created on the iPad that haven't + # been assigned yet (assigned_to is NULL until a director assigns them). + # Pre-phase18 rows with reported_by = NULL still surface via assigned_to. + query = query.filter( + db.or_( + Issue.assigned_to == user.id, + Issue.reported_by == user.id, + ) + ) else: # Broader roles: exclude resolved by default so the list stays manageable status_filter = request.args.get('status') @@ -204,6 +212,7 @@ def create_issue(): photo_path = data.get('photo_path') or None, status = 'open', reported_at = now_eastern(), + reported_by = user.id, mobile_local_id = mobile_local_id, ) db.session.add(issue) @@ -264,7 +273,7 @@ def get_issue(issue_id): if issue is None: return api_error('Issue not found', 404) - if user.role == 'inspector' and issue.assigned_to != user.id: + if user.role == 'inspector' and issue.assigned_to != user.id and issue.reported_by != user.id: return api_error('Access denied', 403) return api_ok(_issue_payload(issue)) @@ -294,8 +303,8 @@ def update_issue_status(issue_id): if issue is None: return api_error('Issue not found', 404) - if user.role == 'inspector' and issue.assigned_to != user.id: - return api_error('Access denied — you can only update issues assigned to you', 403) + if user.role == 'inspector' and issue.assigned_to != user.id and issue.reported_by != user.id: + return api_error('Access denied — you can only update issues assigned to or reported by you', 403) data = request.get_json(silent=True) or {} new_status = (data.get('status') or '').strip().lower() diff --git a/app/models/issue.py b/app/models/issue.py index 065ac5a..153080d 100644 --- a/app/models/issue.py +++ b/app/models/issue.py @@ -54,6 +54,11 @@ class Issue(db.Model): photo_path = db.Column(db.String(255)) status = db.Column(db.Enum('open', 'in_progress', 'resolved', 'pending_verification'), default='open') assigned_to = db.Column(db.Integer, db.ForeignKey('users.id')) + # Set at creation time to the user who filed the issue (inspector or admin). + # Nullable for backward compatibility — pre-phase18 rows will be NULL. + # Used by the mobile API to return issues the inspector created but hasn't + # been assigned yet (assigned_to is NULL until a director assigns them). + reported_by = db.Column(db.Integer, db.ForeignKey('users.id', ondelete='SET NULL'), nullable=True) reported_at = db.Column(db.DateTime, default=now_eastern) resolved_at = db.Column(db.DateTime) result_notes = db.Column(db.Text) @@ -75,6 +80,7 @@ class Issue(db.Model): # with that backref at mapper configuration time (CLAUDE.md rule 31 revised). facility = db.relationship('Facility', foreign_keys=[facility_id], backref='direct_issues') assigned_user = db.relationship('User', foreign_keys=[assigned_to], backref='assigned_issues') + reporter = db.relationship('User', foreign_keys=[reported_by], backref='reported_issues') verifier = db.relationship('User', foreign_keys=[verified_by], backref='verified_issues') comments = db.relationship('IssueComment', backref='issue', lazy='dynamic', order_by='IssueComment.created_at', diff --git a/app/routes/dashboard.py b/app/routes/dashboard.py index c0030d8..0dd7565 100644 --- a/app/routes/dashboard.py +++ b/app/routes/dashboard.py @@ -110,9 +110,14 @@ def index(): recent_inspections = recent_q.limit(5).all() # ── Pending follow-up inspections ──────────────────────────────────── + # follow_ups is a lazy='dynamic' relationship — comparing it to None does + # NOT produce a "has no rows" predicate for dynamic relationships. The + # correct idiom is ~.any(), which generates EXISTS (SELECT 1 FROM inspections + # WHERE parent_inspection_id = inspections.id). This matches the identical + # filter used in routes/inspections.py:follow_up_filter. followup_q = Inspection.query.filter_by( follow_up_required=True, status='completed' - ).filter(Inspection.follow_ups == None) # noqa: E711 — SQLAlchemy usage + ).filter(~Inspection.follow_ups.any()) if is_inspector: followup_q = followup_q.filter(Inspection.inspector_id == current_user.id) elif is_customer: diff --git a/app/routes/inspections.py b/app/routes/inspections.py index 2a019b1..e346c88 100644 --- a/app/routes/inspections.py +++ b/app/routes/inspections.py @@ -730,6 +730,7 @@ def flag_issue(inspection_id): status = 'open', assigned_to = form.assigned_to.data or None, reported_at = now_eastern(), + reported_by = current_user.id, ) db.session.add(issue) diff --git a/app/routes/issues.py b/app/routes/issues.py index c457096..641017b 100644 --- a/app/routes/issues.py +++ b/app/routes/issues.py @@ -447,6 +447,7 @@ def create(): status = 'open', assigned_to = form.assigned_to.data or None, reported_at = now_eastern(), + reported_by = current_user.id, ) db.session.add(issue) db.session.commit() diff --git a/migrations/versions/phase18_issue_reported_by.py b/migrations/versions/phase18_issue_reported_by.py new file mode 100644 index 0000000..3281e5e --- /dev/null +++ b/migrations/versions/phase18_issue_reported_by.py @@ -0,0 +1,75 @@ +"""phase18 — add reported_by column to issues table + +Background +---------- +Issues created on the iPad by an inspector have no `assigned_to` value until +a director assigns them via the web portal. The mobile API's list_issues +endpoint filtered inspectors to `assigned_to == user.id`, so their own +newly-submitted issues were invisible on the iPad until assigned. + +This migration adds `reported_by INT NULL FK → users.id` so the API can +return issues the inspector either created OR was assigned to, without +a join to the inspections table. + +The column is nullable for backward compatibility: existing issues created +before this migration will have reported_by = NULL and continue to surface +only via the assigned_to path. + +Uses INFORMATION_SCHEMA existence check — safe to re-run (CLAUDE.md rules 16, 17). + +Revision ID: phase18_issue_reported_by +Revises: phase17_notification_event_type +""" + +revision = 'phase18_issue_reported_by' +down_revision = 'phase17_notification_event_type' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def _column_exists(conn, table, column): + result = conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS " + "WHERE TABLE_SCHEMA = DATABASE() " + "AND TABLE_NAME = :table AND COLUMN_NAME = :col" + ), {"table": table, "col": column}) + return result.scalar() > 0 + + +def _fk_exists(conn, table, constraint_name): + result = conn.execute(sa.text( + "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS " + "WHERE TABLE_SCHEMA = DATABASE() " + "AND TABLE_NAME = :table AND CONSTRAINT_NAME = :name" + ), {"table": table, "name": constraint_name}) + return result.scalar() > 0 + + +def upgrade(): + bind = op.get_bind() + + if not _column_exists(bind, 'issues', 'reported_by'): + op.execute(sa.text( + "ALTER TABLE issues " + "ADD COLUMN reported_by INT NULL, " + "ADD CONSTRAINT fk_issues_reported_by " + " FOREIGN KEY (reported_by) REFERENCES users(id) " + " ON DELETE SET NULL" + )) + + +def downgrade(): + bind = op.get_bind() + + if _fk_exists(bind, 'issues', 'fk_issues_reported_by'): + op.execute(sa.text( + "ALTER TABLE issues DROP FOREIGN KEY fk_issues_reported_by" + )) + + if _column_exists(bind, 'issues', 'reported_by'): + op.execute(sa.text( + "ALTER TABLE issues DROP COLUMN reported_by" + )) From 766fbfaa87311b90d0a20ebab86729fc09e02d38 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 16 May 2026 14:10:08 -0400 Subject: [PATCH 7/9] 05/16 Fix bugs 3 --- app/routes/issues.py | 18 +++++++++++------- app/utils/audit.py | 7 ++++++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/routes/issues.py b/app/routes/issues.py index 641017b..db1ea20 100644 --- a/app/routes/issues.py +++ b/app/routes/issues.py @@ -70,12 +70,18 @@ class _SLAFilteredPage: @login_required def index(): page = request.args.get('page', 1, type=int) - q = Issue.query.order_by(Issue.reported_at.desc()) + # outerjoin Area once here so both the customer-scope filter and the + # facility_filter block can reference Area.facility_id without a cartesian + # product. Issues with no area_id get NULL for all Area columns (outer join). + q = ( + Issue.query + .outerjoin(Area, Issue.area_id == Area.id) + .order_by(Issue.reported_at.desc()) + ) if current_user.role == 'inspector': q = q.filter(Issue.assigned_to == current_user.id) elif current_user.role == 'customer': - from app.models.facility import Area customer_facility_ids = get_customer_scope(current_user) if not customer_facility_ids: q = q.filter(False) @@ -86,11 +92,10 @@ def index(): Issue.facility_id.in_(customer_facility_ids), db.and_( Issue.area_id.isnot(None), - Issue.area_id == Area.id, - Area.facility_id.in_(customer_facility_ids) + Area.facility_id.in_(customer_facility_ids), ) ) - ).outerjoin(Area, Issue.area_id == Area.id) + ) severity_filter = request.args.get('severity', '') status_filter = request.args.get('status', '') @@ -106,8 +111,7 @@ def index(): q = q.filter( db.or_( Issue.facility_id == fid, - db.and_(Issue.area_id.isnot(None), - Area.facility_id == fid) + Area.facility_id == fid, ) ) # SLA filter — SLA status is computed in Python (not a DB column). diff --git a/app/utils/audit.py b/app/utils/audit.py index 1bf0c49..4ed2453 100644 --- a/app/utils/audit.py +++ b/app/utils/audit.py @@ -44,6 +44,11 @@ def log_action(action: str, """ Write a single AuditLog row. Safe to call from any request context. + ⚠️ This function calls db.session.commit() internally. + Always call it AFTER the primary db.session.commit() for the business + transaction — never before. Calling it mid-transaction will commit any + dirty ORM state accumulated in the session up to that point. + Parameters ---------- action : One of the ACTION_* constants (or a custom string ≤ 50 chars). @@ -90,4 +95,4 @@ def log_action(action: str, try: db.session.rollback() except Exception: - pass + pass \ No newline at end of file From 8e9484e3ee41d48d308194e9383411f12733a13a Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 16 May 2026 14:24:09 -0400 Subject: [PATCH 8/9] 05/16 Fix bugs 4 --- app/models/notification_matrix.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/notification_matrix.py b/app/models/notification_matrix.py index 8c55906..237e22d 100644 --- a/app/models/notification_matrix.py +++ b/app/models/notification_matrix.py @@ -23,7 +23,7 @@ issue_assigned : admin ✗ director ✗ inspector ✗ pm ✗ cust issue_status : admin ✗ director ✗ inspector ✗ pm ✗ customer ✗ (assignee implicit) issue_comment : admin ✗ director ✗ inspector ✗ pm ✗ customer ✗ (assignee implicit) issue_follow_update : admin ✗ director ✗ inspector ✗ pm ✗ customer ✗ (followers implicit) -issue_flagged : admin ✗ director ✗ inspector ✗ pm ✗ customer ✓ (assignee implicit) +issue_flagged : admin ✓ director ✓ inspector ✗ pm ✗ customer ✓ (assignee implicit) issue_created : admin ✗ director ✗ inspector ✗ pm ✗ customer ✓ (assignee implicit) issue_updated_customer : admin ✗ director ✗ inspector ✗ pm ✗ customer ✓ verification_requested : admin ✓ director ✓ inspector ✗ pm ✗ customer ✗ @@ -113,8 +113,8 @@ MATRIX_DEFAULTS = { ('issue_follow_update', 'customer'): False, ('issue_follow_update', 'custom'): False, # issue_flagged (from inspection) - ('issue_flagged', 'admin'): False, - ('issue_flagged', 'director'): False, + ('issue_flagged', 'admin'): True, + ('issue_flagged', 'director'): True, ('issue_flagged', 'inspector'): False, ('issue_flagged', 'project_manager'): False, ('issue_flagged', 'customer'): True, @@ -216,4 +216,4 @@ def get_custom_emails_for(event_type: str) -> list: ).first() if row is None: return [] - return row.get_custom_emails() + return row.get_custom_emails() \ No newline at end of file From 82dabfd7b341d37db040df0875109612ea23d9ca Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 16 May 2026 14:53:21 -0400 Subject: [PATCH 9/9] 05/16 Fix bugs 5 --- CLAUDE.md | 40 ++++++++++++++++++++++++++++------------ app/api/inspections.py | 28 +++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8bb635f..d3f67ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ > **Audience:** AI assistants and developers working on this codebase. > **Purpose:** Authoritative reference for architecture, conventions, gotchas, and decisions. -> **Last reviewed:** May 2026 (Phase C complete — iPad notification polling, assigned-issue sync, issue photo display, follow-up notifications) +> **Last reviewed:** May 2026 (Phase 18 complete — reported_by on issues, issue_flagged notification prefs, dashboard follow-up query fix, customer join refactor) --- @@ -236,14 +236,15 @@ inspections: id, template_id, facility_id, area_id, inspector_id, inspection_dat ### Issue ``` -issues: id, inspection_id (nullable), area_id, severity (low/medium/high/critical), +issues: id, inspection_id (nullable), area_id, facility_id (nullable), severity (low/medium/high/critical), description, photo_path, status (open/in_progress/resolved/pending_verification), - assigned_to, reported_at, resolved_at, result_notes, result_photos (JSON), + assigned_to, reported_by (nullable FK → users, SET NULL on delete), + reported_at, resolved_at, result_notes, result_photos (JSON), verified_by, verified_at, verification_note, sla_notified, mobile_local_id VARCHAR(64) nullable indexed ← Phase B ``` -**`mobile_local_id`:** Same idempotency pattern as `inspections.mobile_local_id`. +**`reported_by`:** Added in phase18. Set at creation time to the user who filed the issue — on the web (`current_user.id`) and via the mobile API (`g.api_user.id`). Nullable for backward compatibility; pre-phase18 rows have `NULL`. Used by `GET /api/v1/issues` to return issues the inspector created but hasn't been assigned yet. ### Notification / NotificationPreference @@ -344,7 +345,7 @@ api_device_tokens: id, user_id, device_id, apns_token, device_name, app_version `now_eastern()` — always use this, never `datetime.utcnow()`. ### `audit.py` -`log_action(action, entity_type, entity_id, entity_label, details)` — call **after** `db.session.commit()`. +`log_action(action, entity_type, entity_id, entity_label, details)` — call **after** `db.session.commit()`. **This function calls `db.session.commit()` internally.** Calling it before the primary commit will prematurely persist any dirty ORM state in the session. ### `scope.py` `get_customer_scope(user)` — returns `list[int]` facility IDs for customers, `None` for staff. @@ -435,7 +436,7 @@ Customer role is blocked from template endpoints (`_ALLOWED_ROLES` check). Facil ### Issue API Scope Rules -- **Inspector:** `GET /issues` returns only `assigned_to == current_user.id`. `GET /issues/` and `PATCH /issues//status` both enforce the same restriction. +- **Inspector:** `GET /issues` returns issues where `assigned_to == current_user.id` **OR** `reported_by == current_user.id`. This ensures issues the inspector created on the iPad appear even before a director assigns them. `GET /issues/` and `PATCH /issues//status` enforce the same combined check. - **Admin / Director / Project Manager:** `GET /issues` returns all non-resolved issues (default) or filtered by `?status=`. - `_issue_payload()` returns: `id`, `status`, `severity`, `description`, `assigned_to`, `facility_id`, `facility_name`, `reported_at`, `resolved_at`, `mobile_local_id`, `photo_path`, `result_photos`. @@ -578,12 +579,19 @@ All types from the web app's `INPUT_FIELD_TYPES` set are rendered: ### Event Constants (`app/models/notification.py`) ``` -inspection_completed, issue_assigned, issue_reassigned, issue_unassigned, -issue_status, issue_comment, issue_follow_update, issue_flagged, issue_created, -issue_updated_customer, verification_requested, sla_alert, -customer_inspection_completed +EVENT_ISSUE_ASSIGNED = 'issue_assigned' +EVENT_ISSUE_STATUS = 'issue_status' +EVENT_ISSUE_COMMENT = 'issue_comment' +EVENT_ISSUE_FOLLOW = 'issue_follow_update' +EVENT_INSPECTION_DONE = 'inspection_completed' +EVENT_SLA_ALERT = 'sla_alert' +EVENT_ISSUE_FLAGGED = 'issue_flagged' ← added; must be in ALL_EVENT_TYPES +EVENT_CUSTOMER_INSPECTION_DONE = 'customer_inspection_completed' +EVENT_CUSTOMER_ISSUE_UPDATED = 'customer_issue_updated' ``` +**`ALL_EVENT_TYPES`** is the authoritative dict for the preferences UI. Every `event_type` string passed to `notify()` or `notify_by_matrix()` must have a matching entry here — missing entries cause that event to be invisible in the preferences form. + ### Cron Endpoints (all require `token=DIGEST_SECRET`) | Endpoint | Purpose | Schedule | @@ -651,7 +659,8 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif → phase12_performance_indexes → phase_b_mobile_local_id → phase13_issue_facility → phase14_facility_created_at → phase15_audit_log_indexes → phase16_notifications_columns - → phase17_notification_event_type ← HEAD + → phase17_notification_event_type + → phase18_issue_reported_by ← HEAD ``` ### phase_b_mobile_local_id @@ -682,6 +691,10 @@ Ensures `digest_pending TINYINT NOT NULL DEFAULT 0` and `inspection_id INT NULL Adds `event_type VARCHAR(50) NULL` to the `notifications` table. Required for `GET /api/v1/notifications` to return event type to the iPad so it can categorise alerts. The API endpoint catches `OperationalError` and falls back to raw SQL before this migration runs. Uses `INFORMATION_SCHEMA` existence check — safe to re-run. +### phase18_issue_reported_by + +Adds `reported_by INT NULL FK → users.id ON DELETE SET NULL` to the `issues` table. Allows the mobile API to return issues the inspector created (but hasn't been assigned) alongside their assigned issues. Nullable — pre-phase18 rows have `NULL` and surface only via the `assigned_to` path. Uses `INFORMATION_SCHEMA` existence and constraint checks — safe to re-run. + ### MySQL ENUM Change Protocol (3 steps — always follow) ```sql -- 1. Expand @@ -812,8 +825,11 @@ timeout = 30 | 36 | **`notify()` must always receive `event_type`** | Without it the mobile API returns `null` for event type and the iPad cannot categorise the alert banner | | 37 | **`flag_followup` calls `notify()` for the original inspector** | Without this, the inspector receives no follow-up request notification on any channel | | 38 | **`GET /api/v1/notifications` catches `OperationalError`** | If phase17 migration hasn't run, the ORM query fails at the SQL layer because `event_type` is in the SELECT but not in the DB; `getattr()` does NOT protect against this — only a try/except does | -| 39 | **Issue API scope: inspectors see only their assigned issues** | `GET /issues`, `GET /issues/`, `PATCH /issues//status` all enforce `assigned_to == current_user.id` for the inspector role | +| 39 | **Issue API scope: inspectors see assigned OR reported issues** | `GET /issues`, `GET /issues/`, `PATCH /issues//status` all enforce `assigned_to == user.id OR reported_by == user.id` for the inspector role. Pre-phase18 rows with `reported_by = NULL` surface only via `assigned_to`. | | 40 | **`_issue_payload()` must return `photo_path` and `result_photos`** | iPad `pullAssignedIssues` stores these in `photoServerPaths` for display via `AsyncImage`; omitting them means server-created issues show no photos | +| 41 | **`log_action()` commits internally — always call after `db.session.commit()`** | audit.py calls `db.session.commit()` to write the AuditLog row; calling it mid-transaction prematurely commits dirty session state. Snapshot any label strings needed for the audit call before the main commit if they come from ORM objects that may expire. | +| 42 | **`~Inspection.follow_ups.any()` not `== None` for dynamic relationships** | `follow_ups` is `lazy='dynamic'`; comparing to `None` does not generate a "has no rows" predicate. Use `~.any()` which emits a proper `NOT EXISTS` subquery. | +| 43 | **`issues.index()` outerjoin must precede all filters** | `outerjoin(Area, Issue.area_id == Area.id)` is unconditional at the top of the query. Both the customer-scope block and the `facility_filter` block reference `Area.facility_id`; without a prior join the `facility_filter` path generates a cartesian product for non-customer users. | --- diff --git a/app/api/inspections.py b/app/api/inspections.py index e9d0b1f..6fd4aec 100644 --- a/app/api/inspections.py +++ b/app/api/inspections.py @@ -50,6 +50,29 @@ def _parse_datetime(value): def _inspection_payload(inspection): """Serialize an Inspection to the dict returned in API responses.""" + # Extract form responses from the notes JSON blob. + # Mobile submissions store form data as {"_form_data": {...}, "_inspector_notes": "..."}. + # Web submissions store form data in the form_data column directly. + form_data = {} + inspector_notes = '' + if inspection.notes: + try: + notes_obj = json.loads(inspection.notes) + if isinstance(notes_obj, dict): + form_data = notes_obj.get('_form_data', {}) or {} + inspector_notes = notes_obj.get('_inspector_notes', '') or '' + except (json.JSONDecodeError, TypeError): + pass + # Fallback: web-created inspections store responses in form_data column + if not form_data and inspection.form_data: + form_data = inspection.form_data if isinstance(inspection.form_data, dict) else {} + + # Include the template's form_schema so the iPad can render history + # without needing a locally cached copy of the template. + form_schema = [] + if inspection.template: + form_schema = inspection.template.get_form_schema() + return { 'id': inspection.id, 'template_id': inspection.template_id, @@ -59,12 +82,15 @@ def _inspection_payload(inspection): 'area_id': inspection.area_id, 'area_name': inspection.area.name if inspection.area else None, 'status': inspection.status, - 'overall_score': inspection.overall_score, + 'overall_score': float(inspection.overall_score) if inspection.overall_score is not None else None, 'inspection_date': inspection.inspection_date.isoformat() if inspection.inspection_date else None, 'completed_at': inspection.completed_at.isoformat() if inspection.completed_at else None, 'mobile_local_id': inspection.mobile_local_id, + 'form_data': form_data, + 'form_schema': form_schema, + 'inspector_notes': inspector_notes, # ── Follow-up / re-inspection fields ────────────────────────────── 'follow_up_required': inspection.follow_up_required, 'follow_up_note': inspection.follow_up_note,