From a41188ee5cae70395e850a9d9d13b4ef5cb8a997 Mon Sep 17 00:00:00 2001 From: Lachezar Lechev Date: Mon, 14 Aug 2023 15:34:57 +0300 Subject: [PATCH] feat: use last_video_released for notifications: - player - update on switch_to_next_video - notifs - update on dismiss and pull Signed-off-by: Lachezar Lechev --- src/models/ctx/update_notifications.rs | 102 +++++++++++++++--- src/models/player.rs | 19 +++- .../ctx/notifications/update_notifications.rs | 44 +++++--- 3 files changed, 132 insertions(+), 33 deletions(-) diff --git a/src/models/ctx/update_notifications.rs b/src/models/ctx/update_notifications.rs index ac85dc451..ac53f17e8 100644 --- a/src/models/ctx/update_notifications.rs +++ b/src/models/ctx/update_notifications.rs @@ -19,7 +19,7 @@ use crate::{ }, types::{ addon::{AggrRequest, ExtraValue}, - library::LibraryBucket, + library::{LibraryBucket, LibraryItem}, notifications::{NotificationItem, NotificationsBucket}, profile::Profile, resource::{MetaItem, MetaItemId, VideoId}, @@ -136,7 +136,7 @@ pub fn update_notifications( .join(notifications_effects) } Msg::Internal(Internal::DismissNotificationItem(id)) => { - dismiss_notification_item::(library, notifications, id) + dismiss_notification_item(library, notifications, id) } Msg::Internal(Internal::NotificationsChanged) => { Effects::one(push_notifications_to_storage::(notifications)).unchanged() @@ -171,6 +171,7 @@ fn update_notification_items( .map(|extra_value| Either::Left(extra_value.value.split(','))) .unwrap_or_else(|| Either::Right(std::iter::empty())); + let mut library_items_effects = vec![]; let next_notification_items = next_notification_ids.fold(HashMap::new(), |mut map, meta_id| { // Get the LibraryItem from user's library // Exit early if library item does not exist in the Library @@ -202,9 +203,9 @@ fn update_notification_items( meta_item .videos_iter() .filter_map(|video| { - match (&library_item.state.last_watched, video.released) { - (Some(last_watched), Some(video_released)) => { - if last_watched < &video_released && + match (&library_item.state.last_video_released, video.released) { + (Some(last_video_released), Some(video_released)) => { + if last_video_released < &video_released && // exclude future videos (i.e. that will air in the future) video_released <= E::now() { @@ -240,14 +241,28 @@ fn update_notification_items( ); // if not videos were added and the hashmap is empty, just remove the MetaItem record all together + // otherwise try to update the last_video_released on the LibraryItem if meta_notifs.is_empty() { map.remove(meta_id); + } else { + // when dismissing notifications, make sure we update the `last_video_released` + // of the LibraryItem this way if we've only `DismissedNotificationItem` + // the next time we `PullNotifications` we won't see the same notifications. + library_items_effects.push(update_library_item(library_item, meta_notifs)); } map }); - eq_update(notification_items, next_notification_items) + // if we have at least 1 LibraryItem effect in the vector + let library_items_effects = library_items_effects.into_iter().fold( + Effects::none().unchanged(), + |effects, library_item_effects| effects.join(library_item_effects), + ); + + let notifications_effects = eq_update(notification_items, next_notification_items); + + library_items_effects.join(notifications_effects) } fn push_notifications_to_storage(notifications: &NotificationsBucket) -> Effect { @@ -266,23 +281,19 @@ fn push_notifications_to_storage(notifications: &Notifications .into() } -fn dismiss_notification_item( +fn dismiss_notification_item( library: &LibraryBucket, notifications: &mut NotificationsBucket, id: &str, ) -> Effects { match notifications.items.remove(id) { - Some(_) => { - // when dismissing notifications, make sure we update the `last_watched` - // of the LibraryItem this way if we've only `DismissedNotificationItem` + Some(library_item_notifications) => { + // when dismissing notifications, make sure we update the `last_video_released` + // of the LibraryItem this way if we've `DismissedNotificationItem` // the next time we `PullNotifications` we won't see the same notifications. let library_item_effects = match library.items.get(id) { Some(library_item) => { - let mut library_item = library_item.to_owned(); - library_item.state.last_watched = Some(E::now()); - - Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item))) - .unchanged() + update_library_item(library_item, &library_item_notifications) } _ => Effects::none().unchanged(), }; @@ -296,3 +307,64 @@ fn dismiss_notification_item( _ => Effects::none().unchanged(), } } + +/// Updates the [`LibraryItem.state.last_video_released`] by triggering an [`Internal::UpdateLibraryItem`]. +/// +/// There should be at least 1 [`NotificationItem.video_released`] > [`LibraryItem.state.last_video_released`] +/// in order for this to happen. +fn update_library_item( + library_item: &LibraryItem, + library_item_notifications: &HashMap, +) -> Effects { + let last_video_released = library_item_notifications + .iter() + .sorted_by(|(_id_a, item_a), (_id_b, item_b)| { + item_b.video_released.cmp(&item_a.video_released) + }) + .map(|(_id, item)| item.video_released) + .next(); + + match last_video_released { + Some(last_video_released) => { + // This should always be the case but to be safe + // we check if the library item's last_video_released is really + // smaller than the notif. video_released before updating the + // LibraryItem + let should_update_library_item = library_item + .state + .last_video_released + .filter(|lib_item| lib_item < &last_video_released) + .is_some(); + + if should_update_library_item { + let mut library_item = library_item.to_owned(); + + library_item.state.last_video_released = Some(last_video_released); + + Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item))).unchanged() + } else { + Effects::none().unchanged() + } + } + _ => Effects::none().unchanged(), + } +} + +#[cfg(test)] +mod test { + use chrono::{TimeZone, Utc}; + use lazysort::SortedBy; + + #[test] + fn test_sort_by_with_datetimes() { + let datetimes = &[ + Utc.with_ymd_and_hms(2022, 6, 10, 10, 0, 0).unwrap(), + Utc.with_ymd_and_hms(2022, 7, 20, 20, 0, 0).unwrap(), + ]; + + // let latest_ab = datetimes.iter().sorted_by(|a, b| a.cmp(b)).next(); + let latest_ba = datetimes.iter().sorted_by(|a, b| b.cmp(a)).next(); + // assert_eq!(latest_ab, Some(&datetimes[1])); + assert_eq!(latest_ba, Some(&datetimes[1])); + } +} diff --git a/src/models/player.rs b/src/models/player.rs index 3cbb92b67..dee1015a9 100644 --- a/src/models/player.rs +++ b/src/models/player.rs @@ -333,7 +333,6 @@ impl UpdateWithCtx for Player { Some(library_item), ) => { let seeking = library_item.state.time_offset.abs_diff(*time) > 1000; - // library_item.state.last_watched = Some(E::now() - chrono::Duration::days(1)); library_item.state.last_watched = Some(E::now()); if library_item.state.video_id != Some(video_id.to_owned()) { library_item.state.video_id = Some(video_id.to_owned()); @@ -568,6 +567,24 @@ fn switch_to_next_video( library_item.state.time_watched = 0; library_item.state.flagged_watched = 0; library_item.state.time_offset = 1; + + match ( + &next_video.released, + &library_item.state.last_video_released, + ) { + // if video's released date is larger than the last_video_released (observed) + // update that too + (Some(released), Some(last_video_released)) + if released > last_video_released => + { + library_item.state.last_video_released = Some(*released) + } + // or if we haven't had a last_video_released up until now. + (Some(released), None) => { + library_item.state.last_video_released = Some(*released) + } + _ => {} + }; }; } _ => {} diff --git a/src/unit_tests/ctx/notifications/update_notifications.rs b/src/unit_tests/ctx/notifications/update_notifications.rs index 3e6e14dd8..3329ce3e6 100644 --- a/src/unit_tests/ctx/notifications/update_notifications.rs +++ b/src/unit_tests/ctx/notifications/update_notifications.rs @@ -40,7 +40,7 @@ use crate::{ streams::StreamsBucket, }, unit_tests::{ - default_fetch_handler, Request, TestEnv, EVENTS, FETCH_HANDLER, NOW, REQUESTS, STATES, + default_fetch_handler, Request, TestEnv, EVENTS, FETCH_HANDLER, REQUESTS, STATES, }, }; @@ -283,6 +283,23 @@ fn test_pull_notifications_and_play_in_player() { meta_notifs.get("tt1:1:7").is_some(), "Should have notification for tt1:1:7" ); + + // last_video-released should be updated + assert_eq!( + Some(Utc.with_ymd_and_hms(2020, 1, 15, 0, 0, 0).unwrap()), + runtime + .model() + .unwrap() + .ctx + .library + .items + .get("tt1") + .unwrap() + .state + .last_video_released, + "Last video released should be updated with the latest episode release date" + ); + // Start watching episode 6 // This should dismiss all notifications for this MetaItem Id. // libraryItem.state.lastWatched dismisses any unwatched episode notifications @@ -449,7 +466,7 @@ fn test_dismiss_notification() { video_id: Some("tt1:1".into()), watched: None, last_video_released: Some( - Utc.with_ymd_and_hms(2023, 7, 10, 0, 0, 0).unwrap(), + Utc.with_ymd_and_hms(2023, 6, 20, 0, 0, 0).unwrap(), ), no_notif: false, }, @@ -478,7 +495,7 @@ fn test_dismiss_notification() { video_id: Some("tt1:1".into()), watched: None, last_video_released: Some( - Utc.with_ymd_and_hms(2023, 8, 14, 0, 0, 0).unwrap(), + Utc.with_ymd_and_hms(2023, 6, 20, 0, 0, 0).unwrap(), ), no_notif: false, }, @@ -508,9 +525,6 @@ fn test_dismiss_notification() { 1000, ); let runtime = Arc::new(RwLock::new(runtime)); - // update now to later date than both last_watched - let expected_last_watched = Utc.with_ymd_and_hms(2023, 8, 14, 0, 0, 0).unwrap(); - *NOW.write().unwrap() = expected_last_watched.clone(); TestEnv::run_with_runtime( rx, @@ -540,29 +554,24 @@ fn test_dismiss_notification() { RuntimeEvent::NewState(fields, model) if fields.len() == 1 && *fields.first().unwrap() == TestModelField::Ctx && model.ctx.notifications.items.len() == 1, "We should have notifications for 1 LibraryItem ids" ); + assert_matches!( events[1], RuntimeEvent::NewState(fields, model) if fields.len() == 1 && *fields.first().unwrap() == TestModelField::Ctx && model.ctx.notifications.items.len() == 1, "We should have notifications for 1 LibraryItem ids" ); - assert_matches!( events[2], - RuntimeEvent::CoreEvent(crate::runtime::msg::Event::NotificationsDismissed { + RuntimeEvent::CoreEvent(Event::NotificationsDismissed { id }) if id == "tt1" ); assert_matches!( events[3], - RuntimeEvent::CoreEvent(crate::runtime::msg::Event::NotificationsPushedToStorage { + RuntimeEvent::CoreEvent(Event::NotificationsPushedToStorage { ids }) if ids == &vec!["tt2".to_string()] ); - assert_matches!( - events[4], - RuntimeEvent::CoreEvent(Event::LibraryItemsPushedToStorage { ids }) if ids == &["tt1".to_string()], - "LibraryItem should be pushed to storage" - ); let states = STATES.read().unwrap(); let states = states @@ -599,11 +608,11 @@ fn test_dismiss_notification() { ); } - // state 2 - `LibraryItem.state.last_watched` of the dismissed item's notifications should be updated too + // state 2 - `LibraryItem.state.last_video_released` of the dismissed item's notifications should be updated too // by the `UpdateLibraryItem` { assert_eq!( - Some(expected_last_watched), + Some(Utc.with_ymd_and_hms(2023, 7, 10, 0, 0, 0).unwrap()), states[2] .ctx .library @@ -611,7 +620,8 @@ fn test_dismiss_notification() { .get("tt1") .unwrap() .state - .last_watched + .last_video_released, + "Last video released should be updated with the latest episode release date" ); } }