Mirrored playlists: custom name alias (overrides display + sync name, survives upstream refresh) — card rename button like the source-ref editor
This commit is contained in:
parent
6366f72b7e
commit
ba5d62946a
7 changed files with 236 additions and 4 deletions
|
|
@ -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,
|
||||
|
|
|
|||
30
core/playlists/naming.py
Normal file
30
core/playlists/naming.py
Normal file
|
|
@ -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']
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
86
tests/test_mirrored_custom_name.py
Normal file
86
tests/test_mirrored_custom_name.py
Normal file
|
|
@ -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']
|
||||
|
|
@ -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/<int:playlist_id>/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/<int:playlist_id>/preferences', methods=['PATCH'])
|
||||
def update_mirrored_playlist_preferences_endpoint(playlist_id):
|
||||
"""Update per-playlist download preferences (e.g. organize by playlist folder)."""
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -509,7 +509,8 @@ function renderMirroredCard(p, container) {
|
|||
card.innerHTML = `
|
||||
<div class="source-icon ${_escAttr(p.source)}">${srcIcon}</div>
|
||||
<div class="mirrored-card-info">
|
||||
<div class="card-name">${_esc(p.name)}</div>
|
||||
<div class="card-name">${_esc(p.display_name || p.name)}</div>
|
||||
${p.custom_name ? `<div class="card-original-name" title="Original (upstream) playlist name — still tracked">↳ ${_esc(p.name)}</div>` : ''}
|
||||
<div class="card-meta">
|
||||
<span class="source-badge ${_escAttr(p.source)}">${_esc(p.source)}</span>
|
||||
<span>${p.track_count} tracks</span>
|
||||
|
|
@ -520,6 +521,7 @@ function renderMirroredCard(p, container) {
|
|||
</div>
|
||||
${disc > 0 ? `<button class="mirrored-card-clear" onclick="event.stopPropagation(); clearMirroredDiscovery(${p.id}, '${_escJs(p.name)}')" title="Clear discovery data">↺</button>` : ''}
|
||||
<button class="mirrored-card-pipeline" onclick="event.stopPropagation(); runMirroredPlaylistPipeline(${p.id}, '${_escJs(p.name)}')" title="Refresh, discover, sync, and queue missing tracks">Auto-Sync</button>
|
||||
<button class="mirrored-card-rename" onclick="event.stopPropagation(); editMirroredCustomName(${p.id}, '${_escJs(p.name)}', '${_escJs(p.custom_name || '')}')" title="Rename (changes the name shown here and used when syncing)">✏️</button>
|
||||
<button class="mirrored-card-link" onclick="event.stopPropagation(); editMirroredSourceRef(${p.id}, '${_escJs(p.name)}', '${_escJs(p.source)}', '${_escJs(sourceRef)}')" title="Edit original playlist link">🔗</button>
|
||||
<button class="mirrored-card-delete" onclick="event.stopPropagation(); deleteMirroredPlaylist(${p.id}, '${_escJs(p.name)}')" title="Delete mirror">✕</button>
|
||||
`;
|
||||
|
|
|
|||
Loading…
Reference in a new issue