max / goingson
5 files changed,
+78 insertions,
-23 deletions
| @@ -118,8 +118,13 @@ | |||
| 118 | 118 | ||
| 119 | 119 | // Date Helpers | |
| 120 | 120 | ||
| 121 | + | /// The first of the user's current month. | |
| 122 | + | /// | |
| 123 | + | /// Local civil date, for the same reason as | |
| 124 | + | /// [`crate::weekly_review::current_week_start`]: the month the heat-map buckets | |
| 125 | + | /// into has to be the month it was derived from. | |
| 121 | 126 | pub fn current_month_start() -> NaiveDate { | |
| 122 | - | let today = Utc::now().date_naive(); | |
| 127 | + | let today = crate::tz::today_local(); | |
| 123 | 128 | NaiveDate::from_ymd_opt(today.year(), today.month(), 1).expect("day 1 is always valid") | |
| 124 | 129 | } | |
| 125 | 130 |
| @@ -22,7 +22,7 @@ | |||
| 22 | 22 | //! zone is the user in" waiting to diverge. | |
| 23 | 23 | ||
| 24 | 24 | use crate::models::TzKind; | |
| 25 | - | use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; | |
| 25 | + | use chrono::{DateTime, NaiveDate, NaiveDateTime, TimeZone, Utc}; | |
| 26 | 26 | use chrono_tz::Tz; | |
| 27 | 27 | ||
| 28 | 28 | /// The user's IANA time zone, resolved from the OS. Falls back to UTC if the OS | |
| @@ -34,6 +34,18 @@ | |||
| 34 | 34 | .unwrap_or(Tz::UTC) | |
| 35 | 35 | } | |
| 36 | 36 | ||
| 37 | + | /// Today's civil date in the user's zone. | |
| 38 | + | /// | |
| 39 | + | /// The date a *period* is derived from -- this week, this month -- has to come | |
| 40 | + | /// from the same zone that later buckets rows into its days, or the two disagree | |
| 41 | + | /// for the hours when the UTC date has turned and the local one has not. Reading | |
| 42 | + | /// it off `Utc::now()` puts an evening west of Greenwich in tomorrow's week while | |
| 43 | + | /// every window query stays in today's, so the period being shown holds none of | |
| 44 | + | /// the rows that were just written into it. | |
| 45 | + | pub fn today_local() -> NaiveDate { | |
| 46 | + | Utc::now().with_timezone(&system_tz()).date_naive() | |
| 47 | + | } | |
| 48 | + | ||
| 37 | 49 | /// The zone an event's civil time should be read in. | |
| 38 | 50 | /// | |
| 39 | 51 | /// `Local` names its own zone, so it resolves the same for every reader; an |
| @@ -172,11 +172,21 @@ | |||
| 172 | 172 | ||
| 173 | 173 | // Date Helpers | |
| 174 | 174 | ||
| 175 | - | /// Gets the Monday of the current ISO week. | |
| 175 | + | /// Gets the Monday of the ISO week containing `date`. | |
| 176 | + | pub fn week_start_containing(date: NaiveDate) -> NaiveDate { | |
| 177 | + | let days_from_monday = date.weekday().num_days_from_monday(); | |
| 178 | + | date - Duration::days(days_from_monday as i64) | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | /// Gets the Monday of the user's current ISO week. | |
| 182 | + | /// | |
| 183 | + | /// Today is the user's civil date, not UTC's. Deriving the week from | |
| 184 | + | /// `Utc::now()` instead put an evening west of Greenwich in *next* week while | |
| 185 | + | /// the fetch window and the timeline buckets stayed in this one, so a review | |
| 186 | + | /// opened after the UTC rollover reported an empty week: the completions it was | |
| 187 | + | /// meant to show fell outside the window entirely. | |
| 176 | 188 | pub fn current_week_start() -> NaiveDate { | |
| 177 | - | let today = Utc::now().date_naive(); | |
| 178 | - | let days_from_monday = today.weekday().num_days_from_monday(); | |
| 179 | - | today - Duration::days(days_from_monday as i64) | |
| 189 | + | week_start_containing(crate::tz::today_local()) | |
| 180 | 190 | } | |
| 181 | 191 | ||
| 182 | 192 | /// Parses a "YYYY-MM-DD" string into the Monday of that ISO week. | |
| @@ -184,8 +194,7 @@ | |||
| 184 | 194 | /// have to pre-compute the boundary. | |
| 185 | 195 | pub fn parse_week_start(s: &str) -> Option<NaiveDate> { | |
| 186 | 196 | let date = NaiveDate::parse_from_str(s, "%Y-%m-%d").ok()?; | |
| 187 | - | let days_from_monday = date.weekday().num_days_from_monday(); | |
| 188 | - | Some(date - Duration::days(days_from_monday as i64)) | |
| 197 | + | Some(week_start_containing(date)) | |
| 189 | 198 | } | |
| 190 | 199 | ||
| 191 | 200 | /// Gets the Sunday of the week starting on the given Monday. | |
| @@ -566,6 +575,39 @@ | |||
| 566 | 575 | assert_eq!(utc[5].completed_count, 1, "UTC bucketing lands on Saturday"); | |
| 567 | 576 | } | |
| 568 | 577 | ||
| 578 | + | #[test] | |
| 579 | + | fn an_evening_completion_stays_in_the_week_it_was_made_in() { | |
| 580 | + | // 19:06 Sunday in Denver (UTC-6) is 01:06 Monday UTC. The week has to | |
| 581 | + | // be the one the user is still in, and the completion has to land on | |
| 582 | + | // its last day rather than outside it. Reading "today" off UTC made the | |
| 583 | + | // review show the *next* week, which held none of these rows. | |
| 584 | + | let denver: Tz = "America/Denver".parse().unwrap(); | |
| 585 | + | let sunday_evening_utc = NaiveDate::from_ymd_opt(2026, 8, 17) | |
| 586 | + | .unwrap() | |
| 587 | + | .and_hms_opt(1, 6, 0) | |
| 588 | + | .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc)) | |
| 589 | + | .unwrap(); | |
| 590 | + | ||
| 591 | + | let today = sunday_evening_utc.with_timezone(&denver).date_naive(); | |
| 592 | + | assert_eq!(today, NaiveDate::from_ymd_opt(2026, 8, 16).unwrap()); | |
| 593 | + | ||
| 594 | + | let week_start = week_start_containing(today); | |
| 595 | + | assert_eq!( | |
| 596 | + | week_start, | |
| 597 | + | NaiveDate::from_ymd_opt(2026, 8, 10).unwrap(), | |
| 598 | + | "the week is the one the user is still in" | |
| 599 | + | ); | |
| 600 | + | ||
| 601 | + | let tasks = vec![make_completed_task(sunday_evening_utc, sunday_evening_utc)]; | |
| 602 | + | let days = build_timeline_days(week_start, today, denver, &[], &tasks, &[], &[], &[], &[]); | |
| 603 | + | assert_eq!(days[6].completed_count, 1, "Sunday holds the completion"); | |
| 604 | + | assert_eq!( | |
| 605 | + | days.iter().map(|d| d.completed_count).sum::<i32>(), | |
| 606 | + | 1, | |
| 607 | + | "and no other day does" | |
| 608 | + | ); | |
| 609 | + | } | |
| 610 | + | ||
| 569 | 611 | #[test] | |
| 570 | 612 | fn test_should_show_nudge_with_review() { | |
| 571 | 613 | let review = Some(WeeklyReview { |
| @@ -6,7 +6,7 @@ | |||
| 6 | 6 | ||
| 7 | 7 | mod common; | |
| 8 | 8 | ||
| 9 | - | use chrono::{Datelike, NaiveDate}; | |
| 9 | + | use chrono::NaiveDate; | |
| 10 | 10 | use goingson_core::WeeklyReviewRepository; | |
| 11 | 11 | use goingson_db_sqlite::SqliteWeeklyReviewRepository; | |
| 12 | 12 | ||
| @@ -83,10 +83,11 @@ | |||
| 83 | 83 | ||
| 84 | 84 | assert!(!repo.is_current_week_completed(user_id).unwrap()); | |
| 85 | 85 | ||
| 86 | - | // Complete the current calendar week's review. | |
| 87 | - | let today = chrono::Utc::now().date_naive(); | |
| 88 | - | let days_since_monday = today.weekday().num_days_from_monday() as i64; | |
| 89 | - | let this_monday = today - chrono::Duration::days(days_since_monday); | |
| 86 | + | // Complete the current calendar week's review. Core's answer, not a | |
| 87 | + | // hand-rolled one off the UTC clock: the two disagree on the evenings when | |
| 88 | + | // the UTC date has turned and the user's has not, and then this asserts | |
| 89 | + | // that writing one week completed another. | |
| 90 | + | let this_monday = goingson_core::weekly_review::current_week_start(); | |
| 90 | 91 | repo.upsert(user_id, this_monday, "done").unwrap(); | |
| 91 | 92 | ||
| 92 | 93 | assert!(repo.is_current_week_completed(user_id).unwrap()); |
| @@ -5,7 +5,7 @@ | |||
| 5 | 5 | //! - Creating/updating reviews | |
| 6 | 6 | //! - Checking if current week is completed | |
| 7 | 7 | ||
| 8 | - | use chrono::{Datelike, NaiveDate, Utc}; | |
| 8 | + | use chrono::{NaiveDate, Utc}; | |
| 9 | 9 | use goingson_core::{ | |
| 10 | 10 | CoreError, Result, UserId, WeeklyReview, WeeklyReviewId, WeeklyReviewRepository, | |
| 11 | 11 | }; | |
| @@ -34,14 +34,6 @@ | |||
| 34 | 34 | } | |
| 35 | 35 | } | |
| 36 | 36 | ||
| 37 | - | /// Gets the Monday of the current ISO week. | |
| 38 | - | fn current_week_start() -> NaiveDate { | |
| 39 | - | let today = Utc::now().date_naive(); | |
| 40 | - | // NaiveDate::week returns the ISO week, which starts on Monday | |
| 41 | - | let days_from_monday = today.weekday().num_days_from_monday(); | |
| 42 | - | today - chrono::Duration::days(days_from_monday as i64) | |
| 43 | - | } | |
| 44 | - | ||
| 45 | 37 | struct WeeklyReviewRow { | |
| 46 | 38 | id: String, | |
| 47 | 39 | user_id: String, | |
| @@ -157,7 +149,10 @@ | |||
| 157 | 149 | ||
| 158 | 150 | #[tracing::instrument(skip_all)] | |
| 159 | 151 | fn is_current_week_completed(&self, user_id: UserId) -> Result<bool> { | |
| 160 | - | let week_start = current_week_start(); | |
| 152 | + | // Core's, not a second copy: this asks the same question the review | |
| 153 | + | // screen asks, and two answers to "which week is it" diverge on the | |
| 154 | + | // evenings when the UTC date has turned and the user's has not. | |
| 155 | + | let week_start = goingson_core::weekly_review::current_week_start(); | |
| 161 | 156 | let review = self.get_for_week(user_id, week_start)?; | |
| 162 | 157 | Ok(review.is_some()) | |
| 163 | 158 | } |