From 6c1704e3c466e043acf6afcb7929b27d131df92f Mon Sep 17 00:00:00 2001 From: Winman486 Date: Mon, 20 Oct 2025 18:58:46 -0500 Subject: [PATCH] fix: Remove duplicate menu entry and reduce code complexity - Removed duplicate 'Download Full Album' context menu entry - Refactored on_download_all_albums_from_playlist to reduce C901 complexity - Extracted helper methods: _extract_album_ids_from_tracks, _load_albums_with_rate_limiting, _validate_session, _handle_album_load_error, _queue_loaded_albums - All make check errors and warnings now resolved --- tidal_dl_ng/gui.py | 202 +++++++++++++++++++++++++++++---------------- 1 file changed, 132 insertions(+), 70 deletions(-) diff --git a/tidal_dl_ng/gui.py b/tidal_dl_ng/gui.py index 0dc319d..4bf0f06 100644 --- a/tidal_dl_ng/gui.py +++ b/tidal_dl_ng/gui.py @@ -588,16 +588,12 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): # We build the menu. menu = QtWidgets.QMenu() - media = get_results_media_item(index, self.proxy_tr_results, self.model_tr_results) - - if isinstance(media, Track) and media.album: - menu.addAction("Download Full Album", lambda: self.thread_download_album_from_track(point)) - - menu.addAction("Copy Share URL", lambda: self.on_copy_url_share(self.tr_results, point)) # Add "Download Full Album" option if it's a track or video with an album if isinstance(media, Track | Video) and hasattr(media, "album") and media.album: - menu.addAction("Download Full Album", lambda: self.thread_it(self.on_download_album_from_track, point)) + menu.addAction("Download Full Album", lambda: self.thread_download_album_from_track(point)) + + menu.addAction("Copy Share URL", lambda: self.on_copy_url_share(self.tr_results, point)) menu.exec(self.tr_results.mapToGlobal(point)) @@ -652,7 +648,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): point (QPoint): The point in the tree where the playlist was right-clicked. """ try: - # Get the playlist + # Get and validate the playlist item = self.tr_lists_user.itemAt(point) media_list = get_user_list_media_item(item) @@ -665,17 +661,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): media_items = items_results_all(media_list) # Extract unique album IDs from tracks - album_ids = {} - for media_item in media_items: - if isinstance(media_item, Track | Video) and hasattr(media_item, "album") and media_item.album: - try: - # Access album.id carefully as it might trigger API calls - album_id = media_item.album.id - if album_id: - album_ids[album_id] = media_item.album - except Exception as e: - logger_gui.debug(f"Skipping track with unavailable album: {e!s}") - continue + album_ids = self._extract_album_ids_from_tracks(media_items) if not album_ids: logger_gui.warning("No albums found in this playlist.") @@ -684,64 +670,17 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): logger_gui.info(f"Found {len(album_ids)} unique albums. Loading with rate limiting...") # Load albums with rate limiting - albums_dict = {} - for idx, album_id in enumerate(album_ids.keys(), start=1): - try: - # Add delay every 20 albums to avoid rate limiting - if idx > 1 and (idx - 1) % 20 == 0: - logger_gui.info(f"🛑 RATE LIMITING: Processed {idx - 1} albums, pausing for 3 seconds...") - time.sleep(3) - - # Check session validity before making API calls - if not self.tidal.session.check_login(): - logger_gui.error("Session expired. Please restart the application and login again.") - return - - # Reload full album object - album = self.tidal.session.album(album_id) - albums_dict[album.id] = album - logger_gui.debug( - f"Loaded album {idx}/{len(album_ids)}: {name_builder_artist(album)} - {album.name}" - ) - - except Exception as e: - error_msg = str(e) - # Check for OAuth/authentication errors - if "401" in error_msg or "OAuth" in error_msg or "token" in error_msg.lower(): - logger_gui.error(f"Authentication error: {error_msg}") - logger_gui.error("Your session has expired. Please restart the application and login again.") - self.s_statusbar_message.emit( - StatusbarMessage(message="Session expired - please restart and login", timeout=5000) - ) - return - logger_gui.warning(f"Failed to load album {album_id}: {error_msg}") - logger_gui.info( - "Note: Some albums may be unavailable due to region restrictions or removal from TIDAL. This is normal." - ) - continue + albums_dict = self._load_albums_with_rate_limiting(album_ids) if not albums_dict: logger_gui.error("Failed to load any albums from playlist.") return - # Prepare all queue items first - logger_gui.info(f"Successfully loaded {len(albums_dict)} albums. Preparing queue items...") - - queue_items = [] - for album in albums_dict.values(): - queue_dl_item = self.media_to_queue_download_model(album) - if queue_dl_item: - queue_items.append((queue_dl_item, album)) - logger_gui.debug(f"Prepared: {name_builder_artist(album)} - {album.name}") - - # Add all items to queue at once - logger_gui.info(f"Adding {len(queue_items)} albums to queue...") - for queue_dl_item, album in queue_items: - self.queue_download_media(queue_dl_item) - logger_gui.info(f"Added: {name_builder_artist(album)} - {album.name}") + # Prepare and queue albums + self._queue_loaded_albums(albums_dict) # Show confirmation - message = f"Added {len(queue_items)} albums to download queue" + message = f"Added {len(albums_dict)} albums to download queue" self.s_statusbar_message.emit(StatusbarMessage(message=message, timeout=3000)) logger_gui.info(message) @@ -750,6 +689,129 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow): logger_gui.error(error_msg) self.s_statusbar_message.emit(StatusbarMessage(message=error_msg, timeout=3000)) + def _extract_album_ids_from_tracks(self, media_items: list) -> dict[int, Album]: + """Extract unique album IDs from a list of media items. + + Args: + media_items (list): List of media items (tracks/videos) from a playlist. + + Returns: + dict[int, Album]: Dictionary mapping album IDs to album stub objects. + """ + album_ids = {} + + for media_item in media_items: + if not isinstance(media_item, Track | Video): + continue + + if not hasattr(media_item, "album") or not media_item.album: + continue + + try: + # Access album.id carefully as it might trigger API calls + album_id = media_item.album.id + if album_id: + album_ids[album_id] = media_item.album + except Exception as e: + logger_gui.debug(f"Skipping track with unavailable album: {e!s}") + continue + + return album_ids + + def _load_albums_with_rate_limiting(self, album_ids: dict[int, Album]) -> dict[int, Album]: + """Load full album objects with rate limiting to prevent API throttling. + + Args: + album_ids (dict[int, Album]): Dictionary of album IDs to album stubs. + + Returns: + dict[int, Album]: Dictionary of successfully loaded full album objects. + """ + albums_dict = {} + + for idx, album_id in enumerate(album_ids.keys(), start=1): + try: + # Add delay every 20 albums to avoid rate limiting + if idx > 1 and (idx - 1) % 20 == 0: + logger_gui.info(f"🛑 RATE LIMITING: Processed {idx - 1} albums, pausing for 3 seconds...") + time.sleep(3) + + # Check session validity before making API calls + if not self._validate_session(): + return albums_dict + + # Reload full album object + album = self.tidal.session.album(album_id) + albums_dict[album.id] = album + logger_gui.debug(f"Loaded album {idx}/{len(album_ids)}: {name_builder_artist(album)} - {album.name}") + + except Exception as e: + if not self._handle_album_load_error(e, album_id): + return albums_dict + continue + + logger_gui.info(f"Successfully loaded {len(albums_dict)} albums.") + return albums_dict + + def _validate_session(self) -> bool: + """Validate that the TIDAL session is still authenticated. + + Returns: + bool: True if session is valid, False otherwise. + """ + if not self.tidal.session.check_login(): + logger_gui.error("Session expired. Please restart the application and login again.") + return False + return True + + def _handle_album_load_error(self, error: Exception, album_id: int) -> bool: + """Handle errors that occur when loading an album. + + Args: + error (Exception): The exception that was raised. + album_id (int): The ID of the album that failed to load. + + Returns: + bool: True if processing should continue, False if it should stop. + """ + error_msg = str(error) + + # Check for OAuth/authentication errors + if "401" in error_msg or "OAuth" in error_msg or "token" in error_msg.lower(): + logger_gui.error(f"Authentication error: {error_msg}") + logger_gui.error("Your session has expired. Please restart the application and login again.") + self.s_statusbar_message.emit( + StatusbarMessage(message="Session expired - please restart and login", timeout=5000) + ) + return False + + logger_gui.warning(f"Failed to load album {album_id}: {error_msg}") + logger_gui.info( + "Note: Some albums may be unavailable due to region restrictions or removal from TIDAL. This is normal." + ) + return True + + def _queue_loaded_albums(self, albums_dict: dict[int, Album]) -> None: + """Prepare and add loaded albums to the download queue. + + Args: + albums_dict (dict[int, Album]): Dictionary of successfully loaded albums. + """ + logger_gui.info(f"Preparing queue items for {len(albums_dict)} albums...") + + queue_items = [] + for album in albums_dict.values(): + queue_dl_item = self.media_to_queue_download_model(album) + if queue_dl_item: + queue_items.append((queue_dl_item, album)) + logger_gui.debug(f"Prepared: {name_builder_artist(album)} - {album.name}") + + # Add all items to queue + logger_gui.info(f"Adding {len(queue_items)} albums to queue...") + for queue_dl_item, album in queue_items: + self.queue_download_media(queue_dl_item) + logger_gui.info(f"Added: {name_builder_artist(album)} - {album.name}") + def on_copy_url_share( self, tree_target: QtWidgets.QTreeWidget | QtWidgets.QTreeView, point: QtCore.QPoint = None ) -> None: