Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: use last_video_released for notifications #501

Closed
wants to merge 1 commit into from
Closed

feat: use last_video_released for notifications #501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 87 additions & 15 deletions src/models/ctx/update_notifications.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
},
types::{
addon::{AggrRequest, ExtraValue},
library::LibraryBucket,
library::{LibraryBucket, LibraryItem},
notifications::{NotificationItem, NotificationsBucket},
profile::Profile,
resource::{MetaItem, MetaItemId, VideoId},
Expand Down Expand Up @@ -136,7 +136,7 @@ pub fn update_notifications<E: Env + 'static>(
.join(notifications_effects)
}
Msg::Internal(Internal::DismissNotificationItem(id)) => {
dismiss_notification_item::<E>(library, notifications, id)
dismiss_notification_item(library, notifications, id)
}
Msg::Internal(Internal::NotificationsChanged) => {
Effects::one(push_notifications_to_storage::<E>(notifications)).unchanged()
Expand Down Expand Up @@ -171,6 +171,7 @@ fn update_notification_items<E: Env + 'static>(
.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
Expand Down Expand Up @@ -202,9 +203,9 @@ fn update_notification_items<E: Env + 'static>(
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()
{
Expand Down Expand Up @@ -240,14 +241,28 @@ fn update_notification_items<E: Env + 'static>(
);

// 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<E: Env + 'static>(notifications: &NotificationsBucket) -> Effect {
Expand All @@ -266,23 +281,19 @@ fn push_notifications_to_storage<E: Env + 'static>(notifications: &Notifications
.into()
}

fn dismiss_notification_item<E: Env + 'static>(
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(),
};
Expand All @@ -296,3 +307,64 @@ fn dismiss_notification_item<E: Env + 'static>(
_ => 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<VideoId, NotificationItem>,
) -> 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]));
}
}
19 changes: 18 additions & 1 deletion src/models/player.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ impl<E: Env + 'static> UpdateWithCtx<E> 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());
Expand Down Expand Up @@ -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)
}
_ => {}
};
};
}
_ => {}
Expand Down
44 changes: 27 additions & 17 deletions src/unit_tests/ctx/notifications/update_notifications.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -599,19 +608,20 @@ 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
.items
.get("tt1")
.unwrap()
.state
.last_watched
.last_video_released,
"Last video released should be updated with the latest episode release date"
);
}
}