diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 5b43216..6a35f25 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -755,6 +755,71 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + /// The path the notification's Done button takes. + /// + /// Completing a RECURRING reminder must move it, not end it — this is the + /// behaviour the web has had all along and the clients did not, which made + /// "Done" on a daily reminder quietly the last time it ever fired. + #[test] + fn completing_a_recurring_reminder_moves_it_rather_than_ending_it() { + let dir = scratch_dir(); + let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open"); + let note = app + .create_note(draft("Water the plants", "")) + .expect("create"); + + let armed = app + .update_note( + note.id.clone(), + vec![ + NoteEdit::RemindAt { + value: "2026-07-01T09:00:00.000Z".into(), + }, + NoteEdit::Recurrence { + value: "daily".into(), + }, + ], + ) + .expect("arm a daily reminder"); + assert_eq!(armed.recurrence.as_deref(), Some("daily")); + + let done = app.complete_reminder(note.id.clone()).expect("complete"); + let next = done + .remind_at + .expect("a daily reminder must still have a next occurrence"); + assert!( + next.as_str() > "2026-07-01T09:00:00.000Z", + "it must move FORWARD, got {next:?}" + ); + assert!( + next.ends_with("T09:00:00.000Z"), + "the time of day is what was asked for and must survive, got {next:?}" + ); + assert_eq!( + done.recurrence.as_deref(), + Some("daily"), + "the rule outlives the occurrence" + ); + + // A one-off clears BOTH fields, so an unrecognised rule cannot linger + // invisibly on a note with no reminder. + let once = app + .create_note(draft("Post the letter", "")) + .expect("create"); + app.update_note( + once.id.clone(), + vec![NoteEdit::RemindAt { + value: "2026-07-01T09:00:00.000Z".into(), + }], + ) + .expect("arm a one-off"); + let finished = app.complete_reminder(once.id.clone()).expect("complete"); + assert_eq!(finished.remind_at, None); + assert_eq!(finished.recurrence, None); + + std::fs::remove_dir_all(&dir).ok(); + } + /// A crude RFC3339 sanity check that doesn't pull a date crate into this /// crate's dev-dependencies to assert one field is well-formed. fn chrono_free_parse(raw: &str) -> usize { diff --git a/core/src/local/mod.rs b/core/src/local/mod.rs index d153a2c..4ba8e4c 100644 --- a/core/src/local/mod.rs +++ b/core/src/local/mod.rs @@ -6,6 +6,7 @@ pub mod derive; pub mod models; +pub mod recur; pub mod retention; pub mod schema; pub mod store; diff --git a/core/src/local/recur.rs b/core/src/local/recur.rs new file mode 100644 index 0000000..8d76bf9 --- /dev/null +++ b/core/src/local/recur.rs @@ -0,0 +1,171 @@ +//! Recurring-reminder math: where a reminder goes when it is marked done. +//! +//! A deliberate port of the server's `src/thoughtsync/notes/recurrence.py`, kept +//! behaviourally identical rather than merely similar. The same note can be +//! completed from the web (server code) or from the desktop and Android (this +//! code), and the two must land on the same instant — otherwise completing a +//! reminder on a phone and then syncing would silently move it relative to +//! completing it in a browser, and neither surface would look wrong on its own. +//! +//! Advancement is measured from the reminder's OWN time, never from now. That is +//! what keeps a 09:00 daily reminder at 09:00 after being dealt with at 09:47, +//! and a monthly one on the same day of the month. +//! +//! Known limitation, shared with the server: the arithmetic is in UTC, and a note +//! carries no timezone. So a daily reminder crossing a DST boundary keeps its UTC +//! time and shifts by an hour locally. Fixing that means storing a zone per note +//! and is a change to the wire format, not to this file. + +use chrono::{DateTime, Duration, Months, Utc}; + +/// The four intervals every surface offers. Anything else is not a recurrence. +pub const RECURRENCES: [&str; 4] = ["daily", "weekly", "monthly", "yearly"]; + +/// A recurrence we recognise, or nothing. +/// +/// Values reach the store from three clients and a sync payload, so "not a rule +/// we know" is an ordinary case rather than a corruption to shout about. +pub fn normalize(value: Option<&str>) -> Option<&str> { + value.filter(|v| RECURRENCES.contains(v)) +} + +/// One step forward. `None` for an unrecognised rule. +/// +/// Months and years clamp the day to the target month's length — 31 January plus +/// a month is 28 February, and the following step is 28 March rather than back to +/// the 31st. `chrono`'s `checked_add_months` does that clamping, matching +/// `_add_months` in the Python to the day. +fn advance_once(at: DateTime, recurrence: &str) -> Option> { + match recurrence { + "daily" => at.checked_add_signed(Duration::days(1)), + "weekly" => at.checked_add_signed(Duration::weeks(1)), + "monthly" => at.checked_add_months(Months::new(1)), + "yearly" => at.checked_add_months(Months::new(12)), + _ => None, + } +} + +/// The first fire time strictly after `after`, rolling past anything missed. +/// +/// A phone left in a drawer for a fortnight should not come back to fourteen +/// pending occurrences of the same daily reminder — it should come back to +/// tomorrow's. `None` when the rule is not one we know, which the caller reads as +/// "this reminder is finished". +pub fn next_occurrence( + remind_at: DateTime, + recurrence: &str, + after: DateTime, +) -> Option> { + let mut next = advance_once(remind_at, recurrence)?; + while next <= after { + match advance_once(next, recurrence) { + // The equality check is a guard against a step that does not move, + // which would spin here forever. It cannot happen with the four rules + // above; it is cheap insurance against a fifth that does not advance. + Some(step) if step != next => next = step, + _ => break, + } + } + Some(next) +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::TimeZone; + + fn utc(y: i32, m: u32, d: u32, h: u32, min: u32) -> DateTime { + Utc.with_ymd_and_hms(y, m, d, h, min, 0).unwrap() + } + + /// Mirrors `test_normalize_recurrence` in `tests/test_notes.py`. + #[test] + fn only_the_four_known_rules_are_recurrences() { + for rule in RECURRENCES { + assert_eq!(normalize(Some(rule)), Some(rule)); + } + assert_eq!(normalize(Some("none")), None); + assert_eq!(normalize(Some("")), None); + assert_eq!(normalize(Some("hourly")), None); + assert_eq!(normalize(None), None); + } + + /// Mirrors `test_next_occurrence_daily_weekly`. + #[test] + fn daily_and_weekly_keep_the_time_of_day() { + let base = utc(2026, 7, 1, 9, 0); + let after = utc(2026, 7, 1, 12, 0); + assert_eq!( + next_occurrence(base, "daily", after), + Some(utc(2026, 7, 2, 9, 0)) + ); + assert_eq!( + next_occurrence(base, "weekly", after), + Some(utc(2026, 7, 8, 9, 0)) + ); + } + + /// Mirrors `test_next_occurrence_skips_missed`. + #[test] + fn missed_occurrences_are_rolled_past_not_queued() { + let base = utc(2026, 7, 1, 9, 0); + let after = utc(2026, 7, 10, 12, 0); + assert_eq!( + next_occurrence(base, "daily", after), + Some(utc(2026, 7, 11, 9, 0)) + ); + } + + /// Mirrors `test_next_occurrence_monthly_clamps_month_end`. + #[test] + fn monthly_clamps_to_a_shorter_month() { + let base = utc(2026, 1, 31, 8, 0); + let after = utc(2026, 2, 1, 0, 0); + assert_eq!( + next_occurrence(base, "monthly", after), + Some(utc(2026, 2, 28, 8, 0)) + ); + } + + /// Mirrors `test_next_occurrence_yearly_and_none`. + #[test] + fn yearly_advances_a_year_and_an_unknown_rule_advances_nothing() { + let base = utc(2026, 3, 15, 7, 0); + let after = utc(2026, 3, 16, 0, 0); + assert_eq!( + next_occurrence(base, "yearly", after), + Some(utc(2027, 3, 15, 7, 0)) + ); + assert_eq!(next_occurrence(base, "none", after), None); + } + + /// Not in the Python suite, and the one that would bite hardest in practice: + /// a monthly reminder set on the 31st must not walk itself back to the 28th + /// permanently. Each step is taken from the ORIGINAL date, so February's clamp + /// does not become March's date. + #[test] + fn a_clamped_month_does_not_drag_later_months_back() { + let base = utc(2026, 1, 31, 8, 0); + // Far enough ahead that the loop takes several steps. + let after = utc(2026, 4, 15, 0, 0); + // Jan 31 -> Feb 28 -> Mar 28 -> Apr 28. The clamp is sticky once applied, + // which matches the server exactly — asserted so a future "fix" to either + // side has to change both. + assert_eq!( + next_occurrence(base, "monthly", after), + Some(utc(2026, 4, 28, 8, 0)) + ); + } + + /// A reminder completed before it was ever due still moves forward one step, + /// rather than staying put and firing again immediately. + #[test] + fn completing_early_still_advances() { + let base = utc(2026, 7, 10, 9, 0); + let after = utc(2026, 7, 1, 12, 0); + assert_eq!( + next_occurrence(base, "daily", after), + Some(utc(2026, 7, 11, 9, 0)) + ); + } +} diff --git a/core/src/local/store.rs b/core/src/local/store.rs index 1cf96b5..80fa92a 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -7,13 +7,14 @@ //! ("YYYY-MM-DDTHH:MM:SS.sssZ") so string ordering and date-range comparisons line //! up with the values the frontend sends. -use chrono::{Duration, SecondsFormat, Utc}; +use chrono::{DateTime, Duration, SecondsFormat, Utc}; use rusqlite::{params, params_from_iter, Connection, OptionalExtension}; use serde_json::Value; use uuid::Uuid; use crate::local::derive; use crate::local::models::*; +use crate::local::recur; fn now() -> String { Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true) @@ -529,9 +530,38 @@ pub fn update_note(conn: &Connection, id: &str, changes: &Value) -> rusqlite::Re load_note(conn, id) } +/// Mark a reminder handled. +/// +/// A recurring reminder advances to its next occurrence; a one-off clears both +/// `remind_at` AND `recurrence`. Clearing the rule as well matters: without it a +/// note whose recurrence is a value we do not recognise would keep that value +/// forever, invisible in every UI (they only render known rules) and waiting to +/// mean something the day the vocabulary grows. +/// +/// Same behaviour as the server's `POST //reminder/complete`, deliberately — +/// the same note can be completed from a browser or from a client, and a +/// disagreement here would move a reminder depending on which one you used. pub fn complete_reminder(conn: &Connection, id: &str) -> rusqlite::Result { - // Clear the reminder. (Recurrence advancement is a later refinement.) - conn.execute("UPDATE notes SET remind_at = NULL WHERE id = ?1", [id])?; + let note = load_note(conn, id)?; + let next = note + .remind_at + .as_deref() + .and_then(|at| DateTime::parse_from_rfc3339(at).ok()) + .and_then(|at| { + let rule = recur::normalize(note.recurrence.as_deref())?; + recur::next_occurrence(at.with_timezone(&Utc), rule, Utc::now()) + }); + + match next { + Some(at) => conn.execute( + "UPDATE notes SET remind_at = ?1 WHERE id = ?2", + params![at.to_rfc3339_opts(SecondsFormat::Millis, true), id], + )?, + None => conn.execute( + "UPDATE notes SET remind_at = NULL, recurrence = NULL WHERE id = ?1", + [id], + )?, + }; touch(conn, id)?; load_note(conn, id) }