05/16 Fix bugs 5
This commit is contained in:
Nguyen Ngo
@@ -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/<id>` and `PATCH /issues/<id>/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/<id>` and `PATCH /issues/<id>/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/<id>`, `PATCH /issues/<id>/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/<id>`, `PATCH /issues/<id>/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. |
|
||||
|
||||
---
|
||||
|
||||
|
||||
+27
-1
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user