From 100cfbc988a3f7280a8ad359cbca07004d7803a4 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Fri, 8 May 2026 13:36:52 -0400 Subject: [PATCH] 05/08 Updated code: fixed some issues 2 --- CLAUDE.md | 10 +- .../versions/phase16_notifications_columns.py | 100 ++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/phase16_notifications_columns.py diff --git a/CLAUDE.md b/CLAUDE.md index 8ede823..8ba9c60 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 | --- diff --git a/migrations/versions/phase16_notifications_columns.py b/migrations/versions/phase16_notifications_columns.py new file mode 100644 index 0000000..b76b30b --- /dev/null +++ b/migrations/versions/phase16_notifications_columns.py @@ -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" + ))