05/08 Updated code: fixed some issues 2

2026-05-08 13:36:52 -04:00
parent 24904c0ec7
commit 100cfbc988
2 changed files with 108 additions and 2 deletions
@@ -620,7 +620,7 @@ 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   ← HEAD
+                → phase15_audit_log_indexes → phase16_notifications_columns   ← HEAD
 ```

 ### phase_b_mobile_local_id
@@ -643,6 +643,10 @@ Uses direct `ALTER TABLE` + `INFORMATION_SCHEMA` check — safe to re-run.
 Adds individual indexes on `audit_logs.action` and `audit_logs.entity_type`.
 Uses `INFORMATION_SCHEMA.STATISTICS` existence checks — safe to re-run.

+### phase16_notifications_columns
+
+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.
+
 ### MySQL ENUM Change Protocol (3 steps — always follow)
 ```sql
 -- 1. Expand
@@ -749,7 +753,7 @@ timeout = 30
 | 12 | **`filter()` before `limit()`** | SQLAlchemy ordering requirement |
 | 13 | **Bulk queries in customer list** | Per-customer loops cause N+1 |
 | 14 | **Email in background thread** | Never block HTTP response |
-| 15 | **Open-redirect guards** | `_safe_next()` in auth.py; `_safe_referrer()` in customers.py |
+| 15 | **Open-redirect guards** | `safe_redirect_url()` in `app/utils/decorators.py` — the single canonical utility, imported by both `auth.py` and `customers.py` |
 | 16 | **`CREATE INDEX IF NOT EXISTS` not on MySQL < 8.0.12** | Use `INFORMATION_SCHEMA.STATISTICS` check |
 | 17 | **`batch_alter_table` is SQLite-only** | Use direct `ALTER TABLE` for MySQL migrations |
 | 18 | **Set `REDIS_URL` in production** | `memory://` is per-process; Gunicorn needs Redis for accurate shared counters |
@@ -768,6 +772,8 @@ timeout = 30
 | 31 | **Do NOT add an explicit `Issue.area` relationship** | `Area.issues` declares `backref='area'`, supplying `Issue.area` automatically. A second declaration on `Issue` raises `ConflictingBackreferences` at startup. The dependency is documented here; do not "fix" it by adding an explicit relationship. |
 | 32 | **Do not sync an issue when its parent `LocalInspection.syncStatus == "failed"`** | Submitting without `inspection_id` creates orphaned server records; mark issue `"failed"` instead |
 | 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 |

 ---

@@ -0,0 +1,100 @@
+"""phase16 — ensure digest_pending and inspection_id columns on notifications
+
+Background
+----------
+The `notifications` table was created before the Alembic migration chain was
+established (pre-phase1 baseline schema).  Two columns added to the model
+after the initial creation were never covered by a migration:
+
+  digest_pending  BOOLEAN NOT NULL DEFAULT 0  (used by the digest email system)
+  inspection_id   INT NULL FK → inspections.id ON DELETE CASCADE
+
+Without this migration, any instance whose `notifications` table was created
+from the original baseline (rather than from the current model) will raise
+`OperationalError: Unknown column 'notifications.digest_pending'` the first
+time a notification is created, and the digest email cron will fail entirely.
+
+All checks use INFORMATION_SCHEMA so the migration is safe to re-run on any
+MySQL version ≥ 5.7 (CLAUDE.md rules 16, 17).
+
+Revision ID: phase16_notifications_columns
+Revises:     phase15_audit_log_indexes
+"""
+
+revision      = 'phase16_notifications_columns'
+down_revision = 'phase15_audit_log_indexes'
+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 _index_exists(conn, table, index_name):
+    result = conn.execute(sa.text(
+        "SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS "
+        "WHERE TABLE_SCHEMA = DATABASE() "
+        "AND TABLE_NAME = :table AND INDEX_NAME = :idx"
+    ), {"table": table, "idx": index_name})
+    return result.scalar() > 0
+
+
+def upgrade():
+    bind = op.get_bind()
+
+    # 1. digest_pending — Boolean NOT NULL DEFAULT 0
+    #    Used by notify() to flag notifications for digest delivery, and by
+    #    send_pending_digests() to find and clear them after delivery.
+    if not _column_exists(bind, 'notifications', 'digest_pending'):
+        op.execute(sa.text(
+            "ALTER TABLE notifications "
+            "ADD COLUMN digest_pending TINYINT(1) NOT NULL DEFAULT 0"
+        ))
+
+    # 2. Index on digest_pending — the digest cron filters on this column
+    if not _index_exists(bind, 'notifications', 'ix_notifications_digest_pending'):
+        op.execute(sa.text(
+            "CREATE INDEX ix_notifications_digest_pending "
+            "ON notifications (digest_pending)"
+        ))
+
+    # 3. inspection_id — nullable FK to inspections, CASCADE on delete
+    #    Allows the notification bell to link directly to an inspection.
+    if not _column_exists(bind, 'notifications', 'inspection_id'):
+        op.execute(sa.text(
+            "ALTER TABLE notifications "
+            "ADD COLUMN inspection_id INT NULL, "
+            "ADD CONSTRAINT fk_notifications_inspection_id "
+            "    FOREIGN KEY (inspection_id) REFERENCES inspections(id) "
+            "    ON DELETE CASCADE"
+        ))
+
+
+def downgrade():
+    bind = op.get_bind()
+
+    if _column_exists(bind, 'notifications', 'inspection_id'):
+        op.execute(sa.text(
+            "ALTER TABLE notifications "
+            "DROP FOREIGN KEY fk_notifications_inspection_id, "
+            "DROP COLUMN inspection_id"
+        ))
+
+    if _index_exists(bind, 'notifications', 'ix_notifications_digest_pending'):
+        op.execute(sa.text(
+            "DROP INDEX ix_notifications_digest_pending ON notifications"
+        ))
+
+    if _column_exists(bind, 'notifications', 'digest_pending'):
+        op.execute(sa.text(
+            "ALTER TABLE notifications DROP COLUMN digest_pending"
+        ))