Merge pull request #494 from Stremio/fix/notifications-and-cw

Fix: Notifications and Continue watching

src/models/continue_watching_preview.rs

@@ -14,18 +14,27 @@ use crate::{
     },
 };
 
+#[derive(Clone, Serialize, Debug, PartialEq, Eq)]
+#[serde(rename_all = "camelCase")]
+pub struct Item {
+    #[serde(flatten)]
+    pub library_item: LibraryItem,
+    /// a count of the total notifications we have for this item
+    pub notifications: usize,
+}
+
 /// The continue watching section in the app
 #[derive(Default, Clone, Serialize, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct ContinueWatchingPreview {
-    pub library_items: Vec<LibraryItem>,
+    pub items: Vec<Item>,
 }
 
 impl ContinueWatchingPreview {
     pub fn new(library: &LibraryBucket, notifications: &NotificationsBucket) -> (Self, Effects) {
-        let mut library_items = vec![];
-        let effects = library_items_update(&mut library_items, library, notifications);
-        (Self { library_items }, effects.unchanged())
+        let mut items = vec![];
+        let effects = library_items_update(&mut items, library, notifications);
+        (Self { items }, effects.unchanged())
     }
 }
 
@@ -37,70 +46,80 @@ impl<E: Env + 'static> UpdateWithCtx<E> for ContinueWatchingPreview {
             Msg::Internal(Internal::LibraryChanged(true))
             // notifications have been updated
             | Msg::Internal(Internal::NotificationsChanged) => {
-                library_items_update(&mut self.library_items, &ctx.library, &ctx.notifications)
+                library_items_update(&mut self.items, &ctx.library, &ctx.notifications)
             }
             _ => Effects::none().unchanged(),
         }
     }
 }
 
 fn library_items_update(
-    library_items: &mut Vec<LibraryItem>,
+    cw_items: &mut Vec<Item>,
     library: &LibraryBucket,
     notifications: &NotificationsBucket,
 ) -> Effects {
-    let next_library_items = library
+    let next_cw_items = library
         .items
         .values()
-        .filter(|library_item| {
+        .filter_map(|library_item| {
+            let library_notification = notifications
+                .items
+                .get(&library_item.id)
+                .filter(|meta_notifs| !meta_notifs.is_empty());
+
             // either the library item is in CW
-            library_item.is_in_continue_watching()
+            if library_item.is_in_continue_watching()
             // or there's a new notification for it
-                || notifications
-                    .items
-                    .get(&library_item.id)
-                    .filter(|meta_notifs| !meta_notifs.is_empty())
-                    .is_some()
+                || library_notification.is_some()
+            {
+                Some((
+                    library_item,
+                    library_notification
+                        .map(|notifs| notifs.len())
+                        .unwrap_or_default(),
+                ))
+            } else {
+                None
+            }
         })
         // either take the oldest video released date or the modification date of the LibraryItem
-        .sorted_by(|a, b| {
+        .sorted_by(|(item_a, _), (item_b, _)| {
             let a_time = notifications
                 .items
-                .get(&a.id)
+                .get(&item_a.id)
                 .and_then(|notifs| {
                     notifs
                         .values()
-                        // take the released date of the video if there is one, or skip this notification
-                        .filter_map(|notification| notification.video.released)
-                        .sorted_by(|a_released, b_released| {
-                            // order by the oldest video released!
-                            b_released.cmp(a_released).reverse()
-                        })
+                        // take the video released date of the notification
+                        .map(|notification| notification.video_released)
+                        // order by the newest video released!
+                        .sorted_by(|a_released, b_released| b_released.cmp(a_released))
                         .next()
                 })
-                .unwrap_or(a.mtime);
+                .unwrap_or(item_a.mtime);
 
             let b_time = notifications
                 .items
-                .get(&b.id)
+                .get(&item_b.id)
                 .and_then(|notifs| {
                     notifs
                         .values()
-                        // take the released date of the video if there is one, or skip this notification
-                        .filter_map(|notification| notification.video.released)
-                        .sorted_by(|a_released, b_released| {
-                            // order by the oldest video released!
-                            b_released.cmp(a_released).reverse()
-                        })
+                        // take the video released date of the notification
+                        .map(|notification| notification.video_released)
+                        // order by the newest video released!
+                        .sorted_by(|a_released, b_released| b_released.cmp(a_released))
                         .next()
                 })
-                .unwrap_or(b.mtime);
+                .unwrap_or(item_b.mtime);
 
             b_time.cmp(&a_time)
         })
         .take(CATALOG_PREVIEW_SIZE)
-        .cloned()
+        .map(|(library_item, notifications)| Item {
+            library_item: library_item.clone(),
+            notifications,
+        })
         .collect::<Vec<_>>();
 
-    eq_update(library_items, next_library_items)
+    eq_update(cw_items, next_cw_items)
 }

src/models/ctx/update_library.rs

@@ -1,19 +1,29 @@
-use crate::constants::{
-    LIBRARY_COLLECTION_NAME, LIBRARY_RECENT_COUNT, LIBRARY_RECENT_STORAGE_KEY, LIBRARY_STORAGE_KEY,
+use std::{collections::HashMap, marker::PhantomData};
+
+use futures::{
+    future::{self, Either},
+    FutureExt, TryFutureExt,
 };
-use crate::models::ctx::{CtxError, CtxStatus, OtherError};
-use crate::runtime::msg::{Action, ActionCtx, Event, Internal, Msg};
-use crate::runtime::{Effect, EffectFuture, Effects, Env, EnvFutureExt};
-use crate::types::api::{
-    fetch_api, APIResult, DatastoreCommand, DatastoreRequest, LibraryItemModified,
-    LibraryItemsResponse, SuccessResponse,
+
+use crate::{
+    constants::{
+        LIBRARY_COLLECTION_NAME, LIBRARY_RECENT_COUNT, LIBRARY_RECENT_STORAGE_KEY,
+        LIBRARY_STORAGE_KEY,
+    },
+    models::ctx::{CtxError, CtxStatus, OtherError},
+    runtime::{
+        msg::{Action, ActionCtx, Event, Internal, Msg},
+        Effect, EffectFuture, Effects, Env, EnvFutureExt,
+    },
+    types::{
+        api::{
+            fetch_api, APIResult, DatastoreCommand, DatastoreRequest, LibraryItemModified,
+            LibraryItemsResponse, SuccessResponse,
+        },
+        library::{LibraryBucket, LibraryBucketRef, LibraryItem},
+        profile::{AuthKey, Profile},
+    },
 };
-use crate::types::library::{LibraryBucket, LibraryBucketRef, LibraryItem};
-use crate::types::profile::{AuthKey, Profile};
-use futures::future::Either;
-use futures::{future, FutureExt, TryFutureExt};
-use std::collections::HashMap;
-use std::marker::PhantomData;
 
 pub fn update_library<E: Env + 'static>(
     library: &mut LibraryBucket,
@@ -50,7 +60,19 @@ pub fn update_library<E: Env + 'static>(
                 let mut library_item = library_item.to_owned();
                 library_item.removed = true;
                 library_item.temp = false;
+
+                // Dismiss any notification for the LibraryItem
+                let notifications_effects = if library_item.state.no_notif {
+                    Effects::msg(Msg::Internal(Internal::DismissNotificationItem(
+                        id.to_owned(),
+                    )))
+                    .unchanged()
+                } else {
+                    Effects::none().unchanged()
+                };
+
                 Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item)))
+                    .join(notifications_effects)
                     .join(Effects::msg(Msg::Event(Event::LibraryItemRemoved {
                         id: id.to_owned(),
                     })))
@@ -66,7 +88,7 @@ pub fn update_library<E: Env + 'static>(
             Some(library_item) => {
                 let mut library_item = library_item.to_owned();
                 library_item.state.time_offset = 0;
-                library_item.state.last_watched = Some(E::now());
+
                 Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item)))
                     .join(Effects::msg(Msg::Event(Event::LibraryItemRewinded {
                         id: id.to_owned(),
@@ -84,10 +106,23 @@ pub fn update_library<E: Env + 'static>(
                 Some(library_item) => {
                     let mut library_item = library_item.to_owned();
                     library_item.state.no_notif = *state;
+
+                    // if we have `no_notif` set to `true` (we don't want notifications for the LibraryItem)
+                    // we want to dismiss any notifications for the LibraryItem that exist
+                    let notifications_effects = if library_item.state.no_notif {
+                        Effects::msg(Msg::Internal(Internal::DismissNotificationItem(
+                            id.to_owned(),
+                        )))
+                        .unchanged()
+                    } else {
+                        Effects::none().unchanged()
+                    };
+
                     Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item)))
                         .join(Effects::msg(Msg::Event(
                             Event::LibraryItemNotificationsToggled { id: id.to_owned() },
                         )))
+                        .join(notifications_effects)
                         .unchanged()
                 }
                 _ => Effects::msg(Msg::Event(Event::Error {
@@ -111,6 +146,7 @@ pub fn update_library<E: Env + 'static>(
         Msg::Internal(Internal::UpdateLibraryItem(library_item)) => {
             let mut library_item = library_item.to_owned();
             library_item.mtime = E::now();
+
             let push_to_api_effects = match auth_key {
                 Some(auth_key) => Effects::one(push_items_to_api::<E>(
                     vec![library_item.to_owned()],
@@ -119,10 +155,12 @@ pub fn update_library<E: Env + 'static>(
                 .unchanged(),
                 _ => Effects::none().unchanged(),
             };
+
             let push_to_storage_effects = Effects::one(update_and_push_items_to_storage::<E>(
                 library,
                 vec![library_item],
             ));
+
             push_to_api_effects
                 .join(push_to_storage_effects)
                 .join(Effects::msg(Msg::Internal(Internal::LibraryChanged(true))))

src/models/ctx/update_notifications.rs

@@ -76,9 +76,10 @@ pub fn update_notifications<E: Env + 'static>(
                 .join(notification_items_effects)
                 .unchanged()
         }
-        Msg::Action(Action::Ctx(ActionCtx::DismissNotificationItem(id))) => {
-            remove_notification_item(notifications, id)
-        }
+        Msg::Action(Action::Ctx(ActionCtx::DismissNotificationItem(id))) => Effects::msg(
+            Msg::Internal(Internal::DismissNotificationItem(id.to_owned())),
+        )
+        .unchanged(),
         Msg::Action(Action::Ctx(ActionCtx::Logout)) | Msg::Internal(Internal::Logout) => {
             let notification_catalogs_effects = eq_update(notification_catalogs, vec![]);
             let next_notifications = NotificationsBucket::new::<E>(profile.uid(), vec![]);
@@ -135,7 +136,7 @@ pub fn update_notifications<E: Env + 'static>(
                 .join(notifications_effects)
         }
         Msg::Internal(Internal::DismissNotificationItem(id)) => {
-            remove_notification_item(notifications, id)
+            dismiss_notification_item::<E>(library, notifications, id)
         }
         Msg::Internal(Internal::NotificationsChanged) => {
             Effects::one(push_notifications_to_storage::<E>(notifications)).unchanged()
@@ -200,35 +201,43 @@ fn update_notification_items<E: Env + 'static>(
         // meta items videos
         meta_item
             .videos_iter()
-            .filter(|video| {
-                match (&library_item.state.last_watched, &video.released) {
+            .filter_map(|video| {
+                match (&library_item.state.last_watched, video.released) {
                     (Some(last_watched), Some(video_released)) => {
-                        last_watched < video_released &&
+                        if last_watched < &video_released &&
                                     // exclude future videos (i.e. that will air in the future)
-                                    video_released <= &E::now()
+                                    video_released <= E::now()
+                        {
+                            Some((&library_item.id, &video.id, video_released))
+                        } else {
+                            None
+                        }
                     }
-                    _ => false,
+                    _ => None,
                 }
             })
             // We need to manually fold, otherwise the last seen element with a given key
             // will be present in the final HashMap instead of the first occurrence.
-            .fold(&mut meta_notifs, |meta_notifs, video| {
-                let notif_entry = meta_notifs.entry(video.id.clone());
+            .fold(
+                &mut meta_notifs,
+                |meta_notifs, (meta_id, video_id, video_released)| {
+                    let notif_entry = meta_notifs.entry(video_id.to_owned());
 
-                // for now just skip same videos that already exist
-                // leave the first one found in the Vec.
-                if let Entry::Vacant(new) = notif_entry {
-                    let notification = NotificationItem {
-                        meta_id: meta_id.to_owned(),
-                        video_id: video.id.to_owned(),
-                        video: video.to_owned(),
-                    };
+                    // for now just skip same videos that already exist
+                    // leave the first one found in the Vec.
+                    if let Entry::Vacant(new) = notif_entry {
+                        let notification = NotificationItem {
+                            meta_id: meta_id.to_owned(),
+                            video_id: video_id.to_owned(),
+                            video_released,
+                        };
 
-                    new.insert(notification);
-                }
+                        new.insert(notification);
+                    }
 
-                meta_notifs
-            });
+                    meta_notifs
+                },
+            );
 
         // if not videos were added and the hashmap is empty, just remove the MetaItem record all together
         if meta_notifs.is_empty() {
@@ -257,9 +266,33 @@ fn push_notifications_to_storage<E: Env + 'static>(notifications: &Notifications
     .into()
 }
 
-fn remove_notification_item(notifications: &mut NotificationsBucket, id: &String) -> Effects {
+fn dismiss_notification_item<E: Env + 'static>(
+    library: &LibraryBucket,
+    notifications: &mut NotificationsBucket,
+    id: &str,
+) -> Effects {
     match notifications.items.remove(id) {
-        Some(_) => Effects::msg(Msg::Internal(Internal::NotificationsChanged)).unchanged(),
+        Some(_) => {
+            // when dismissing notifications, make sure we update the `last_watched`
+            // of the LibraryItem this way if we've only `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()
+                }
+                _ => Effects::none().unchanged(),
+            };
+
+            Effects::msg(Msg::Internal(Internal::NotificationsChanged))
+                .join(library_item_effects)
+                .join(Effects::msg(Msg::Event(Event::NotificationsDismissed {
+                    id: id.to_owned(),
+                })))
+        }
         _ => Effects::none().unchanged(),
     }
 }