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
This commit is contained in:
+132
-70
@@ -588,16 +588,12 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
|
|||||||
|
|
||||||
# We build the menu.
|
# We build the menu.
|
||||||
menu = QtWidgets.QMenu()
|
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
|
# 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:
|
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))
|
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.
|
point (QPoint): The point in the tree where the playlist was right-clicked.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
# Get the playlist
|
# Get and validate the playlist
|
||||||
item = self.tr_lists_user.itemAt(point)
|
item = self.tr_lists_user.itemAt(point)
|
||||||
media_list = get_user_list_media_item(item)
|
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)
|
media_items = items_results_all(media_list)
|
||||||
|
|
||||||
# Extract unique album IDs from tracks
|
# Extract unique album IDs from tracks
|
||||||
album_ids = {}
|
album_ids = self._extract_album_ids_from_tracks(media_items)
|
||||||
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
|
|
||||||
|
|
||||||
if not album_ids:
|
if not album_ids:
|
||||||
logger_gui.warning("No albums found in this playlist.")
|
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...")
|
logger_gui.info(f"Found {len(album_ids)} unique albums. Loading with rate limiting...")
|
||||||
|
|
||||||
# Load albums with rate limiting
|
# Load albums with rate limiting
|
||||||
albums_dict = {}
|
albums_dict = self._load_albums_with_rate_limiting(album_ids)
|
||||||
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
|
|
||||||
|
|
||||||
if not albums_dict:
|
if not albums_dict:
|
||||||
logger_gui.error("Failed to load any albums from playlist.")
|
logger_gui.error("Failed to load any albums from playlist.")
|
||||||
return
|
return
|
||||||
|
|
||||||
# Prepare all queue items first
|
# Prepare and queue albums
|
||||||
logger_gui.info(f"Successfully loaded {len(albums_dict)} albums. Preparing queue items...")
|
self._queue_loaded_albums(albums_dict)
|
||||||
|
|
||||||
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}")
|
|
||||||
|
|
||||||
# Show confirmation
|
# 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))
|
self.s_statusbar_message.emit(StatusbarMessage(message=message, timeout=3000))
|
||||||
logger_gui.info(message)
|
logger_gui.info(message)
|
||||||
|
|
||||||
@@ -750,6 +689,129 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow):
|
|||||||
logger_gui.error(error_msg)
|
logger_gui.error(error_msg)
|
||||||
self.s_statusbar_message.emit(StatusbarMessage(message=error_msg, timeout=3000))
|
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(
|
def on_copy_url_share(
|
||||||
self, tree_target: QtWidgets.QTreeWidget | QtWidgets.QTreeView, point: QtCore.QPoint = None
|
self, tree_target: QtWidgets.QTreeWidget | QtWidgets.QTreeView, point: QtCore.QPoint = None
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user