From 9646f6ca7f76c6ffe53dcc5d72263eb062c044ce Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 10:36:50 +0300 Subject: [PATCH 1/8] Clarify Spotify auth actions - Hide the auth button when a Spotify session is active - Treat disconnect as a session change, not a provider swap - Share metadata source labels in the registry - Tighten rate-limit copy around Spotify-specific behavior --- core/metadata/registry.py | 18 +++++++++++++ tests/conftest.py | 2 +- tests/metadata/test_metadata_registry.py | 30 +++++++++++++++++++++ tests/test_websocket_infrastructure.py | 1 + web_server.py | 30 ++++++++++++++------- webui/index.html | 14 ++++------ webui/static/settings.js | 34 +++++------------------- webui/static/shared-helpers.js | 9 +++++-- 8 files changed, 88 insertions(+), 50 deletions(-) create mode 100644 tests/metadata/test_metadata_registry.py diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 595c8549..d1162b35 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -18,6 +18,13 @@ logger = get_logger("metadata.registry") MetadataClientFactory = Callable[[], Any] METADATA_SOURCE_PRIORITY = ("deezer", "itunes", "spotify", "discogs", "hydrabase") +METADATA_SOURCE_LABELS = { + "spotify": "Spotify", + "itunes": "iTunes", + "deezer": "Deezer", + "discogs": "Discogs", + "hydrabase": "Hydrabase", +} _UNSET = object() _client_cache_lock = threading.RLock() @@ -293,6 +300,17 @@ def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = return source +def get_spotify_disconnect_source() -> str: + """Return the active metadata source after Spotify is disconnected.""" + source = get_primary_source() + return "deezer" if source == "spotify" else source + + +def get_metadata_source_label(source: str) -> str: + """Return a human-readable label for a metadata source.""" + return METADATA_SOURCE_LABELS.get(source, source.replace("_", " ").title()) + + def get_source_priority(preferred_source: str): """Return source priority with preferred source first.""" ordered = [] diff --git a/tests/conftest.py b/tests/conftest.py index a9976c6b..390252ea 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,7 +17,7 @@ from flask_socketio import SocketIO, join_room, leave_room # --------------------------------------------------------------------------- _DEFAULT_STATUS_CACHE = { - 'spotify': {'connected': True, 'response_time': 12.5, 'source': 'spotify'}, + 'spotify': {'connected': True, 'authenticated': True, 'response_time': 12.5, 'source': 'spotify'}, 'media_server': {'connected': True, 'response_time': 8.1, 'type': 'plex'}, 'soulseek': {'connected': True, 'response_time': 5.3, 'source': 'soulseek'}, } diff --git a/tests/metadata/test_metadata_registry.py b/tests/metadata/test_metadata_registry.py new file mode 100644 index 00000000..ea3a33d0 --- /dev/null +++ b/tests/metadata/test_metadata_registry.py @@ -0,0 +1,30 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from core.metadata import registry + + +def test_spotify_disconnect_source_uses_deezer_when_spotify_is_primary(monkeypatch): + monkeypatch.setattr(registry, "get_primary_source", lambda: "spotify") + + assert registry.get_spotify_disconnect_source() == "deezer" + + +def test_spotify_disconnect_source_keeps_non_spotify_primary(monkeypatch): + monkeypatch.setattr(registry, "get_primary_source", lambda: "discogs") + + assert registry.get_spotify_disconnect_source() == "discogs" + + +def test_metadata_source_label_maps_known_sources(): + assert registry.get_metadata_source_label("spotify") == "Spotify" + assert registry.get_metadata_source_label("itunes") == "iTunes" + assert registry.get_metadata_source_label("deezer") == "Deezer" + assert registry.get_metadata_source_label("discogs") == "Discogs" + assert registry.get_metadata_source_label("hydrabase") == "Hydrabase" + + +def test_metadata_source_label_falls_back_to_title_case(): + assert registry.get_metadata_source_label("apple_music") == "Apple Music" diff --git a/tests/test_websocket_infrastructure.py b/tests/test_websocket_infrastructure.py index 8d1f4658..392f80aa 100644 --- a/tests/test_websocket_infrastructure.py +++ b/tests/test_websocket_infrastructure.py @@ -56,6 +56,7 @@ class TestServiceStatus: assert 'media_server' in data assert 'soulseek' in data assert 'active_media_server' in data + assert 'authenticated' in data['spotify'] def test_status_matches_http(self, test_app, shared_state): """Socket event data matches HTTP endpoint response exactly.""" diff --git a/web_server.py b/web_server.py index ace196fc..d42781f8 100644 --- a/web_server.py +++ b/web_server.py @@ -98,7 +98,9 @@ from core.metadata.cache import get_metadata_cache from core.metadata import registry as metadata_registry from core.metadata.registry import ( clear_cached_metadata_client, + get_metadata_source_label, get_spotify_client, + get_spotify_disconnect_source, register_runtime_clients, ) from core.imports.context import ( @@ -800,11 +802,11 @@ _idle_since = {} _IDLE_GRACE_SECONDS = 5 _status_cache = { - 'spotify': {'connected': False, 'response_time': 0, 'source': 'itunes'}, + 'spotify': {'connected': False, 'authenticated': False, 'response_time': 0, 'source': 'itunes'}, 'media_server': {'connected': False, 'response_time': 0, 'type': None}, 'soulseek': {'connected': False, 'response_time': 0}, } -_status_cache_timestamps = { +_status_cache_timestamps: dict[str, float] = { 'spotify': 0, 'media_server': 0, 'soulseek': 0, @@ -3424,6 +3426,7 @@ def get_status(): is_rate_limited = spotify_client.is_rate_limited() if spotify_client else False rate_limit_info = spotify_client.get_rate_limit_info() if (spotify_client and is_rate_limited) else None cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0 + spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) # Read configured source once — no auth validation here, we do that explicitly below configured_source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer' @@ -3445,6 +3448,7 @@ def get_status(): _status_cache['spotify'] = { 'connected': True, # Always true — iTunes fallback is always available + 'authenticated': spotify_session_active, 'response_time': round(spotify_response_time, 1), 'source': music_source, 'rate_limited': is_rate_limited, @@ -4773,7 +4777,9 @@ def test_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': + spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) _status_cache['spotify']['connected'] = True + _status_cache['spotify']['authenticated'] = spotify_session_active _status_cache['spotify']['source'] = _get_metadata_fallback_source() _status_cache_timestamps['spotify'] = current_time logger.info("Updated Spotify status cache after successful test") @@ -4939,7 +4945,9 @@ def test_dashboard_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': + spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) _status_cache['spotify']['connected'] = True + _status_cache['spotify']['authenticated'] = spotify_session_active _status_cache['spotify']['source'] = _get_metadata_fallback_source() _status_cache_timestamps['spotify'] = current_time logger.info("Updated Spotify status cache after successful dashboard test") @@ -5838,7 +5846,7 @@ def spotify_callback(): @app.route('/api/spotify/disconnect', methods=['POST']) def spotify_disconnect(): - """Disconnect Spotify and fall back to iTunes/Apple Music""" + """Disconnect Spotify and keep using the active primary metadata source.""" global spotify_client try: # Pause enrichment worker before disconnecting to prevent it from hammering API @@ -5846,18 +5854,20 @@ def spotify_disconnect(): spotify_enrichment_worker.pause() spotify_client.disconnect() # Immediately update status cache so UI reflects the change - fallback_src = _get_metadata_fallback_source() + active_source = get_spotify_disconnect_source() + source_label = get_metadata_source_label(active_source) _status_cache['spotify'] = { - 'connected': True, # Fallback source is always available + 'connected': False, + 'authenticated': False, 'response_time': 0, - 'source': fallback_src, + 'source': active_source, 'rate_limited': False, - 'rate_limit': None + 'rate_limit': None, + 'post_ban_cooldown': None } _status_cache_timestamps['spotify'] = time.time() - fallback_label = 'Deezer' if fallback_src == 'deezer' else 'Discogs' if fallback_src == 'discogs' else 'iTunes' - add_activity_item("", "Spotify Disconnected", f"Switched to {fallback_label} metadata source", "Now") - return jsonify({'success': True, 'message': f'Spotify disconnected. Now using {fallback_label}.'}) + add_activity_item("", "Spotify Disconnected", f"Using {source_label} for metadata", "Now") + return jsonify({'success': True, 'message': f'Spotify disconnected. Using {source_label} for metadata.', 'source': active_source, 'authenticated': False}) except Exception as e: logger.error(f"Error disconnecting Spotify: {e}") return jsonify({'success': False, 'error': str(e)}), 500 diff --git a/webui/index.html b/webui/index.html index 87fe7938..7cd8bb17 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3693,7 +3693,7 @@

Metadata Source

- +
-
The primary source for artist, album, and track metadata. Spotify requires authentication below. Discogs requires a personal token.
+
Choose the primary source for artist, album, and track metadata. Spotify auth is optional and only needed for Spotify-specific actions. Discogs requires a personal token.
@@ -3741,10 +3741,6 @@ - @@ -7864,13 +7860,13 @@ — -

You can wait for the ban to expire (the app uses Apple Music in the meantime) or disconnect Spotify to clear the ban immediately.

+

While rate limiting is active, Spotify-specific features are unavailable. You can wait for the ban to expire or disconnect Spotify to clear it immediately.

diff --git a/webui/static/settings.js b/webui/static/settings.js index 4f2ade67..cd74a3e8 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -3066,8 +3066,10 @@ async function authenticateSpotify() { } async function disconnectSpotify() { - const fallbackName = currentMusicSourceName !== 'Spotify' ? currentMusicSourceName : 'the configured fallback source'; - if (!await showConfirmDialog({ title: 'Disconnect Spotify', message: `Disconnect Spotify? The app will switch to ${fallbackName} for metadata.` })) { + if (!await showConfirmDialog({ + title: 'Disconnect Spotify', + message: 'Disconnect Spotify? Spotify-specific actions will stop until you reauthenticate.' + })) { return; } try { @@ -3075,7 +3077,7 @@ async function disconnectSpotify() { const response = await fetch('/api/spotify/disconnect', { method: 'POST' }); const data = await response.json(); if (data.success) { - showToast(`Spotify disconnected. Now using ${fallbackName}.`, 'success'); + showToast(data.message || 'Spotify disconnected.', 'success'); // Immediately refresh status to update UI await fetchAndUpdateServiceStatus(); } else { @@ -3089,29 +3091,6 @@ async function disconnectSpotify() { } } -async function clearSpotifyCacheAndFallback() { - const fallbackName = currentMusicSourceName !== 'Spotify' ? currentMusicSourceName : 'the configured fallback source'; - if (!await showConfirmDialog({ - title: 'Clear Spotify Cache', - message: `This will clear the Spotify token cache and switch metadata to ${fallbackName}. You can re-authenticate later.` - })) return; - try { - showLoadingOverlay('Clearing Spotify cache...'); - const response = await fetch('/api/spotify/disconnect', { method: 'POST' }); - const data = await response.json(); - if (data.success) { - showToast(data.message || `Switched to ${fallbackName}`, 'success'); - await fetchAndUpdateServiceStatus(); - } else { - showToast(`Failed: ${data.error}`, 'error'); - } - } catch (error) { - showToast('Failed to clear Spotify cache', 'error'); - } finally { - hideLoadingOverlay(); - } -} - // ── Spotify Rate Limit Handling ─────────────────────────────────────────── let _spotifyRateLimitShown = false; let _spotifyInCooldown = false; @@ -3198,7 +3177,7 @@ async function disconnectSpotifyFromRateLimit() { const data = await response.json(); if (data.success) { _spotifyRateLimitShown = false; - showToast(`Spotify disconnected. Now using ${currentMusicSourceName}.`, 'success'); + showToast(data.message || 'Spotify disconnected.', 'success'); await fetchAndUpdateServiceStatus(); if (currentPage === 'discover') { loadDiscoverPage(); @@ -3879,4 +3858,3 @@ function togglePathLock(pathType, btn) { // =============================== - diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 26b1c060..a0f822a0 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3194,10 +3194,15 @@ function updateServiceStatus(service, statusData) { currentMusicSourceName = sourceName; } - // Show/hide Spotify disconnect button based on connection state + // Keep the Spotify action buttons aligned with the actual auth session. + const spotifySessionActive = statusData.authenticated === true || (statusData.authenticated === undefined && statusData.source === 'spotify'); + const authBtn = document.querySelector('button[onclick="authenticateSpotify()"]'); const disconnectBtn = document.getElementById('spotify-disconnect-btn'); + if (authBtn) { + authBtn.style.display = spotifySessionActive ? 'none' : ''; + } if (disconnectBtn) { - disconnectBtn.style.display = statusData.source === 'spotify' ? '' : 'none'; + disconnectBtn.style.display = spotifySessionActive ? '' : 'none'; } } From 55603be14c482871867da1b03d64f76543a82c11 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 11:23:00 +0300 Subject: [PATCH 2/8] Clarify Spotify auth flow and sync UI - Send Spotify auth completion back to the opener so the settings page refreshes immediately - Make the local auth flow go straight through to Spotify instead of showing the temporary instruction page - Keep the remote/docker instruction page available for manual callback setups - Sync Spotify status, connect/disconnect buttons, and metadata source selection after auth and disconnect - Keep the disconnect behavior aligned with the active primary metadata source --- core/metadata/registry.py | 5 +- tests/metadata/test_metadata_registry.py | 12 +- web_server.py | 222 ++++++++++++----------- webui/index.html | 2 +- webui/static/helper.js | 4 +- webui/static/init.js | 4 +- webui/static/settings.js | 27 ++- webui/static/shared-helpers.js | 33 ++++ 8 files changed, 193 insertions(+), 116 deletions(-) diff --git a/core/metadata/registry.py b/core/metadata/registry.py index d1162b35..214aa3f6 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -300,9 +300,10 @@ def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = return source -def get_spotify_disconnect_source() -> str: +def get_spotify_disconnect_source(configured_source: Optional[str] = None) -> str: """Return the active metadata source after Spotify is disconnected.""" - source = get_primary_source() + source = configured_source if configured_source is not None else _get_config_value("metadata.fallback_source", "deezer") + source = source or "deezer" return "deezer" if source == "spotify" else source diff --git a/tests/metadata/test_metadata_registry.py b/tests/metadata/test_metadata_registry.py index ea3a33d0..e4701d99 100644 --- a/tests/metadata/test_metadata_registry.py +++ b/tests/metadata/test_metadata_registry.py @@ -6,16 +6,12 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[2])) from core.metadata import registry -def test_spotify_disconnect_source_uses_deezer_when_spotify_is_primary(monkeypatch): - monkeypatch.setattr(registry, "get_primary_source", lambda: "spotify") - - assert registry.get_spotify_disconnect_source() == "deezer" +def test_spotify_disconnect_source_uses_deezer_when_spotify_is_primary(): + assert registry.get_spotify_disconnect_source("spotify") == "deezer" -def test_spotify_disconnect_source_keeps_non_spotify_primary(monkeypatch): - monkeypatch.setattr(registry, "get_primary_source", lambda: "discogs") - - assert registry.get_spotify_disconnect_source() == "discogs" +def test_spotify_disconnect_source_keeps_non_spotify_primary(): + assert registry.get_spotify_disconnect_source("discogs") == "discogs" def test_metadata_source_label_maps_known_sources(): diff --git a/web_server.py b/web_server.py index d42781f8..1dfd3ad0 100644 --- a/web_server.py +++ b/web_server.py @@ -3426,7 +3426,7 @@ def get_status(): is_rate_limited = spotify_client.is_rate_limited() if spotify_client else False rate_limit_info = spotify_client.get_rate_limit_info() if (spotify_client and is_rate_limited) else None cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0 - spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) + spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False # Read configured source once — no auth validation here, we do that explicitly below configured_source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer' @@ -3447,7 +3447,7 @@ def get_status(): music_source = configured_source _status_cache['spotify'] = { - 'connected': True, # Always true — iTunes fallback is always available + 'connected': spotify_session_active, 'authenticated': spotify_session_active, 'response_time': round(spotify_response_time, 1), 'source': music_source, @@ -4777,7 +4777,7 @@ def test_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': - spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) + spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False _status_cache['spotify']['connected'] = True _status_cache['spotify']['authenticated'] = spotify_session_active _status_cache['spotify']['source'] = _get_metadata_fallback_source() @@ -4945,7 +4945,7 @@ def test_dashboard_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': - spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) + spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False _status_cache['spotify']['connected'] = True _status_cache['spotify']['authenticated'] = spotify_session_active _status_cache['spotify']['source'] = _get_metadata_fallback_source() @@ -5480,101 +5480,85 @@ def auth_spotify(): '127.0.0.1' not in configured_uri and 'localhost' not in configured_uri ) - if is_remote or is_docker: - # Show instructions for remote/docker access - if uses_main_port: - # redirect_uri already points to port 8008 or a custom domain — - # callback will come through the main Flask app, no manual steps needed - return f''' - - - - - -

Spotify Authentication

-

Click the link below to authenticate with Spotify:

-

Authenticate with Spotify

-
- Redirect URI: {configured_uri}
- After authorizing, Spotify will redirect back automatically. Make sure this URL matches your Spotify Dashboard redirect URI. -
-

After authentication completes, you can close this window and return to SoulSync.

- - - ''' - else: - # redirect_uri points to the standalone callback server — show manual steps AND suggest switching - import re as _re - _port_match = _re.search(r':(\d+)/', configured_uri) - callback_server_port = _port_match.group(1) if _port_match else str(os.environ.get('SOULSYNC_SPOTIFY_CALLBACK_PORT', '8888')) - return f''' - - - - - -

Spotify Authentication (Remote/Docker)

+ if not (is_remote or is_docker): + return redirect(auth_url) -
- Using a reverse proxy? Your redirect URI is set to {configured_uri} - which uses port {callback_server_port}. If you're behind a reverse proxy (Caddy, Nginx, Traefik), change the - redirect URI in SoulSync settings to use your proxy URL on the main port instead, e.g.:
- https://{host}/callback
- Then update the same URI in your Spotify Dashboard. - This avoids the need for manual URL editing below. -
+ if uses_main_port: + # The OAuth callback returns to the app itself, so there is no + # need to keep an intermediate page open. + return redirect(auth_url) -

Step 1: Click the link below to authenticate with Spotify

-

{auth_url}

-
-

Step 2: After authorizing, you'll see a blank page. The URL will look like:

- http://127.0.0.1:{callback_server_port}/callback?code=... -

Step 3: Change 127.0.0.1 to {host} and press Enter: - -

- http://{host}:{callback_server_port}/callback?code=... -

Authentication will then complete!

+ # redirect_uri points to the standalone callback server — show manual steps AND suggest switching + import re as _re + _port_match = _re.search(r':(\d+)/', configured_uri) + callback_server_port = _port_match.group(1) if _port_match else str(os.environ.get('SOULSYNC_SPOTIFY_CALLBACK_PORT', '8888')) + return f''' + + + + + +

Spotify Authentication (Remote/Docker)

- - - - ''' - else: - # Local access - simple message - return f'

Spotify Authentication

Click the link below to authenticate:

{auth_url}

After authentication, return to the app.

' +
+ Using a reverse proxy? Your redirect URI is set to {configured_uri} + which uses port {callback_server_port}. If you're behind a reverse proxy (Caddy, Nginx, Traefik), change the + redirect URI in SoulSync settings to use your proxy URL on the main port instead, e.g.:
+ https://{host}/callback
+ Then update the same URI in your Spotify Dashboard. + This avoids the need for manual URL editing below. +
+ +

Step 1: Click the link below to authenticate with Spotify

+

{auth_url}

+
+

Step 2: After authorizing, you'll see a blank page. The URL will look like:

+ http://127.0.0.1:{callback_server_port}/callback?code=... +

Step 3: Change 127.0.0.1 to {host} and press Enter: + +

+ http://{host}:{callback_server_port}/callback?code=... +

Authentication will then complete!

+ + + + + ''' else: return "

Spotify Authentication Failed

Could not initialize Spotify client. Check your credentials.

", 400 except Exception as e: @@ -5735,6 +5719,33 @@ def auth_tidal(): return f"

Tidal Authentication Error

{str(e)}

", 500 +def _spotify_auth_success_page(detail_text: str) -> str: + """Return the post-auth success page and notify the opener.""" + return f""" + + + + Spotify Authentication Successful + + +

Spotify Authentication Successful

+

{detail_text}

+ + +""" + + @app.route('/callback') def spotify_callback(): """ @@ -5796,7 +5807,7 @@ def spotify_callback(): # Invalidate cached profile client so it gets recreated with new tokens metadata_registry.clear_cached_profile_spotify_client(profile_id_from_state) add_activity_item("", "Spotify Auth Complete", f"Profile {profile_id_from_state} authenticated with Spotify", "Now") - return "

Spotify Authentication Successful!

Your personal Spotify account is now connected. You can close this window.

" + return _spotify_auth_success_page("Your personal Spotify account is now connected. You can close this window.") else: raise Exception("Failed to exchange authorization code for access token") @@ -5833,7 +5844,7 @@ def spotify_callback(): spotify_enrichment_worker.client.reload_config() spotify_enrichment_worker.client._invalidate_auth_cache() add_activity_item("", "Spotify Auth Complete", "Successfully authenticated with Spotify", "Now") - return "

Spotify Authentication Successful!

You can close this window.

" + return _spotify_auth_success_page("You can close this window.") else: raise Exception("Token exchange succeeded but authentication validation failed") else: @@ -5849,13 +5860,16 @@ def spotify_disconnect(): """Disconnect Spotify and keep using the active primary metadata source.""" global spotify_client try: + configured_source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer' # Pause enrichment worker before disconnecting to prevent it from hammering API if spotify_enrichment_worker: spotify_enrichment_worker.pause() spotify_client.disconnect() # Immediately update status cache so UI reflects the change - active_source = get_spotify_disconnect_source() + active_source = get_spotify_disconnect_source(configured_source) source_label = get_metadata_source_label(active_source) + if configured_source == 'spotify': + config_manager.set('metadata.fallback_source', active_source) _status_cache['spotify'] = { 'connected': False, 'authenticated': False, @@ -5867,7 +5881,13 @@ def spotify_disconnect(): } _status_cache_timestamps['spotify'] = time.time() add_activity_item("", "Spotify Disconnected", f"Using {source_label} for metadata", "Now") - return jsonify({'success': True, 'message': f'Spotify disconnected. Using {source_label} for metadata.', 'source': active_source, 'authenticated': False}) + return jsonify({ + 'success': True, + 'message': f'Spotify disconnected. Using {source_label} for metadata.', + 'source': active_source, + 'authenticated': False, + 'primary_source_changed': configured_source == 'spotify' + }) except Exception as e: logger.error(f"Error disconnecting Spotify: {e}") return jsonify({'success': False, 'error': str(e)}), 500 @@ -31970,7 +31990,7 @@ def start_oauth_callback_servers(): self.send_response(200) self.send_header('Content-type', 'text/html') self.end_headers() - self.wfile.write(b'

Spotify Authentication Successful!

You can close this window.

') + self.wfile.write(_spotify_auth_success_page("You can close this window.").encode("utf-8")) else: raise Exception("Token exchange succeeded but authentication validation failed") else: diff --git a/webui/index.html b/webui/index.html index 7cd8bb17..4652f7e6 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3702,7 +3702,7 @@
-
Choose the primary source for artist, album, and track metadata. Spotify auth is optional and only needed for Spotify-specific actions. Discogs requires a personal token.
+
Choose the primary source for artist, album, and track metadata. Spotify can only be selected while an active Spotify session exists. Discogs requires a personal token.
diff --git a/webui/static/helper.js b/webui/static/helper.js index 46994799..6be9cfb0 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -2979,8 +2979,8 @@ async function _checkSetupStatus() { const resp = await fetch('/status'); if (resp.ok) { const data = await resp.json(); - // Metadata source: spotify.connected is always true (iTunes fallback), check .source - if (data.spotify?.connected && data.spotify?.source) { + // Metadata source is available when status reports a source. + if (data.spotify?.source) { results['metadata-source'] = results['metadata-source'] || Date.now(); _markSetupComplete('metadata-source'); } diff --git a/webui/static/init.js b/webui/static/init.js index 2680518a..6b3b11eb 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -1970,6 +1970,9 @@ function initApp() { initExpandedPlayer(); initializeSyncPage(); initializeWatchlist(); + if (typeof initializeSpotifyAuthCompletionListener === 'function') { + initializeSpotifyAuthCompletionListener(); + } // Initialize WebSocket connection (falls back to HTTP polling if unavailable) @@ -2371,4 +2374,3 @@ async function loadPageData(pageId) { // Old updateStatusIndicator function removed - replaced by updateSidebarServiceStatus // =============================== - diff --git a/webui/static/settings.js b/webui/static/settings.js index cd74a3e8..bb4e1e9d 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -50,6 +50,13 @@ function handleManualSaveClick() { saveSettings(false); } +function syncMetadataSourceSelection(source) { + const select = document.getElementById('metadata-fallback-source'); + if (!select || !source) return; + const option = select.querySelector(`option[value="${source}"]`); + if (option) select.value = source; +} + function initializeSettings() { // This function is called when the settings page is loaded. // It attaches event listeners to all interactive elements on the page. @@ -102,6 +109,11 @@ function initializeSettings() { // Test connection buttons // Test button event listeners removed - they use onclick attributes in HTML to avoid double firing + + if (typeof syncSpotifyMetadataSourceAvailability === 'function') { + syncSpotifyMetadataSourceAvailability(_lastServiceStatus?.spotify || null); + } + syncMetadataSourceSelection(_lastServiceStatus?.spotify?.source); } function resetFileOrganizationTemplates() { @@ -2400,6 +2412,17 @@ async function saveSettings(quiet = false) { activeServer = 'soulsync'; } + const metadataSourceSelect = document.getElementById('metadata-fallback-source'); + let metadataSource = metadataSourceSelect?.value || 'itunes'; + const spotifyDisconnected = _lastServiceStatus?.spotify?.connected === false; + if (metadataSource === 'spotify' && spotifyDisconnected) { + metadataSource = 'deezer'; + if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; + if (!quiet) { + showToast('Spotify is disconnected, so Deezer is used as the primary metadata source.', 'warning'); + } + } + const settings = { active_media_server: activeServer, spotify: { @@ -2472,7 +2495,7 @@ async function saveSettings(quiet = false) { token: document.getElementById('discogs-token').value, }, metadata: { - fallback_source: document.getElementById('metadata-fallback-source').value || 'itunes' + fallback_source: metadataSource }, hydrabase: { url: document.getElementById('hydrabase-url').value, @@ -3078,6 +3101,7 @@ async function disconnectSpotify() { const data = await response.json(); if (data.success) { showToast(data.message || 'Spotify disconnected.', 'success'); + syncMetadataSourceSelection(data.source || 'deezer'); // Immediately refresh status to update UI await fetchAndUpdateServiceStatus(); } else { @@ -3178,6 +3202,7 @@ async function disconnectSpotifyFromRateLimit() { if (data.success) { _spotifyRateLimitShown = false; showToast(data.message || 'Spotify disconnected.', 'success'); + syncMetadataSourceSelection(data.source || 'deezer'); await fetchAndUpdateServiceStatus(); if (currentPage === 'discover') { loadDiscoverPage(); diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index a0f822a0..299f90ac 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3107,6 +3107,24 @@ async function _forceServiceStatusRefresh() { } } +let _spotifyAuthCompletionListenerInstalled = false; + +function initializeSpotifyAuthCompletionListener() { + if (_spotifyAuthCompletionListenerInstalled) return; + _spotifyAuthCompletionListenerInstalled = true; + + window.addEventListener('message', async event => { + if (event.origin !== window.location.origin) return; + if (!event.data || event.data.type !== 'spotify-auth-complete') return; + + try { + await _forceServiceStatusRefresh(); + } catch (error) { + console.warn('Could not refresh Spotify status after auth completion:', error); + } + }); +} + async function fetchAndUpdateServiceStatus() { if (document.hidden) return; // Skip polling when tab is not visible if (socketConnected) return; // WebSocket is pushing updates — skip HTTP poll @@ -3160,6 +3178,19 @@ async function fetchAndUpdateServiceStatus() { } } +function syncSpotifyMetadataSourceAvailability(statusData) { + const select = document.getElementById('metadata-fallback-source'); + if (!select) return; + if (!statusData) return; + + const spotifyOption = select.querySelector('option[value="spotify"]'); + if (!spotifyOption) return; + + const spotifyAvailable = statusData?.connected === true; + spotifyOption.disabled = !spotifyAvailable; + spotifyOption.dataset.unavailable = spotifyAvailable ? 'false' : 'true'; +} + function updateServiceStatus(service, statusData) { const indicator = document.getElementById(`${service}-status-indicator`); const statusText = document.getElementById(`${service}-status-text`); @@ -3204,6 +3235,8 @@ function updateServiceStatus(service, statusData) { if (disconnectBtn) { disconnectBtn.style.display = spotifySessionActive ? '' : 'none'; } + + syncSpotifyMetadataSourceAvailability(statusData); } // Update download source title on dashboard card From 74e3cc460c3807648ead6ebb1021fd37ed7defcc Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 12:05:46 +0300 Subject: [PATCH 3/8] Simplify service status and labels - Flatten the Spotify service-status rendering so it shows rate-limit and recovery states explicitly, while otherwise displaying the active metadata provider directly. - Keep the Spotify auth controls and metadata-source picker aligned with the real session state after authenticate and disconnect flows. - Return "Unmapped" for unknown metadata source labels instead of implying iTunes. - Update the metadata registry tests to cover the new label fallback. --- core/metadata/registry.py | 2 +- tests/metadata/test_metadata_registry.py | 4 +- webui/static/settings.js | 4 +- webui/static/shared-helpers.js | 107 +++++++++++++++++------ 4 files changed, 83 insertions(+), 34 deletions(-) diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 214aa3f6..71206cfe 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -309,7 +309,7 @@ def get_spotify_disconnect_source(configured_source: Optional[str] = None) -> st def get_metadata_source_label(source: str) -> str: """Return a human-readable label for a metadata source.""" - return METADATA_SOURCE_LABELS.get(source, source.replace("_", " ").title()) + return METADATA_SOURCE_LABELS.get(source, "Unmapped") def get_source_priority(preferred_source: str): diff --git a/tests/metadata/test_metadata_registry.py b/tests/metadata/test_metadata_registry.py index e4701d99..cc83af52 100644 --- a/tests/metadata/test_metadata_registry.py +++ b/tests/metadata/test_metadata_registry.py @@ -22,5 +22,5 @@ def test_metadata_source_label_maps_known_sources(): assert registry.get_metadata_source_label("hydrabase") == "Hydrabase" -def test_metadata_source_label_falls_back_to_title_case(): - assert registry.get_metadata_source_label("apple_music") == "Apple Music" +def test_metadata_source_label_falls_back_to_unmapped(): + assert registry.get_metadata_source_label("apple_music") == "Unmapped" diff --git a/webui/static/settings.js b/webui/static/settings.js index bb4e1e9d..dce37adc 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -2414,8 +2414,8 @@ async function saveSettings(quiet = false) { const metadataSourceSelect = document.getElementById('metadata-fallback-source'); let metadataSource = metadataSourceSelect?.value || 'itunes'; - const spotifyDisconnected = _lastServiceStatus?.spotify?.connected === false; - if (metadataSource === 'spotify' && spotifyDisconnected) { + const spotifySessionActive = _lastServiceStatus?.spotify?.authenticated === true; + if (metadataSource === 'spotify' && !spotifySessionActive) { metadataSource = 'deezer'; if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; if (!quiet) { diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 299f90ac..928588fe 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3186,33 +3186,86 @@ function syncSpotifyMetadataSourceAvailability(statusData) { const spotifyOption = select.querySelector('option[value="spotify"]'); if (!spotifyOption) return; - const spotifyAvailable = statusData?.connected === true; + const spotifyAvailable = statusData?.authenticated === true; spotifyOption.disabled = !spotifyAvailable; spotifyOption.dataset.unavailable = spotifyAvailable ? 'false' : 'true'; } +function getMetadataSourceLabel(source) { + if (source === 'deezer') return 'Deezer'; + if (source === 'discogs') return 'Discogs'; + if (source === 'itunes') return 'iTunes'; + if (source === 'spotify') return 'Spotify'; + return 'Unmapped'; +} + +function getSpotifyStatusPresentation(statusData) { + const sourceLabel = getMetadataSourceLabel(statusData?.source); + const rateLimited = !!(statusData?.rate_limited && statusData?.rate_limit); + const cooldown = !!(statusData?.post_ban_cooldown > 0); + const sessionActive = statusData?.authenticated === true || (statusData?.authenticated === undefined && statusData?.source === 'spotify'); + + if (rateLimited) { + const remaining = statusData.rate_limit?.remaining_seconds || 0; + return { + statusClass: 'rate-limited', + statusText: `Spotify paused \u2014 ${formatRateLimitDuration(remaining)}`, + dotClass: 'rate-limited', + dotTitle: `Spotify paused \u2014 ${formatRateLimitDuration(remaining)} remaining`, + sessionActive + }; + } + + if (cooldown) { + const remaining = statusData.post_ban_cooldown; + return { + statusClass: 'rate-limited', + statusText: `Spotify recovering \u2014 ${formatRateLimitDuration(remaining)}`, + dotClass: 'rate-limited', + dotTitle: `Spotify recovering \u2014 ${formatRateLimitDuration(remaining)} cooldown`, + sessionActive + }; + } + + if (statusData?.source && statusData.source !== 'spotify') { + return { + statusClass: 'connected', + statusText: sourceLabel, + dotClass: 'connected', + dotTitle: sourceLabel, + sessionActive + }; + } + + return { + statusClass: 'connected', + statusText: `Connected (${statusData?.response_time}ms)`, + dotClass: 'connected', + dotTitle: '', + sessionActive + }; +} + function updateServiceStatus(service, statusData) { const indicator = document.getElementById(`${service}-status-indicator`); const statusText = document.getElementById(`${service}-status-text`); if (indicator && statusText) { - if (service === 'spotify' && (statusData.rate_limited || statusData.post_ban_cooldown)) { - indicator.className = 'service-card-indicator rate-limited'; - const remaining = statusData.rate_limited - ? formatRateLimitDuration(statusData.rate_limit?.remaining_seconds || 0) - : formatRateLimitDuration(statusData.post_ban_cooldown); - const phase = statusData.rate_limited ? 'paused' : 'recovering'; - const fallbackLabel = statusData.source === 'deezer' ? 'Deezer' : 'iTunes'; - statusText.textContent = `${fallbackLabel} (Spotify ${phase} \u2014 ${remaining})`; - statusText.className = 'service-card-status-text rate-limited'; - } else if (statusData.connected) { - indicator.className = 'service-card-indicator connected'; - statusText.textContent = `Connected (${statusData.response_time}ms)`; - statusText.className = 'service-card-status-text connected'; + if (service === 'spotify') { + const presentation = getSpotifyStatusPresentation(statusData || {}); + indicator.className = `service-card-indicator ${presentation.statusClass}`; + statusText.textContent = presentation.statusText; + statusText.className = `service-card-status-text ${presentation.statusClass}`; } else { - indicator.className = 'service-card-indicator disconnected'; - statusText.textContent = 'Disconnected'; - statusText.className = 'service-card-status-text disconnected'; + if (statusData.connected) { + indicator.className = 'service-card-indicator connected'; + statusText.textContent = `Connected (${statusData.response_time}ms)`; + statusText.className = 'service-card-status-text connected'; + } else { + indicator.className = 'service-card-indicator disconnected'; + statusText.textContent = 'Disconnected'; + statusText.className = 'service-card-status-text disconnected'; + } } } @@ -3220,13 +3273,13 @@ function updateServiceStatus(service, statusData) { if (service === 'spotify' && statusData.source) { const musicSourceTitleElement = document.getElementById('music-source-title'); if (musicSourceTitleElement) { - const sourceName = statusData.source === 'spotify' ? 'Spotify' : statusData.source === 'deezer' ? 'Deezer' : statusData.source === 'discogs' ? 'Discogs' : 'iTunes'; + const sourceName = getMetadataSourceLabel(statusData.source); musicSourceTitleElement.textContent = sourceName; currentMusicSourceName = sourceName; } // Keep the Spotify action buttons aligned with the actual auth session. - const spotifySessionActive = statusData.authenticated === true || (statusData.authenticated === undefined && statusData.source === 'spotify'); + const spotifySessionActive = getSpotifyStatusPresentation(statusData || {}).sessionActive; const authBtn = document.querySelector('button[onclick="authenticateSpotify()"]'); const disconnectBtn = document.getElementById('spotify-disconnect-btn'); if (authBtn) { @@ -3255,16 +3308,12 @@ function updateSidebarServiceStatus(service, statusData) { const nameElement = indicator.querySelector('.status-name'); if (dot) { - if (service === 'spotify' && (statusData.rate_limited || statusData.post_ban_cooldown)) { - dot.className = 'status-dot rate-limited'; - dot.title = statusData.rate_limited - ? `Spotify paused \u2014 ${formatRateLimitDuration(statusData.rate_limit?.remaining_seconds || 0)} remaining` - : `Spotify recovering \u2014 ${formatRateLimitDuration(statusData.post_ban_cooldown)} cooldown`; - } else if (statusData.connected) { - dot.className = 'status-dot connected'; - dot.title = ''; + if (service === 'spotify') { + const presentation = getSpotifyStatusPresentation(statusData || {}); + dot.className = `status-dot ${presentation.dotClass}`; + dot.title = presentation.dotTitle; } else { - dot.className = 'status-dot disconnected'; + dot.className = statusData?.connected ? 'status-dot connected' : 'status-dot disconnected'; dot.title = ''; } } @@ -3282,7 +3331,7 @@ function updateSidebarServiceStatus(service, statusData) { if (service === 'spotify' && statusData.source) { const musicSourceNameElement = document.getElementById('music-source-name'); if (musicSourceNameElement) { - const sourceName = statusData.source === 'spotify' ? 'Spotify' : statusData.source === 'deezer' ? 'Deezer' : statusData.source === 'discogs' ? 'Discogs' : 'iTunes'; + const sourceName = getMetadataSourceLabel(statusData.source); musicSourceNameElement.textContent = sourceName; } } From f733744f911d116c8bb81724f67c743e1470c0a9 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 12:14:13 +0300 Subject: [PATCH 4/8] Fix Spotify auth completion sync - Make the Spotify auth completion popup notify the opener across callback origins. - Refresh service status in the settings UI after auth completes so the button flips to Disconnect immediately. - Keep the standalone callback instruction page and the main app flow working with the same completion signal. --- web_server.py | 3 +-- webui/static/settings.js | 2 +- webui/static/shared-helpers.js | 4 +++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/web_server.py b/web_server.py index 1dfd3ad0..7e2cd7e0 100644 --- a/web_server.py +++ b/web_server.py @@ -5539,7 +5539,6 @@ def auth_spotify(): @@ -5803,10 +5818,22 @@ def spotify_callback(): ) token_info = auth_manager.get_access_token(auth_code) if token_info: - # Invalidate cached profile client so it gets recreated with new tokens metadata_registry.clear_cached_profile_spotify_client(profile_id_from_state) - add_activity_item("", "Spotify Auth Complete", f"Profile {profile_id_from_state} authenticated with Spotify", "Now") - return _spotify_auth_success_page("Your personal Spotify account is now connected. You can close this window.") + profile_client = metadata_registry.get_spotify_client_for_profile(profile_id_from_state) + profile_authenticated = bool(profile_client and profile_client.is_spotify_authenticated()) + if profile_authenticated: + if profile_client: + profile_client._invalidate_auth_cache() + add_activity_item("", "Spotify Auth Complete", f"Profile {profile_id_from_state} authenticated with Spotify", "Now") + return _spotify_auth_result_page("Your personal Spotify account is now connected. You can close this window.", authenticated=True) + if profile_client: + profile_client._invalidate_auth_cache() + _status_cache_timestamps['spotify'] = 0 + add_activity_item("", "Spotify Auth Warning", f"Profile {profile_id_from_state} completed OAuth but Spotify did not confirm an authenticated session", "Now") + return _spotify_auth_result_page( + "Spotify authorization completed, but SoulSync could not confirm an authenticated Spotify session for this profile. You can close this window and try Authenticate again.", + authenticated=False, + ) else: raise Exception("Failed to exchange authorization code for access token") @@ -5843,9 +5870,16 @@ def spotify_callback(): spotify_enrichment_worker.client.reload_config() spotify_enrichment_worker.client._invalidate_auth_cache() add_activity_item("", "Spotify Auth Complete", "Successfully authenticated with Spotify", "Now") - return _spotify_auth_success_page("You can close this window.") + return _spotify_auth_result_page("You can close this window.", authenticated=True) else: - raise Exception("Token exchange succeeded but authentication validation failed") + logger.warning("Spotify OAuth token exchange succeeded but authentication validation failed") + spotify_client._invalidate_auth_cache() + _status_cache_timestamps['spotify'] = 0 + add_activity_item("", "Spotify Auth Warning", "OAuth completed, but Spotify did not confirm an authenticated session", "Now") + return _spotify_auth_result_page( + "Spotify authorization completed, but SoulSync could not confirm an authenticated Spotify session. You can close this window and try Authenticate again.", + authenticated=False, + ) else: raise Exception("Failed to exchange authorization code for access token") except Exception as e: @@ -31989,9 +32023,19 @@ def start_oauth_callback_servers(): self.send_response(200) self.send_header('Content-type', 'text/html') self.end_headers() - self.wfile.write(_spotify_auth_success_page("You can close this window.").encode("utf-8")) + self.wfile.write(_spotify_auth_result_page("You can close this window.", authenticated=True).encode("utf-8")) else: - raise Exception("Token exchange succeeded but authentication validation failed") + _oauth_logger.warning("Spotify token exchange succeeded but authentication validation failed") + spotify_client._invalidate_auth_cache() + _status_cache_timestamps['spotify'] = 0 + add_activity_item("", "Spotify Auth Warning", "OAuth completed, but Spotify did not confirm an authenticated session", "Now") + self.send_response(200) + self.send_header('Content-type', 'text/html') + self.end_headers() + self.wfile.write(_spotify_auth_result_page( + "Spotify authorization completed, but SoulSync could not confirm an authenticated Spotify session. You can close this window and try Authenticate again.", + authenticated=False, + ).encode("utf-8")) else: raise Exception("Failed to exchange authorization code for access token") except Exception as e: diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 92e5246b..6cad27f8 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3121,6 +3121,12 @@ function initializeSpotifyAuthCompletionListener() { try { window._spotifyAuthWindow = null; await _forceServiceStatusRefresh(); + if (event.data.authenticated === false) { + showToast( + event.data.detail || 'Spotify authorization completed, but no authenticated session was detected.', + 'warning' + ); + } } catch (error) { console.warn('Could not refresh Spotify status after auth completion:', error); } From 287c9601fca087a1e4021c77676d8988bebc4379 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 12:41:22 +0300 Subject: [PATCH 6/8] Mark Spotify settings as needing auth - Drive the Spotify settings accordion from live auth state instead of treating it as configured/healthy when the session is missing. - Reuse the existing yellow missing-state styling so unauthenticated Spotify is visually distinct from active Spotify. - Keep the shared status refresh path updating the settings view immediately after auth changes. --- webui/static/core.js | 5 ++++- webui/static/settings.js | 25 +++++++++++++++++++++++++ webui/static/shared-helpers.js | 4 ++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/webui/static/core.js b/webui/static/core.js index 32a75406..9257e233 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -473,6 +473,10 @@ function handleServiceStatusUpdate(data) { // Cache for library status card _lastServiceStatus = data; + if (typeof syncSpotifySettingsAuthState === 'function') { + syncSpotifySettingsAuthState(data?.spotify || null); + } + // Same logic as fetchAndUpdateServiceStatus response handler updateServiceStatus('spotify', data.spotify); updateServiceStatus('media-server', data.media_server); @@ -876,4 +880,3 @@ let _lastServiceStatus = null; let _isSoulsyncStandalone = false; // Global flag: true when no media server (sync buttons hidden) // =============================== - diff --git a/webui/static/settings.js b/webui/static/settings.js index 4f4f0327..5c84b53b 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -113,6 +113,7 @@ function initializeSettings() { if (typeof syncSpotifyMetadataSourceAvailability === 'function') { syncSpotifyMetadataSourceAvailability(_lastServiceStatus?.spotify || null); } + syncSpotifySettingsAuthState(_lastServiceStatus?.spotify || null); syncMetadataSourceSelection(_lastServiceStatus?.spotify?.source); } @@ -307,11 +308,35 @@ async function applyServiceStatusGradients() { else header.appendChild(spinner); } }); + syncSpotifySettingsAuthState(_lastServiceStatus?.spotify || null); } catch (e) { console.warn('[Settings Status] Failed to apply gradients:', e); } } +function syncSpotifySettingsAuthState(statusData) { + const card = document.querySelector('#settings-page .stg-service[data-service="spotify"]'); + if (!card) return; + + const header = card.querySelector('.stg-service-header'); + const dot = card.querySelector('.stg-service-dot'); + if (!header && !dot) return; + + const authenticated = statusData?.authenticated === true; + const rateLimited = !!(statusData?.rate_limited && statusData?.rate_limit); + const cooldown = !!(statusData?.post_ban_cooldown > 0); + const needsAttention = !authenticated || rateLimited || cooldown; + + if (header) { + header.classList.toggle('status-configured', !needsAttention); + header.classList.toggle('status-missing', needsAttention); + } + + if (dot) { + dot.style.color = needsAttention ? '#f1c40f' : '#1DB954'; + } +} + function _stgSetCheckingState(service, isChecking) { const card = document.querySelector(`#settings-page .stg-service[data-service="${service}"]`); if (!card) return; diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 6cad27f8..359fbd21 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3145,6 +3145,10 @@ async function fetchAndUpdateServiceStatus() { // Cache for library status card _lastServiceStatus = data; + if (typeof syncSpotifySettingsAuthState === 'function') { + syncSpotifySettingsAuthState(data?.spotify || null); + } + // Update service status indicators and text (dashboard) updateServiceStatus('spotify', data.spotify); updateServiceStatus('media-server', data.media_server); From 5ff20fbfec21adc298c9edc9f12492580de9f3e2 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 12:51:51 +0300 Subject: [PATCH 7/8] Polish Spotify source selection - Show Spotify with a lock icon when it is not currently selectable. - Keep the explanation in the hover title instead of cluttering the dropdown label. - Redirect users to the Spotify settings section when they try to pick a locked source. --- webui/static/settings.js | 53 ++++++++++++++++++++++++++++++++++ webui/static/shared-helpers.js | 5 +++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/webui/static/settings.js b/webui/static/settings.js index 5c84b53b..c034298f 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -55,6 +55,49 @@ function syncMetadataSourceSelection(source) { if (!select || !source) return; const option = select.querySelector(`option[value="${source}"]`); if (option) select.value = source; + select.dataset.lastValidSource = source; +} + +function focusSpotifySettingsSection() { + const card = document.querySelector('#settings-page .stg-service[data-service="spotify"]'); + if (!card) return; + + const header = card.querySelector('.stg-service-header'); + if (!card.classList.contains('expanded') && header) { + toggleStgService(header); + } + + card.scrollIntoView({ behavior: 'smooth', block: 'center' }); + + const firstControl = card.querySelector('input, button'); + if (firstControl) { + firstControl.focus({ preventScroll: true }); + } + + showToast('Spotify must be authenticated before it can be selected as the primary metadata source.', 'warning'); +} + +function handleMetadataSourceChange(event) { + const select = event.target; + if (!select || select.id !== 'metadata-fallback-source') return; + + const selectedSource = select.value; + if (selectedSource !== 'spotify') { + select.dataset.lastValidSource = selectedSource; + return; + } + + const spotifySessionActive = _lastServiceStatus?.spotify?.authenticated === true; + if (spotifySessionActive) { + select.dataset.lastValidSource = selectedSource; + return; + } + + const fallbackSource = select.dataset.lastValidSource || _lastServiceStatus?.spotify?.source || 'deezer'; + if (fallbackSource && fallbackSource !== 'spotify') { + select.value = fallbackSource; + } + focusSpotifySettingsSection(); } function initializeSettings() { @@ -83,6 +126,11 @@ function initializeSettings() { }); } + const metadataSourceSelect = document.getElementById('metadata-fallback-source'); + if (metadataSourceSelect) { + metadataSourceSelect.addEventListener('change', handleMetadataSourceChange); + } + // Server toggle buttons const plexToggle = document.getElementById('plex-toggle'); if (plexToggle) { @@ -115,6 +163,9 @@ function initializeSettings() { } syncSpotifySettingsAuthState(_lastServiceStatus?.spotify || null); syncMetadataSourceSelection(_lastServiceStatus?.spotify?.source); + if (metadataSourceSelect) { + metadataSourceSelect.dataset.lastValidSource = metadataSourceSelect.value; + } } function resetFileOrganizationTemplates() { @@ -315,6 +366,8 @@ async function applyServiceStatusGradients() { } function syncSpotifySettingsAuthState(statusData) { + if (!statusData) return; + const card = document.querySelector('#settings-page .stg-service[data-service="spotify"]'); if (!card) return; diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 359fbd21..2262b40f 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3199,8 +3199,11 @@ function syncSpotifyMetadataSourceAvailability(statusData) { if (!spotifyOption) return; const spotifyAvailable = statusData?.authenticated === true; - spotifyOption.disabled = !spotifyAvailable; spotifyOption.dataset.unavailable = spotifyAvailable ? 'false' : 'true'; + spotifyOption.textContent = spotifyAvailable ? 'Spotify' : '🔒 Spotify'; + spotifyOption.title = spotifyAvailable + ? 'Spotify' + : 'Spotify authentication is required before this source can be selected.'; } function getMetadataSourceLabel(source) { From 4e40bce3e9791d875ed182eda0a189069424c8e1 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 12:59:38 +0300 Subject: [PATCH 8/8] Gate Discogs primary source by token - Show Discogs with a lock icon until a personal access token is present. - Prevent selecting locked Discogs and steer users to the Discogs settings section. - Keep metadata-source availability and selection state synced as the token changes. --- webui/static/core.js | 6 +++ webui/static/settings.js | 91 +++++++++++++++++++++++++++------- webui/static/shared-helpers.js | 34 ++++++++++--- 3 files changed, 106 insertions(+), 25 deletions(-) diff --git a/webui/static/core.js b/webui/static/core.js index 9257e233..3a23cb53 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -476,6 +476,12 @@ function handleServiceStatusUpdate(data) { if (typeof syncSpotifySettingsAuthState === 'function') { syncSpotifySettingsAuthState(data?.spotify || null); } + if (typeof syncPrimaryMetadataSourceAvailability === 'function') { + syncPrimaryMetadataSourceAvailability(data?.spotify || null); + } + if (typeof sanitizeMetadataSourceSelection === 'function') { + sanitizeMetadataSourceSelection({ quiet: true }); + } // Same logic as fetchAndUpdateServiceStatus response handler updateServiceStatus('spotify', data.spotify); diff --git a/webui/static/settings.js b/webui/static/settings.js index c034298f..dfa092bb 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -58,8 +58,25 @@ function syncMetadataSourceSelection(source) { select.dataset.lastValidSource = source; } -function focusSpotifySettingsSection() { - const card = document.querySelector('#settings-page .stg-service[data-service="spotify"]'); +function _isMetadataSourceSelectable(source) { + if (source === 'spotify') { + return _lastServiceStatus?.spotify?.authenticated === true; + } + if (source === 'discogs') { + const token = document.getElementById('discogs-token'); + return !!token?.value?.trim(); + } + return true; +} + +function _metadataSourceFallback(source) { + if (source === 'spotify') return 'deezer'; + if (source === 'discogs') return 'itunes'; + return 'itunes'; +} + +function focusServiceSettingsSection(service, message) { + const card = document.querySelector(`#settings-page .stg-service[data-service="${service}"]`); if (!card) return; const header = card.querySelector('.stg-service-header'); @@ -74,7 +91,39 @@ function focusSpotifySettingsSection() { firstControl.focus({ preventScroll: true }); } - showToast('Spotify must be authenticated before it can be selected as the primary metadata source.', 'warning'); + if (message) { + showToast(message, 'warning'); + } +} + +function sanitizeMetadataSourceSelection({ quiet = true } = {}) { + const select = document.getElementById('metadata-fallback-source'); + if (!select) return false; + + const selectedSource = select.value || 'itunes'; + if (_isMetadataSourceSelectable(selectedSource)) { + select.dataset.lastValidSource = selectedSource; + return false; + } + + const lastValid = select.dataset.lastValidSource; + const fallbackSource = lastValid && lastValid !== selectedSource && _isMetadataSourceSelectable(lastValid) + ? lastValid + : _metadataSourceFallback(selectedSource); + + if (fallbackSource && fallbackSource !== selectedSource) { + select.value = fallbackSource; + } + select.dataset.lastValidSource = fallbackSource; + + if (!quiet) { + const message = selectedSource === 'discogs' + ? 'Discogs requires a personal access token before it can be selected as the primary metadata source.' + : 'Spotify must be authenticated before it can be selected as the primary metadata source.'; + focusServiceSettingsSection(selectedSource, message); + } + + return true; } function handleMetadataSourceChange(event) { @@ -82,22 +131,12 @@ function handleMetadataSourceChange(event) { if (!select || select.id !== 'metadata-fallback-source') return; const selectedSource = select.value; - if (selectedSource !== 'spotify') { + if (_isMetadataSourceSelectable(selectedSource)) { select.dataset.lastValidSource = selectedSource; return; } - const spotifySessionActive = _lastServiceStatus?.spotify?.authenticated === true; - if (spotifySessionActive) { - select.dataset.lastValidSource = selectedSource; - return; - } - - const fallbackSource = select.dataset.lastValidSource || _lastServiceStatus?.spotify?.source || 'deezer'; - if (fallbackSource && fallbackSource !== 'spotify') { - select.value = fallbackSource; - } - focusSpotifySettingsSection(); + sanitizeMetadataSourceSelection({ quiet: false }); } function initializeSettings() { @@ -130,6 +169,15 @@ function initializeSettings() { if (metadataSourceSelect) { metadataSourceSelect.addEventListener('change', handleMetadataSourceChange); } + const discogsTokenInput = document.getElementById('discogs-token'); + if (discogsTokenInput) { + discogsTokenInput.addEventListener('input', () => { + if (typeof syncPrimaryMetadataSourceAvailability === 'function') { + syncPrimaryMetadataSourceAvailability(_lastServiceStatus?.spotify || null); + } + sanitizeMetadataSourceSelection({ quiet: true }); + }); + } // Server toggle buttons const plexToggle = document.getElementById('plex-toggle'); @@ -158,11 +206,12 @@ function initializeSettings() { // Test connection buttons // Test button event listeners removed - they use onclick attributes in HTML to avoid double firing - if (typeof syncSpotifyMetadataSourceAvailability === 'function') { - syncSpotifyMetadataSourceAvailability(_lastServiceStatus?.spotify || null); + if (typeof syncPrimaryMetadataSourceAvailability === 'function') { + syncPrimaryMetadataSourceAvailability(_lastServiceStatus?.spotify || null); } syncSpotifySettingsAuthState(_lastServiceStatus?.spotify || null); syncMetadataSourceSelection(_lastServiceStatus?.spotify?.source); + sanitizeMetadataSourceSelection({ quiet: true }); if (metadataSourceSelect) { metadataSourceSelect.dataset.lastValidSource = metadataSourceSelect.value; } @@ -2491,6 +2540,8 @@ async function saveSettings(quiet = false) { } const metadataSourceSelect = document.getElementById('metadata-fallback-source'); + const discogsTokenInput = document.getElementById('discogs-token'); + const discogsTokenPresent = !!discogsTokenInput?.value?.trim(); let metadataSource = metadataSourceSelect?.value || 'itunes'; const spotifySessionActive = _lastServiceStatus?.spotify?.authenticated === true; if (metadataSource === 'spotify' && !spotifySessionActive) { @@ -2499,6 +2550,12 @@ async function saveSettings(quiet = false) { if (!quiet) { showToast('Spotify is disconnected, so Deezer is used as the primary metadata source.', 'warning'); } + } else if (metadataSource === 'discogs' && !discogsTokenPresent) { + metadataSource = 'itunes'; + if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; + if (!quiet) { + showToast('Discogs requires a personal access token before it can be selected as the primary metadata source.', 'warning'); + } } const settings = { diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 2262b40f..6fed9d88 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3148,6 +3148,12 @@ async function fetchAndUpdateServiceStatus() { if (typeof syncSpotifySettingsAuthState === 'function') { syncSpotifySettingsAuthState(data?.spotify || null); } + if (typeof syncPrimaryMetadataSourceAvailability === 'function') { + syncPrimaryMetadataSourceAvailability(data?.spotify || null); + } + if (typeof sanitizeMetadataSourceSelection === 'function') { + sanitizeMetadataSourceSelection({ quiet: true }); + } // Update service status indicators and text (dashboard) updateServiceStatus('spotify', data.spotify); @@ -3190,20 +3196,32 @@ async function fetchAndUpdateServiceStatus() { } } -function syncSpotifyMetadataSourceAvailability(statusData) { +function syncPrimaryMetadataSourceAvailability(statusData) { const select = document.getElementById('metadata-fallback-source'); if (!select) return; if (!statusData) return; const spotifyOption = select.querySelector('option[value="spotify"]'); - if (!spotifyOption) return; + const discogsOption = select.querySelector('option[value="discogs"]'); const spotifyAvailable = statusData?.authenticated === true; - spotifyOption.dataset.unavailable = spotifyAvailable ? 'false' : 'true'; - spotifyOption.textContent = spotifyAvailable ? 'Spotify' : '🔒 Spotify'; - spotifyOption.title = spotifyAvailable - ? 'Spotify' - : 'Spotify authentication is required before this source can be selected.'; + if (spotifyOption) { + spotifyOption.dataset.unavailable = spotifyAvailable ? 'false' : 'true'; + spotifyOption.textContent = spotifyAvailable ? 'Spotify' : '🔒 Spotify'; + spotifyOption.title = spotifyAvailable + ? 'Spotify' + : 'Spotify authentication is required before this source can be selected.'; + } + + if (discogsOption) { + const discogsToken = document.getElementById('discogs-token'); + const discogsAvailable = !!discogsToken?.value?.trim(); + discogsOption.dataset.unavailable = discogsAvailable ? 'false' : 'true'; + discogsOption.textContent = discogsAvailable ? 'Discogs' : '🔒 Discogs'; + discogsOption.title = discogsAvailable + ? 'Discogs' + : 'Discogs personal access token is required before this source can be selected.'; + } } function getMetadataSourceLabel(source) { @@ -3304,7 +3322,7 @@ function updateServiceStatus(service, statusData) { disconnectBtn.style.display = spotifySessionActive ? '' : 'none'; } - syncSpotifyMetadataSourceAvailability(statusData); + syncPrimaryMetadataSourceAvailability(statusData); } // Update download source title on dashboard card