From 219a36c05bf4f63bca09b58ace893fca9166ff1e Mon Sep 17 00:00:00 2001 From: Broque Thomas Date: Tue, 9 Sep 2025 18:28:29 -0700 Subject: [PATCH] Improve wishlist processing and modal sync logic Sanitizes wishlist track data for consistent backend/frontend handling, enhances wishlist process state reporting for better UI sync, and refactors frontend modal logic to prevent auto-show conflicts and ensure user-driven visibility. Also improves polling and rehydration to keep frontend state in sync with server-side auto-processing. --- web_server.py | 112 +++++++++++++-- webui/static/script.js | 301 ++++++++++++++++++++++++----------------- 2 files changed, 279 insertions(+), 134 deletions(-) diff --git a/web_server.py b/web_server.py index ec823f3d..1d79b064 100644 --- a/web_server.py +++ b/web_server.py @@ -4641,10 +4641,57 @@ def start_simple_background_monitor(): # == AUTOMATIC WISHLIST PROCESSING == # =============================== +def _sanitize_track_data_for_processing(track_data): + """ + Sanitizes track data from wishlist service to ensure consistent format. + Handles album field conversion from dict to string and artist field normalization. + """ + if not isinstance(track_data, dict): + print(f"⚠️ [Sanitize] Unexpected track data type: {type(track_data)}") + return track_data + + # Create a copy to avoid modifying original data + sanitized = track_data.copy() + + # Handle album field - convert dictionary to string if needed + raw_album = sanitized.get('album', '') + if isinstance(raw_album, dict) and 'name' in raw_album: + sanitized['album'] = raw_album['name'] + print(f"🔧 [Sanitize] Converted album from dict to string: '{raw_album['name']}'") + elif not isinstance(raw_album, str): + sanitized['album'] = str(raw_album) + print(f"🔧 [Sanitize] Converted album to string: '{sanitized['album']}'") + + # Handle artists field - ensure it's a list of strings + raw_artists = sanitized.get('artists', []) + if isinstance(raw_artists, list): + processed_artists = [] + for artist in raw_artists: + if isinstance(artist, str): + processed_artists.append(artist) + elif isinstance(artist, dict) and 'name' in artist: + processed_artists.append(artist['name']) + print(f"🔧 [Sanitize] Converted artist from dict to string: '{artist['name']}'") + else: + processed_artists.append(str(artist)) + print(f"🔧 [Sanitize] Converted artist to string: '{str(artist)}'") + sanitized['artists'] = processed_artists + else: + print(f"⚠️ [Sanitize] Unexpected artists format: {type(raw_artists)}") + sanitized['artists'] = [str(raw_artists)] if raw_artists else [] + + return sanitized + def start_wishlist_auto_processing(): """Start automatic wishlist processing with 1-minute initial delay.""" global wishlist_auto_timer + # Skip if this is Flask's debug reloader process + import os + if os.environ.get('WERKZEUG_RUN_MAIN') != 'true': + print("🔄 Skipping auto-processing start in Flask debug reloader") + return + with wishlist_timer_lock: # Stop any existing timer to prevent duplicates if wishlist_auto_timer is not None: @@ -4721,14 +4768,22 @@ def _process_wishlist_automatically(): return # Get wishlist tracks for processing - wishlist_tracks = wishlist_service.get_wishlist_tracks_for_download() - if not wishlist_tracks: + raw_wishlist_tracks = wishlist_service.get_wishlist_tracks_for_download() + if not raw_wishlist_tracks: print("⚠️ No tracks returned from wishlist service.") with wishlist_timer_lock: wishlist_auto_processing = False schedule_next_wishlist_processing() return + # SANITIZE: Ensure consistent data format from wishlist service + wishlist_tracks = [] + for track in raw_wishlist_tracks: + sanitized_track = _sanitize_track_data_for_processing(track) + wishlist_tracks.append(sanitized_track) + + print(f"🔧 [Auto-Wishlist] Sanitized {len(wishlist_tracks)} tracks from wishlist service") + # Create batch for automatic processing batch_id = str(uuid.uuid4()) playlist_name = "Wishlist (Auto)" @@ -4857,8 +4912,15 @@ def get_wishlist_tracks(): try: from core.wishlist_service import get_wishlist_service wishlist_service = get_wishlist_service() - tracks = wishlist_service.get_wishlist_tracks_for_download() - return jsonify({"tracks": tracks}) + raw_tracks = wishlist_service.get_wishlist_tracks_for_download() + + # SANITIZE: Ensure consistent data format for frontend + sanitized_tracks = [] + for track in raw_tracks: + sanitized_track = _sanitize_track_data_for_processing(track) + sanitized_tracks.append(sanitized_track) + + return jsonify({"tracks": sanitized_tracks}) except Exception as e: print(f"Error getting wishlist tracks: {e}") return jsonify({"error": str(e)}), 500 @@ -4874,10 +4936,18 @@ def start_wishlist_missing_downloads(): wishlist_service = get_wishlist_service() # Get wishlist tracks formatted for download modal - wishlist_tracks = wishlist_service.get_wishlist_tracks_for_download() - if not wishlist_tracks: + raw_wishlist_tracks = wishlist_service.get_wishlist_tracks_for_download() + if not raw_wishlist_tracks: return jsonify({"success": False, "error": "No tracks in wishlist"}), 400 + # SANITIZE: Ensure consistent data format from wishlist service + wishlist_tracks = [] + for track in raw_wishlist_tracks: + sanitized_track = _sanitize_track_data_for_processing(track) + wishlist_tracks.append(sanitized_track) + + print(f"🔧 [Manual-Wishlist] Sanitized {len(wishlist_tracks)} tracks from wishlist service") + batch_id = str(uuid.uuid4()) # Use "wishlist" as the playlist_id for consistency in the modal system @@ -6418,13 +6488,28 @@ def get_active_processes(): with tasks_lock: for batch_id, batch_data in download_batches.items(): if batch_data.get('phase') not in ['complete', 'error', 'cancelled']: - active_processes.append({ + process_info = { "type": "batch", "playlist_id": batch_data.get('playlist_id'), "playlist_name": batch_data.get('playlist_name'), "batch_id": batch_id, "phase": batch_data.get('phase') - }) + } + + # Enhanced wishlist information for better frontend state management + if batch_data.get('playlist_id') == 'wishlist': + process_info.update({ + "auto_initiated": batch_data.get('auto_initiated', False), + "auto_processing_timestamp": batch_data.get('auto_processing_timestamp'), + "should_show_modal": True, # Wishlist processes should always be visible + "is_background_process": batch_data.get('auto_initiated', False) + }) + + # Add current auto-processing state for frontend awareness + with wishlist_timer_lock: + process_info["auto_processing_active"] = wishlist_auto_processing + + active_processes.append(process_info) # Add YouTube playlists in non-fresh phases for rehydration for url_hash, state in youtube_playlist_states.items(): @@ -8759,12 +8844,21 @@ def _run_sync_task(playlist_id, playlist_name, tracks_json): print(f"🔄 Converting JSON tracks to SpotifyTrack objects...") tracks = [] for i, t in enumerate(tracks_json): + # Handle album field - extract name if it's a dictionary + raw_album = t.get('album', '') + if isinstance(raw_album, dict) and 'name' in raw_album: + album_name = raw_album['name'] + elif isinstance(raw_album, str): + album_name = raw_album + else: + album_name = str(raw_album) + # Create SpotifyTrack objects with proper default values for missing fields track = SpotifyTrack( id=t.get('id', ''), # Provide default empty string name=t.get('name', ''), artists=t.get('artists', []), - album=t.get('album', ''), + album=album_name, duration_ms=t.get('duration_ms', 0), popularity=t.get('popularity', 0), # Default value preview_url=t.get('preview_url'), diff --git a/webui/static/script.js b/webui/static/script.js index 63ffae4f..e05ff9df 100644 --- a/webui/static/script.js +++ b/webui/static/script.js @@ -1975,41 +1975,69 @@ async function rehydrateModal(processInfo, userRequested = false) { // Handle wishlist processes specially if (playlist_id === "wishlist") { - console.log(`🔍 Current activeDownloadProcesses keys: [${Object.keys(activeDownloadProcesses).join(', ')}]`); + console.log(`💧 [Rehydrate] Handling wishlist modal for active process: ${batch_id}`); - await openDownloadMissingWishlistModal(); - const process = activeDownloadProcesses[playlist_id]; - if (!process) { - console.error('❌ Failed to create wishlist process in activeDownloadProcesses'); - return; - } - - console.log(`✅ Setting wishlist process state - batchId: ${batch_id}, status: running`); - process.status = 'running'; - process.batchId = batch_id; - // Note: Wishlist processes don't affect sync page refresh button state - - const beginBtn = document.getElementById(`begin-analysis-btn-${playlist_id}`); - const cancelBtn = document.getElementById(`cancel-all-btn-${playlist_id}`); + // Check if modal already exists and is visible + const existingProcess = activeDownloadProcesses[playlist_id]; + const modalAlreadyOpen = existingProcess && existingProcess.modalElement && + existingProcess.modalElement.style.display === 'flex'; - if (beginBtn) beginBtn.style.display = 'none'; - if (cancelBtn) cancelBtn.style.display = 'inline-block'; - - startModalDownloadPolling(playlist_id); - - // Enhanced logic: Show modal if user-requested OR if it was previously visible before page refresh - const wasModalPreviouslyVisible = WishlistModalState.wasVisible(); - const shouldShowModal = userRequested || (wasModalPreviouslyVisible && !WishlistModalState.wasUserClosed()); - - if (shouldShowModal) { - process.modalElement.style.display = 'flex'; - WishlistModalState.setVisible(); // Track that modal is now visible - WishlistModalState.clearUserClosed(); // Clear any user closed state since we're showing it - console.log(`🔍 Showing wishlist modal - User requested: ${userRequested}, Previously visible: ${wasModalPreviouslyVisible}`); + if (modalAlreadyOpen) { + console.log(`💧 [Rehydrate] Wishlist modal already open - updating existing modal with auto-process state`); + + // Update existing process with new batch info + existingProcess.status = 'running'; + existingProcess.batchId = batch_id; + + // Update UI to reflect running state + const beginBtn = document.getElementById(`begin-analysis-btn-${playlist_id}`); + const cancelBtn = document.getElementById(`cancel-all-btn-${playlist_id}`); + if (beginBtn) beginBtn.style.display = 'none'; + if (cancelBtn) cancelBtn.style.display = 'inline-block'; + + // Ensure polling is active for live updates + if (!existingProcess.intervalId) { + console.log(`💧 [Rehydrate] Starting polling for existing modal`); + startModalDownloadPolling(playlist_id); + } + + console.log(`✅ [Rehydrate] Successfully updated existing wishlist modal for auto-process`); } else { - process.modalElement.style.display = 'none'; - WishlistModalState.setHidden(); // Track that modal is hidden - console.log('🔍 Hiding wishlist modal for background rehydration'); + console.log(`💧 [Rehydrate] Creating new wishlist modal for active process: ${batch_id}`); + + // Create the modal with current server state + await openDownloadMissingWishlistModal(); + const process = activeDownloadProcesses[playlist_id]; + if (!process) { + console.error('❌ [Rehydrate] Failed to create wishlist process in activeDownloadProcesses'); + return; + } + + // Sync process state with server + console.log(`✅ [Rehydrate] Syncing wishlist process state - batchId: ${batch_id}, status: running`); + process.status = 'running'; + process.batchId = batch_id; + + // Update UI to reflect running state + const beginBtn = document.getElementById(`begin-analysis-btn-${playlist_id}`); + const cancelBtn = document.getElementById(`cancel-all-btn-${playlist_id}`); + if (beginBtn) beginBtn.style.display = 'none'; + if (cancelBtn) cancelBtn.style.display = 'inline-block'; + + // Start polling for live updates + startModalDownloadPolling(playlist_id); + + // SIMPLIFIED VISIBILITY LOGIC: Show modal if user requested it, otherwise keep hidden for background sync + if (userRequested) { + console.log('👤 [Rehydrate] User requested - showing wishlist modal'); + process.modalElement.style.display = 'flex'; + WishlistModalState.setVisible(); + WishlistModalState.clearUserClosed(); + } else { + console.log('🔄 [Rehydrate] Background sync - keeping modal hidden until user interaction'); + process.modalElement.style.display = 'none'; + WishlistModalState.setHidden(); + } } return; } @@ -3524,14 +3552,41 @@ function startGlobalDownloadPolling() { // Get all active processes that need polling const activeBatchIds = []; const batchToPlaylistMap = {}; + let hasOpenWishlistModal = false; Object.entries(activeDownloadProcesses).forEach(([playlistId, process]) => { if (process.batchId && process.status === 'running') { activeBatchIds.push(process.batchId); batchToPlaylistMap[process.batchId] = playlistId; } + + // Check if there's an open wishlist modal (visible and idle/waiting) + if (playlistId === 'wishlist' && process.modalElement && + process.modalElement.style.display === 'flex' && + (!process.batchId || process.status !== 'running')) { + hasOpenWishlistModal = true; + } }); + // Special handling for open wishlist modal - check for new auto-processing + if (hasOpenWishlistModal) { + try { + const response = await fetch('/api/active-processes'); + if (response.ok) { + const data = await response.json(); + const processes = data.active_processes || []; + const serverWishlistProcess = processes.find(p => p.playlist_id === 'wishlist'); + + if (serverWishlistProcess) { + console.log('🔄 [Global Polling] Detected auto-processing for open wishlist modal - rehydrating'); + await rehydrateModal(serverWishlistProcess, false); // false = not user-requested + } + } + } catch (error) { + console.debug('⚠️ [Global Polling] Failed to check for wishlist auto-processing:', error); + } + } + if (activeBatchIds.length === 0) { console.debug('📊 [Global Polling] No active processes, continuing polling'); return; @@ -3609,14 +3664,41 @@ function startGlobalDownloadPollingWithInterval(interval) { globalDownloadStatusPoller = setInterval(async () => { const activeBatchIds = []; const batchToPlaylistMap = {}; + let hasOpenWishlistModal = false; Object.entries(activeDownloadProcesses).forEach(([playlistId, process]) => { if (process.batchId && process.status === 'running') { activeBatchIds.push(process.batchId); batchToPlaylistMap[process.batchId] = playlistId; } + + // Check if there's an open wishlist modal (visible and idle/waiting) + if (playlistId === 'wishlist' && process.modalElement && + process.modalElement.style.display === 'flex' && + (!process.batchId || process.status !== 'running')) { + hasOpenWishlistModal = true; + } }); + // Special handling for open wishlist modal - check for new auto-processing + if (hasOpenWishlistModal) { + try { + const response = await fetch('/api/active-processes'); + if (response.ok) { + const data = await response.json(); + const processes = data.active_processes || []; + const serverWishlistProcess = processes.find(p => p.playlist_id === 'wishlist'); + + if (serverWishlistProcess) { + console.log('🔄 [Global Polling] Detected auto-processing for open wishlist modal - rehydrating'); + await rehydrateModal(serverWishlistProcess, false); // false = not user-requested + } + } + } catch (error) { + console.debug('⚠️ [Global Polling] Failed to check for wishlist auto-processing:', error); + } + } + if (activeBatchIds.length === 0) { console.debug('📊 [Global Polling] No active processes, continuing polling'); return; @@ -3701,16 +3783,8 @@ function processModalStatusUpdate(playlistId, data) { console.debug(`📊 [Status Update] Processing update for ${playlistId}: phase=${data.phase}, tasks=${(data.tasks || []).length}`); - // Auto-show wishlist modal during active auto-processing - const isWishlist = (playlistId === 'wishlist'); - const isAutoInitiated = data.auto_initiated || false; - const isModalHidden = process.modalElement && process.modalElement.style.display === 'none'; - - if (isWishlist && isAutoInitiated && isModalHidden && currentPage === 'dashboard' && !WishlistModalState.wasUserClosed()) { - console.log('🤖 [Status Update] Auto-showing wishlist modal during active auto-processing'); - process.modalElement.style.display = 'flex'; - WishlistModalState.setVisible(); - } + // Note: Wishlist modal visibility is now managed by handleWishlistButtonClick() only + // Auto-show logic has been simplified to prevent conflicts if (data.phase === 'analysis') { const progress = data.analysis_progress; @@ -3859,13 +3933,7 @@ function processModalStatusUpdate(playlistId, data) { const isAutoInitiated = data.auto_initiated || false; // Server indicates if batch was auto-started const isBackgroundWishlist = isWishlist && (isModalHidden || isAutoInitiated); - // Auto-show modal for wishlist auto-processing if user is on dashboard and hasn't closed it - if (isWishlist && isAutoInitiated && isModalHidden && currentPage === 'dashboard' && !WishlistModalState.wasUserClosed()) { - console.log('🤖 [Status Update] Auto-showing wishlist modal for live updates during auto-processing'); - process.modalElement.style.display = 'flex'; - WishlistModalState.setVisible(); - showToast('Auto-processing wishlist - showing live updates', 'info', 2000); - } + // Note: Auto-show logic removed - wishlist modal visibility managed by user interaction only if (data.phase === 'cancelled') { process.status = 'cancelled'; @@ -6919,34 +6987,19 @@ async function checkForAutoInitiatedWishlistProcess() { if (serverWishlistProcess && serverWishlistProcess.auto_initiated) { console.log('🤖 [Auto-Processing] Detected auto-initiated wishlist process during polling'); - // Check if we need to show the modal (no existing modal or modal is hidden) - const needsModalDisplay = !clientWishlistProcess || + // Only sync frontend state if needed, but don't auto-show modal + const needsSync = !clientWishlistProcess || + clientWishlistProcess.batchId !== serverWishlistProcess.batch_id || !clientWishlistProcess.modalElement || - clientWishlistProcess.modalElement.style.display === 'none' || !document.body.contains(clientWishlistProcess.modalElement); - if (needsModalDisplay) { - console.log('🤖 [Auto-Processing] Auto-showing wishlist modal for auto-processing'); - - // Rehydrate or create the modal - if (clientWishlistProcess && clientWishlistProcess.modalElement && document.body.contains(clientWishlistProcess.modalElement)) { - // Just show existing modal - clientWishlistProcess.modalElement.style.display = 'flex'; - WishlistModalState.setVisible(); - } else { - // Rehydrate the modal with userRequested=false but show it due to auto-processing - await rehydrateModal(serverWishlistProcess, false); // Don't mark as user-requested - // But then force show it since it's auto-initiated and should be visible - if (activeDownloadProcesses[playlistId] && activeDownloadProcesses[playlistId].modalElement) { - activeDownloadProcesses[playlistId].modalElement.style.display = 'flex'; - WishlistModalState.setVisible(); - console.log('🤖 [Auto-Processing] Forced display of rehydrated modal for auto-processing'); - } - } - - // Show a toast to inform user about auto-processing - showToast('Auto-processing wishlist tracks...', 'info', 3000); + if (needsSync) { + console.log('🔄 [Auto-Processing] Syncing frontend state for auto-processing (background mode)'); + await rehydrateModal(serverWishlistProcess, false); // Background sync only } + + // Note: Modal visibility is controlled by user interaction only + // User must click wishlist button to see auto-processing progress } } catch (error) { @@ -8369,70 +8422,68 @@ async function handleWishlistButtonClick() { try { const playlistId = 'wishlist'; - // Always check server state first to detect any active wishlist processes + console.log('🎵 [Wishlist Button] User clicked wishlist button - checking server state first'); + + // STEP 1: Always check server state first to detect any active wishlist processes const response = await fetch('/api/active-processes'); - if (response.ok) { - const data = await response.json(); - const processes = data.active_processes || []; - - // Look for active wishlist process on server - const serverWishlistProcess = processes.find(p => p.playlist_id === playlistId); - const clientWishlistProcess = activeDownloadProcesses[playlistId]; - - if (serverWishlistProcess) { - console.log('Server has active wishlist process:', serverWishlistProcess.batch_id); - - // Check if this is an auto-initiated batch (from auto-processing) - const isAutoInitiated = serverWishlistProcess.auto_initiated || false; - if (isAutoInitiated) { - console.log('🤖 [Auto-Processing] Detected auto-initiated wishlist batch'); - // Clear any user closed state since user is actively requesting to see it - WishlistModalState.clearUserClosed(); - } - - // Check if client state is properly synchronized with server - const isClientStateMismatched = !clientWishlistProcess || - clientWishlistProcess.batchId !== serverWishlistProcess.batch_id || - clientWishlistProcess.status !== 'running'; - - const isModalElementMissing = !clientWishlistProcess?.modalElement || - !document.body.contains(clientWishlistProcess.modalElement); - - if (isClientStateMismatched || isModalElementMissing) { - console.log('Server/client wishlist state mismatch or missing modal, rehydrating...'); - console.log('Client batch ID:', clientWishlistProcess?.batchId, 'Server batch ID:', serverWishlistProcess.batch_id); - console.log('Client status:', clientWishlistProcess?.status, 'Modal exists:', !isModalElementMissing); - - await rehydrateModal(serverWishlistProcess, true); // Force rehydration with user request - return; - } else { - // Client state is properly synchronized, just show existing modal - console.log('Found properly synced wishlist process, showing modal'); - if (clientWishlistProcess.modalElement) { - clientWishlistProcess.modalElement.style.display = 'flex'; - WishlistModalState.setVisible(); // Track that modal is now visible - return; - } - } - } + if (!response.ok) { + throw new Error(`Failed to fetch active processes: ${response.status}`); } - // No active wishlist process on server, proceed with normal modal creation - // First check if there are any tracks in the wishlist - const countResponse = await fetch('/api/wishlist/count'); - const countData = await countResponse.json(); + const data = await response.json(); + const processes = data.active_processes || []; + const serverWishlistProcess = processes.find(p => p.playlist_id === playlistId); + // STEP 2: Handle active server process - show current state immediately + if (serverWishlistProcess) { + console.log('🎯 [Wishlist Button] Server has active wishlist process:', { + batch_id: serverWishlistProcess.batch_id, + phase: serverWishlistProcess.phase, + auto_initiated: serverWishlistProcess.auto_initiated, + should_show: serverWishlistProcess.should_show_modal + }); + + // Clear any user-closed state since user explicitly requested to see modal + WishlistModalState.clearUserClosed(); + + // Check if we need to create/sync the frontend modal + const clientWishlistProcess = activeDownloadProcesses[playlistId]; + const needsRehydration = !clientWishlistProcess || + clientWishlistProcess.batchId !== serverWishlistProcess.batch_id || + !clientWishlistProcess.modalElement || + !document.body.contains(clientWishlistProcess.modalElement); + + if (needsRehydration) { + console.log('🔄 [Wishlist Button] Frontend modal needs sync/creation'); + await rehydrateModal(serverWishlistProcess, true); // user-requested = true + } else { + console.log('✅ [Wishlist Button] Frontend modal already synced, showing existing modal'); + clientWishlistProcess.modalElement.style.display = 'flex'; + WishlistModalState.setVisible(); + } + return; + } + + // STEP 3: No active server process - check wishlist count and create fresh modal + console.log('📭 [Wishlist Button] No active server process, checking wishlist content'); + + const countResponse = await fetch('/api/wishlist/count'); + if (!countResponse.ok) { + throw new Error(`Failed to fetch wishlist count: ${countResponse.status}`); + } + + const countData = await countResponse.json(); if (countData.count === 0) { showToast('Wishlist is empty. No tracks to download.', 'info'); return; } - // Open the wishlist download modal for new process - console.log('No active server process, creating new wishlist modal'); + // STEP 4: Create fresh modal for new wishlist process + console.log(`🆕 [Wishlist Button] Creating fresh modal for ${countData.count} wishlist tracks`); await openDownloadMissingWishlistModal(); } catch (error) { - console.error('Error handling wishlist button click:', error); + console.error('❌ [Wishlist Button] Error handling wishlist button click:', error); showToast(`Error opening wishlist: ${error.message}`, 'error'); } }