Christophe Vila
b08f0152ef
Migration 0026 (activities table rebuild for the per-user unique
constraint) issued its own PRAGMA foreign_keys = OFF/ON inside the
migration's SQL content, but the whole migration file runs inside one
db.go tx.Begin()/tx.Exec()/tx.Commit() transaction, and SQLite documents
PRAGMA foreign_keys as a no-op once a transaction is open. As a result FK
enforcement never actually got disabled, so DROP TABLE activities
triggered SQLite's implicit DELETE FROM semantics, firing ON DELETE
CASCADE on every row in laps, activity_samples, and kind_assignments for
every activity -- silently, with no error. On a real upgrade with synced
data this would have permanently destroyed all lap/sample/classification
history. It was masked because every existing migration test runs
migrations back-to-back on an empty temp database with no pre-existing
child rows.
Fix: add "0026_activities_unique_constraint.sql" to db.go's
tableRebuildMigrations map so it gets the same autocommit-mode FK
disable/enable toggle (before/after the transaction) already used for
migrations 0023/0024/0025, and remove the now-redundant/misleading
mid-transaction PRAGMA lines from the migration file itself, matching
the established pattern.
Also restore the DEFAULT '' on event_type_key in migration 0026's
rebuilt activities table -- it was dropped from migration 0007's
original column definition during the rebuild, which broke every insert
that omits event_type_key and relies on that default
(TestClaimLegacyOwner_BindsExistingSingletonRowsToOneNewUser and others).
Extend TestRebuildMigrationsPreserveForeignKeyReferences to cover 0026:
seed a laps row and an activity_samples row (in addition to the existing
kind_assignments row) against a pre-existing activities row before the
rebuild migrations run, then verify after 0023-0026 complete that all
three child rows still exist and still reference the same activity, and
that FK enforcement rejects bogus activity_id/workout_kind_id afterward.
This is the regression guard that would have caught the original bug.
Note: TestClaimLegacyOwner_NoOpOnGenuinelyFreshInstall still fails on
this branch; verified it fails identically at the prior commit
(d8b7228), so it's a pre-existing, unrelated bug in ClaimLegacyOwner
(not migration 0026 or this FK-toggle bug) and out of scope for this fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
56 lines
3.2 KiB
SQL
56 lines
3.2 KiB
SQL
-- activities.garmin_activity_id's UNIQUE constraint (migration 0001) must become
|
|
-- per-user (UNIQUE(user_id, garmin_activity_id)) now that the per-user-profile
|
|
-- design allows multiple users to share the same Garmin activity ID. The old
|
|
-- constraint alone was intentionally left in place by migration 0022 as a
|
|
-- belts-and-suspenders safeguard (Garmin IDs are globally unique in practice),
|
|
-- but it must be removed now to allow Task 6's cross-user test to pass.
|
|
--
|
|
-- The FK enforcement toggle needed for this rebuild (DROP TABLE on a table
|
|
-- with real inbound foreign keys) happens in Go code around this migration's
|
|
-- execution (db.go's tableRebuildMigrations map), in autocommit mode before
|
|
-- the transaction begins -- a mid-transaction PRAGMA foreign_keys statement
|
|
-- is a documented no-op with modernc.org/sqlite, so it must not appear here.
|
|
--
|
|
-- event_type_key keeps the DEFAULT '' that migration 0007 originally gave
|
|
-- it (ALTER TABLE ... ADD COLUMN event_type_key TEXT NOT NULL DEFAULT '');
|
|
-- dropping the default here (as an earlier draft of this migration did)
|
|
-- broke every INSERT that omits event_type_key and relies on that default,
|
|
-- which several existing tests (e.g. TestClaimLegacyOwner_*) do.
|
|
|
|
CREATE TABLE activities_new (
|
|
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
|
user_id INTEGER REFERENCES users(id),
|
|
garmin_activity_id INTEGER NOT NULL,
|
|
event_type_key TEXT NOT NULL DEFAULT '',
|
|
workout_id INTEGER,
|
|
start_time_utc TEXT NOT NULL,
|
|
duration_seconds REAL NOT NULL,
|
|
distance_meters REAL NOT NULL,
|
|
avg_hr REAL,
|
|
max_hr REAL,
|
|
avg_speed_mps REAL,
|
|
elevation_gain_m REAL,
|
|
aerobic_training_effect REAL,
|
|
anaerobic_training_effect REAL,
|
|
vo2max_value REAL,
|
|
raw_json TEXT NOT NULL,
|
|
details_fetched_at TEXT,
|
|
details_raw_json TEXT,
|
|
splits_fetched_at TEXT,
|
|
workout_raw_json TEXT,
|
|
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
|
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
|
|
UNIQUE(user_id, garmin_activity_id)
|
|
);
|
|
|
|
INSERT INTO activities_new (id, user_id, garmin_activity_id, event_type_key, workout_id, start_time_utc, duration_seconds, distance_meters, avg_hr, max_hr, avg_speed_mps, elevation_gain_m, aerobic_training_effect, anaerobic_training_effect, vo2max_value, raw_json, details_fetched_at, details_raw_json, splits_fetched_at, workout_raw_json, created_at, updated_at)
|
|
SELECT id, user_id, garmin_activity_id, event_type_key, workout_id, start_time_utc, duration_seconds, distance_meters, avg_hr, max_hr, avg_speed_mps, elevation_gain_m, aerobic_training_effect, anaerobic_training_effect, vo2max_value, raw_json, details_fetched_at, details_raw_json, splits_fetched_at, workout_raw_json, created_at, updated_at
|
|
FROM activities;
|
|
|
|
DROP TABLE activities;
|
|
|
|
ALTER TABLE activities_new RENAME TO activities;
|
|
|
|
CREATE INDEX idx_activities_start_time ON activities(start_time_utc);
|
|
CREATE INDEX idx_activities_user_garmin_id ON activities(user_id, garmin_activity_id);
|