fix: address PR review comments:

- Keep the results instead of discarding errors on authentication
- add error logs
- Profile - remove library_missing and update migration

Signed-off-by: Lachezar Lechev <[email protected]>

src/models/ctx/ctx.rs

@@ -1,4 +1,4 @@
-use crate::constants::{LIBRARY_COLLECTION_NAME, OFFICIAL_ADDONS};
+use crate::constants::LIBRARY_COLLECTION_NAME;
 use crate::models::common::{DescriptorLoadable, Loadable, ResourceLoadable};
 use crate::models::ctx::{
     update_events, update_library, update_notifications, update_profile, update_search_history,
@@ -24,7 +24,7 @@ use enclose::enclose;
 use futures::{future, FutureExt, TryFutureExt};
 use serde::Serialize;
 
-use tracing::trace;
+use tracing::{error, trace};
 
 use super::OtherError;
 
@@ -175,33 +175,40 @@ impl<E: Env + 'static> Update<E> for Ctx {
                                     .unchanged();
 
                                 let addons_locked_event = Event::UserAddonsLocked {
-                                    addons_locked: ctx_auth.addons_locked,
+                                    addons_locked: ctx_auth.addons_result.is_err(),
                                 };
-                                let addons_locked_effects = if ctx_auth.addons_locked {
-                                    Effects::msg(Msg::Event(Event::Error {
+
+                                let addons_effects = match ctx_auth.addons_result.as_ref() {
+                                    Ok(_addons) => {
+                                        Effects::msg(Msg::Event(addons_locked_event)).unchanged()
+                                    }
+                                    Err(_err) => Effects::msg(Msg::Event(Event::Error {
                                         error: CtxError::Other(OtherError::UserAddonsAreLocked),
                                         source: Box::new(addons_locked_event),
                                     }))
-                                    .unchanged()
-                                } else {
-                                    Effects::msg(Msg::Event(addons_locked_event)).unchanged()
+                                    .unchanged(),
                                 };
 
                                 let library_missing_event = Event::UserLibraryMissing {
-                                    library_missing: ctx_auth.library_missing,
+                                    library_missing: ctx_auth.library_items_result.is_err(),
                                 };
-                                let library_missing_effects = if ctx_auth.library_missing {
-                                    Effects::msg(Msg::Event(Event::Error {
+
+                                let library_missing_effects = match ctx_auth
+                                    .library_items_result
+                                    .as_ref()
+                                {
+                                    Ok(_library) => {
+                                        Effects::msg(Msg::Event(library_missing_event)).unchanged()
+                                    }
+                                    Err(_err) => Effects::msg(Msg::Event(Event::Error {
                                         error: CtxError::Other(OtherError::UserLibraryIsMissing),
                                         source: Box::new(library_missing_event),
                                     }))
-                                    .unchanged()
-                                } else {
-                                    Effects::msg(Msg::Event(library_missing_event)).unchanged()
+                                    .unchanged(),
                                 };
 
                                 authentication_effects
-                                    .join(addons_locked_effects)
+                                    .join(addons_effects)
                                     .join(library_missing_effects)
                             }
                             Err(error) => Effects::msg(Msg::Event(Event::Error {
@@ -321,16 +328,17 @@ fn authenticate<E: Env + 'static>(auth_request: &AuthRequest) -> Effect {
             let (addon_collection_result, datastore_library_result) =
                 future::join(addon_collection_fut, datastore_library_fut).await;
 
-            // lock if the result from fetching the addons has failed
-            let addons_locked = addon_collection_result.is_err();
-            // set the flag to true if fetching of the library has failed
-            let library_missing = datastore_library_result.is_err();
+            if let Err(error) = addon_collection_result.as_ref() {
+                error!("Failed to fetch Addon collection from API: {error:?}");
+            }
+            if let Err(error) = datastore_library_result.as_ref() {
+                error!("Failed to fetch LibraryItems for user from API: {error:?}");
+            }
+
             Ok(CtxAuthResponse {
                 auth,
-                addons: addon_collection_result.unwrap_or(OFFICIAL_ADDONS.clone()),
-                addons_locked,
-                library_items: datastore_library_result.unwrap_or_default(),
-                library_missing,
+                addons_result: addon_collection_result,
+                library_items_result: datastore_library_result,
             })
         }
         .map(enclose!((auth_request) move |result| {

src/models/ctx/update_library.rs

@@ -179,12 +179,14 @@ pub fn update_library<E: Env + 'static>(
                 CtxStatus::Loading(loading_auth_request),
                 Ok(CtxAuthResponse {
                     auth,
-                    library_items,
+                    library_items_result,
                     ..
                 }),
             ) if loading_auth_request == auth_request => {
-                let next_library =
-                    LibraryBucket::new(Some(auth.user.id.to_owned()), library_items.to_owned());
+                let next_library = LibraryBucket::new(
+                    Some(auth.user.id.to_owned()),
+                    library_items_result.to_owned().unwrap_or_default(),
+                );
                 if *library != next_library {
                     *library = next_library;
                     Effects::msg(Msg::Internal(Internal::LibraryChanged(false)))
@@ -249,10 +251,9 @@ pub fn update_library<E: Env + 'static>(
             result,
         )) if Some(loading_auth_key) == auth_key => match result {
             Ok(items) => {
-                // override the missing library flag to indicate that we've successfully updated the local library
-                profile.library_missing = false;
+                // send an event that the missing library is now present
                 let library_missing_effects = Effects::msg(Msg::Event(Event::UserLibraryMissing {
-                    library_missing: profile.library_missing,
+                    library_missing: false,
                 }))
                 .unchanged();
 

src/models/ctx/update_profile.rs

@@ -275,17 +275,14 @@ pub fn update_profile<E: Env + 'static>(
                 CtxStatus::Loading(loading_auth_request),
                 Ok(CtxAuthResponse {
                     auth,
-                    addons,
-                    addons_locked,
-                    library_missing,
+                    addons_result,
                     ..
                 }),
             ) if loading_auth_request == auth_request => {
                 let next_profile = Profile {
                     auth: Some(auth.to_owned()),
-                    addons: addons.to_owned(),
-                    addons_locked: *addons_locked,
-                    library_missing: *library_missing,
+                    addons: addons_result.to_owned().unwrap_or(OFFICIAL_ADDONS.clone()),
+                    addons_locked: addons_result.is_err(),
                     settings: Settings::default(),
                 };
                 if *profile != next_profile {

src/runtime/env.rs

@@ -554,7 +554,6 @@ fn migrate_storage_schema_to_v13<E: Env>() -> TryEnvFuture<()> {
             match profile.as_mut().and_then(|profile| profile.as_object_mut()) {
                 Some(profile) => {
                     profile.insert("addonsLocked".to_owned(), serde_json::Value::Bool(false));
-                    profile.insert("libraryMissing".to_owned(), serde_json::Value::Bool(false));
                     E::set_storage(PROFILE_STORAGE_KEY, Some(&profile))
                 }
                 _ => E::set_storage::<()>(PROFILE_STORAGE_KEY, None),
@@ -1028,7 +1027,6 @@ mod test {
 
             let migrated_profile = json!({
                 "addonsLocked": false,
-                "libraryMissing": false,
             });
 
             // setup storage for migration

src/runtime/msg/internal.rs

@@ -26,15 +26,8 @@ pub type CtxStorageResponse = (
 #[derive(Debug)]
 pub struct CtxAuthResponse {
     pub auth: Auth,
-    pub addons: Vec<Descriptor>,
-    /// If the addon get request fails, this flag will be `true`
-    /// to disallow the user to install addons and override his addons in the API
-    pub addons_locked: bool,
-    pub library_items: Vec<LibraryItem>,
-    /// If the Library datastore Get request fails on initial logging
-    /// this flag will be set to `true` to indicate why the user doesn't see
-    /// their library items.
-    pub library_missing: bool,
+    pub addons_result: Result<Vec<Descriptor>, CtxError>,
+    pub library_items_result: Result<Vec<LibraryItem>, CtxError>,
 }
 
 pub type LibraryPlanResponse = (Vec<String>, Vec<String>);

src/types/profile/profile.rs

@@ -25,10 +25,6 @@ pub struct Profile {
     /// if they install a new addon locally when we have defaulted to the official ones
     #[serde(default)]
     pub addons_locked: bool,
-    /// This missing library flag is raised when the API Library Collection fetch request on login
-    /// has failed to indicate why the user has an empty Library
-    #[serde(default)]
-    pub library_missing: bool,
     pub settings: Settings,
 }
 
@@ -38,7 +34,6 @@ impl Default for Profile {
             auth: None,
             addons: OFFICIAL_ADDONS.to_owned(),
             addons_locked: false,
-            library_missing: false,
             settings: Settings::default(),
         }
     }

src/unit_tests/serde/profile.rs

@@ -10,14 +10,12 @@ fn profile() {
                 auth: Some(Auth::default()),
                 addons: vec![],
                 addons_locked: false,
-                library_missing: false,
                 settings: Settings::default(),
             },
             Profile {
                 auth: None,
                 addons: vec![],
                 addons_locked: false,
-                library_missing: false,
                 settings: Settings::default(),
             },
         ],
@@ -26,7 +24,7 @@ fn profile() {
                 Token::Seq { len: Some(2) },
                 Token::Struct {
                     name: "Profile",
-                    len: 5,
+                    len: 4,
                 },
                 Token::Str("auth"),
                 Token::Some,
@@ -38,16 +36,14 @@ fn profile() {
                 Token::SeqEnd,
                 Token::Str("addonsLocked"),
                 Token::Bool(false),
-                Token::Str("libraryMissing"),
-                Token::Bool(false),
                 Token::Str("settings"),
             ],
             Settings::default_tokens(),
             vec![
                 Token::StructEnd,
                 Token::Struct {
                     name: "Profile",
-                    len: 5,
+                    len: 4,
                 },
                 Token::Str("auth"),
                 Token::None,
@@ -56,8 +52,6 @@ fn profile() {
                 Token::SeqEnd,
                 Token::Str("addonsLocked"),
                 Token::Bool(false),
-                Token::Str("libraryMissing"),
-                Token::Bool(false),
                 Token::Str("settings"),
             ],
             Settings::default_tokens(),
@@ -70,22 +64,19 @@ fn profile() {
             auth: None,
             addons: vec![],
             addons_locked: false,
-            library_missing: false,
             settings: Settings::default(),
         },
         &[
             vec![
                 Token::Struct {
                     name: "Profile",
-                    len: 4,
+                    len: 3,
                 },
                 Token::Str("addons"),
                 Token::Seq { len: Some(0) },
                 Token::SeqEnd,
                 Token::Str("addonsLocked"),
                 Token::Bool(false),
-                Token::Str("libraryMissing"),
-                Token::Bool(false),
                 Token::Str("settings"),
             ],
             Settings::default_tokens(),