diff --git a/.env.example b/.env.example index 33f30524..0fdad9f4 100644 --- a/.env.example +++ b/.env.example @@ -47,6 +47,8 @@ ALLOW_REGISTRATION=true # SECURE_COOKIES=true # Handler-level logs come from the `presentation` crate; startup/wiring logs come # from `server`. Include both — `server=info` alone silently drops handler logs. +# Crates not named here (adapters, etc.) still log warnings and errors unless +# you set a bare level yourself (e.g. `info,presentation=debug`). # RUST_LOG=server=info,presentation=info,tower_http=info,worker=info # CORS (for SPA development only) diff --git a/Cargo.lock b/Cargo.lock index d3fc10fa..efa04004 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1067,6 +1067,16 @@ dependencies = [ "windows-link", ] +[[package]] +name = "chrono-tz" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6139a8597ed92cf816dfb33f5dd6cf0bb93a6adc938f11039f371bc5bcd26c3" +dependencies = [ + "chrono", + "phf 0.12.1", +] + [[package]] name = "cipher" version = "0.4.4" @@ -1321,7 +1331,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eb2a7d3066da2de787b7f032c736763eb7ae5d355f81a68bab2675a96008b0bf" dependencies = [ "lab", - "phf", + "phf 0.11.3", ] [[package]] @@ -1333,7 +1343,7 @@ dependencies = [ "cssparser-macros", "dtoa-short", "itoa", - "phf", + "phf 0.11.3", "smallvec", ] @@ -3751,7 +3761,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1fd6780a80ae0c52cc120a26a1a42c1ae51b247a253e4e06113d23d2c2edd078" dependencies = [ "phf_macros", - "phf_shared", + "phf_shared 0.11.3", +] + +[[package]] +name = "phf" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "913273894cec178f401a31ec4b656318d95473527be05c0752cc41cdc32be8b7" +dependencies = [ + "phf_shared 0.12.1", ] [[package]] @@ -3761,7 +3780,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aef8048c789fa5e851558d709946d6d79a8ff88c0440c587967f8e94bfb1216a" dependencies = [ "phf_generator", - "phf_shared", + "phf_shared 0.11.3", ] [[package]] @@ -3770,7 +3789,7 @@ version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c80231409c20246a13fddb31776fb942c38553c51e871f8cbd687a4cfb5843d" dependencies = [ - "phf_shared", + "phf_shared 0.11.3", "rand 0.8.6", ] @@ -3781,7 +3800,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f84ac04429c13a7ff43785d75ad27569f2951ce0ffd30a3321230db2fc727216" dependencies = [ "phf_generator", - "phf_shared", + "phf_shared 0.11.3", "proc-macro2", "quote", "syn 2.0.117", @@ -3796,6 +3815,15 @@ dependencies = [ "siphasher", ] +[[package]] +name = "phf_shared" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06005508882fb681fd97892ecff4b7fd0fee13ef1aa569f8695dae7ab9099981" +dependencies = [ + "siphasher", +] + [[package]] name = "pin-project" version = "1.1.13" @@ -4047,6 +4075,7 @@ dependencies = [ "axum-governor", "bytes", "chrono", + "chrono-tz", "composition", "domain", "dotenvy", @@ -5545,7 +5574,7 @@ checksum = "bf776ba3fa74f83bf4b63c3dcbbf82173db2632ed8452cb2d891d33f459de70f" dependencies = [ "new_debug_unreachable", "parking_lot", - "phf_shared", + "phf_shared 0.11.3", "precomputed-hash", "serde", ] @@ -5557,7 +5586,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c711928715f1fe0fe509c53b43e993a9a557babc2d0a3567d0a3006f1ac931a0" dependencies = [ "phf_generator", - "phf_shared", + "phf_shared 0.11.3", "proc-macro2", "quote", ] @@ -5748,7 +5777,7 @@ checksum = "d4ea810f0692f9f51b382fff5893887bb4580f5fa246fde546e0b13e7fcee662" dependencies = [ "fnv", "nom 7.1.3", - "phf", + "phf 0.11.3", "phf_codegen", ] @@ -5785,7 +5814,7 @@ dependencies = [ "ordered-float", "pest", "pest_derive", - "phf", + "phf 0.11.3", "sha2", "signal-hook", "siphasher", @@ -6665,7 +6694,7 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57ffde1dc01240bdf9992e3205668b235e59421fd085e8a317ed98da0178d414" dependencies = [ - "phf", + "phf 0.11.3", "phf_codegen", "string_cache", "string_cache_codegen", diff --git a/Cargo.toml b/Cargo.toml index af16eb35..59e44179 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,6 +55,7 @@ tracing-subscriber = { version = "0.3.23", features = ["env-filter"] } async-trait = "0.1.92" uuid = { version = "1.23.0", features = ["v4", "v5", "serde"] } chrono = { version = "0.4", features = ["serde"] } +chrono-tz = "0.10" sqlx = { version = "0.8.6", features = [ "runtime-tokio-rustls", "sqlite", diff --git a/crates/adapters/postgres/src/insights.rs b/crates/adapters/postgres/src/insights.rs index a7474e65..09bfe971 100644 --- a/crates/adapters/postgres/src/insights.rs +++ b/crates/adapters/postgres/src/insights.rs @@ -30,7 +30,7 @@ impl InsightsQuery for PostgresInsightsQuery { to_char(watched_at AT TIME ZONE 'UTC', 'YYYY-MM-DD HH24:MI:SS') AS watched_at, \ watch_medium \ FROM reviews \ - WHERE user_id = $1 \ + WHERE user_id = $1 AND remote_actor_url IS NULL \ ORDER BY watched_at ASC", ) .bind(user_id.value().to_string()) diff --git a/crates/adapters/sqlite/src/insights.rs b/crates/adapters/sqlite/src/insights.rs index 6762b851..2224901f 100644 --- a/crates/adapters/sqlite/src/insights.rs +++ b/crates/adapters/sqlite/src/insights.rs @@ -8,8 +8,6 @@ use sqlx::{Row, SqlitePool}; use adapter_common::{parse_datetime, parse_uuid}; -use crate::wrapup::fetch_genres_sqlite; - /// SQLite read side of Insights (ADR-0010). Deliberately thin: one query for /// review basics, one for genres — every threshold and comparison lives in /// the pure analyzer, not here. @@ -29,7 +27,7 @@ impl InsightsQuery for SqliteInsightsQuery { let rows = sqlx::query( "SELECT movie_id, rating, watched_at, watch_medium \ FROM reviews \ - WHERE user_id = ? \ + WHERE user_id = ? AND remote_actor_url IS NULL \ ORDER BY watched_at ASC", ) .bind(user_id.value().to_string()) @@ -37,20 +35,32 @@ impl InsightsQuery for SqliteInsightsQuery { .await .map_err(adapter_common::map_sqlx_error)?; - let mut movie_ids: Vec = Vec::new(); - let mut seen = std::collections::HashSet::new(); - for row in &rows { + // Subquery rather than an `IN (?, …)` bind list: a whole diary can + // exceed SQLite's bound-parameter limit. + let genre_rows = sqlx::query( + "SELECT movie_id, name FROM movie_genres \ + WHERE movie_id IN ( \ + SELECT movie_id FROM reviews \ + WHERE user_id = ? AND remote_actor_url IS NULL \ + ) \ + ORDER BY name", + ) + .bind(user_id.value().to_string()) + .fetch_all(&self.pool) + .await + .map_err(adapter_common::map_sqlx_error)?; + + let mut genres_map: HashMap> = HashMap::new(); + for row in genre_rows { let mid: String = row .try_get("movie_id") .map_err(adapter_common::map_sqlx_error)?; - if seen.insert(mid.clone()) { - movie_ids.push(mid); - } + let name: String = row + .try_get("name") + .map_err(adapter_common::map_sqlx_error)?; + genres_map.entry(mid).or_default().push(name); } - let genres_map: HashMap> = - fetch_genres_sqlite(&self.pool, &movie_ids).await?; - let mut result = Vec::with_capacity(rows.len()); for row in &rows { let movie_id: String = row diff --git a/crates/api-types/src/insights.rs b/crates/api-types/src/insights.rs index 45aed22a..912e3e12 100644 --- a/crates/api-types/src/insights.rs +++ b/crates/api-types/src/insights.rs @@ -17,7 +17,7 @@ pub enum InsightDto { sample_count: u32, }, /// `genre`'s share of watches in `month` (1–12) is `multiplier` times the - /// uniform 1/12 baseline. + /// user's own share of watches in that month. Seasonality { genre: String, month: u32, @@ -47,3 +47,11 @@ pub enum InsightDto { pub struct InsightsResponse { pub insights: Vec, } + +#[derive(Debug, Clone, Deserialize, utoipa::IntoParams)] +#[into_params(parameter_in = Query)] +pub struct InsightsQueryParams { + /// IANA time zone (e.g. `Europe/Warsaw`) months, weekdays and hours are + /// bucketed in. Defaults to UTC. + pub tz: Option, +} diff --git a/crates/application/src/insights/get.rs b/crates/application/src/insights/get.rs index d831ca2a..ae610a9a 100644 --- a/crates/application/src/insights/get.rs +++ b/crates/application/src/insights/get.rs @@ -3,13 +3,20 @@ //! thresholds. No caching, no snapshots — the same on-the-fly shape as the //! existing live stats. +use chrono::TimeZone; use domain::{errors::DomainError, models::Insight, value_objects::UserId}; use crate::insights::deps::GetInsightsDeps; -pub async fn execute(deps: &GetInsightsDeps, user_id: UserId) -> Result, DomainError> { +pub async fn execute( + deps: &GetInsightsDeps, + user_id: UserId, + tz: &Tz, +) -> Result, DomainError> { let rows = deps.insights.get_watch_rows(&user_id).await?; - Ok(domain::services::insights_analyzer::build_insights(&rows)) + Ok(domain::services::insights_analyzer::build_insights( + &rows, tz, + )) } #[cfg(test)] diff --git a/crates/application/src/insights/tests/get.rs b/crates/application/src/insights/tests/get.rs index 5a65e6b3..c053e507 100644 --- a/crates/application/src/insights/tests/get.rs +++ b/crates/application/src/insights/tests/get.rs @@ -31,7 +31,9 @@ async fn returns_analyzer_output_for_the_user() { insights: InMemoryInsightsQuery::with_rows(user, rows), }; - let insights = execute(&deps, UserId::from_uuid(user)).await.unwrap(); + let insights = execute(&deps, UserId::from_uuid(user), &chrono::Utc) + .await + .unwrap(); assert!(insights.iter().any( |i| matches!(i, domain::models::Insight::RatingLift { genre, .. } if genre == "Thriller") )); @@ -49,7 +51,9 @@ async fn other_users_rows_are_not_seen() { insights: InMemoryInsightsQuery::with_rows(other, rows), }; - let insights = execute(&deps, UserId::from_uuid(user)).await.unwrap(); + let insights = execute(&deps, UserId::from_uuid(user), &chrono::Utc) + .await + .unwrap(); assert!(insights.is_empty()); } @@ -60,7 +64,9 @@ async fn empty_diary_returns_empty_list() { insights: InMemoryInsightsQuery::new(), }; - let insights = execute(&deps, UserId::from_uuid(user)).await.unwrap(); + let insights = execute(&deps, UserId::from_uuid(user), &chrono::Utc) + .await + .unwrap(); assert!(insights.is_empty()); } @@ -83,7 +89,7 @@ async fn port_errors_propagate() { insights: Arc::new(Failing), }; - let err = execute(&deps, UserId::from_uuid(Uuid::new_v4())) + let err = execute(&deps, UserId::from_uuid(Uuid::new_v4()), &chrono::Utc) .await .unwrap_err(); assert!(matches!( diff --git a/crates/domain/src/models/insights.rs b/crates/domain/src/models/insights.rs index 2ee261d5..bb3954b1 100644 --- a/crates/domain/src/models/insights.rs +++ b/crates/domain/src/models/insights.rs @@ -35,7 +35,7 @@ pub enum Insight { sample_count: u32, }, /// The genre's share of watches in one month of the year is `multiplier` - /// times the uniform 1/12 baseline. + /// times the user's own share of watches in that month. Seasonality { genre: String, /// 1–12 diff --git a/crates/domain/src/services/insights_analyzer.rs b/crates/domain/src/services/insights_analyzer.rs index 04ee8a2e..706e1ecd 100644 --- a/crates/domain/src/services/insights_analyzer.rs +++ b/crates/domain/src/services/insights_analyzer.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; -use chrono::{Datelike, Timelike, Weekday}; +use chrono::{Datelike, NaiveDateTime, TimeZone, Timelike, Weekday}; use crate::models::{Insight, WatchRow}; @@ -21,8 +21,11 @@ const MIN_BASELINE_RATINGS: usize = 10; const MIN_RATING_DELTA: f64 = 0.3; /// Rows a genre needs before month-of-year distribution is more than noise. const MIN_GENRE_SEASON_ROWS: usize = 15; -/// Minimum lift over the uniform 1/12 month baseline. +/// Minimum lift over the user's own share of watches in that month. const MIN_SEASON_MULTIPLIER: f64 = 1.5; +/// Genre watches the peak month needs — a lift built on one or two rows in a +/// quiet month is noise, however large the ratio. +const MIN_SEASON_PEAK_ROWS: u32 = 4; /// Rows a medium needs before its weekend share means anything. const MIN_MEDIUM_ROWS: usize = 5; /// Minimum lift over the user's own weekend share. @@ -40,15 +43,21 @@ const MAX_PER_CATEGORY: usize = 3; /// Width of the peak-hour window in the rhythm insight. const RHYTHM_WINDOW_HOURS: u32 = 3; -pub fn build_insights(rows: &[WatchRow]) -> Vec { +/// `rows` carry `watched_at` in UTC as stored; `tz` is the viewer's zone, so +/// months, weekdays and hours are bucketed as the user lived them. +pub fn build_insights(rows: &[WatchRow], tz: &Tz) -> Vec { let mut insights = Vec::new(); insights.extend(rating_lifts(rows)); - insights.extend(seasonalities(rows)); - insights.extend(medium_timings(rows)); - insights.extend(rhythm(rows)); + insights.extend(seasonalities(rows, tz)); + insights.extend(medium_timings(rows, tz)); + insights.extend(rhythm(rows, tz)); insights } +fn local(tz: &Tz, utc: &NaiveDateTime) -> NaiveDateTime { + tz.from_utc_datetime(utc).naive_local() +} + // ── Rating lift ────────────────────────────────────────────────────────────── fn rating_lifts(rows: &[WatchRow]) -> Vec { @@ -75,20 +84,13 @@ fn rating_lifts(rows: &[WatchRow]) -> Vec { if delta.abs() < MIN_RATING_DELTA { return None; } - Some(( - strength(delta.abs(), ratings.len()), - genre, - genre_avg, + Some(Insight::RatingLift { + genre: genre.to_string(), + genre_avg: round2(genre_avg), + user_avg: round2(user_avg), delta, - ratings.len(), - )) - }) - .map(|(_, genre, genre_avg, delta, n)| Insight::RatingLift { - genre: genre.to_string(), - genre_avg: round2(genre_avg), - user_avg: round2(user_avg), - delta, - sample_count: n as u32, + sample_count: ratings.len() as u32, + }) }) .collect(); @@ -128,50 +130,55 @@ fn avg_rating(rows: &[&WatchRow]) -> f64 { // ── Seasonality ────────────────────────────────────────────────────────────── -fn seasonalities(rows: &[WatchRow]) -> Vec { - let mut by_genre: HashMap<&str, Vec> = HashMap::new(); +fn seasonalities(rows: &[WatchRow], tz: &Tz) -> Vec { + if rows.is_empty() { + return vec![]; + } + // The user's own month-of-year distribution is the baseline: a diary + // started in October, or a user who simply watches more in winter, must + // not make every big genre look seasonal. + let mut user_months = [0u32; 12]; + let mut by_genre: HashMap<&str, [u32; 12]> = HashMap::new(); for row in rows { + let m = local(tz, &row.watched_at).month0() as usize; + user_months[m] += 1; for genre in &row.genres { - by_genre - .entry(genre) - .or_default() - .push(row.watched_at.month()); + by_genre.entry(genre).or_insert([0; 12])[m] += 1; } } + let user_total = rows.len() as f64; let mut candidates: Vec = by_genre .into_iter() - .filter(|(_, months)| months.len() >= MIN_GENRE_SEASON_ROWS) - .filter_map(|(genre, months)| { - let total = months.len() as f64; - let mut histogram = [0u32; 12]; - for &m in &months { - histogram[(m - 1) as usize] += 1; + .filter_map(|(genre, histogram)| { + let genre_total: u32 = histogram.iter().sum(); + if (genre_total as usize) < MIN_GENRE_SEASON_ROWS { + return None; } - // Smallest month number wins ties — deterministic output. - let (peak_month, peak_count) = histogram - .into_iter() + // Month with the largest lift over the user's share for that + // month; smallest month number wins ties — deterministic output. + let (peak_month, multiplier) = histogram + .iter() .enumerate() - .max_by_key(|(i, c)| (*c, std::cmp::Reverse(*i))) - .map(|(i, c)| (i as u32 + 1, c))?; - // Baseline is a uniform year: each month expects total/12 watches. - let multiplier = round2(f64::from(peak_count) * 12.0 / total); + .filter(|(_, c)| **c >= MIN_SEASON_PEAK_ROWS) + .map(|(i, &c)| { + let genre_share = f64::from(c) / f64::from(genre_total); + let user_share = f64::from(user_months[i]) / user_total; + (i as u32 + 1, round2(genre_share / user_share)) + }) + .fold(None, |best: Option<(u32, f64)>, cur| match best { + Some(b) if b.1 >= cur.1 => Some(b), + _ => Some(cur), + })?; if multiplier < MIN_SEASON_MULTIPLIER { return None; } - Some(( - strength(multiplier - 1.0, months.len()), - genre, - peak_month, + Some(Insight::Seasonality { + genre: genre.to_string(), + month: peak_month, multiplier, - months.len(), - )) - }) - .map(|(_, genre, month, multiplier, n)| Insight::Seasonality { - genre: genre.to_string(), - month, - multiplier, - sample_count: n as u32, + sample_count: genre_total, + }) }) .collect(); @@ -182,7 +189,7 @@ fn seasonalities(rows: &[WatchRow]) -> Vec { // ── Medium timing ──────────────────────────────────────────────────────────── -fn medium_timings(rows: &[WatchRow]) -> Vec { +fn medium_timings(rows: &[WatchRow], tz: &Tz) -> Vec { if rows.is_empty() { return vec![]; } @@ -196,7 +203,10 @@ fn medium_timings(rows: &[WatchRow]) -> Vec { let mut by_medium: HashMap<&str, (usize, usize)> = HashMap::new(); let mut weekend_total = 0usize; for row in rows.iter().filter(|r| r.watch_medium.is_some()) { - let weekend = matches!(row.watched_at.weekday(), Weekday::Sat | Weekday::Sun); + let weekend = matches!( + local(tz, &row.watched_at).weekday(), + Weekday::Sat | Weekday::Sun + ); weekend_total += usize::from(weekend); let entry = by_medium .entry(row.watch_medium.as_deref().expect("filtered above")) @@ -220,23 +230,14 @@ fn medium_timings(rows: &[WatchRow]) -> Vec { if multiplier < MIN_MEDIUM_MULTIPLIER { return None; } - Some(( - strength(multiplier - 1.0, count), - medium, - weekend_share, - multiplier, - count, - )) - }) - .map( - |(_, medium, weekend_share, multiplier, n)| Insight::MediumTiming { + Some(Insight::MediumTiming { medium: medium.to_string(), multiplier, weekend_share: round2(weekend_share), baseline_share: round2(baseline_share), - sample_count: n as u32, - }, - ) + sample_count: count as u32, + }) + }) .collect(); candidates.sort_by(strongest_first); @@ -246,10 +247,14 @@ fn medium_timings(rows: &[WatchRow]) -> Vec { // ── Rhythm ─────────────────────────────────────────────────────────────────── -fn rhythm(rows: &[WatchRow]) -> Vec { - let real: Vec<&WatchRow> = rows +fn rhythm(rows: &[WatchRow], tz: &Tz) -> Vec { + // Import noise is detected on the stored value: date-only imports land at + // exactly 00:00 as stored, which no longer reads as midnight once shifted + // into the viewer's zone. + let real: Vec = rows .iter() .filter(|r| !(r.watched_at.hour() == 0 && r.watched_at.minute() == 0)) + .map(|r| local(tz, &r.watched_at)) .collect(); let midnight_share = 1.0 - real.len() as f64 / rows.len().max(1) as f64; if rows.is_empty() || real.len() < MIN_RHYTHM_ROWS || midnight_share > MAX_MIDNIGHT_SHARE { @@ -258,9 +263,9 @@ fn rhythm(rows: &[WatchRow]) -> Vec { let mut hours = [0u32; 24]; let mut days = [0u32; 7]; // index 0 = Monday (chrono weekday number − 1) - for row in &real { - hours[row.watched_at.hour() as usize] += 1; - days[row.watched_at.weekday().number_from_monday() as usize - 1] += 1; + for at in &real { + hours[at.hour() as usize] += 1; + days[at.weekday().number_from_monday() as usize - 1] += 1; } // Circular 3-hour window with the most watches; smallest start hour wins ties. diff --git a/crates/domain/src/services/tests/insights_analyzer.rs b/crates/domain/src/services/tests/insights_analyzer.rs index 4a9595b0..27597bab 100644 --- a/crates/domain/src/services/tests/insights_analyzer.rs +++ b/crates/domain/src/services/tests/insights_analyzer.rs @@ -1,4 +1,4 @@ -use chrono::{Datelike, NaiveDate, Weekday}; +use chrono::{Datelike, FixedOffset, NaiveDate, Utc, Weekday}; use uuid::Uuid; use crate::models::{Insight, WatchRow}; @@ -41,7 +41,7 @@ fn rating_lift_compares_genre_to_personal_average() { .collect(); rows.extend((0..12).map(|i| watch(&["Comedy"], 2, date(2024, 3, 1 + i), 20))); - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); let thrill = insights.iter().find_map(|i| match i { Insight::RatingLift { genre, @@ -81,7 +81,7 @@ fn rating_lift_ignores_rewatches() { rows.push(rewatch); } - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); let thrill = insights.iter().find_map(|i| match i { Insight::RatingLift { genre, @@ -107,7 +107,7 @@ fn rating_lift_below_sample_threshold_is_absent() { // high they are rated, the genre must not yield an insight. rows.extend((0..4).map(|i| watch(&["Thriller"], 5, date(2024, 2, 1 + i), 20))); - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); assert!( !insights .iter() @@ -126,14 +126,14 @@ fn rating_lift_below_delta_threshold_is_absent() { rows.extend((0..3).map(|i| watch(&["Drama"], 3, date(2024, 1, 13 + i), 20))); rows.extend((0..5).map(|i| watch(&["Thriller"], 4, date(2024, 2, 1 + i), 20))); - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); assert!(!kinds(&insights).contains(&"rating_lift")); } // ── Seasonality ────────────────────────────────────────────────────────────── #[test] -fn seasonality_detects_month_peak_against_uniform_baseline() { +fn seasonality_detects_month_peak_against_own_monthly_baseline() { // 16 horrors: 10 in October across two years, 6 scattered. let mut rows: Vec = Vec::new(); for i in 0..10u32 { @@ -146,7 +146,7 @@ fn seasonality_detects_month_peak_against_uniform_baseline() { // A perfectly spread genre (2 watches per month) must NOT appear. rows.extend((0..24).map(|i| watch(&["Drama"], 3, date(2024, 1 + i % 12, 5), 21))); - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); let horror = insights.iter().find_map(|i| match i { Insight::Seasonality { genre, @@ -159,9 +159,10 @@ fn seasonality_detects_month_peak_against_uniform_baseline() { }); let (month, multiplier, count) = horror.expect("Horror seasonality expected"); assert_eq!(month, 10); - // 10 of 16 watches in one month → (10/16)·12 = 7.5× the uniform baseline. + // Horror: 10 of 16 watches in October (62.5%); the whole diary puts + // 12 of 40 watches there (30%) → 0.625 / 0.3 ≈ 2.08× the user's own share. assert!( - (multiplier - 7.5).abs() < 1e-9, + (multiplier - 2.08).abs() < 1e-9, "multiplier was {multiplier}" ); assert_eq!(count, 16); @@ -174,12 +175,26 @@ fn seasonality_detects_month_peak_against_uniform_baseline() { ); } +#[test] +fn seasonality_ignores_when_the_diary_itself_is_concentrated() { + // A diary started in October: every genre peaks in Oct/Nov only because + // the user logged nothing else. Relative to their own months, nothing is + // seasonal. + let mut rows: Vec = Vec::new(); + for i in 0..20u32 { + let month = if i % 2 == 0 { 10 } else { 11 }; + rows.push(watch(&["Drama"], 3, date(2024, month, 1 + i), 21)); + rows.push(watch(&["Comedy"], 3, date(2024, month, 1 + i), 21)); + } + assert!(!kinds(&build_insights(&rows, &Utc)).contains(&"seasonality")); +} + #[test] fn seasonality_below_sample_threshold_is_absent() { let rows: Vec = (0..14) .map(|i| watch(&["Horror"], 4, date(2024, 10, 1 + i), 21)) .collect(); - assert!(!kinds(&build_insights(&rows)).contains(&"seasonality")); + assert!(!kinds(&build_insights(&rows, &Utc)).contains(&"seasonality")); } // ── Medium timing ──────────────────────────────────────────────────────────── @@ -216,7 +231,7 @@ fn medium_timing_detects_weekend_lift() { rows.push(row); } - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); let cinema = insights.iter().find_map(|i| match i { Insight::MediumTiming { medium, @@ -255,7 +270,7 @@ fn medium_timing_skipped_when_medium_mostly_missing() { rows.push(row); } - assert!(!kinds(&build_insights(&rows)).contains(&"medium_timing")); + assert!(!kinds(&build_insights(&rows, &Utc)).contains(&"medium_timing")); } // ── Rhythm ─────────────────────────────────────────────────────────────────── @@ -280,7 +295,7 @@ fn rhythm_finds_peak_window_and_days() { } } - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); let rhythm = insights.iter().find_map(|i| match i { Insight::Rhythm { peak_hour_start, @@ -305,6 +320,61 @@ fn rhythm_finds_peak_window_and_days() { assert_eq!(count, 12); } +#[test] +fn rhythm_buckets_in_the_viewers_time_zone() { + // 12 watches stored at 22:30 UTC on Tue/Wed/Thu are 00:30 the next day + // for a viewer at UTC+2. + let first_tuesday = date(2024, 3, 5); + let mut rows: Vec = Vec::new(); + for day in 0..3u32 { + for week in 0..(5 - day) { + rows.push(watch( + &["Drama"], + 4, + first_tuesday + + chrono::Duration::days(i64::from(day)) + + chrono::Duration::weeks(i64::from(week)), + 22, + )); + } + } + let plus_two = FixedOffset::east_opt(2 * 3600).unwrap(); + + let insights = build_insights(&rows, &plus_two); + let (start, end, days) = insights + .iter() + .find_map(|i| match i { + Insight::Rhythm { + peak_hour_start, + peak_hour_end, + top_days, + .. + } => Some((*peak_hour_start, *peak_hour_end, top_days.clone())), + _ => None, + }) + .expect("rhythm expected"); + // Local hour 0; the scan starts at 0, so the first window covering it is 0–3. + assert_eq!((start, end), (0, 3)); + assert_eq!(days, vec![3, 4, 5], "Wed=3, Thu=4, Fri=5 locally"); +} + +#[test] +fn rhythm_import_noise_detected_before_zone_shift() { + // Date-only imports are stored at 00:00 UTC; at UTC+2 they read 02:00, + // but they are still noise. + let rows: Vec = (0..12u32) + .map(|i| WatchRow { + movie_id: Uuid::new_v4(), + rating: 4, + watched_at: date(2024, 1, 1 + i).and_hms_opt(0, 0, 0).unwrap(), + watch_medium: None, + genres: vec!["Drama".to_string()], + }) + .collect(); + let plus_two = FixedOffset::east_opt(2 * 3600).unwrap(); + assert!(!kinds(&build_insights(&rows, &plus_two)).contains(&"rhythm")); +} + #[test] fn rhythm_skipped_when_timestamps_are_import_noise() { let mut rows: Vec = Vec::new(); @@ -320,14 +390,14 @@ fn rhythm_skipped_when_timestamps_are_import_noise() { rows.push(row); } - assert!(!kinds(&build_insights(&rows)).contains(&"rhythm")); + assert!(!kinds(&build_insights(&rows, &Utc)).contains(&"rhythm")); } // ── Overall ────────────────────────────────────────────────────────────────── #[test] fn empty_diary_yields_nothing() { - assert!(build_insights(&[]).is_empty()); + assert!(build_insights(&[], &Utc).is_empty()); } #[test] @@ -335,7 +405,7 @@ fn small_diary_yields_nothing() { let rows: Vec = (0..4) .map(|i| watch(&["Drama"], 4, date(2024, 1, 1 + i), 20)) .collect(); - assert!(build_insights(&rows).is_empty()); + assert!(build_insights(&rows, &Utc).is_empty()); } #[test] @@ -356,7 +426,7 @@ fn max_three_insights_per_category() { // Baseline dramas rated 1 make every lift large. rows.extend((0..10).map(|i| watch(&["Drama"], 1, date(2024, 3, 1 + i), 20))); - let insights = build_insights(&rows); + let insights = build_insights(&rows, &Utc); let rating_lifts = insights .iter() .filter(|i| matches!(i, Insight::RatingLift { .. })) diff --git a/crates/presentation/Cargo.toml b/crates/presentation/Cargo.toml index f145fdaa..56d14455 100644 --- a/crates/presentation/Cargo.toml +++ b/crates/presentation/Cargo.toml @@ -26,6 +26,7 @@ tokio = { workspace = true } dotenvy = { workspace = true } uuid = { workspace = true } chrono = { workspace = true } +chrono-tz = { workspace = true } async-trait = { workspace = true } futures = { workspace = true } diff --git a/crates/presentation/src/handlers/diary.rs b/crates/presentation/src/handlers/diary.rs index 1ac9bbea..ad7d09f5 100644 --- a/crates/presentation/src/handlers/diary.rs +++ b/crates/presentation/src/handlers/diary.rs @@ -26,7 +26,7 @@ use api_types::{ }; use template_askama::{ActivityFeedTemplate, NewReviewTemplate, build_page_items}; -use super::helpers::{build_export_response, build_page_context, encode_error}; +use super::helpers::{build_export_response, build_page_context, encode_domain_error}; // ── API ────────────────────────────────────────────────────────────────────── @@ -238,7 +238,7 @@ pub async fn post_review_html( { Ok(_) => Redirect::to("/").into_response(), Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!("/reviews/new?error={}", msg)).into_response() } } diff --git a/crates/presentation/src/handlers/helpers.rs b/crates/presentation/src/handlers/helpers.rs index 5c73a8d9..7a0daf06 100644 --- a/crates/presentation/src/handlers/helpers.rs +++ b/crates/presentation/src/handlers/helpers.rs @@ -8,6 +8,19 @@ pub(crate) fn encode_error(msg: &str) -> String { utf8_percent_encode(msg, NON_ALPHANUMERIC).to_string() } +/// Error text for a classic-page `?error=` redirect. Infrastructure failures +/// are logged here — the redirect is their only exit, so nothing else would — +/// and shown to the user generically, never with internal detail. +pub(crate) fn encode_domain_error(e: &domain::errors::DomainError) -> String { + match e { + domain::errors::DomainError::InfrastructureError(_) => { + tracing::error!("Internal error: {e}"); + encode_error("Something went wrong, please try again") + } + _ => encode_error(&e.to_string()), + } +} + pub(crate) fn build_export_response( format_str: &str, user_id: uuid::Uuid, diff --git a/crates/presentation/src/handlers/import.rs b/crates/presentation/src/handlers/import.rs index 452a6d94..33061e0f 100644 --- a/crates/presentation/src/handlers/import.rs +++ b/crates/presentation/src/handlers/import.rs @@ -40,7 +40,7 @@ use crate::{ state::AppState, }; -use super::helpers::encode_error; +use super::helpers::encode_domain_error; fn str_to_domain_field(field: &str) -> Option { match field { @@ -171,7 +171,7 @@ pub async fn post_upload( { Ok(r) => Redirect::to(&format!("/import/{}/mapping", r.session_id.value())).into_response(), Err(e) => { - Redirect::to(&format!("/import?error={}", encode_error(&e.to_string()))).into_response() + Redirect::to(&format!("/import?error={}", encode_domain_error(&e))).into_response() } } } @@ -258,7 +258,7 @@ pub async fn post_mapping( Err(e) => Redirect::to(&format!( "/import/{}/mapping?error={}", session_id_str, - encode_error(&e.to_string()) + encode_domain_error(&e) )) .into_response(), } @@ -375,7 +375,7 @@ pub async fn post_confirm( )) .into_response(), Err(e) => { - Redirect::to(&format!("/import?error={}", encode_error(&e.to_string()))).into_response() + Redirect::to(&format!("/import?error={}", encode_domain_error(&e))).into_response() } } } diff --git a/crates/presentation/src/handlers/insights.rs b/crates/presentation/src/handlers/insights.rs index 1403ba4f..c71ccad8 100644 --- a/crates/presentation/src/handlers/insights.rs +++ b/crates/presentation/src/handlers/insights.rs @@ -1,10 +1,13 @@ -use axum::{Json, extract::State}; +use axum::{ + Json, + extract::{Query, State}, +}; use application::insights::get as get_insights_uc; -use domain::models::Insight; +use domain::{errors::DomainError, models::Insight}; use crate::{errors::ApiError, extractors::AuthenticatedUser, state::AppState}; -use api_types::insights::{InsightDto, InsightsResponse}; +use api_types::insights::{InsightDto, InsightsQueryParams, InsightsResponse}; fn to_dto(insight: Insight) -> InsightDto { match insight { @@ -64,8 +67,10 @@ fn to_dto(insight: Insight) -> InsightDto { /// statistics already public on profiles (ADR-0010). #[utoipa::path( get, path = "/api/v1/insights", + params(InsightsQueryParams), responses( (status = 200, body = InsightsResponse), + (status = 400, description = "Unknown time zone"), (status = 401, description = "Unauthorized"), ), security(("bearer_auth" = [])) @@ -73,8 +78,17 @@ fn to_dto(insight: Insight) -> InsightDto { pub async fn get_insights( State(state): State, user: AuthenticatedUser, + Query(params): Query, ) -> Result, ApiError> { - let insights = get_insights_uc::execute(&state.app_ctx.deps.insights.get, user.0).await?; + let tz: chrono_tz::Tz = match params.tz.as_deref() { + None => chrono_tz::UTC, + Some(name) => name.parse().map_err(|_| { + ApiError(DomainError::ValidationError(format!( + "unknown time zone: {name}" + ))) + })?, + }; + let insights = get_insights_uc::execute(&state.app_ctx.deps.insights.get, user.0, &tz).await?; Ok(Json(InsightsResponse { insights: insights.into_iter().map(to_dto).collect(), })) diff --git a/crates/presentation/src/handlers/integrations.rs b/crates/presentation/src/handlers/integrations.rs index 4ffdcce1..6325461b 100644 --- a/crates/presentation/src/handlers/integrations.rs +++ b/crates/presentation/src/handlers/integrations.rs @@ -24,7 +24,7 @@ use crate::{ }; use template_askama::{IntegrationsTemplate, WatchQueueTemplate}; -use super::helpers::{build_page_context, encode_error}; +use super::helpers::{build_page_context, encode_domain_error}; // ── HTML ───────────────────────────────────────────────────────────────────── @@ -184,7 +184,7 @@ pub async fn post_confirm_single( { Ok(_) => Redirect::to("/watch-queue").into_response(), Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!("/watch-queue?error={msg}")).into_response() } } @@ -211,7 +211,7 @@ pub async fn post_dismiss_single( { Ok(_) => Redirect::to("/watch-queue").into_response(), Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!("/watch-queue?error={msg}")).into_response() } } diff --git a/crates/presentation/src/handlers/social.rs b/crates/presentation/src/handlers/social.rs index 3b257747..cf978a89 100644 --- a/crates/presentation/src/handlers/social.rs +++ b/crates/presentation/src/handlers/social.rs @@ -30,7 +30,7 @@ use domain::value_objects::{FollowTarget, InstanceIdentity, SocialActor, SocialI use template_askama::{BlockedActorsTemplate, BlockedDomainsTemplate}; use template_askama::{FollowersTemplate, FollowingTemplate, RemoteActorData}; -use super::helpers::{build_page_context, encode_error}; +use super::helpers::{build_page_context, encode_domain_error}; fn social_actor_to_dto(actor: SocialActor, instance: &InstanceIdentity) -> RemoteActorDto { RemoteActorDto { @@ -576,7 +576,7 @@ pub async fn follow_user( Ok(()) => Redirect::to(&redirect_base).into_response(), Err(e) => { tracing::error!("follow error: {:?}", e); - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); let sep = if redirect_base.contains('?') { '&' } else { @@ -614,7 +614,7 @@ pub async fn unfollow_user( Redirect::to(&format!("/users/{}/following-list", profile_user_uuid)).into_response() } Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!( "/users/{}/following-list?error={}", profile_user_uuid, msg @@ -649,7 +649,7 @@ pub async fn accept_follower_html( { Ok(_) => Redirect::to(&format!("/users/{}", profile_user_uuid)).into_response(), Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!("/users/{}?error={}", profile_user_uuid, msg)).into_response() } } @@ -680,7 +680,7 @@ pub async fn reject_follower_html( { Ok(_) => Redirect::to(&format!("/users/{}", profile_user_uuid)).into_response(), Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!("/users/{}?error={}", profile_user_uuid, msg)).into_response() } } @@ -896,7 +896,7 @@ pub async fn remove_follower_html( Redirect::to(&format!("/users/{}/followers-list", profile_user_uuid)).into_response() } Err(e) => { - let msg = encode_error(&e.to_string()); + let msg = encode_domain_error(&e); Redirect::to(&format!( "/users/{}/followers-list?error={}", profile_user_uuid, msg diff --git a/crates/presentation/src/handlers/users.rs b/crates/presentation/src/handlers/users.rs index e9d22b77..ba5cc638 100644 --- a/crates/presentation/src/handlers/users.rs +++ b/crates/presentation/src/handlers/users.rs @@ -829,7 +829,7 @@ pub async fn post_profile_settings( tracing::error!("update_profile error: {:?}", e); return axum::response::Redirect::to(&format!( "/settings/profile?error={}", - super::helpers::encode_error(&e.to_string()) + super::helpers::encode_domain_error(&e) )) .into_response(); } @@ -860,7 +860,7 @@ pub async fn post_profile_settings( tracing::error!("update_profile_fields error: {:?}", e); return axum::response::Redirect::to(&format!( "/settings/profile?error={}", - super::helpers::encode_error(&e.to_string()) + super::helpers::encode_domain_error(&e) )) .into_response(); } diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index a4f2eabd..5d73a0f7 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -313,9 +313,23 @@ async fn create_event_publisher( fn init_tracing() { tracing_subscriber::registry() - .with(tracing_subscriber::EnvFilter::new( - std::env::var("RUST_LOG").unwrap_or_else(|_| "server=debug,tower_http=debug".into()), - )) + .with(log_filter("server=debug,tower_http=debug")) .with(tracing_subscriber::fmt::layer()) .init(); } + +/// `RUST_LOG` names the crates to trace in detail; anything it doesn't name +/// still reports warnings and errors. Without a baseline, a filter like +/// `presentation=debug` silently drops adapter errors (e.g. the sqlx detail +/// behind "Database operation failed"). +fn log_filter(default: &str) -> tracing_subscriber::EnvFilter { + let directives = std::env::var("RUST_LOG").unwrap_or_else(|_| default.to_string()); + let has_baseline = directives + .split(',') + .any(|d| !d.trim().is_empty() && !d.contains('=')); + if has_baseline { + tracing_subscriber::EnvFilter::new(directives) + } else { + tracing_subscriber::EnvFilter::new(format!("warn,{directives}")) + } +} diff --git a/crates/server/tests/api_test.rs b/crates/server/tests/api_test.rs index 40ab7fbd..a8698c22 100644 --- a/crates/server/tests/api_test.rs +++ b/crates/server/tests/api_test.rs @@ -744,3 +744,103 @@ async fn get_api_insights_returns_rating_lift_for_own_user() { assert_eq!(rhythm["peak_hour_end"], 14); assert_eq!(rhythm["top_days"], serde_json::json!([5, 7])); } + +async fn get_insights_json( + app: Router, + user_id: Uuid, + query: &str, +) -> (StatusCode, serde_json::Value) { + let response = app + .oneshot(with_ip( + Request::builder() + .uri(format!("/api/v1/insights{query}")) + .header("authorization", format!("Bearer {user_id}")) + .body(Body::empty()) + .unwrap(), + )) + .await + .unwrap(); + let status = response.status(); + let bytes = response.into_body().collect().await.unwrap().to_bytes(); + let json = serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null); + (status, json) +} + +#[tokio::test] +async fn get_api_insights_ignores_remote_reviews() { + let (app, pool) = test_app_with(domain::testing::FakeAuthService).await; + let user_id = Uuid::new_v4(); + seed_insights_diary(&pool, user_id).await; + // Federated rows stored under the same user_id are someone else's watches. + for i in 0..12u32 { + let movie_id = uuid::Uuid::new_v4().to_string(); + sqlx::query("INSERT INTO movies (id, title, release_year) VALUES (?, ?, ?)") + .bind(&movie_id) + .bind(format!("Horror {i}")) + .bind(2019) + .execute(&pool) + .await + .unwrap(); + sqlx::query("INSERT INTO movie_genres (movie_id, tmdb_id, name) VALUES (?, ?, ?)") + .bind(&movie_id) + .bind(66600 + i as i64) + .bind("Horror") + .execute(&pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO reviews (id, movie_id, user_id, rating, watched_at, created_at, remote_actor_url) \ + VALUES (?, ?, ?, ?, ?, ?, ?)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(&movie_id) + .bind(user_id.to_string()) + .bind(5) + .bind("2024-05-05 09:00:00") + .bind("2024-05-05 09:00:00") + .bind("https://remote.example/users/someone") + .execute(&pool) + .await + .unwrap(); + } + + let (status, json) = get_insights_json(app, user_id, "").await; + assert_eq!(status, StatusCode::OK); + let insights = json["insights"].as_array().unwrap(); + assert!( + !insights.iter().any(|i| i["genre"] == "Horror"), + "remote reviews must not feed insights: {json}" + ); + let thriller = insights + .iter() + .find(|i| i["kind"] == "rating_lift" && i["genre"] == "Thriller") + .unwrap(); + assert_eq!(thriller["delta"], 1.5); +} + +#[tokio::test] +async fn get_api_insights_buckets_rhythm_in_requested_time_zone() { + let (app, pool) = test_app_with(domain::testing::FakeAuthService).await; + let user_id = Uuid::new_v4(); + seed_insights_diary(&pool, user_id).await; + + // January in Warsaw is UTC+1: 22:30 → 23:30 and 13:00 → 14:00, so the + // earliest tied window moves from 11–14 to 12–15. + let (status, json) = get_insights_json(app, user_id, "?tz=Europe/Warsaw").await; + assert_eq!(status, StatusCode::OK); + let rhythm = json["insights"] + .as_array() + .unwrap() + .iter() + .find(|i| i["kind"] == "rhythm") + .expect("rhythm expected"); + assert_eq!(rhythm["peak_hour_start"], 12); + assert_eq!(rhythm["peak_hour_end"], 15); +} + +#[tokio::test] +async fn get_api_insights_rejects_unknown_time_zone() { + let (app, _pool) = test_app_with(domain::testing::FakeAuthService).await; + let (status, _) = get_insights_json(app, Uuid::new_v4(), "?tz=Mars/Olympus").await; + assert_eq!(status, StatusCode::BAD_REQUEST); +} diff --git a/crates/worker/src/main.rs b/crates/worker/src/main.rs index 7e70ade6..867b309f 100644 --- a/crates/worker/src/main.rs +++ b/crates/worker/src/main.rs @@ -307,10 +307,24 @@ async fn main() -> anyhow::Result<()> { } fn init_tracing() { - let filter = std::env::var("RUST_LOG") - .unwrap_or_else(|_| "worker=info,application=info,k_ap=info".to_string()); tracing_subscriber::registry() - .with(tracing_subscriber::EnvFilter::new(filter)) + .with(log_filter("worker=info,application=info,k_ap=info")) .with(tracing_subscriber::fmt::layer()) .init(); } + +/// `RUST_LOG` names the crates to trace in detail; anything it doesn't name +/// still reports warnings and errors. Without a baseline, a filter like +/// `presentation=debug` silently drops adapter errors (e.g. the sqlx detail +/// behind "Database operation failed"). +fn log_filter(default: &str) -> tracing_subscriber::EnvFilter { + let directives = std::env::var("RUST_LOG").unwrap_or_else(|_| default.to_string()); + let has_baseline = directives + .split(',') + .any(|d| !d.trim().is_empty() && !d.contains('=')); + if has_baseline { + tracing_subscriber::EnvFilter::new(directives) + } else { + tracing_subscriber::EnvFilter::new(format!("warn,{directives}")) + } +} diff --git a/spa/src/components/review-form-fields.tsx b/spa/src/components/review-form-fields.tsx index 0d6f846a..2a05e07c 100644 --- a/spa/src/components/review-form-fields.tsx +++ b/spa/src/components/review-form-fields.tsx @@ -8,6 +8,7 @@ import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover import { Calendar } from "@/components/ui/calendar" import { StarRating } from "@/components/star-rating" import { WatchMediumPicker } from "@/components/watch-medium-picker" +import { withPickedDay } from "@/lib/date" type ReviewFormFieldsProps = { rating: number @@ -55,7 +56,7 @@ export function ReviewFormFields({ mode="single" fixedWeeks selected={watchedAt} - onSelect={(d) => d && onWatchedAtChange(d)} + onSelect={(d) => d && onWatchedAtChange(withPickedDay(watchedAt, d))} disabled={(d) => d > new Date()} autoFocus /> diff --git a/spa/src/components/review-sheet.tsx b/spa/src/components/review-sheet.tsx index 6e7c3585..cd397000 100644 --- a/spa/src/components/review-sheet.tsx +++ b/spa/src/components/review-sheet.tsx @@ -12,7 +12,7 @@ import { Trash2 } from "lucide-react" import { toast } from "sonner" import { posterUrl } from "@/lib/api/client" import { hapticMedium } from "@/lib/haptics" -import { parseLocalDate, formatLocalDateTime } from "@/lib/date" +import { formatWatchedAt, isDateOnly, parseWatchedAt } from "@/lib/date" import type { EditReviewRequest } from "@/features/diary" import type { MovieDto, ReviewDto } from "@/lib/api/common" @@ -77,7 +77,7 @@ function LogMode({ open, onOpenChange }: { open: boolean; onOpenChange: (open: b manual_director: movie.director, rating, comment: comment || undefined, - watched_at: watchedAt.toISOString().replace("Z", "").split(".")[0]!, + watched_at: formatWatchedAt(watchedAt), watch_medium: watchMedium, }, { @@ -145,7 +145,7 @@ function EditMode({ const { t } = useTranslation() const [rating, setRating] = useState(review.rating) const [comment, setComment] = useState(review.comment ?? "") - const [watchedAt, setWatchedAt] = useState(() => parseLocalDate(review.watched_at)) + const [watchedAt, setWatchedAt] = useState(() => parseWatchedAt(review.watched_at)) const [dateChanged, setDateChanged] = useState(false) const [watchMedium, setWatchMedium] = useState(review.watch_medium) const [confirmDelete, setConfirmDelete] = useState(false) @@ -164,7 +164,7 @@ function EditMode({ if (rating !== review.rating) data.rating = rating const newComment = comment || null if (newComment !== (review.comment ?? null)) data.comment = newComment - if (dateChanged) data.watched_at = formatLocalDateTime(watchedAt) + if (dateChanged) data.watched_at = formatWatchedAt(watchedAt, isDateOnly(review.watched_at)) if (watchMedium !== review.watch_medium) data.watch_medium = watchMedium ?? null if (Object.keys(data).length === 0) { diff --git a/spa/src/features/insights.ts b/spa/src/features/insights.ts index 66d25bab..73329bf5 100644 --- a/spa/src/features/insights.ts +++ b/spa/src/features/insights.ts @@ -47,15 +47,23 @@ export type InsightsResponse = { export const insightsKeys = { all: ["insights"] as const, + forZone: (tz: string) => [...insightsKeys.all, tz] as const, } -function getInsights() { - return get("/insights") +// Months, weekdays and hours are bucketed server-side in this zone, so the +// rhythm card's hours read as the viewer's local time. +function browserTimeZone(): string { + return Intl.DateTimeFormat().resolvedOptions().timeZone +} + +function getInsights(tz: string) { + return get("/insights", { tz }) } export function useInsights() { + const tz = browserTimeZone() return useQuery({ - queryKey: insightsKeys.all, - queryFn: getInsights, + queryKey: insightsKeys.forZone(tz), + queryFn: () => getInsights(tz), }) } diff --git a/spa/src/lib/date.ts b/spa/src/lib/date.ts index f239e202..7572764d 100644 --- a/spa/src/lib/date.ts +++ b/spa/src/lib/date.ts @@ -18,18 +18,34 @@ export function shortDate(dateStr: string): string { } } -export function parseLocalDate(s: string): Date { - const [datePart, timePart] = s.split("T") - if (!datePart) return new Date() - const [y, m, d] = datePart.split("-").map(Number) - if (timePart) { - const [h, min, sec] = timePart.split(":").map(Number) - return new Date(y!, m! - 1, d!, h, min, sec) - } - return new Date(y!, m! - 1, d!) +// `watched_at` is stored as naive UTC ("YYYY-MM-DDTHH:MM:SS", no zone). +// Date-only imports carry no time and are stored at exactly 00:00:00; those +// are calendar dates, not instants, and must not be shifted between zones. + +export function isDateOnly(s: string): boolean { + return /^\d{4}-\d{2}-\d{2}([T ]00:00(:00)?)?$/.test(s) } -export function formatLocalDateTime(d: Date): string { - const pad = (n: number) => n.toString().padStart(2, "0") - return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}` +export function parseWatchedAt(s: string): Date { + const [y, m, d] = s.slice(0, 10).split("-").map(Number) + if (isDateOnly(s)) return new Date(y!, m! - 1, d!) + const parsed = new Date(`${s.replace(" ", "T")}Z`) + return Number.isNaN(parsed.getTime()) ? new Date() : parsed +} + +export function formatWatchedAt(d: Date, dateOnly = false): string { + const pad = (n: number) => n.toString().padStart(2, "0") + if (dateOnly) { + return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T00:00:00` + } + return d.toISOString().split(".")[0]! +} + +// Swap the calendar day of `current` for `picked` (the date picker yields local +// midnight), keeping the time of day so the stored UTC instant stays on the +// chosen day. +export function withPickedDay(current: Date, picked: Date): Date { + const next = new Date(current) + next.setFullYear(picked.getFullYear(), picked.getMonth(), picked.getDate()) + return next } diff --git a/spa/src/locales/en.json b/spa/src/locales/en.json index 385b36cc..da95f0c8 100644 --- a/spa/src/locales/en.json +++ b/spa/src/locales/en.json @@ -275,7 +275,10 @@ "weekends": "weekends", "yourBaseline": "your baseline", "rhythmBody": "You watch mostly {{start}}–{{end}} on {{days}}", - "filmsSample": "{{count}} films" + "filmsSample_one": "{{count}} film", + "filmsSample_other": "{{count}} films", + "watchesSample_one": "{{count}} watch", + "watchesSample_other": "{{count}} watches" }, "wrapup": { "title": "Year Wrap-Up", diff --git a/spa/src/routes/_app/insights.tsx b/spa/src/routes/_app/insights.tsx index 2584fcee..7eafd9c2 100644 --- a/spa/src/routes/_app/insights.tsx +++ b/spa/src/routes/_app/insights.tsx @@ -2,6 +2,7 @@ import { createFileRoute } from "@tanstack/react-router" import type { ReactNode } from "react" import { Trans, useTranslation } from "react-i18next" import { + AlertTriangle, CalendarDays, Clapperboard, Clock, @@ -12,6 +13,7 @@ import { } from "lucide-react" import { BackButton } from "@/components/back-button" import { Badge } from "@/components/ui/badge" +import { Button } from "@/components/ui/button" import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card" import { EmptyState } from "@/components/empty-state" import { Skeleton } from "@/components/ui/skeleton" @@ -54,7 +56,7 @@ function hourLabel(hour: number): string { function InsightsPage() { const { t } = useTranslation() - const { data, isPending } = useInsights() + const { data, isPending, isError, refetch } = useInsights() useDocumentTitle(t("insights.title")) if (isPending) return @@ -72,7 +74,14 @@ function InsightsPage() {

{t("insights.subtitle")}

- {insights.length === 0 ? ( + {isError ? ( +
+ + +
+ ) : insights.length === 0 ? ( {t("insights.yourAverage")}: {insight.user_avg.toFixed(1)}★ - + @@ -193,7 +202,7 @@ function SeasonalityCard({ insight }: SeasonalityCardProps) { {insight.multiplier.toFixed(1)}× · {monthName(insight.month)} {insight.genre} - + @@ -227,7 +236,7 @@ function MediumTimingCard({ insight }: MediumTimingCardProps) { {t("insights.yourBaseline")}: {Math.round(insight.baseline_share * 100)}% - + @@ -259,16 +268,19 @@ function RhythmCard({ insight }: RhythmCardProps) { {hourLabel(insight.peak_hour_start)}–{hourLabel(insight.peak_hour_end)} {dayNames(insight.top_days).join(", ")} - + ) } -function SampleBadge({ count }: { count: number }) { +// Rating lift counts distinct films (first watches only); every other +// category counts diary entries, rewatches included. +function SampleBadge({ unit, count }: { unit: "films" | "watches"; count: number }) { const { t } = useTranslation() - return {t("insights.filmsSample", { count })} + const key = unit === "films" ? "insights.filmsSample" : "insights.watchesSample" + return {t(key, { count })} } function InsightsSkeleton() {