From ba5d62946a6e40e99f61c1bf43e3ad5bdd2b9db2 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 00:23:56 -0700 Subject: [PATCH] =?UTF-8?q?Mirrored=20playlists:=20custom=20name=20alias?= =?UTF-8?q?=20(overrides=20display=20+=20sync=20name,=20survives=20upstrea?= =?UTF-8?q?m=20refresh)=20=E2=80=94=20card=20rename=20button=20like=20the?= =?UTF-8?q?=20source-ref=20editor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/automation/handlers/sync_playlist.py | 11 ++- core/playlists/naming.py | 30 ++++++++ database/music_database.py | 44 ++++++++++++ tests/test_mirrored_custom_name.py | 86 +++++++++++++++++++++++ web_server.py | 32 +++++++++ webui/static/auto-sync.js | 33 +++++++++ webui/static/stats-automations.js | 4 +- 7 files changed, 236 insertions(+), 4 deletions(-) create mode 100644 core/playlists/naming.py create mode 100644 tests/test_mirrored_custom_name.py diff --git a/core/automation/handlers/sync_playlist.py b/core/automation/handlers/sync_playlist.py index 509c25ef..854d44d3 100644 --- a/core/automation/handlers/sync_playlist.py +++ b/core/automation/handlers/sync_playlist.py @@ -203,10 +203,15 @@ def auto_sync_playlist(config: Dict[str, Any], deps: AutomationDeps) -> Dict[str except Exception as e: deps.logger.debug("mirror sync last-status read: %s", e) + # Sync under the user's custom alias when set, else the upstream name (#865 + # follow-up). The server-side playlist is named with this. + from core.playlists.naming import effective_mirrored_name + sync_name = effective_mirrored_name(pl) or pl.get('name') or 'Playlist' + deps.update_progress( auto_id, progress=50, - phase=f'Syncing "{pl["name"]}"', + phase=f'Syncing "{sync_name}"', log_line=f'{len(tracks_json)} discovered, {skipped_count} skipped', log_type='info', ) @@ -221,14 +226,14 @@ def auto_sync_playlist(config: Dict[str, Any], deps: AutomationDeps) -> Dict[str skip_wishlist_add = bool(pl.get('organize_by_playlist')) threading.Thread( target=deps.run_sync_task, - args=(sync_id, pl['name'], tracks_json, auto_id, 1, pl.get('image_url', '')), + args=(sync_id, sync_name, tracks_json, auto_id, 1, pl.get('image_url', '')), kwargs={'skip_wishlist_add': skip_wishlist_add}, daemon=True, name=f'auto-sync-{playlist_id}', ).start() return { 'status': 'started', - 'playlist_name': pl['name'], + 'playlist_name': sync_name, 'discovered_tracks': str(len(tracks_json)), 'skipped_tracks': str(skipped_count), '_manages_own_progress': True, diff --git a/core/playlists/naming.py b/core/playlists/naming.py new file mode 100644 index 00000000..a588b122 --- /dev/null +++ b/core/playlists/naming.py @@ -0,0 +1,30 @@ +"""Effective name for a mirrored playlist. + +A mirrored playlist tracks an upstream playlist, so its ``name`` column is +rewritten from upstream on every refresh (see ``mirror_playlist``). Users can set +a ``custom_name`` alias that overrides what's shown in the UI and what's used when +syncing the playlist to the media server — while staying tied to the original +(the upstream ``name`` keeps tracking; the alias just overrides the visible/synced +label, and lives in its own column so refresh never clobbers it). + +This module is the single, pure source of truth for "which name wins", so the API +payload, the card, and the sync path all agree. +""" + +from __future__ import annotations + +from typing import Any, Mapping + + +def effective_mirrored_name(playlist: Mapping[str, Any]) -> str: + """Return the name to DISPLAY + SYNC: the user's ``custom_name`` alias when + set (non-blank), otherwise the upstream ``name``. Always returns a string.""" + if not isinstance(playlist, Mapping): + return '' + custom = str(playlist.get('custom_name') or '').strip() + if custom: + return custom + return str(playlist.get('name') or '').strip() + + +__all__ = ['effective_mirrored_name'] diff --git a/database/music_database.py b/database/music_database.py index ef0705e1..518ac116 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -579,6 +579,7 @@ class MusicDatabase: # Add explored_at to mirrored_playlists (migration) self._add_mirrored_playlist_explored_column(cursor) self._add_mirrored_playlist_organize_column(cursor) + self._add_mirrored_playlist_custom_name_column(cursor) # Add notification columns to automations (migration) self._add_automation_notify_columns(cursor) @@ -1251,6 +1252,23 @@ class MusicDatabase: except Exception as e: logger.error(f"Error adding organize_by_playlist column to mirrored_playlists: {e}") + def _add_mirrored_playlist_custom_name_column(self, cursor): + """Add custom_name (a user alias) for a mirrored playlist. + + Overrides the upstream ``name`` for both UI display and sync-to-server, + while staying tied to the original — the upstream ``name`` keeps tracking + on refresh, ``custom_name`` just overrides what's shown/synced. Stored in + its OWN column (not ``name``) precisely so ``mirror_playlist`` — which + rewrites ``name`` from upstream on every refresh — never clobbers it.""" + try: + cursor.execute("PRAGMA table_info(mirrored_playlists)") + cols = [c[1] for c in cursor.fetchall()] + if 'custom_name' not in cols: + cursor.execute("ALTER TABLE mirrored_playlists ADD COLUMN custom_name TEXT DEFAULT NULL") + logger.info("Added custom_name column to mirrored_playlists table") + except Exception as e: + logger.error(f"Error adding custom_name column to mirrored_playlists: {e}") + def _add_automation_notify_columns(self, cursor): """Add notification and result columns to automations table.""" try: @@ -13735,6 +13753,32 @@ class MusicDatabase: logger.error(f"Error updating organize_by_playlist for playlist {playlist_id}: {e}") return False + def set_mirrored_playlist_custom_name(self, playlist_id: int, custom_name) -> bool: + """Set or clear a user alias for a mirrored playlist. + + A blank/None value CLEARS the alias (display + sync fall back to the + upstream name). Touches only ``custom_name`` + ``updated_at``, leaving the + upstream ``name`` and the tracks untouched — so the alias survives upstream + refresh and never disturbs anything else (mirrors the source-ref/organize + update pattern).""" + value = (str(custom_name).strip() or None) if custom_name is not None else None + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute( + """ + UPDATE mirrored_playlists + SET custom_name = ?, updated_at = CURRENT_TIMESTAMP + WHERE id = ? + """, + (value, playlist_id), + ) + conn.commit() + return cursor.rowcount > 0 + except Exception as e: + logger.error(f"Error updating custom_name for playlist {playlist_id}: {e}") + return False + def get_mirrored_playlist_tracks(self, playlist_id: int) -> List[Dict]: """Return all tracks for a mirrored playlist ordered by position.""" try: diff --git a/tests/test_mirrored_custom_name.py b/tests/test_mirrored_custom_name.py new file mode 100644 index 00000000..0a69f1d1 --- /dev/null +++ b/tests/test_mirrored_custom_name.py @@ -0,0 +1,86 @@ +"""Mirrored-playlist custom name (alias) — seam + DB regression tests. + +Users can rename a mirrored playlist; the alias overrides the name shown in the +UI and used when syncing, while the playlist stays tied to its upstream source. +The non-negotiable guarantee: the alias must SURVIVE an upstream refresh (which +rewrites the upstream `name`). +""" + +from __future__ import annotations + +from core.playlists.naming import effective_mirrored_name +from database.music_database import MusicDatabase + + +# ── pure seam ──────────────────────────────────────────────────────────────── + +def test_custom_name_wins_when_set(): + assert effective_mirrored_name({'name': 'Discover Weekly', 'custom_name': 'My Jams'}) == 'My Jams' + + +def test_falls_back_to_upstream_name_when_no_alias(): + assert effective_mirrored_name({'name': 'Discover Weekly', 'custom_name': None}) == 'Discover Weekly' + assert effective_mirrored_name({'name': 'Discover Weekly', 'custom_name': ''}) == 'Discover Weekly' + assert effective_mirrored_name({'name': 'Discover Weekly', 'custom_name': ' '}) == 'Discover Weekly' + assert effective_mirrored_name({'name': 'Discover Weekly'}) == 'Discover Weekly' + + +def test_safe_on_bad_input(): + assert effective_mirrored_name(None) == '' + assert effective_mirrored_name('nope') == '' + assert effective_mirrored_name({}) == '' + + +# ── DB behaviour ───────────────────────────────────────────────────────────── + +def _mk(tmp_path): + db = MusicDatabase(str(tmp_path / 'm.db')) + pk = db.mirror_playlist(source='spotify', source_playlist_id='PL1', + name='Original Name', tracks=[], profile_id=1) + assert pk + return db, pk + + +def test_set_and_clear_custom_name(tmp_path): + db, pk = _mk(tmp_path) + assert db.get_mirrored_playlist(pk).get('custom_name') in (None, '') + + assert db.set_mirrored_playlist_custom_name(pk, 'My Alias') is True + assert db.get_mirrored_playlist(pk)['custom_name'] == 'My Alias' + + # Blank clears it back to upstream. + assert db.set_mirrored_playlist_custom_name(pk, ' ') is True + assert db.get_mirrored_playlist(pk).get('custom_name') is None + + # None clears too. + db.set_mirrored_playlist_custom_name(pk, 'Again') + assert db.set_mirrored_playlist_custom_name(pk, None) is True + assert db.get_mirrored_playlist(pk).get('custom_name') is None + + +def test_alias_survives_upstream_refresh(tmp_path): + """THE regression: re-mirroring (refresh) rewrites the upstream `name` but + must NOT touch the custom alias.""" + db, pk = _mk(tmp_path) + db.set_mirrored_playlist_custom_name(pk, 'My Alias') + + # Upstream renamed the playlist + added tracks → refresh. + db.mirror_playlist(source='spotify', source_playlist_id='PL1', + name='Upstream Renamed', tracks=[ + {'track_name': 'A', 'artist_name': 'X'}, + ], profile_id=1) + + row = db.get_mirrored_playlist(pk) + assert row['name'] == 'Upstream Renamed' # upstream name keeps tracking + assert row['custom_name'] == 'My Alias' # alias preserved + assert effective_mirrored_name(row) == 'My Alias' + + +def test_set_custom_name_does_not_touch_other_fields(tmp_path): + db, pk = _mk(tmp_path) + before = db.get_mirrored_playlist(pk) + db.set_mirrored_playlist_custom_name(pk, 'Alias') + after = db.get_mirrored_playlist(pk) + assert after['name'] == before['name'] + assert after['source'] == before['source'] + assert after['source_playlist_id'] == before['source_playlist_id'] diff --git a/web_server.py b/web_server.py index 3c86e85b..1443e3b3 100644 --- a/web_server.py +++ b/web_server.py @@ -33841,6 +33841,7 @@ def get_mirrored_playlists_endpoint(): """List all mirrored playlists for the active profile.""" try: from core.playlists.source_refs import describe_mirrored_source_ref + from core.playlists.naming import effective_mirrored_name database = get_database() profile_id = get_current_profile_id() playlists = database.get_mirrored_playlists(profile_id=profile_id) @@ -33859,6 +33860,9 @@ def get_mirrored_playlists_endpoint(): pl['source_ref_kind'] = source_ref.source_ref_kind pl['source_ref_status'] = source_ref.source_ref_status pl['source_ref_error'] = source_ref.source_ref_error + # The name the UI should show / sync uses: custom alias if set, else + # the upstream name. Single source of truth so card + sync agree. + pl['display_name'] = effective_mirrored_name(pl) pl['pipeline_state'] = _snapshot_playlist_pipeline_state(pl['id']) return jsonify(playlists) except Exception as e: @@ -33870,6 +33874,7 @@ def get_mirrored_playlist_endpoint(playlist_id): """Get a mirrored playlist with its tracks.""" try: from core.playlists.source_refs import describe_mirrored_source_ref + from core.playlists.naming import effective_mirrored_name database = get_database() playlist = database.get_mirrored_playlist(playlist_id) if not playlist: @@ -33879,6 +33884,7 @@ def get_mirrored_playlist_endpoint(playlist_id): playlist['source_ref_kind'] = source_ref.source_ref_kind playlist['source_ref_status'] = source_ref.source_ref_status playlist['source_ref_error'] = source_ref.source_ref_error + playlist['display_name'] = effective_mirrored_name(playlist) playlist['pipeline_state'] = _snapshot_playlist_pipeline_state(playlist_id) playlist['tracks'] = database.get_mirrored_playlist_tracks(playlist_id) return jsonify(playlist) @@ -33936,6 +33942,32 @@ def update_mirrored_playlist_source_ref_endpoint(playlist_id): return jsonify({"error": str(e)}), 500 +@app.route('/api/mirrored-playlists//custom-name', methods=['PATCH']) +def update_mirrored_playlist_custom_name_endpoint(playlist_id): + """Set or clear a user alias (custom display + sync name) for a mirrored + playlist. A blank/missing custom_name CLEARS the alias (falls back to the + upstream name). The upstream name keeps tracking on refresh either way.""" + try: + from core.playlists.naming import effective_mirrored_name + data = request.get_json() or {} + database = get_database() + playlist = database.get_mirrored_playlist(playlist_id) + if not playlist: + return jsonify({"error": "Playlist not found"}), 404 + + # `custom_name` may be '' / null to CLEAR the alias. + ok = database.set_mirrored_playlist_custom_name(playlist_id, data.get('custom_name')) + if not ok: + return jsonify({"error": "Failed to update name"}), 500 + + updated = database.get_mirrored_playlist(playlist_id) or {} + updated['display_name'] = effective_mirrored_name(updated) + return jsonify({"success": True, "playlist": updated}) + except Exception as e: + logger.error(f"Error updating mirrored playlist custom name: {e}") + return jsonify({"error": str(e)}), 500 + + @app.route('/api/mirrored-playlists//preferences', methods=['PATCH']) def update_mirrored_playlist_preferences_endpoint(playlist_id): """Update per-playlist download preferences (e.g. organize by playlist folder).""" diff --git a/webui/static/auto-sync.js b/webui/static/auto-sync.js index fd243ed1..8a91d3ad 100644 --- a/webui/static/auto-sync.js +++ b/webui/static/auto-sync.js @@ -2014,6 +2014,39 @@ async function parseMirroredPipelineResponse(res, fallbackMessage) { return data; } +async function editMirroredCustomName(playlistId, originalName, currentCustom) { + // Custom alias: changes the name shown in SoulSync AND used when syncing to + // the media server, while staying tied to the original upstream playlist. + // Blank clears the alias (falls back to the original name). + const nextName = window.prompt( + `Rename "${originalName}"\n\nThis name is shown here and used when syncing. ` + + `Leave blank to use the original name.`, + currentCustom || '' + ); + if (nextName === null) return; // cancelled + const trimmed = nextName.trim(); + try { + const res = await fetch(`/api/mirrored-playlists/${playlistId}/custom-name`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ custom_name: trimmed }) + }); + const data = await res.json(); + if (!res.ok || data.error) { + throw new Error(data.error || 'Failed to update name'); + } + showToast(trimmed ? `Renamed to "${trimmed}"` : `Reverted to "${originalName}"`, 'success'); + loadMirroredPlaylists(); + const openModal = document.getElementById('mirrored-track-modal'); + if (openModal) { + closeMirroredModal(); + openMirroredPlaylistModal(playlistId); + } + } catch (err) { + showToast(`Error: ${err.message}`, 'error'); + } +} + async function editMirroredSourceRef(playlistId, name, source, currentRef) { const label = (source === 'spotify_public' || source === 'youtube') ? 'original playlist URL' diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index 68e1adea..395ba528 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -509,7 +509,8 @@ function renderMirroredCard(p, container) { card.innerHTML = `
${srcIcon}
-
${_esc(p.name)}
+
${_esc(p.display_name || p.name)}
+ ${p.custom_name ? `
↳ ${_esc(p.name)}
` : ''}
${_esc(p.source)} ${p.track_count} tracks @@ -520,6 +521,7 @@ function renderMirroredCard(p, container) {
${disc > 0 ? `` : ''} + `;