diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardViewModel.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardViewModel.kt index f9f4002..ca644fd 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardViewModel.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BoardViewModel.kt @@ -354,7 +354,6 @@ class BoardViewModel( is EditorAction.SaveText -> mutate { it.updateNote(id, listOf(NoteEdit.Body(action.body))) } - // Pinning re-sorts the board rather than emptying it, and on a phone // you often pin while still reading — so unlike the three below, it // deliberately leaves the editor open. diff --git a/core/src/local/schema.rs b/core/src/local/schema.rs index c16ec6c..7cd9867 100644 --- a/core/src/local/schema.rs +++ b/core/src/local/schema.rs @@ -257,15 +257,21 @@ fn migrate_v8(conn: &Connection) -> rusqlite::Result<()> { // The saved-filter sweep is the second half and not optional. `params` is opaque JSON // and a stored view could carry `"color": "teal"`; with the facet gone that key would // sit there forever, and a view that silently filters on a field the app no longer has -// is worse than one that lost a criterion. Guarded on `json_valid` because a corrupt -// blob must lose a key, not become NULL. +// is worse than one that visibly lost a criterion. Guarded on `json_valid` because a +// corrupt blob must keep whatever it holds, not become NULL. +// +// The second guard is a LIKE and not `json_extract(...) IS NOT NULL`, which is the +// obvious way to write it and is a trap: SQLite does not promise to short-circuit AND, +// so `json_extract` can be evaluated against the very rows `json_valid` was there to +// exclude — and on malformed input it does not return NULL, it RAISES, which would +// abort the migration for every other row too. `LIKE` is total over any text. const SCHEMA_V9: &str = r#" ALTER TABLE notes DROP COLUMN color; UPDATE saved_filters SET params = json_remove(params, '$.color') WHERE json_valid(params) - AND json_extract(params, '$.color') IS NOT NULL; + AND params LIKE '%"color"%'; "#; pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { @@ -415,12 +421,79 @@ mod tests { } #[test] - fn a_fresh_database_reaches_v8() { + fn a_fresh_database_reaches_the_latest_version() { let conn = Connection::open_in_memory().expect("open"); migrate(&conn).expect("migrate"); let version: i64 = conn .query_row("PRAGMA user_version", [], |r| r.get(0)) .expect("version"); - assert_eq!(version, 8); + assert_eq!(version, 9); + } + + /// The column is gone, not merely unread. Asserted by asking SQLite rather than by + /// reading a row: a SELECT that omits `color` would pass either way. + #[test] + fn v9_drops_the_note_colour_column() { + let conn = Connection::open_in_memory().expect("open"); + migrate(&conn).expect("migrate"); + let mut stmt = conn.prepare("PRAGMA table_info(notes)").expect("pragma"); + let columns: Vec = stmt + .query_map([], |r| r.get::<_, String>(1)) + .expect("query") + .collect::>>() + .expect("collect"); + assert!(!columns.iter().any(|c| c == "color")); + // The one that survived. Getting this wrong would take every tag's colour with + // it, which is the whole thing M315 was keeping. + let mut stmt = conn.prepare("PRAGMA table_info(labels)").expect("pragma"); + let label_columns: Vec = stmt + .query_map([], |r| r.get::<_, String>(1)) + .expect("query") + .collect::>>() + .expect("collect"); + assert!(label_columns.iter().any(|c| c == "color")); + } + + /// A stored view that filtered on colour loses that criterion and keeps the rest. + /// The alternative — leaving the key — is a lens that silently narrows on a field + /// the app no longer has and never says why it returned nothing. + #[test] + fn v9_sweeps_colour_out_of_saved_filters() { + let conn = Connection::open_in_memory().expect("open"); + conn.execute_batch("PRAGMA foreign_keys = ON;").expect("fk"); + for batch in [ + SCHEMA_V1, SCHEMA_V2, SCHEMA_V3, SCHEMA_V4, SCHEMA_V5, SCHEMA_V6, SCHEMA_V7, + ] { + conn.execute_batch(batch).expect("schema"); + } + conn.execute_batch("PRAGMA user_version = 8;").expect("v8"); + for (id, params) in [ + ("a", r#"{"color":"teal","q":"milk"}"#), + ("b", r#"{"q":"eggs"}"#), + // Not JSON at all. It must come out UNCHANGED rather than NULL — a blob + // this migration cannot read is not a blob it gets to destroy. + ("c", "not json"), + ] { + conn.execute( + "INSERT INTO saved_filters (id, name, params, created_at) + VALUES (?1, ?1, ?2, '2026-08-28T00:00:00.000Z')", + params![id, params], + ) + .expect("seed"); + } + + migrate(&conn).expect("migrate"); + + let read = |id: &str| -> String { + conn.query_row( + "SELECT params FROM saved_filters WHERE id = ?1", + [id], + |r| r.get(0), + ) + .expect("read") + }; + assert_eq!(read("a"), r#"{"q":"milk"}"#); + assert_eq!(read("b"), r#"{"q":"eggs"}"#); + assert_eq!(read("c"), "not json"); } }