Make artist_name Optional in find_library_artist_for_source
Cin's review note: typing artist_name as plain `str` forced callers that didn't have a name to pass `""` as a placeholder, which leaks the parameter's emptiness contract into every call site and reads badly in tests. Switching to `Optional[str] = None` lets callers omit it. The function body's `if artist_name and active_server:` check already handles None and "" identically, so no body changes were needed. Tests that previously passed `artist_name=""` drop the argument; one new test covers the omitted-arg path explicitly. The web_server.py wrapper takes the same default for symmetry.
This commit is contained in:
parent
a097cf3d5a
commit
e66af77ff6
3 changed files with 12 additions and 6 deletions
|
|
@ -43,7 +43,7 @@ def find_library_artist_for_source(
|
|||
database,
|
||||
source: str,
|
||||
source_artist_id: str,
|
||||
artist_name: str,
|
||||
artist_name: Optional[str] = None,
|
||||
active_server: Optional[str] = None,
|
||||
) -> Optional[str]:
|
||||
"""Return the library PK of an artist matching the source-aware click.
|
||||
|
|
|
|||
|
|
@ -109,9 +109,7 @@ class TestFindLibraryArtistForSource:
|
|||
**{column: source_value},
|
||||
)
|
||||
|
||||
result = find_library_artist_for_source(
|
||||
db, source, source_value, artist_name=""
|
||||
)
|
||||
result = find_library_artist_for_source(db, source, source_value)
|
||||
assert result == f"pk-{source}"
|
||||
|
||||
def test_unknown_source_returns_none(self, db):
|
||||
|
|
@ -121,8 +119,16 @@ class TestFindLibraryArtistForSource:
|
|||
|
||||
def test_lookup_misses_when_source_id_unknown(self, db):
|
||||
_insert_artist(db, artist_id="pk-real", name="Real Artist", deezer_id="dz-real")
|
||||
assert find_library_artist_for_source(db, "deezer", "dz-not-real") is None
|
||||
|
||||
def test_artist_name_is_optional(self, db):
|
||||
"""Callers that don't have a name handy should be able to omit it
|
||||
without falling through to the name-fallback branch."""
|
||||
_insert_artist(db, artist_id="pk-q", name="Some Artist", server_source="plex")
|
||||
# No source-id match, no name passed → must return None even when
|
||||
# active_server is set (otherwise we'd risk matching by None name).
|
||||
assert find_library_artist_for_source(
|
||||
db, "deezer", "dz-not-real", artist_name=""
|
||||
db, "deezer", "no-id-match", active_server="plex"
|
||||
) is None
|
||||
|
||||
def test_name_fallback_matches_within_active_server(self, db):
|
||||
|
|
|
|||
|
|
@ -11323,7 +11323,7 @@ from core.artist_source_lookup import (
|
|||
)
|
||||
|
||||
|
||||
def _find_library_artist_for_source(database, source, source_artist_id, artist_name):
|
||||
def _find_library_artist_for_source(database, source, source_artist_id, artist_name=None):
|
||||
"""Thin wrapper that injects the active-server context for the core lookup."""
|
||||
try:
|
||||
active_server = config_manager.get_active_media_server()
|
||||
|
|
|
|||
Loading…
Reference in a new issue