Skip to main content

max / goingson

Data/Testing: classify committed-HLC table, persist recurrence_rule on edit, cover review repos - restore.rs: add sync_committed_hlc to EXCLUDED_TABLES (device-local sync internal) — fixes the schema_tables_are_all_classified guard that migration 056 left red on main. - Thread recurrence_rule through the task update path: add the field to UpdateTask, set it in update_task, bind it in the UPDATE. The JS form already sent it on edit, so a changed custom recurrence no longer keeps the stale rule. - Tests: new weekly_review_repo_tests + monthly_review_repo_tests (upsert idempotency/atomicity), core test modules for RecurrenceRule::display and Project* enum round-trips, and test_update_persists_recurrence_rule. Suite: 785 passed, 0 failed; clippy --workspace --all-targets clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-02 21:35 UTC
Commit: 9873b320ac2d6286af9bb64930d66e7c86ca3dfb
Parent: 24c6429
10 files changed, +399 insertions, -3 deletions
@@ -178,3 +178,53 @@ pub struct UpdateProject {
178 178 pub project_type: ProjectType,
179 179 pub status: ProjectStatus,
180 180 }
181 +
182 + #[cfg(test)]
183 + mod tests {
184 + use super::*;
185 + use crate::models::shared::{DbValue, ParseableEnum};
186 +
187 + #[test]
188 + fn project_type_db_value_round_trips() {
189 + for pt in [
190 + ProjectType::Job,
191 + ProjectType::SideProject,
192 + ProjectType::Company,
193 + ProjectType::Essay,
194 + ProjectType::Article,
195 + ProjectType::Painting,
196 + ProjectType::Other,
197 + ] {
198 + let parsed = ProjectType::from_str_or_default(pt.db_value());
199 + assert_eq!(parsed, pt, "db_value must round-trip for {pt:?}");
200 + }
201 + }
202 +
203 + #[test]
204 + fn project_status_db_value_round_trips() {
205 + for st in [
206 + ProjectStatus::Active,
207 + ProjectStatus::OnHold,
208 + ProjectStatus::Completed,
209 + ProjectStatus::Archived,
210 + ] {
211 + let parsed = ProjectStatus::from_str_or_default(st.db_value());
212 + assert_eq!(parsed, st, "db_value must round-trip for {st:?}");
213 + }
214 + }
215 +
216 + #[test]
217 + fn unknown_project_enum_strings_fall_back_to_default() {
218 + assert_eq!(ProjectType::from_str_or_default("bogus"), ProjectType::Other);
219 + assert_eq!(ProjectStatus::from_str_or_default("bogus"), ProjectStatus::Active);
220 + }
221 +
222 + #[test]
223 + fn display_and_db_value_diverge_where_expected() {
224 + // as_str is human-facing ("Side Project"); db_value is the stored token ("SideProject").
225 + assert_eq!(ProjectType::SideProject.as_str(), "Side Project");
226 + assert_eq!(ProjectType::SideProject.db_value(), "SideProject");
227 + assert_eq!(ProjectStatus::OnHold.as_str(), "On Hold");
228 + assert_eq!(ProjectStatus::OnHold.db_value(), "OnHold");
229 + }
230 + }
@@ -283,3 +283,93 @@ impl SortDirection {
283 283 }
284 284 }
285 285 }
286 +
287 + #[cfg(test)]
288 + mod tests {
289 + use super::*;
290 +
291 + fn rule(pattern: Recurrence, interval: u32, weekdays: Vec<u8>, monthly_spec: Option<MonthlySpec>) -> RecurrenceRule {
292 + RecurrenceRule { pattern, interval, weekdays, monthly_spec }
293 + }
294 +
295 + #[test]
296 + fn display_simple_intervals() {
297 + assert_eq!(rule(Recurrence::Daily, 1, vec![], None).display(), "Every day");
298 + assert_eq!(rule(Recurrence::Weekly, 1, vec![], None).display(), "Every week");
299 + assert_eq!(rule(Recurrence::Monthly, 1, vec![], None).display(), "Every month");
300 + assert_eq!(rule(Recurrence::Weekly, 2, vec![], None).display(), "Every 2 weeks");
301 + assert_eq!(rule(Recurrence::Daily, 3, vec![], None).display(), "Every 3 days");
302 + }
303 +
304 + #[test]
305 + fn display_weekly_with_weekdays() {
306 + assert_eq!(
307 + rule(Recurrence::Weekly, 2, vec![0, 2, 4], None).display(),
308 + "Every 2 weeks on Mon, Wed, Fri"
309 + );
310 + assert_eq!(
311 + rule(Recurrence::Weekly, 1, vec![6], None).display(),
312 + "Every week on Sun"
313 + );
314 + }
315 +
316 + #[test]
317 + fn display_ignores_out_of_range_weekday_bytes() {
318 + // A hostile/corrupt weekday byte (>6) must be filtered, not panic or index out of bounds.
319 + assert_eq!(
320 + rule(Recurrence::Weekly, 1, vec![9, 1], None).display(),
321 + "Every week on Tue"
322 + );
323 + // Weekdays are only meaningful for Weekly; a Daily rule ignores them.
324 + assert_eq!(rule(Recurrence::Daily, 1, vec![0, 1], None).display(), "Every day");
325 + }
326 +
327 + #[test]
328 + fn display_monthly_specs() {
329 + assert_eq!(
330 + rule(Recurrence::Monthly, 1, vec![], Some(MonthlySpec::DayOfMonth { day: 15 })).display(),
331 + "Every month on day 15"
332 + );
333 + assert_eq!(
334 + rule(Recurrence::Monthly, 1, vec![], Some(MonthlySpec::NthWeekday { week: 2, weekday: 4 })).display(),
335 + "Every month on the 2nd Fri"
336 + );
337 + assert_eq!(
338 + rule(Recurrence::Monthly, 1, vec![], Some(MonthlySpec::NthWeekday { week: -1, weekday: 0 })).display(),
339 + "Every month on the last Mon"
340 + );
341 + }
342 +
343 + #[test]
344 + fn display_none_pattern_is_empty() {
345 + assert_eq!(rule(Recurrence::None, 1, vec![], None).display(), "");
346 + }
347 +
348 + #[test]
349 + fn ordinal_suffix_edge_cases() {
350 + assert_eq!(ordinal_suffix(1), "st");
351 + assert_eq!(ordinal_suffix(2), "nd");
352 + assert_eq!(ordinal_suffix(3), "rd");
353 + assert_eq!(ordinal_suffix(4), "th");
354 + assert_eq!(ordinal_suffix(11), "th");
355 + assert_eq!(ordinal_suffix(12), "th");
356 + assert_eq!(ordinal_suffix(13), "th");
357 + assert_eq!(ordinal_suffix(21), "st");
358 + }
359 +
360 + #[test]
361 + fn from_legacy_and_effective() {
362 + assert!(RecurrenceRule::from_legacy(&Recurrence::None).is_none());
363 + let weekly = RecurrenceRule::from_legacy(&Recurrence::Weekly).unwrap();
364 + assert_eq!(weekly.pattern, Recurrence::Weekly);
365 + assert_eq!(weekly.interval, 1);
366 +
367 + // effective prefers an explicit rule over the legacy column.
368 + let explicit = rule(Recurrence::Weekly, 3, vec![1], None);
369 + let eff = RecurrenceRule::effective(Some(&explicit), &Recurrence::Daily).unwrap();
370 + assert_eq!(eff.interval, 3);
371 + // with no explicit rule, it synthesizes from the legacy value.
372 + let eff = RecurrenceRule::effective(None, &Recurrence::Monthly).unwrap();
373 + assert_eq!(eff.pattern, Recurrence::Monthly);
374 + }
375 + }
@@ -712,6 +712,9 @@ pub struct UpdateTask {
712 712 pub tags: Vec<String>,
713 713 /// Recurrence pattern (None, Daily, Weekly, Monthly).
714 714 pub recurrence: Recurrence,
715 + /// Rich recurrence configuration (JSON). Threaded on edit so a changed custom
716 + /// rule is persisted rather than leaving the stored rule stale.
717 + pub recurrence_rule: Option<RecurrenceRule>,
715 718 /// Re-calculated urgency score based on priority, due date, age, and tags.
716 719 pub urgency: f64,
717 720 /// Scheduled start time for time-blocking.
@@ -482,6 +482,7 @@ mod tests {
482 482 due: Some(Utc::now() + Duration::days(1)),
483 483 tags: vec!["updated".to_string()],
484 484 recurrence: Recurrence::Weekly,
485 + recurrence_rule: None,
485 486 urgency: 7.0,
486 487 scheduled_start: None,
487 488 scheduled_duration: None,
@@ -63,6 +63,7 @@ pub const EXCLUDED_TABLES: &[&str] = &[
63 63 "sync_changelog", // sync internals, regenerated
64 64 "sync_state", // sync cursors/flags, internal
65 65 "hlc_state", // hybrid logical clock, device-local sync internal
66 + "sync_committed_hlc", // per-row committed HLC clock store, device-local sync internal
66 67 ];
67 68
68 69 /// Restore a backup into the database as a single transaction.
@@ -567,7 +567,7 @@ impl TaskRepository for SqliteTaskRepository {
567 567 let result = sqlx::query(
568 568 r#"
569 569 UPDATE tasks
570 - SET project_id = ?, contact_id = ?, milestone_id = ?, description = ?, status = ?, priority = ?, due = ?, tags = ?, recurrence = ?, urgency = ?, scheduled_start = ?, scheduled_duration = ?, estimated_minutes = ?, completed_at = ?
570 + SET project_id = ?, contact_id = ?, milestone_id = ?, description = ?, status = ?, priority = ?, due = ?, tags = ?, recurrence = ?, recurrence_rule = ?, urgency = ?, scheduled_start = ?, scheduled_duration = ?, estimated_minutes = ?, completed_at = ?
571 571 WHERE id = ? AND user_id = ?
572 572 "#,
573 573 )
@@ -580,6 +580,7 @@ impl TaskRepository for SqliteTaskRepository {
580 580 .bind(&due_str)
581 581 .bind(&tags_json)
582 582 .bind(task.recurrence.db_value())
583 + .bind(task.recurrence_rule.as_ref().map(|r| serde_json::to_string(r).unwrap_or_default()))
583 584 .bind(task.urgency)
584 585 .bind(&scheduled_start_str)
585 586 .bind(task.scheduled_duration)
@@ -0,0 +1,88 @@
1 + //! Integration tests for SqliteMonthlyReviewRepository.
2 + //!
3 + //! Focus: the goal upsert-by-position path (a transaction-guarded
4 + //! get-then-insert, since `(user_id, month, position)` has no unique key) and
5 + //! the reflection `ON CONFLICT(user_id, month)` upsert — the review-repo
6 + //! atomicity the audit flagged as untested.
7 +
8 + mod common;
9 +
10 + use goingson_core::{MonthlyGoalStatus, MonthlyReviewRepository};
11 + use goingson_db_sqlite::SqliteMonthlyReviewRepository;
12 +
13 + const MONTH: &str = "2026-06";
14 +
15 + #[tokio::test]
16 + async fn test_upsert_goal_by_position_replaces_in_place() {
17 + let pool = common::setup_test_db().await;
18 + let user_id = common::create_test_user(&pool).await;
19 + let repo = SqliteMonthlyReviewRepository::new(pool);
20 +
21 + let first = repo.upsert_goal(user_id, MONTH, "ship v1", 1).await.unwrap();
22 + assert_eq!(first.text, "ship v1");
23 + assert_eq!(first.position, 1);
24 +
25 + // Same (user, month, position) must update the existing goal, not add a second at position 1.
26 + let second = repo.upsert_goal(user_id, MONTH, "ship v2", 1).await.unwrap();
27 + assert_eq!(second.id, first.id, "upsert must reuse the goal at that position");
28 + assert_eq!(second.text, "ship v2");
29 +
30 + let goals = repo.list_goals(user_id, MONTH).await.unwrap();
31 + assert_eq!(goals.len(), 1, "repeated upsert at one position must not duplicate");
32 + }
33 +
34 + #[tokio::test]
35 + async fn test_upsert_goal_distinct_positions_coexist() {
36 + let pool = common::setup_test_db().await;
37 + let user_id = common::create_test_user(&pool).await;
38 + let repo = SqliteMonthlyReviewRepository::new(pool);
39 +
40 + repo.upsert_goal(user_id, MONTH, "goal one", 1).await.unwrap();
41 + repo.upsert_goal(user_id, MONTH, "goal two", 2).await.unwrap();
42 + repo.upsert_goal(user_id, MONTH, "goal three", 3).await.unwrap();
43 +
44 + let goals = repo.list_goals(user_id, MONTH).await.unwrap();
45 + assert_eq!(goals.len(), 3);
46 + assert_eq!(goals[0].position, 1);
47 + assert_eq!(goals[2].text, "goal three");
48 + }
49 +
50 + #[tokio::test]
51 + async fn test_update_goal_status_and_delete() {
52 + let pool = common::setup_test_db().await;
53 + let user_id = common::create_test_user(&pool).await;
54 + let repo = SqliteMonthlyReviewRepository::new(pool);
55 +
56 + let goal = repo.upsert_goal(user_id, MONTH, "finish audit", 1).await.unwrap();
57 + assert_eq!(goal.status, MonthlyGoalStatus::Active);
58 +
59 + let updated = repo
60 + .update_goal_status(goal.id, user_id, &MonthlyGoalStatus::Done)
61 + .await
62 + .unwrap()
63 + .expect("goal exists");
64 + assert_eq!(updated.status, MonthlyGoalStatus::Done);
65 +
66 + assert!(repo.delete_goal(goal.id, user_id).await.unwrap());
67 + assert!(repo.list_goals(user_id, MONTH).await.unwrap().is_empty());
68 + }
69 +
70 + #[tokio::test]
71 + async fn test_upsert_reflection_is_idempotent() {
72 + let pool = common::setup_test_db().await;
73 + let user_id = common::create_test_user(&pool).await;
74 + let repo = SqliteMonthlyReviewRepository::new(pool);
75 +
76 + repo.upsert_reflection(user_id, MONTH, "highlight A", "change A").await.unwrap();
77 + // Second upsert for the same month must overwrite, not create a second reflection row.
78 + let second = repo
79 + .upsert_reflection(user_id, MONTH, "highlight B", "change B")
80 + .await
81 + .unwrap();
82 + assert_eq!(second.highlight_text, "highlight B");
83 + assert_eq!(second.change_text, "change B");
84 +
85 + let got = repo.get_reflection(user_id, MONTH).await.unwrap().unwrap();
86 + assert_eq!(got.highlight_text, "highlight B");
87 + assert_eq!(repo.list_all_reflections(user_id).await.unwrap().len(), 1);
88 + }
@@ -4,8 +4,8 @@ mod common;
4 4
5 5 use chrono::{Duration, Utc};
6 6 use goingson_core::{
7 - NewProject, NewTask, Priority, ProjectRepository, Recurrence, TaskFilterQuery, TaskRepository,
8 - TaskStatus, UpdateTask,
7 + NewProject, NewTask, Priority, ProjectRepository, Recurrence, RecurrenceRule, TaskFilterQuery,
8 + TaskRepository, TaskStatus, UpdateTask,
9 9 };
10 10 use goingson_db_sqlite::{SqliteProjectRepository, SqliteTaskRepository};
11 11
@@ -270,6 +270,7 @@ async fn test_update_to_completed_sets_completed_at() {
270 270 due: task.due,
271 271 tags: task.tags.clone(),
272 272 recurrence: Recurrence::None,
273 + recurrence_rule: None,
273 274 urgency: task.urgency,
274 275 scheduled_start: None,
275 276 scheduled_duration: None,
@@ -311,6 +312,7 @@ async fn test_update_from_completed_clears_completed_at() {
311 312 due: task.due,
312 313 tags: task.tags.clone(),
313 314 recurrence: Recurrence::None,
315 + recurrence_rule: None,
314 316 urgency: task.urgency,
315 317 scheduled_start: None,
316 318 scheduled_duration: None,
@@ -525,3 +527,73 @@ async fn test_bulk_set_project() {
525 527 assert_eq!(n2, 1);
526 528 assert_eq!(tasks.get_by_id(a.id, user_id).await.unwrap().unwrap().project_id, None);
527 529 }
530 +
531 + #[tokio::test]
532 + async fn test_update_persists_recurrence_rule() {
533 + let pool = common::setup_test_db().await;
534 + let user_id = common::create_test_user(&pool).await;
535 + let repo = SqliteTaskRepository::new(pool);
536 +
537 + // Create a task with a custom weekly rule (every 2 weeks, Mon/Wed).
538 + let new_task = NewTask::builder("Recurring standup")
539 + .recurrence(Recurrence::Weekly)
540 + .recurrence_rule(RecurrenceRule {
541 + pattern: Recurrence::Weekly,
542 + interval: 2,
543 + weekdays: vec![0, 2],
544 + monthly_spec: None,
545 + })
546 + .build();
547 + let task = repo.create(user_id, new_task).await.expect("create");
548 + let stored = task.recurrence_rule.expect("rule persisted on create");
549 + assert_eq!(stored.interval, 2);
550 + assert_eq!(stored.weekdays, vec![0, 2]);
551 +
552 + // Edit the rule to every 3 weeks on Friday. The update path must persist the new rule,
553 + // not leave the stale create-time rule (GO-31-11).
554 + let update = UpdateTask {
555 + project_id: None,
556 + milestone_id: None,
557 + contact_id: None,
558 + description: task.description.clone(),
559 + status: task.status.clone(),
560 + priority: task.priority.clone(),
561 + due: task.due,
562 + tags: task.tags.clone(),
563 + recurrence: Recurrence::Weekly,
564 + recurrence_rule: Some(RecurrenceRule {
565 + pattern: Recurrence::Weekly,
566 + interval: 3,
567 + weekdays: vec![4],
568 + monthly_spec: None,
569 + }),
570 + urgency: task.urgency,
571 + scheduled_start: None,
572 + scheduled_duration: None,
573 + estimated_minutes: None,
574 + };
575 + let updated = repo.update(task.id, user_id, update).await.expect("update").unwrap();
576 + let rule = updated.recurrence_rule.expect("rule present after update");
577 + assert_eq!(rule.interval, 3, "edited interval must persist");
578 + assert_eq!(rule.weekdays, vec![4], "edited weekdays must persist");
579 +
580 + // Clearing the rule on update must null it out, not retain the old one.
581 + let clear = UpdateTask {
582 + project_id: None,
583 + milestone_id: None,
584 + contact_id: None,
585 + description: task.description.clone(),
586 + status: task.status.clone(),
587 + priority: task.priority.clone(),
588 + due: task.due,
589 + tags: task.tags.clone(),
590 + recurrence: Recurrence::None,
591 + recurrence_rule: None,
592 + urgency: task.urgency,
593 + scheduled_start: None,
594 + scheduled_duration: None,
595 + estimated_minutes: None,
596 + };
597 + let cleared = repo.update(task.id, user_id, clear).await.expect("update").unwrap();
598 + assert!(cleared.recurrence_rule.is_none(), "cleared rule must not persist stale value");
599 + }
@@ -0,0 +1,89 @@
1 + //! Integration tests for SqliteWeeklyReviewRepository.
2 + //!
3 + //! Focus: upsert idempotency (the `ON CONFLICT(user_id, week_start_date)` path)
4 + //! and vacation-day round-trips — the review-repo atomicity the audit flagged as
5 + //! untested.
6 +
7 + mod common;
8 +
9 + use chrono::{Datelike, NaiveDate};
10 + use goingson_core::WeeklyReviewRepository;
11 + use goingson_db_sqlite::SqliteWeeklyReviewRepository;
12 +
13 + fn monday() -> NaiveDate {
14 + NaiveDate::from_ymd_opt(2026, 6, 15).unwrap()
15 + }
16 +
17 + #[tokio::test]
18 + async fn test_upsert_creates_then_updates_in_place() {
19 + let pool = common::setup_test_db().await;
20 + let user_id = common::create_test_user(&pool).await;
21 + let repo = SqliteWeeklyReviewRepository::new(pool);
22 + let week = monday();
23 +
24 + let first = repo.upsert(user_id, week, "initial notes").await.unwrap();
25 + assert_eq!(first.notes, "initial notes");
26 + assert_eq!(first.week_start_date, week);
27 +
28 + // Second upsert for the same (user, week) must UPDATE the same row, not insert a duplicate.
29 + let second = repo.upsert(user_id, week, "revised notes").await.unwrap();
30 + assert_eq!(second.id, first.id, "upsert must reuse the existing row id");
31 + assert_eq!(second.notes, "revised notes");
32 +
33 + let all = repo.list_all(user_id).await.unwrap();
34 + assert_eq!(all.len(), 1, "repeated upsert must not create duplicate weeks");
35 + assert_eq!(all[0].notes, "revised notes");
36 + }
37 +
38 + #[tokio::test]
39 + async fn test_upsert_is_idempotent_under_repeated_writes() {
40 + let pool = common::setup_test_db().await;
41 + let user_id = common::create_test_user(&pool).await;
42 + let repo = SqliteWeeklyReviewRepository::new(pool);
43 + let week = monday();
44 +
45 + for i in 0..5 {
46 + repo.upsert(user_id, week, &format!("pass {i}")).await.unwrap();
47 + }
48 +
49 + let all = repo.list_all(user_id).await.unwrap();
50 + assert_eq!(all.len(), 1);
51 + assert_eq!(all[0].notes, "pass 4");
52 + }
53 +
54 + #[tokio::test]
55 + async fn test_set_vacation_days_round_trip_and_overwrite() {
56 + let pool = common::setup_test_db().await;
57 + let user_id = common::create_test_user(&pool).await;
58 + let repo = SqliteWeeklyReviewRepository::new(pool);
59 + let week = monday();
60 +
61 + repo.upsert(user_id, week, "notes").await.unwrap();
62 + repo.set_vacation_days(user_id, week, &[0, 2, 4]).await.unwrap();
63 +
64 + let review = repo.get_for_week(user_id, week).await.unwrap().unwrap();
65 + assert_eq!(review.vacation_days, vec![0, 2, 4]);
66 +
67 + // Overwriting vacation days must replace, not append or duplicate the row.
68 + repo.set_vacation_days(user_id, week, &[6]).await.unwrap();
69 + let review = repo.get_for_week(user_id, week).await.unwrap().unwrap();
70 + assert_eq!(review.vacation_days, vec![6]);
71 + assert_eq!(repo.list_all(user_id).await.unwrap().len(), 1);
72 + }
73 +
74 + #[tokio::test]
75 + async fn test_is_current_week_completed_reflects_upsert() {
76 + let pool = common::setup_test_db().await;
77 + let user_id = common::create_test_user(&pool).await;
78 + let repo = SqliteWeeklyReviewRepository::new(pool);
79 +
80 + assert!(!repo.is_current_week_completed(user_id).await.unwrap());
81 +
82 + // Complete the current calendar week's review.
83 + let today = chrono::Utc::now().date_naive();
84 + let days_since_monday = today.weekday().num_days_from_monday() as i64;
85 + let this_monday = today - chrono::Duration::days(days_since_monday);
86 + repo.upsert(user_id, this_monday, "done").await.unwrap();
87 +
88 + assert!(repo.is_current_week_completed(user_id).await.unwrap());
89 + }
@@ -489,6 +489,7 @@ pub async fn update_task(state: State<'_, Arc<AppState>>, id: TaskId, input: Tas
489 489 due: input.due,
490 490 tags,
491 491 recurrence,
492 + recurrence_rule: input.recurrence_rule.clone(),
492 493 urgency,
493 494 scheduled_start: ctx.scheduled_start,
494 495 scheduled_duration: ctx.scheduled_duration,