diff --git a/.sqlx/query-3a02f30ced2765dcdbf7575d680c1b4ab1db895065f65db4a99bb0a4dc6e71ed.json b/.sqlx/query-3a02f30ced2765dcdbf7575d680c1b4ab1db895065f65db4a99bb0a4dc6e71ed.json new file mode 100644 index 0000000..207dad9 --- /dev/null +++ b/.sqlx/query-3a02f30ced2765dcdbf7575d680c1b4ab1db895065f65db4a99bb0a4dc6e71ed.json @@ -0,0 +1,86 @@ +{ + "db_name": "SQLite", + "query": "SELECT se.series_id AS \"series_id!: i64\", e.id AS \"id!: i64\", e.season_id AS \"season_id!: i64\", se.number AS \"season_number!: i64\", e.number AS \"number!: i64\", e.title AS \"title!: String\", e.air_date, e.wanted AS \"wanted!: bool\", e.state AS \"state!: String\", e.vanished AS \"vanished!: bool\", e.search_attempts AS \"search_attempts!: i64\", e.last_searched_at\n FROM episodes e JOIN seasons se ON se.id = e.season_id WHERE e.season_id = ?", + "describe": { + "columns": [ + { + "name": "series_id!: i64", + "ordinal": 0, + "type_info": "Integer" + }, + { + "name": "id!: i64", + "ordinal": 1, + "type_info": "Integer" + }, + { + "name": "season_id!: i64", + "ordinal": 2, + "type_info": "Integer" + }, + { + "name": "season_number!: i64", + "ordinal": 3, + "type_info": "Integer" + }, + { + "name": "number!: i64", + "ordinal": 4, + "type_info": "Integer" + }, + { + "name": "title!: String", + "ordinal": 5, + "type_info": "Text" + }, + { + "name": "air_date", + "ordinal": 6, + "type_info": "Text" + }, + { + "name": "wanted!: bool", + "ordinal": 7, + "type_info": "Integer" + }, + { + "name": "state!: String", + "ordinal": 8, + "type_info": "Text" + }, + { + "name": "vanished!: bool", + "ordinal": 9, + "type_info": "Integer" + }, + { + "name": "search_attempts!: i64", + "ordinal": 10, + "type_info": "Integer" + }, + { + "name": "last_searched_at", + "ordinal": 11, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + true, + false, + false, + false, + false, + true, + false, + false, + false, + false, + true + ] + }, + "hash": "3a02f30ced2765dcdbf7575d680c1b4ab1db895065f65db4a99bb0a4dc6e71ed" +} diff --git a/crates/arr-api/src/series.rs b/crates/arr-api/src/series.rs index bace102..632799c 100644 --- a/crates/arr-api/src/series.rs +++ b/crates/arr-api/src/series.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use arr_core::tracking::{apply_auto_track, RefreshedSeason}; +use arr_core::tracking::{apply_auto_track, apply_tracked, RefreshedSeason}; use arr_core::{ derive_series_status, EpisodeId, Language, MediaState, RootId, SeasonId, SeriesId, SeriesStatus, TitleOverrides, @@ -153,9 +153,11 @@ pub struct CreateEpisode { /// Both fields of a season a person can set by hand. /// /// Turning `tracked` on marks every already-revealed episode wanted, and -/// episodes revealed later follow while it stays on; turning it off withdraws -/// nothing (§4.1). `wanted` is the one-click "grab this season", and writes -/// intent onto every episode already in it — the two are deliberately +/// episodes revealed later follow while it stays on; turning it off clears +/// `wanted` on all of them (§4.1, #171) — idempotently, so re-sending +/// `tracked: false` to an already-untracked season still clears its +/// stranded wanted rows. `wanted` is the one-click "grab this season", and +/// writes intent onto every episode already in it — the two are deliberately /// separate (§4.1). #[derive(Debug, Deserialize, ToSchema)] pub struct UpdateSeason { @@ -870,24 +872,40 @@ pub async fn update_season( .ok_or(ApiError::SeasonNotFound)?; let season_id = season.id; + // One transaction: the flag write and the episode intent it implies + // land together or not at all (#171). + let mut transaction = pool(&state)?.begin().await?; + if let Some(tracked) = input.tracked { sqlx::query!( "UPDATE seasons SET tracked = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", tracked, season_id ) - .execute(pool(&state)?) + .execute(&mut *transaction) .await?; // §4.1, as `arr_core::tracking::apply_tracked` decides it: turning - // tracking on marks every already-revealed episode wanted; turning it - // off withdraws nothing. Only the off -> on transition writes intent. - if tracked && !season.tracked { + // tracking on marks every already-revealed episode wanted; turning + // it off clears them all, season 0 included. The rule runs even when + // the flag already holds this value (#171): re-sending `false` to an + // untracked season is how stranded wanted rows get cleared. + let rows = sqlx::query_as!( + EpisodeRow, + r#"SELECT se.series_id AS "series_id!: i64", e.id AS "id!: i64", e.season_id AS "season_id!: i64", se.number AS "season_number!: i64", e.number AS "number!: i64", e.title AS "title!: String", e.air_date, e.wanted AS "wanted!: bool", e.state AS "state!: String", e.vanished AS "vanished!: bool", e.search_attempts AS "search_attempts!: i64", e.last_searched_at + FROM episodes e JOIN seasons se ON se.id = e.season_id WHERE e.season_id = ?"#, + season_id + ) + .fetch_all(&mut *transaction) + .await?; + let mut episodes: Vec<_> = rows.iter().map(core_episode).collect(); + apply_tracked(tracked, &mut episodes); + for episode in &episodes { sqlx::query!( - "UPDATE episodes SET wanted = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE season_id = ?", - true, - season_id + "UPDATE episodes SET wanted = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", + episode.wanted, + episode.id.0 ) - .execute(pool(&state)?) + .execute(&mut *transaction) .await?; } } @@ -899,9 +917,10 @@ pub async fn update_season( wanted, season_id ) - .execute(pool(&state)?) + .execute(&mut *transaction) .await?; } + transaction.commit().await?; let seasons = load_seasons(&state, series_id).await?; seasons @@ -1768,8 +1787,12 @@ mod tests { assert_eq!(episode["wanted"], false); } + /// #171. The §4.1 rule runs whenever the flag is sent: off clears + /// wanted — including on a season that was already untracked, which is + /// how rows stranded by the old ruling get out — and on marks every + /// revealed episode wanted. #[tokio::test] - async fn tracking_a_season_marks_revealed_episodes_and_off_withdraws_nothing() { + async fn untracking_a_season_clears_its_episodes_wanted() { async fn patch_season( base: &str, series_id: i64, @@ -1803,6 +1826,26 @@ mod tests { assert_eq!(season["tracked"], false); assert_eq!(season["episodes"][0]["wanted"], false); + // A season stranded by the old ruling: untracked, yet its episodes + // are all wanted. Re-sending the flag it already holds must still + // do the work (#171). + let updated = patch_season(&base, series_id, serde_json::json!({"wanted": true})).await; + assert!(updated["episodes"] + .as_array() + .expect("episodes") + .iter() + .all(|episode| episode["wanted"] == true)); + let updated = patch_season(&base, series_id, serde_json::json!({"tracked": false})).await; + assert_eq!(updated["tracked"], false); + assert!( + updated["episodes"] + .as_array() + .expect("episodes") + .iter() + .all(|episode| episode["wanted"] == false), + "#171: clearing an already-untracked season is the way out" + ); + let updated = patch_season(&base, series_id, serde_json::json!({"tracked": true})).await; assert_eq!(updated["tracked"], true); assert!( @@ -1814,7 +1857,6 @@ mod tests { "§4.1: turning tracked on marks every revealed episode wanted" ); - // Turning it off withdraws nothing. let updated = patch_season(&base, series_id, serde_json::json!({"tracked": false})).await; assert_eq!(updated["tracked"], false); assert!( @@ -1822,11 +1864,31 @@ mod tests { .as_array() .expect("episodes") .iter() - .all(|episode| episode["wanted"] == true), - "§4.1: leaf intent is never removed implicitly" + .all(|episode| episode["wanted"] == false), + "§4.1: turning tracked off clears every revealed episode" ); } + /// #171. Untracking a season with no episodes touches nothing and is not + /// an error. + #[tokio::test] + async fn untracking_an_empty_season_is_a_no_op() { + let (_dir, state, base) = application().await; + let root_id = tv_root(&state, "main").await; + let series = add_series(&base, root_id, false).await; + let series_id = series["id"].as_i64().expect("id"); + let season = add_season(&base, series_id, 1, serde_json::json!([])).await; + assert_eq!(season["tracked"], false); + + let response = reqwest::Client::new() + .patch(format!("{base}/api/series/{series_id}/seasons/1")) + .json(&serde_json::json!({"tracked": false})) + .send() + .await + .expect("update season"); + assert_eq!(response.status(), StatusCode::OK); + } + #[tokio::test] async fn listed_series_carry_a_derived_status() { let (_dir, state, base) = application().await; diff --git a/crates/arr-core/src/tracking.rs b/crates/arr-core/src/tracking.rs index e6513ec..9370828 100644 --- a/crates/arr-core/src/tracking.rs +++ b/crates/arr-core/src/tracking.rs @@ -38,14 +38,16 @@ pub fn apply_auto_track(series: &Series, ever_refreshed: bool, seasons: &mut [Re /// Applies a season's tracking rule (`DESIGN.md` §4.1) to its /// already-revealed episodes. /// -/// Turning tracking on marks every revealed episode wanted; future reveals -/// follow while the season stays tracked. Turning it off withdraws nothing: -/// leaf intent is never removed implicitly. +/// Turning tracking on marks every revealed episode wanted, and future +/// reveals follow while the season stays tracked. Turning it off clears +/// `wanted` on all of them: toggling the flag is itself the explicit act, +/// and the product offers no other way to withdraw a season's worth of +/// intent. The rule is idempotent and must run even when the flag already +/// has the value asked for (#171) — an operator re-sending `tracked: false` +/// to an already-untracked season is clearing stranded intent. pub fn apply_tracked(tracked: bool, episodes: &mut [Episode]) { - if tracked { - for episode in episodes { - episode.wanted = true; - } + for episode in episodes { + episode.wanted = tracked; } } @@ -216,8 +218,12 @@ mod tests { } } + /// #171. Toggling the flag is the explicit act, so `false` clears + /// wanted on every revealed episode — including when it is re-sent to a + /// season that was already untracked, which is the only way out for + /// rows stranded by the old ruling. #[test] - fn tracked_off_withdraws_nothing() { + fn tracked_off_clears_revealed_episodes_wanted() { let cases: [&[bool]; 4] = [ &[], &[false], @@ -236,18 +242,13 @@ mod tests { ) }) .collect::>(); - let before = episodes - .iter() - .map(|episode| episode.wanted) - .collect::>(); apply_tracked(false, &mut episodes); - let after = episodes - .iter() - .map(|episode| episode.wanted) - .collect::>(); - assert_eq!(before, after, "tracked off must not touch intent"); + assert!( + episodes.iter().all(|episode| !episode.wanted), + "tracked off must clear every revealed episode, input {wanted:?}" + ); } } }