From efefdd64ff4bf09462ab4dbe492d2a833bb38f6b Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 29 Jun 2026 08:35:25 -0700 Subject: [PATCH] spotify export: clickable authorize link (popup-block safe) + endpoint pre-check tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Double-checking the on-demand auth flow: the needs_auth handler called window.open() AFTER an await, which breaks the user-gesture chain so browsers popup-block it — the user would see "approve in the new tab" with no tab. Replaced with a clickable authorize link (a direct click is never blocked). Adds two endpoint tests via the Flask test client: Spotify export returns needs_auth + the /auth/spotify/export url (and short-circuits before the DB) when the token lacks write scope, and does NOT short-circuit when write scope is present. 10 service-export tests green, 64 script-integrity green, ruff clean. --- tests/test_service_playlist_export.py | 24 ++++++++++++++++++++++++ webui/static/stats-automations.js | 8 ++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/tests/test_service_playlist_export.py b/tests/test_service_playlist_export.py index e9bc7b8d..056a80fd 100644 --- a/tests/test_service_playlist_export.py +++ b/tests/test_service_playlist_export.py @@ -128,3 +128,27 @@ def test_spotify_backfill_search_disables_cross_service_fallback(monkeypatch): assert fn is not None fn('Kendrick Lamar', 'Not Like Us') # drives the search assert seen['allow_fallback'] is False + + +def test_spotify_export_endpoint_demands_auth_when_no_write_scope(monkeypatch): + """The export endpoint must return needs_auth (not start a doomed job) when the Spotify + token lacks write scope — and it must short-circuit BEFORE touching the DB.""" + import types + monkeypatch.setattr(ws, 'spotify_client', + types.SimpleNamespace(has_write_scope=lambda: False)) + resp = ws.app.test_client().post('/api/playlists/5/export/service/spotify') + data = resp.get_json() + assert data['needs_auth'] is True + assert data['auth_url'] == '/auth/spotify/export' + assert data['success'] is False + + +def test_spotify_export_endpoint_proceeds_when_write_scope_present(monkeypatch): + """With write scope, the spotify path must NOT short-circuit on needs_auth (it goes on to + start a job — here it just must not be a needs_auth response).""" + import types + monkeypatch.setattr(ws, 'spotify_client', + types.SimpleNamespace(has_write_scope=lambda: True)) + resp = ws.app.test_client().post('/api/playlists/999999/export/service/spotify') + data = resp.get_json() + assert not data.get('needs_auth') diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index 6f2a5c9c..83b551b2 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -730,11 +730,11 @@ async function _startPlaylistExport(playlistId, mode, name, backfill) { body: isService ? JSON.stringify({ backfill: !!backfill }) : JSON.stringify({ mode }), }); const data = await resp.json(); - // Spotify export needs a one-time write-permission grant. Open the consent in a new tab - // and tell the user to try again once they've authorized. + // Spotify export needs a one-time write-permission grant. Surface a clickable link (a + // direct user click avoids popup-blocking; window.open after this await would be blocked) + // and tell the user to retry once they've authorized. if (data.needs_auth && data.auth_url) { - window.open(data.auth_url, '_blank'); - _setExportStatus(playlistId, `Spotify needs permission to create playlists — approve it in the new tab, then click Export again.`, 15000); + _setExportStatus(playlistId, `Spotify needs permission to create playlists — authorize, then click Export again.`, 20000); return; } if (!data.success || !data.job_id) {