diff --git a/core/discovery/endpoints.py b/core/discovery/endpoints.py index db6e53aa..5b2f1312 100644 --- a/core/discovery/endpoints.py +++ b/core/discovery/endpoints.py @@ -398,6 +398,65 @@ def get_playlist_states( return {"error": str(e)}, 500 +def update_playlist_phase( + states: Dict[str, Any], + key: str, + get_json, + *, + not_found_message: str, + error_label: str, + valid_phases: List[str], + apply_extra_fields: bool, +) -> Tuple[Dict[str, Any], int]: + """Update a discovery playlist's phase (used when the modal closes, e.g. to + reset download_complete -> discovered). + + 1:1 lift of the ``update__playlist_phase`` bodies for the five + sources with the identical validation + full-message response (Tidal, + Deezer, Qobuz, Spotify-Public, YouTube). Per-source params: + + - ``valid_phases`` — YouTube's list additionally includes 'parsed'. + - ``apply_extra_fields`` — Deezer/Qobuz/Spotify-Public also persist + download_process_id / converted_spotify_playlist_id from the body; + Tidal/YouTube do NOT (so pass False to keep them 1:1). + - ``not_found_message`` / ``error_label``; ``get_json`` invoked inside the + try like the original ``request.get_json()``. + + Returns ``(payload, status_code)``. + + NOT folded in: iTunes-Link — it uses ``data.get('phase')`` (no separate + "Phase not provided" 400) and returns a no-message payload. + """ + try: + if key not in states: + return {"error": not_found_message}, 404 + + data = get_json() + if not data or 'phase' not in data: + return {"error": "Phase not provided"}, 400 + + new_phase = data['phase'] + if new_phase not in valid_phases: + return {"error": f"Invalid phase. Must be one of: {', '.join(valid_phases)}"}, 400 + + state = states[key] + old_phase = state.get('phase', 'unknown') + state['phase'] = new_phase + state['last_accessed'] = time.time() + + if apply_extra_fields: + if 'download_process_id' in data: + state['download_process_id'] = data['download_process_id'] + if 'converted_spotify_playlist_id' in data: + state['converted_spotify_playlist_id'] = data['converted_spotify_playlist_id'] + + logger.info(f"Updated {error_label} playlist {key} phase: {old_phase} → {new_phase}") + return {"success": True, "message": f"Phase updated to {new_phase}", "old_phase": old_phase, "new_phase": new_phase}, 200 + except Exception as e: + logger.error(f"Error updating {error_label} playlist phase: {e}") + return {"error": str(e)}, 500 + + def first_artist_str_or_obj(original_track: Dict[str, Any]) -> str: """Tidal: first artist from an artists list that may hold strings OR objects ({'name': ...}); '' when empty.""" diff --git a/tests/discovery/test_discovery_endpoints.py b/tests/discovery/test_discovery_endpoints.py index 74932087..618e18cc 100644 --- a/tests/discovery/test_discovery_endpoints.py +++ b/tests/discovery/test_discovery_endpoints.py @@ -790,3 +790,90 @@ def test_update_match_get_json_raises_returns_500(): _, kw, _ = _update_kwargs(json_data={}) body, code = update_discovery_match({}, boom, **kw) assert code == 500 and 'error' in body + + +# --------------------------------------------------------------------------- +# update_playlist_phase +# --------------------------------------------------------------------------- + +_PHASES = ['fresh', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] + + +def test_update_phase_not_found(): + from core.discovery.endpoints import update_playlist_phase + body, code = update_playlist_phase({}, 'k', lambda: {'phase': 'fresh'}, + not_found_message='Tidal playlist not found', + error_label='Tidal', valid_phases=_PHASES, + apply_extra_fields=False) + assert code == 404 and body == {"error": "Tidal playlist not found"} + + +def test_update_phase_missing_phase(): + from core.discovery.endpoints import update_playlist_phase + states = {'k': {'phase': 'discovered'}} + body, code = update_playlist_phase(states, 'k', lambda: {}, + not_found_message='nf', error_label='Tidal', + valid_phases=_PHASES, apply_extra_fields=False) + assert code == 400 and body == {"error": "Phase not provided"} + + +def test_update_phase_invalid(): + from core.discovery.endpoints import update_playlist_phase + states = {'k': {'phase': 'discovered'}} + body, code = update_playlist_phase(states, 'k', lambda: {'phase': 'bogus'}, + not_found_message='nf', error_label='Tidal', + valid_phases=_PHASES, apply_extra_fields=False) + assert code == 400 and 'Invalid phase' in body['error'] + + +def test_update_phase_happy_no_extra_fields(): + from core.discovery.endpoints import update_playlist_phase + state = {'phase': 'download_complete', 'last_accessed': 0} + body, code = update_playlist_phase({'k': state}, 'k', + lambda: {'phase': 'discovered', + 'download_process_id': 'dp'}, + not_found_message='nf', error_label='Tidal', + valid_phases=_PHASES, apply_extra_fields=False) + assert code == 200 + assert body == {"success": True, "message": "Phase updated to discovered", + "old_phase": "download_complete", "new_phase": "discovered"} + assert state['phase'] == 'discovered' + assert state['last_accessed'] != 0 + # apply_extra_fields=False -> download_process_id NOT applied (1:1 for Tidal/YouTube) + assert 'download_process_id' not in state + + +def test_update_phase_applies_extra_fields_when_enabled(): + from core.discovery.endpoints import update_playlist_phase + state = {'phase': 'discovered'} + body, code = update_playlist_phase({'k': state}, 'k', + lambda: {'phase': 'downloading', + 'download_process_id': 'dp7', + 'converted_spotify_playlist_id': 'cv7'}, + not_found_message='nf', error_label='Deezer', + valid_phases=_PHASES, apply_extra_fields=True) + assert code == 200 + assert state['download_process_id'] == 'dp7' + assert state['converted_spotify_playlist_id'] == 'cv7' + + +def test_update_phase_youtube_parsed_allowed(): + from core.discovery.endpoints import update_playlist_phase + yt_phases = ['fresh', 'parsed', 'discovering', 'discovered', 'syncing', + 'sync_complete', 'downloading', 'download_complete'] + state = {'phase': 'discovered'} + body, code = update_playlist_phase({'k': state}, 'k', lambda: {'phase': 'parsed'}, + not_found_message='nf', error_label='YouTube', + valid_phases=yt_phases, apply_extra_fields=False) + assert code == 200 and state['phase'] == 'parsed' + + +def test_update_phase_exception_500(): + from core.discovery.endpoints import update_playlist_phase + def boom(): + raise ValueError('bad') + states = {'k': {'phase': 'discovered'}} + body, code = update_playlist_phase(states, 'k', boom, not_found_message='nf', + error_label='Tidal', valid_phases=_PHASES, + apply_extra_fields=False) + assert code == 500 and 'error' in body diff --git a/web_server.py b/web_server.py index de9809dd..a4cc81e5 100644 --- a/web_server.py +++ b/web_server.py @@ -20620,31 +20620,7 @@ def delete_tidal_playlist(playlist_id): @app.route('/api/tidal/update_phase/', methods=['POST']) def update_tidal_playlist_phase(playlist_id): """Update Tidal playlist phase (used when modal closes to reset from download_complete to discovered)""" - try: - if playlist_id not in tidal_discovery_states: - return jsonify({"error": "Tidal playlist not found"}), 404 - - data = request.get_json() - if not data or 'phase' not in data: - return jsonify({"error": "Phase not provided"}), 400 - - new_phase = data['phase'] - valid_phases = ['fresh', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] - - if new_phase not in valid_phases: - return jsonify({"error": f"Invalid phase. Must be one of: {', '.join(valid_phases)}"}), 400 - - state = tidal_discovery_states[playlist_id] - old_phase = state.get('phase', 'unknown') - state['phase'] = new_phase - state['last_accessed'] = time.time() - - logger.info(f"Updated Tidal playlist {playlist_id} phase: {old_phase} → {new_phase}") - return jsonify({"success": True, "message": f"Phase updated to {new_phase}", "old_phase": old_phase, "new_phase": new_phase}) - - except Exception as e: - logger.error(f"Error updating Tidal playlist phase: {e}") - return jsonify({"error": str(e)}), 500 + return _update_source_playlist_phase(tidal_discovery_states, playlist_id, "Tidal playlist not found", "Tidal", _PHASE_LIST, False) _playlist_discovery_cancelled = set() # Set of automation_ids that have been cancelled @@ -20879,6 +20855,7 @@ from core.discovery.endpoints import ( get_playlist_states as _get_playlist_states_core, start_sync as _start_sync_core, update_discovery_match as _update_discovery_match_core, + update_playlist_phase as _update_playlist_phase_core, playlist_name_attr_or_unknown as _pl_name_attr_or_unknown, playlist_name_strict as _pl_name_strict, playlist_name_safe as _pl_name_safe, @@ -20991,6 +20968,23 @@ def _update_source_discovery_match(states, source_log_label, error_label, return jsonify(body), code +# Valid phase lists for update_*_playlist_phase (YouTube additionally allows 'parsed'). +_PHASE_LIST = ['fresh', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] +_PHASE_LIST_YT = ['fresh', 'parsed', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] + + +def _update_source_playlist_phase(states, key, not_found_message, error_label, + valid_phases, apply_extra_fields): + """Thin glue for the per-source update_*_playlist_phase routes + (Tidal/Deezer/Qobuz/Spotify-Public/YouTube).""" + body, code = _update_playlist_phase_core( + states, key, lambda: request.get_json(), + not_found_message=not_found_message, error_label=error_label, + valid_phases=valid_phases, apply_extra_fields=apply_extra_fields, + ) + return jsonify(body), code + + def _build_tidal_discovery_deps(): """Build the TidalDiscoveryDeps bundle from web_server.py globals on each call.""" return _discovery_tidal.TidalDiscoveryDeps( @@ -21326,39 +21320,7 @@ def delete_deezer_playlist(playlist_id): @app.route('/api/deezer/update_phase/', methods=['POST']) def update_deezer_playlist_phase(playlist_id): """Update Deezer playlist phase (used when modal closes to reset from download_complete to discovered)""" - try: - if playlist_id not in deezer_discovery_states: - return jsonify({"error": "Deezer playlist not found"}), 404 - - data = request.get_json() - if not data or 'phase' not in data: - return jsonify({"error": "Phase not provided"}), 400 - - new_phase = data['phase'] - valid_phases = ['fresh', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] - - if new_phase not in valid_phases: - return jsonify({"error": f"Invalid phase. Must be one of: {', '.join(valid_phases)}"}), 400 - - state = deezer_discovery_states[playlist_id] - old_phase = state.get('phase', 'unknown') - state['phase'] = new_phase - state['last_accessed'] = time.time() - - # Update download process ID if provided (for download persistence) - if 'download_process_id' in data: - state['download_process_id'] = data['download_process_id'] - - # Update converted Spotify playlist ID if provided (for download persistence) - if 'converted_spotify_playlist_id' in data: - state['converted_spotify_playlist_id'] = data['converted_spotify_playlist_id'] - - logger.info(f"Updated Deezer playlist {playlist_id} phase: {old_phase} → {new_phase}") - return jsonify({"success": True, "message": f"Phase updated to {new_phase}", "old_phase": old_phase, "new_phase": new_phase}) - - except Exception as e: - logger.error(f"Error updating Deezer playlist phase: {e}") - return jsonify({"error": str(e)}), 500 + return _update_source_playlist_phase(deezer_discovery_states, playlist_id, "Deezer playlist not found", "Deezer", _PHASE_LIST, True) # Deezer discovery worker logic lives in core/discovery/deezer.py. @@ -21673,36 +21635,7 @@ def delete_qobuz_playlist(playlist_id): @app.route('/api/qobuz/update_phase/', methods=['POST']) def update_qobuz_playlist_phase(playlist_id): """Update Qobuz playlist phase (used when modal closes to reset from download_complete to discovered).""" - try: - if playlist_id not in qobuz_discovery_states: - return jsonify({"error": "Qobuz playlist not found"}), 404 - - data = request.get_json() - if not data or 'phase' not in data: - return jsonify({"error": "Phase not provided"}), 400 - - new_phase = data['phase'] - valid_phases = ['fresh', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] - - if new_phase not in valid_phases: - return jsonify({"error": f"Invalid phase. Must be one of: {', '.join(valid_phases)}"}), 400 - - state = qobuz_discovery_states[playlist_id] - old_phase = state.get('phase', 'unknown') - state['phase'] = new_phase - state['last_accessed'] = time.time() - - if 'download_process_id' in data: - state['download_process_id'] = data['download_process_id'] - if 'converted_spotify_playlist_id' in data: - state['converted_spotify_playlist_id'] = data['converted_spotify_playlist_id'] - - logger.info(f"Updated Qobuz playlist {playlist_id} phase: {old_phase} → {new_phase}") - return jsonify({"success": True, "message": f"Phase updated to {new_phase}", "old_phase": old_phase, "new_phase": new_phase}) - - except Exception as e: - logger.error(f"Error updating Qobuz playlist phase: {e}") - return jsonify({"error": str(e)}), 500 + return _update_source_playlist_phase(qobuz_discovery_states, playlist_id, "Qobuz playlist not found", "Qobuz", _PHASE_LIST, True) # Qobuz discovery worker logic lives in core/discovery/qobuz.py. @@ -22284,39 +22217,7 @@ def delete_spotify_public_playlist(url_hash): @app.route('/api/spotify-public/update_phase/', methods=['POST']) def update_spotify_public_playlist_phase(url_hash): """Update Spotify Public playlist phase (used when modal closes to reset from download_complete to discovered)""" - try: - if url_hash not in spotify_public_discovery_states: - return jsonify({"error": "Spotify Public playlist not found"}), 404 - - data = request.get_json() - if not data or 'phase' not in data: - return jsonify({"error": "Phase not provided"}), 400 - - new_phase = data['phase'] - valid_phases = ['fresh', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] - - if new_phase not in valid_phases: - return jsonify({"error": f"Invalid phase. Must be one of: {', '.join(valid_phases)}"}), 400 - - state = spotify_public_discovery_states[url_hash] - old_phase = state.get('phase', 'unknown') - state['phase'] = new_phase - state['last_accessed'] = time.time() - - # Update download process ID if provided (for download persistence) - if 'download_process_id' in data: - state['download_process_id'] = data['download_process_id'] - - # Update converted Spotify playlist ID if provided (for download persistence) - if 'converted_spotify_playlist_id' in data: - state['converted_spotify_playlist_id'] = data['converted_spotify_playlist_id'] - - logger.info(f"Updated Spotify Public playlist {url_hash} phase: {old_phase} → {new_phase}") - return jsonify({"success": True, "message": f"Phase updated to {new_phase}", "old_phase": old_phase, "new_phase": new_phase}) - - except Exception as e: - logger.error(f"Error updating Spotify Public playlist phase: {e}") - return jsonify({"error": str(e)}), 500 + return _update_source_playlist_phase(spotify_public_discovery_states, url_hash, "Spotify Public playlist not found", "Spotify Public", _PHASE_LIST, True) # Spotify Public discovery worker logic lives in core/discovery/spotify_public.py. @@ -23423,31 +23324,7 @@ def delete_youtube_playlist(url_hash): @app.route('/api/youtube/update_phase/', methods=['POST']) def update_youtube_playlist_phase(url_hash): """Update YouTube playlist phase (used when modal closes to reset from download_complete to discovered)""" - try: - if url_hash not in youtube_playlist_states: - return jsonify({"error": "YouTube playlist not found"}), 404 - - data = request.get_json() - if not data or 'phase' not in data: - return jsonify({"error": "Phase not provided"}), 400 - - new_phase = data['phase'] - valid_phases = ['fresh', 'parsed', 'discovering', 'discovered', 'syncing', 'sync_complete', 'downloading', 'download_complete'] - - if new_phase not in valid_phases: - return jsonify({"error": f"Invalid phase. Must be one of: {', '.join(valid_phases)}"}), 400 - - state = youtube_playlist_states[url_hash] - old_phase = state.get('phase', 'unknown') - state['phase'] = new_phase - state['last_accessed'] = time.time() - - logger.info(f"Updated YouTube playlist {url_hash} phase: {old_phase} → {new_phase}") - return jsonify({"success": True, "message": f"Phase updated to {new_phase}", "old_phase": old_phase, "new_phase": new_phase}) - - except Exception as e: - logger.error(f"Error updating YouTube playlist phase: {e}") - return jsonify({"error": str(e)}), 500 + return _update_source_playlist_phase(youtube_playlist_states, url_hash, "YouTube playlist not found", "YouTube", _PHASE_LIST_YT, False) def convert_youtube_results_to_spotify_tracks(discovery_results): """Convert YouTube discovery results to Spotify tracks format for sync"""