Resolve code review comments

aome510 · Aug 1, 2024 · 9f93feb · 9f93feb
1 parent ed5b5c4
commit 9f93feb
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 18 deletions.
diff --git a/spotify_player/src/client/mod.rs b/spotify_player/src/client/mod.rs
@@ -300,11 +300,11 @@ impl Client {
             }
             ClientRequest::GetUserPlaylists => {
                 let playlists = self.current_user_playlists().await?;
-                let nodes = state.data.read().user_data.playlist_folder_nodes.clone();
-                let playlists = if nodes.is_empty() {
-                    playlists
+                let node = state.data.read().user_data.playlist_folder_node.clone();
+                let playlists = if let Some(node) = node.filter(|n| !n.children.is_empty()) {
+                    crate::playlist_folders::structurize(playlists, node.children)
                 } else {
-                    crate::playlist_folders::structurize(&playlists, nodes)
+                    playlists
                 };
                 store_data_into_file_cache(
                     FileCacheKey::Playlists,

diff --git a/spotify_player/src/playlist_folders.rs b/spotify_player/src/playlist_folders.rs
@@ -5,13 +5,13 @@ use rspotify::model::{Id, PlaylistId, UserId};
 use crate::state::{Playlist, PlaylistFolderNode};
 
 /// Structurizes a flat input playlist according to the playlist folder nodes
-pub fn structurize(playlists: &Vec<Playlist>, nodes: Vec<PlaylistFolderNode>) -> Vec<Playlist> {
+pub fn structurize(playlists: Vec<Playlist>, nodes: Vec<PlaylistFolderNode>) -> Vec<Playlist> {
     // 1. Collect playlist ids from inner nodes
     let mut playlist_ids: HashSet<String> = HashSet::new();
     get_playlist_ids_from_nodes(&nodes, &mut playlist_ids);
     // 2. Add root playlists that don't belong to folders
     let mut playlist_folders: Vec<Playlist> = Vec::new();
-    for playlist in playlists {
+    for playlist in &playlists {
         if !playlist_ids.contains(playlist.id.id()) {
             let mut p = playlist.clone();
             p.is_folder = false;
@@ -21,7 +21,6 @@ pub fn structurize(playlists: &Vec<Playlist>, nodes: Vec<PlaylistFolderNode>) ->
     }
     // 3. Add the rest
     let by_ids: HashMap<String, Playlist> = playlists
-        .clone()
         .into_iter()
         .map(|p| (p.id.id().to_string(), p))
         .collect();
@@ -42,7 +41,7 @@ fn get_playlist_ids_from_nodes(nodes: &Vec<PlaylistFolderNode>, acc: &mut HashSe
 fn add_playlist_folders(
     nodes: &Vec<PlaylistFolderNode>,
     by_ids: &HashMap<String, Playlist>,
-    folder_level: &mut i32,
+    folder_level: &mut usize,
     acc: &mut Vec<Playlist>,
 ) {
     let level = *folder_level;

diff --git a/spotify_player/src/state/data.rs b/spotify_player/src/state/data.rs
@@ -33,7 +33,7 @@ pub struct AppData {
 pub struct UserData {
     pub user: Option<rspotify_model::PrivateUser>,
     pub playlists: Vec<Playlist>,
-    pub playlist_folder_nodes: Vec<PlaylistFolderNode>,
+    pub playlist_folder_node: Option<PlaylistFolderNode>,
     pub followed_artists: Vec<Artist>,
     pub saved_albums: Vec<Album>,
     pub saved_tracks: HashMap<String, Track>,
@@ -109,11 +109,10 @@ impl UserData {
             user: None,
             playlists: load_data_from_file_cache(FileCacheKey::Playlists, cache_folder)
                 .unwrap_or_default(),
-            playlist_folder_nodes: load_data_from_file_cache(
+            playlist_folder_node: load_data_from_file_cache(
                 FileCacheKey::PlaylistFolders,
                 cache_folder,
-            )
-            .unwrap_or_default(),
+            ),
             followed_artists: load_data_from_file_cache(
                 FileCacheKey::FollowedArtists,
                 cache_folder,
@@ -139,7 +138,7 @@ impl UserData {
     }
 
     /// Get a list of playlists for the given folder level
-    pub fn folder_playlists(&self, level: i32) -> Vec<&Playlist> {
+    pub fn folder_playlists(&self, level: usize) -> Vec<&Playlist> {
         self.playlists
             .iter()
             .filter(|p| p.level.0 == level)

diff --git a/spotify_player/src/state/model.rs b/spotify_player/src/state/model.rs
@@ -158,15 +158,17 @@ pub struct Playlist {
     #[serde(default)]
     pub is_folder: bool,
     #[serde(default)]
-    pub level: (i32, i32), // current + target
+    pub level: (usize, usize), // current, target
 }
 
 #[derive(Deserialize, Debug, Clone)]
-/// A node to help building a playlist folder hierarchy
+/// A reference node retrieved by running https://github.com/mikez/spotify-folders
+/// Helps building a playlist folder hierarchy
 pub struct PlaylistFolderNode {
     pub name: Option<String>,
     #[serde(rename = "type")]
     pub node_type: String,
+    #[serde(default)]
     pub uri: String,
     #[serde(default = "Vec::new")]
     pub children: Vec<PlaylistFolderNode>,

diff --git a/spotify_player/src/state/ui/page.rs b/spotify_player/src/state/ui/page.rs
@@ -89,7 +89,7 @@ pub enum ContextPageUIState {
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum LibraryFocusState {
-    Playlists(i32),
+    Playlists(usize), // Playlists in the folder level
     SavedAlbums,
     FollowedArtists,
 }

diff --git a/spotify_player/src/state/ui/popup.rs b/spotify_player/src/state/ui/popup.rs
@@ -37,8 +37,8 @@ pub enum ActionListItem {
 /// An action on an item in a playlist popup list
 #[derive(Debug)]
 pub enum PlaylistPopupAction {
-    Browse(i32),
-    AddTrack(i32, TrackId<'static>),
+    Browse(usize), // Browse playlists by folder level
+    AddTrack(usize, TrackId<'static>),
 }
 
 /// An action on an item in an artist popup list