From 668f7faf034c5c69ea7d001a0baf9ffdca637091 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 24 Aug 2026 00:41:23 -0400 Subject: [PATCH] core: the body is the checklist, and checklist_items is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M304 steps 2 and the client half of 4, together — they cannot be separated. A commit where the store writes items into the body while push.rs still reads them from a table is one that silently pushes the wrong list, and dev publishes to the dev channel on every green build. Store: * load_items becomes items_of(body) — a parse, not a query. An item's id is its ORDINAL, which is all it ever amounted to: push.rs sent text and checked and never an id, and both sides replaced the whole list on every sync. * add_item / update_item / delete_item route through update_note, so they get revision snapshotting, #tag re-derivation and the dirty/updated_at bookkeeping without any of it being written a second time. * create_note folds its items: input into the body, and syncs tags from the FOLDED body — an item can carry a #tag too. * display_title no longer takes items, because items ARE body lines now. It strips the task marker instead: a list-only note is still named by its first item, and calling that note "- [ ] milk" would show someone the storage. Wire: items leave it. A second copy of data already in the body field of the same message is how the two come to disagree. CLIENT_PROTOCOL_VERSION and MIN_SERVER_PROTOCOL_VERSION go to 3, which is what makes this safe to land before the server: a v3 client refuses a v2 server outright rather than pushing a body whose list the old _apply_note_items would then delete. Schema v8 folds every existing row into its note's body before dropping the table. Written in Rust, not SQL: the fold has to produce exactly what derive::append_item produces, and group_concat only gained a guaranteed ORDER BY in SQLite 3.44 — a checklist that quietly reordered itself during a migration would be a poor way to learn that. updated_at and dirty are deliberately left alone, because the server's migration folds the same rows the same way and both sides land on identical bodies; marking every note dirty would push a body the server already has, from every device at once. NOT deployable yet. The server still speaks v2 and still has note_items, so a client built from this will refuse to sync until the server half lands. --- android/ffi/src/lib.rs | 5 +- core/src/local/derive.rs | 35 +++++-- core/src/local/schema.rs | 193 ++++++++++++++++++++++++++++++++++++++- core/src/local/store.rs | 149 +++++++++++++++++------------- core/src/sync/compat.rs | 4 +- core/src/sync/pull.rs | 92 +++++-------------- core/src/sync/push.rs | 24 ----- core/src/sync/wire.rs | 13 --- 8 files changed, 331 insertions(+), 184 deletions(-) diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 9adfc1a..a5a9ebd 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -682,8 +682,9 @@ mod tests { assert!(ticked.items[1].checked); assert_eq!( ticked.items[1].text, "charger", - "ticking a box must not disturb its text — the two setters write \ - different columns and neither may clear the other" + "ticking a box must not disturb its text — both setters rewrite the \ + same line of the body now, so one clobbering the other is a live risk \ + rather than a theoretical one" ); let renamed = app diff --git a/core/src/local/derive.rs b/core/src/local/derive.rs index 11aca4a..835e9a4 100644 --- a/core/src/local/derive.rs +++ b/core/src/local/derive.rs @@ -155,6 +155,17 @@ fn render_task_line(indent: &str, bullet: char, checked: bool, text: &str) -> St } } +/// The text of a line with its task marker removed, or the line as it was. +/// +/// For naming a note: a list-only note is named by its first item, and calling one +/// "- [ ] milk" would be showing someone the storage instead of the note. +pub fn strip_marker(line: &str) -> &str { + match parse_task_line(line) { + Some(t) => t.text, + None => line, + } +} + /// Every checklist item in `body`, in the order they appear. pub fn extract_items(body: &str) -> Vec { let mut out = Vec::new(); @@ -237,8 +248,12 @@ pub fn remove_item(body: &str, index: usize) -> String { /// cosmetic: the server migration folds existing rows into bodies using the same /// layout, so an export taken before the migration and one taken after have to /// agree byte for byte. -pub fn append_item(body: &str, text: &str) -> String { - let line = render_task_line("", '-', false, text.trim()); +/// +/// `checked` is a parameter rather than always false because the two migrations that +/// fold existing rows into bodies have to carry the state those rows were in. A new +/// item from the UI passes false. +pub fn append_item(body: &str, text: &str, checked: bool) -> String { + let line = render_task_line("", '-', checked, text.trim()); let trimmed = body.trim_end_matches('\n'); if trimmed.trim().is_empty() { return line; @@ -378,13 +393,21 @@ mod tests { // Prose then a blank line then the list — byte-for-byte what // import_export.py:_note_markdown writes, which is what the server // migration will fold existing rows into. - assert_eq!(append_item("a note", "milk"), "a note\n\n- [ ] milk"); + assert_eq!(append_item("a note", "milk", false), "a note\n\n- [ ] milk"); // Nothing between consecutive items. let one = "a note\n\n- [ ] milk"; - assert_eq!(append_item(one, "eggs"), format!("{one}\n- [ ] eggs")); + assert_eq!(append_item(one, "eggs", false), format!("{one}\n- [ ] eggs")); // A list-only note starts at the first line. - assert_eq!(append_item("", "milk"), "- [ ] milk"); - assert_eq!(append_item("\n\n", "milk"), "- [ ] milk"); + assert_eq!(append_item("", "milk", false), "- [ ] milk"); + assert_eq!(append_item("\n\n", "milk", false), "- [ ] milk"); + // Carries state, which is what the two migrations need of it. + assert_eq!(append_item("", "done", true), "- [x] done"); + } + + #[test] + fn strip_marker_names_a_list_only_note() { + assert_eq!(strip_marker("- [x] milk"), "milk"); + assert_eq!(strip_marker("just prose"), "just prose"); } #[test] diff --git a/core/src/local/schema.rs b/core/src/local/schema.rs index 8b5decc..b65ea08 100644 --- a/core/src/local/schema.rs +++ b/core/src/local/schema.rs @@ -6,7 +6,9 @@ //! //! Migrations are gated on `PRAGMA user_version`; bump it and add a block per change. -use rusqlite::Connection; +use rusqlite::{params, Connection, OptionalExtension}; + +use crate::local::derive; const SCHEMA_V1: &str = r#" CREATE TABLE notes ( @@ -54,6 +56,8 @@ CREATE TABLE checklist_items ( position INTEGER NOT NULL DEFAULT 0 ); CREATE INDEX idx_items_note ON checklist_items (note_id); +-- Both dropped in v8; kept here so an existing database has something to migrate +-- FROM, exactly as `kind` above is kept for v6. CREATE TABLE attachments ( id TEXT PRIMARY KEY, @@ -180,6 +184,73 @@ ALTER TABLE notes DROP COLUMN title; ALTER TABLE note_revisions DROP COLUMN title; "#; +// v8 (M304): `checklist_items` is gone. The body IS the checklist — a `- [ ] milk` +// line is the item — so a list can sit between two paragraphs instead of only after +// them, which a side table could never express no matter how it was styled. +// +// Rust rather than a SQL const, for two reasons. The fold has to produce EXACTLY what +// `derive::append_item` produces, and expressing that in SQL would be a second +// implementation of the layout rule. And `group_concat` only gained a guaranteed +// ORDER BY in SQLite 3.44 — a checklist that silently reordered itself during the +// migration would be a poor way to find that out. +// +// `updated_at` and `dirty` are deliberately NOT touched. The server's Alembic +// migration folds the same rows with the same spacing, so both sides land on +// identical bodies and this needs no sync at all; marking every note dirty would +// push a body the server already has, and would do it for every device at once. +fn migrate_v8(conn: &Connection) -> rusqlite::Result<()> { + // Grouped in one pass — the query is ordered by note, so a change of note_id is + // the group boundary. `rowid` breaks ties, because `position` was only ever + // advisory and two rows sharing one is not a reason to reorder someone's list. + let mut grouped: Vec<(String, Vec<(String, bool)>)> = Vec::new(); + { + let mut stmt = conn.prepare( + "SELECT note_id, text, checked FROM checklist_items + ORDER BY note_id ASC, position ASC, rowid ASC", + )?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + let note_id: String = row.get(0)?; + let text: String = row.get(1)?; + let checked: bool = row.get(2)?; + match grouped.last_mut() { + Some((id, items)) if *id == note_id => items.push((text, checked)), + _ => grouped.push((note_id, vec![(text, checked)])), + } + } + } + + for (note_id, items) in grouped { + let existing: Option = conn + .query_row( + "SELECT body FROM notes WHERE id = ?1", + [¬e_id], + |r| r.get(0), + ) + .optional()?; + // An item whose note is already gone has nothing to fold into. The foreign key + // should make this impossible; skipping costs nothing, and failing here would + // leave the only copy of someone's notes half-migrated. + let mut body = match existing { + Some(b) => b, + None => continue, + }; + for (text, checked) in items { + body = derive::append_item(&body, &text, checked); + } + conn.execute( + "UPDATE notes SET body = ?1 WHERE id = ?2", + params![body, note_id], + )?; + } + + conn.execute_batch( + "DROP INDEX IF EXISTS idx_items_note; + DROP TABLE checklist_items;", + )?; + Ok(()) +} + /// Bring the database up to the latest schema. Idempotent. pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { conn.execute_batch("PRAGMA foreign_keys = ON;")?; @@ -212,5 +283,125 @@ pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { conn.execute_batch(SCHEMA_V7)?; conn.execute_batch("PRAGMA user_version = 7;")?; } + if version < 8 { + migrate_v8(conn)?; + conn.execute_batch("PRAGMA user_version = 8;")?; + } Ok(()) } + + +#[cfg(test)] +mod tests { + use super::*; + + /// A database as it stood before M304 — items still in their own table. + fn v7_db() -> Connection { + 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("batch"); + } + conn.execute_batch("PRAGMA user_version = 7;").expect("v7"); + conn + } + + fn add_note(conn: &Connection, id: &str, body: &str) { + conn.execute( + "INSERT INTO notes (id, body, created_at, updated_at) + VALUES (?1, ?2, '2026-01-01T00:00:00.000Z', '2026-01-01T00:00:00.000Z')", + params![id, body], + ) + .expect("note"); + } + + fn add_item(conn: &Connection, note: &str, text: &str, checked: bool, pos: i64) { + conn.execute( + "INSERT INTO checklist_items (id, note_id, text, checked, position) + VALUES (?1, ?2, ?3, ?4, ?5)", + params![format!("{note}-{pos}"), note, text, checked, pos], + ) + .expect("item"); + } + + fn body_of(conn: &Connection, id: &str) -> String { + conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0)) + .expect("body") + } + + #[test] + fn v8_folds_items_into_the_body() { + let conn = v7_db(); + add_note(&conn, "n1", "shopping"); + add_item(&conn, "n1", "milk", false, 0); + add_item(&conn, "n1", "eggs", true, 1); + + migrate(&conn).expect("migrate"); + + // Prose, blank line, list — the layout _note_markdown already exports, so an + // export taken before this migration and one taken after agree byte for byte. + assert_eq!(body_of(&conn, "n1"), "shopping\n\n- [ ] milk\n- [x] eggs"); + } + + #[test] + fn v8_keeps_a_list_only_note_whole() { + let conn = v7_db(); + add_note(&conn, "n1", ""); + add_item(&conn, "n1", "milk", false, 0); + + migrate(&conn).expect("migrate"); + + assert_eq!(body_of(&conn, "n1"), "- [ ] milk"); + } + + #[test] + fn v8_leaves_timestamps_alone() { + // The whole reason this needs no sync: the server folds the same rows the same + // way, so both sides already agree. Marking notes dirty would push a body the + // server has, from every device at once. + let conn = v7_db(); + add_note(&conn, "n1", "note"); + add_item(&conn, "n1", "milk", false, 0); + + migrate(&conn).expect("migrate"); + + let (updated, dirty): (String, i64) = conn + .query_row( + "SELECT updated_at, dirty FROM notes WHERE id = 'n1'", + [], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .expect("row"); + assert_eq!(updated, "2026-01-01T00:00:00.000Z"); + assert_eq!(dirty, 1); // as inserted, not raised by the migration + } + + #[test] + fn v8_drops_the_table_and_is_idempotent() { + let conn = v7_db(); + add_note(&conn, "n1", "note"); + migrate(&conn).expect("migrate"); + migrate(&conn).expect("again"); + + let exists: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='checklist_items'", + [], + |r| r.get(0), + ) + .expect("count"); + assert_eq!(exists, 0); + } + + #[test] + fn a_fresh_database_reaches_v8() { + 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); + } +} diff --git a/core/src/local/store.rs b/core/src/local/store.rs index 9c98e56..07a8ee9 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -9,7 +9,7 @@ use chrono::{DateTime, Duration, SecondsFormat, Utc}; use rusqlite::{params, params_from_iter, Connection, OptionalExtension}; -use serde_json::Value; +use serde_json::{json, Value}; use uuid::Uuid; use crate::local::derive; @@ -24,24 +24,25 @@ fn new_id() -> String { Uuid::new_v4().to_string() } -/// The note's NAME: its first non-blank body line, else its first checklist item. +/// The note's NAME: the first line of its body that says anything. /// /// Mirrors `derive_display_title` in the server's notes/helpers.py — one rule written /// twice, and they have to agree or a synced note is called different things on either /// side of the wire. /// -/// Pure, and given the items rather than fetching them: every caller has already -/// loaded them, so a query here would be a second trip for something already in hand. -fn display_title(body: &str, items: &[ChecklistItem]) -> String { - if let Some(line) = body.lines().map(str::trim).find(|l| !l.is_empty()) { - return line.to_string(); +/// It no longer needs the items, because the items ARE lines of the body now (M304). +/// What it needs instead is to strip the task marker off: a list-only note is still +/// named by its first item, and calling that note "- [ ] milk" would be showing +/// someone the storage rather than the note. An empty item is skipped rather than +/// naming the note "", which is what a half-typed list would otherwise do. +fn display_title(body: &str) -> String { + for line in body.lines() { + let text = derive::strip_marker(line.trim()).trim(); + if !text.is_empty() { + return text.to_string(); + } } - items - .iter() - .map(|i| i.text.trim()) - .find(|t| !t.is_empty()) - .unwrap_or("") - .to_string() + String::new() } fn escape_like(s: &str) -> String { @@ -69,19 +70,24 @@ fn load_labels(conn: &Connection, note_id: &str) -> rusqlite::Result rusqlite::Result> { - let mut stmt = conn.prepare( - "SELECT id, text, checked, position FROM checklist_items WHERE note_id = ?1 ORDER BY position ASC", - )?; - let rows = stmt.query_map([note_id], |r| { - Ok(ChecklistItem { - id: r.get(0)?, - text: r.get(1)?, - checked: r.get(2)?, - position: r.get(3)?, +/// The note's checklist, read out of its body. No query, because there is no table. +/// +/// A `- [ ] milk` line IS the item (M304). The id is the item's ORDINAL rather than a +/// uuid — which is all it ever amounted to anyway, since `push.rs` sent text and +/// checked and never an id, and both sides replaced the whole list on every sync. It +/// is also exactly what the rewriters in `derive` take, so a UI holding an id can act +/// on it directly. +fn items_of(body: &str) -> Vec { + derive::extract_items(body) + .into_iter() + .enumerate() + .map(|(i, item)| ChecklistItem { + id: i.to_string(), + text: item.text, + checked: item.checked, + position: i as i64, }) - })?; - rows.collect() + .collect() } fn load_attachments(conn: &Connection, note_id: &str) -> rusqlite::Result> { @@ -162,11 +168,10 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result { }, )?; note.labels = load_labels(conn, id)?; - note.items = load_items(conn, id)?; + note.items = items_of(¬e.body); note.attachments = load_attachments(conn, id)?; note.previews = load_previews(conn, id)?; - // After the items, because a body-only-empty note is named by its first one. - note.display_title = display_title(¬e.body, ¬e.items); + note.display_title = display_title(¬e.body); Ok(note) } @@ -358,20 +363,22 @@ pub fn create_note(conn: &Connection, input: &NoteCreateInput) -> rusqlite::Resu [], |r| r.get(0), )?; + // Items fold into the body rather than into rows of their own. Callers still hand + // them over separately — the importer has a list, not a blob — but where they end + // up is one place. + let mut body = input.body.clone(); + if let Some(items) = &input.items { + for text in items { + body = derive::append_item(&body, text, false); + } + } conn.execute( "INSERT INTO notes (id, body, color, position, created_at, updated_at, dirty) VALUES (?1, ?2, ?3, ?4, ?5, ?5, 1)", - params![id, input.body, input.color, position, ts], + params![id, body, input.color, position, ts], )?; - if let Some(items) = &input.items { - for (i, text) in items.iter().enumerate() { - conn.execute( - "INSERT INTO checklist_items (id, note_id, text, position) VALUES (?1, ?2, ?3, ?4)", - params![new_id(), id, text, i as i64], - )?; - } - } - sync_tags(conn, &id, &input.body)?; + // The FOLDED body, not the input one: an item can carry a #tag too. + sync_tags(conn, &id, &body)?; load_note(conn, &id) } @@ -548,18 +555,32 @@ pub fn set_labels(conn: &Connection, id: &str, label_ids: &[String]) -> rusqlite load_note(conn, id) } +// ---- checklist items: every one of these is a body edit --------------------- +// +// They keep their own names and signatures because the FFI, the Tauri commands and +// the REST shape all speak in items, and a checklist is still a thing a note HAS. +// What changed is where it is kept. Routing all three through `update_note` rather +// than writing the body directly is what gives them revision snapshotting, `#tag` +// re-derivation and the dirty/updated_at bookkeeping without any of it being +// written a second time here. + +fn note_body(conn: &Connection, id: &str) -> rusqlite::Result { + conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0)) +} + +/// An item's id is its ordinal (see [items_of]). Anything else is a stale id from a +/// UI that has not reloaded, and the right answer to those is to do nothing. +fn item_index(item_id: &str) -> Option { + item_id.parse::().ok() +} + +fn set_body(conn: &Connection, id: &str, body: String) -> rusqlite::Result { + update_note(conn, id, &json!({ "body": body })) +} + pub fn add_item(conn: &Connection, id: &str, text: &str) -> rusqlite::Result { - let pos: i64 = conn.query_row( - "SELECT COALESCE(MAX(position), -1) + 1 FROM checklist_items WHERE note_id = ?1", - [id], - |r| r.get(0), - )?; - conn.execute( - "INSERT INTO checklist_items (id, note_id, text, position) VALUES (?1, ?2, ?3, ?4)", - params![new_id(), id, text, pos], - )?; - touch(conn, id)?; - load_note(conn, id) + let body = note_body(conn, id)?; + set_body(conn, id, derive::append_item(&body, text, false)) } pub fn update_item( @@ -568,29 +589,27 @@ pub fn update_item( item_id: &str, changes: &Value, ) -> rusqlite::Result { + let index = match item_index(item_id) { + Some(i) => i, + None => return load_note(conn, id), + }; + let mut body = note_body(conn, id)?; if let Some(text) = changes.get("text").and_then(Value::as_str) { - conn.execute( - "UPDATE checklist_items SET text = ?1 WHERE id = ?2 AND note_id = ?3", - params![text, item_id, id], - )?; + body = derive::set_item_text(&body, index, text); } if let Some(checked) = changes.get("checked").and_then(Value::as_bool) { - conn.execute( - "UPDATE checklist_items SET checked = ?1 WHERE id = ?2 AND note_id = ?3", - params![checked, item_id, id], - )?; + body = derive::set_item_checked(&body, index, checked); } - touch(conn, id)?; - load_note(conn, id) + set_body(conn, id, body) } pub fn delete_item(conn: &Connection, id: &str, item_id: &str) -> rusqlite::Result { - conn.execute( - "DELETE FROM checklist_items WHERE id = ?1 AND note_id = ?2", - params![item_id, id], - )?; - touch(conn, id)?; - load_note(conn, id) + let index = match item_index(item_id) { + Some(i) => i, + None => return load_note(conn, id), + }; + let body = note_body(conn, id)?; + set_body(conn, id, derive::remove_item(&body, index)) } pub fn delete_attachment(conn: &Connection, id: &str, att_id: &str) -> rusqlite::Result { diff --git a/core/src/sync/compat.rs b/core/src/sync/compat.rs index 42104e1..46a2f52 100644 --- a/core/src/sync/compat.rs +++ b/core/src/sync/compat.rs @@ -19,11 +19,11 @@ use serde::{Deserialize, Serialize}; /// The sync wire protocol this client speaks. -pub const CLIENT_PROTOCOL_VERSION: u32 = 2; +pub const CLIENT_PROTOCOL_VERSION: u32 = 3; /// The oldest server protocol this client can drive — the symmetric half of the /// server's `min_client_protocol_version`. -pub const MIN_SERVER_PROTOCOL_VERSION: u32 = 2; +pub const MIN_SERVER_PROTOCOL_VERSION: u32 = 3; /// Capabilities without which syncing is meaningless, so their absence BLOCKS the /// link rather than degrading it. diff --git a/core/src/sync/pull.rs b/core/src/sync/pull.rs index 977618c..103b51f 100644 --- a/core/src/sync/pull.rs +++ b/core/src/sync/pull.rs @@ -277,34 +277,12 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { // Children are replaced wholesale: a delta carries the note's FULL current state, // so "what the server sent" IS the complete set. Diffing would be more code and // could leave behind a row the server no longer has. - replace_items(conn, note)?; replace_attachments(conn, note)?; replace_previews(conn, note)?; replace_labels(conn, note)?; Ok(()) } -fn replace_items(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { - conn.execute( - "DELETE FROM checklist_items WHERE note_id = ?1", - params![note.id], - )?; - for (index, item) in note.items.iter().enumerate() { - conn.execute( - "INSERT INTO checklist_items (id, note_id, text, checked, position) - VALUES (?1, ?2, ?3, ?4, ?5)", - params![ - item.id, - note.id, - item.text, - item.checked, - position_of(item.position, index) - ], - )?; - } - Ok(()) -} - fn replace_attachments(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { conn.execute( "DELETE FROM attachments WHERE note_id = ?1", @@ -393,16 +371,6 @@ fn ensure_label_stub(conn: &Connection, label: &wire::NoteLabel) -> rusqlite::Re Ok(()) } -/// Trust an explicit position; fall back to arrival order when the server sent 0 for -/// everything (which is what an unordered list looks like on the wire). -fn position_of(explicit: i64, index: usize) -> i64 { - if explicit > 0 { - explicit - } else { - index as i64 - } -} - /// Loop the feed to exhaustion, starting from the persisted cursor. /// /// NOTE ON ORDERING: the full cycle is push-then-pull (docs/sync.md). Running this @@ -508,12 +476,22 @@ mod tests { sync_revision: revision, purged_at: None, labels: vec![], - items: vec![], attachments: vec![], previews: vec![], } } + fn attachment(id: &str) -> wire::Attachment { + wire::Attachment { + id: id.to_string(), + url: "/blob/x".into(), + filename: None, + mime: "image/png".into(), + size: None, + sha256: None, + } + } + fn page(notes: Vec, labels: Vec, cursor: i64) -> wire::ChangesPage { wire::ChangesPage { notes, @@ -612,35 +590,20 @@ mod tests { #[test] fn children_are_replaced_not_merged() { + // Was written over checklist items; they are lines of the body now (M304), so + // attachments carry the point instead. It is the same property either way: a + // delta is the note's FULL current state, so a child the server dropped has to + // disappear locally rather than linger. let conn = db(); let mut first = note("n1", 1); - first.items = vec![ - wire::Item { - id: "i1".into(), - text: "one".into(), - checked: false, - position: 0, - }, - wire::Item { - id: "i2".into(), - text: "two".into(), - checked: false, - position: 1, - }, - ]; + first.attachments = vec![attachment("a1"), attachment("a2")]; apply_page(&conn, &page(vec![first], vec![], 1)).expect("apply"); - assert_eq!(count(&conn, "SELECT COUNT(*) FROM checklist_items"), 2); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM attachments"), 2); - // The server dropped an item; the local copy must drop it too. let mut second = note("n1", 2); - second.items = vec![wire::Item { - id: "i1".into(), - text: "one".into(), - checked: true, - position: 0, - }]; + second.attachments = vec![attachment("a1")]; apply_page(&conn, &page(vec![second], vec![], 2)).expect("apply"); - assert_eq!(count(&conn, "SELECT COUNT(*) FROM checklist_items"), 1); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM attachments"), 1); } #[test] @@ -810,23 +773,10 @@ mod tests { fn a_page_that_fails_leaves_the_cursor_untouched() { // Atomicity is the whole resumability story: a cursor committed ahead of its // data would skip those rows forever. Force a failure with a duplicate - // checklist-item id inside one page. + // attachment id inside one page. let conn = db(); let mut n = note("n1", 3); - n.items = vec![ - wire::Item { - id: "dup".into(), - text: "one".into(), - checked: false, - position: 0, - }, - wire::Item { - id: "dup".into(), - text: "two".into(), - checked: false, - position: 1, - }, - ]; + n.attachments = vec![attachment("dup"), attachment("dup")]; assert!(apply_page(&conn, &page(vec![n], vec![], 3)).is_err()); assert_eq!(state::read(&conn).expect("state").last_cursor, 0); assert_eq!(count(&conn, "SELECT COUNT(*) FROM notes"), 0); diff --git a/core/src/sync/push.rs b/core/src/sync/push.rs index 831629e..d5858ce 100644 --- a/core/src/sync/push.rs +++ b/core/src/sync/push.rs @@ -79,8 +79,6 @@ pub struct Change { #[serde(skip_serializing_if = "Option::is_none")] pub position: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub items: Option>, - #[serde(skip_serializing_if = "Option::is_none")] pub label_ids: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub created_at: Option, @@ -103,7 +101,6 @@ impl Change { remind_at: None, recurrence: None, position: None, - items: None, label_ids: None, created_at: None, name: None, @@ -111,12 +108,6 @@ impl Change { } } -#[derive(Debug, Serialize)] -pub struct ItemOut { - pub text: String, - pub checked: bool, -} - // --- incoming results -------------------------------------------------------- #[derive(Debug, Deserialize)] @@ -201,7 +192,6 @@ fn collect_labels(conn: &Connection, out: &mut Vec, limit: usize) -> rus remind_at: None, recurrence: None, position: None, - items: None, label_ids: None, created_at: None, }) @@ -267,19 +257,6 @@ fn note_row(conn: &Connection, id: &str) -> rusqlite::Result { fn note_change(conn: &Connection, id: &str) -> rusqlite::Result { let row = note_row(conn, id)?; - let items = { - let mut stmt = conn.prepare( - "SELECT text, checked FROM checklist_items WHERE note_id = ?1 ORDER BY position", - )?; - let rows = stmt.query_map(params![id], |r| { - Ok(ItemOut { - text: r.get(0)?, - checked: r.get::<_, i64>(1)? != 0, - }) - })?; - rows.collect::>>()? - }; - // MANUAL memberships only. Tag-sourced ones (`via_tag = 1`) are re-derived by the // server from the body; sending them as label_ids would convert them into manual // assignments that no longer disappear when the #tag is removed from the text. @@ -305,7 +282,6 @@ fn note_change(conn: &Connection, id: &str) -> rusqlite::Result { remind_at: row.remind_at, recurrence: row.recurrence, position: Some(row.position), - items: Some(items), label_ids: Some(label_ids), created_at: Some(row.created_at), name: None, diff --git a/core/src/sync/wire.rs b/core/src/sync/wire.rs index 87eb9d9..be4f930 100644 --- a/core/src/sync/wire.rs +++ b/core/src/sync/wire.rs @@ -58,8 +58,6 @@ pub struct Note { #[serde(default)] pub labels: Vec, #[serde(default)] - pub items: Vec, - #[serde(default)] pub attachments: Vec, #[serde(default)] pub previews: Vec, @@ -87,17 +85,6 @@ pub struct NoteLabel { pub via_tag: bool, } -#[derive(Debug, Clone, Deserialize)] -pub struct Item { - pub id: String, - #[serde(default)] - pub text: String, - #[serde(default)] - pub checked: bool, - #[serde(default)] - pub position: i64, -} - #[derive(Debug, Clone, Deserialize)] pub struct Attachment { pub id: String,