#917: 'I have this' reuses the album's existing folder year instead of dropping it
The import rebuilds the destination path from album metadata. When the albums row has no year, release_date is empty, the path template drops $year, and the copied file lands in a NEW yearless directory instead of the album's existing 'Album (YYYY)' folder. (The code logically forces this: the year only drops when album.year is empty.) Fix: when album.year is empty, recover it from a sibling track — its own year column, else a (YYYY)/[YYYY] in the album folder name — so the rebuilt path matches the existing directory. No-op when album.year is already set. Tests: _existing_album_year_from_sibling covers year-column, paren folder, bracket folder, no-signal, and target-slot exclusion.
This commit is contained in:
parent
2934903874
commit
600a744f7f
2 changed files with 103 additions and 0 deletions
|
|
@ -10,6 +10,7 @@ from __future__ import annotations
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
import uuid
|
import uuid
|
||||||
from typing import Any, Callable, Dict, Optional
|
from typing import Any, Callable, Dict, Optional
|
||||||
|
|
@ -121,6 +122,20 @@ def import_existing_track_for_album_slot(album_id: str, payload: dict, deps: Mis
|
||||||
if album_data.get("server_source") and source_track.get("server_source") and album_data["server_source"] != source_track["server_source"]:
|
if album_data.get("server_source") and source_track.get("server_source") and album_data["server_source"] != source_track["server_source"]:
|
||||||
raise MissingTrackImportError("Selected track belongs to a different library source", 400)
|
raise MissingTrackImportError("Selected track belongs to a different library source", 400)
|
||||||
|
|
||||||
|
# #917: "I have this" rebuilds the destination path from album metadata. When the album row
|
||||||
|
# has no year, the rebuilt path drops the $year and the copied file lands in a NEW, yearless
|
||||||
|
# directory instead of the album's existing folder. Recover the year from a sibling track so
|
||||||
|
# the import reuses the same directory.
|
||||||
|
if not album_data.get("year"):
|
||||||
|
recovered_year = _existing_album_year_from_sibling(
|
||||||
|
database, album_id, deps.resolve_library_file_path_fn,
|
||||||
|
int(expected.get("disc_number") or 1), int(expected.get("track_number") or 1),
|
||||||
|
)
|
||||||
|
if recovered_year:
|
||||||
|
album_data["year"] = recovered_year
|
||||||
|
logger.info("[I Have This] recovered album year %s from existing folder for album %s",
|
||||||
|
recovered_year, album_id)
|
||||||
|
|
||||||
source_path = deps.resolve_library_file_path_fn(source_track.get("file_path"))
|
source_path = deps.resolve_library_file_path_fn(source_track.get("file_path"))
|
||||||
if not source_path:
|
if not source_path:
|
||||||
raise MissingTrackImportError(_file_not_found_message(source_track.get("file_path")), 404)
|
raise MissingTrackImportError(_file_not_found_message(source_track.get("file_path")), 404)
|
||||||
|
|
@ -426,6 +441,50 @@ def _sync_imported_track(deps: MissingTrackImportDeps, track_id, expected_title:
|
||||||
logger.debug("Existing-track import server sync skipped/failed: %s", sync_err)
|
logger.debug("Existing-track import server sync skipped/failed: %s", sync_err)
|
||||||
|
|
||||||
|
|
||||||
|
def _existing_album_year_from_sibling(
|
||||||
|
database,
|
||||||
|
album_id: str,
|
||||||
|
resolve_library_file_path_fn: Callable[[Optional[str]], Optional[str]],
|
||||||
|
target_disc: int,
|
||||||
|
target_track: int,
|
||||||
|
) -> Optional[str]:
|
||||||
|
"""Find the release year already baked into this album's on-disk folder (#917).
|
||||||
|
|
||||||
|
Read from a sibling track — its own ``year`` column first, else a ``(YYYY)`` /
|
||||||
|
``[YYYY]`` in the album folder name — so an "I have this" import reuses the album's
|
||||||
|
existing directory instead of rebuilding a yearless one. Returns the 4-digit year
|
||||||
|
string, or None when no signal exists.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
with database._get_connection() as conn:
|
||||||
|
cursor = conn.cursor()
|
||||||
|
cursor.execute(
|
||||||
|
"""
|
||||||
|
SELECT file_path, year FROM tracks
|
||||||
|
WHERE album_id = ?
|
||||||
|
AND file_path IS NOT NULL AND file_path != ''
|
||||||
|
AND NOT (COALESCE(disc_number, 1) = ? AND track_number = ?)
|
||||||
|
ORDER BY COALESCE(disc_number, 1), track_number
|
||||||
|
LIMIT 12
|
||||||
|
""",
|
||||||
|
(album_id, target_disc, target_track),
|
||||||
|
)
|
||||||
|
rows = cursor.fetchall()
|
||||||
|
for row in rows:
|
||||||
|
year = row["year"]
|
||||||
|
if year is not None and str(year).strip()[:4].isdigit():
|
||||||
|
return str(year).strip()[:4]
|
||||||
|
resolved = resolve_library_file_path_fn(row["file_path"])
|
||||||
|
if resolved:
|
||||||
|
folder = os.path.basename(os.path.dirname(resolved))
|
||||||
|
match = re.search(r"[(\[](\d{4})[)\]]", folder) # "Album (2019)" / "Album [2019]"
|
||||||
|
if match:
|
||||||
|
return match.group(1)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.debug("Could not recover album year from sibling for %s: %s", album_id, exc)
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def copy_album_identity_from_target_sibling(
|
def copy_album_identity_from_target_sibling(
|
||||||
database,
|
database,
|
||||||
album_id: str,
|
album_id: str,
|
||||||
|
|
|
||||||
|
|
@ -258,3 +258,47 @@ def test_import_rejects_missing_expected_track_context(tmp_path):
|
||||||
|
|
||||||
assert exc.value.status_code == 400
|
assert exc.value.status_code == 400
|
||||||
assert "expected_track" in str(exc.value)
|
assert "expected_track" in str(exc.value)
|
||||||
|
|
||||||
|
|
||||||
|
# ── #917: recover album year from existing folder so "I have this" reuses the dir ──
|
||||||
|
def _year_db(rows):
|
||||||
|
conn = sqlite3.connect(":memory:")
|
||||||
|
conn.row_factory = sqlite3.Row
|
||||||
|
conn.execute("CREATE TABLE tracks (album_id TEXT, disc_number INTEGER, track_number INTEGER, file_path TEXT, year INTEGER)")
|
||||||
|
conn.executemany(
|
||||||
|
"INSERT INTO tracks (album_id, disc_number, track_number, file_path, year) VALUES (?,?,?,?,?)",
|
||||||
|
[(r.get("album_id", "A1"), r.get("disc_number", 1), r.get("track_number", 1),
|
||||||
|
r.get("file_path"), r.get("year")) for r in rows],
|
||||||
|
)
|
||||||
|
conn.commit()
|
||||||
|
return _FakeDB(conn)
|
||||||
|
|
||||||
|
|
||||||
|
def _ident(p):
|
||||||
|
return p
|
||||||
|
|
||||||
|
|
||||||
|
def test_year_recovered_from_sibling_year_column():
|
||||||
|
db = _year_db([{"track_number": 3, "file_path": "/m/Artist/Album/03.flac", "year": 2024}])
|
||||||
|
assert mti._existing_album_year_from_sibling(db, "A1", _ident, 1, 5) == "2024"
|
||||||
|
|
||||||
|
|
||||||
|
def test_year_recovered_from_paren_folder_name():
|
||||||
|
db = _year_db([{"track_number": 3, "file_path": "/music/Artist/Album (2019)/03 - Song.flac", "year": None}])
|
||||||
|
assert mti._existing_album_year_from_sibling(db, "A1", _ident, 1, 5) == "2019"
|
||||||
|
|
||||||
|
|
||||||
|
def test_year_recovered_from_bracket_folder_name():
|
||||||
|
db = _year_db([{"track_number": 3, "file_path": "/music/Artist/Album [2008]/03.flac", "year": None}])
|
||||||
|
assert mti._existing_album_year_from_sibling(db, "A1", _ident, 1, 5) == "2008"
|
||||||
|
|
||||||
|
|
||||||
|
def test_year_none_when_no_signal():
|
||||||
|
db = _year_db([{"track_number": 3, "file_path": "/music/Artist/Album/03.flac", "year": None}])
|
||||||
|
assert mti._existing_album_year_from_sibling(db, "A1", _ident, 1, 5) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_year_ignores_the_target_slot_itself():
|
||||||
|
# The only sibling row IS the slot being imported -> excluded -> no year.
|
||||||
|
db = _year_db([{"disc_number": 1, "track_number": 5, "file_path": "/m/Album (2030)/05.flac", "year": 2030}])
|
||||||
|
assert mti._existing_album_year_from_sibling(db, "A1", _ident, 1, 5) is None
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue