From 0063feaf0fcf41bf389737376afaec425bf70b15 Mon Sep 17 00:00:00 2001 From: tcconnally Date: Wed, 15 Jul 2026 08:09:34 -0500 Subject: [PATCH] =?UTF-8?q?fix(tags):=20read=20Emby=20tags=20from=20TagIte?= =?UTF-8?q?ms=20=E2=80=94=20ToDelete=20state=20was=20wrong=20on=20the=20wa?= =?UTF-8?q?ll?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emby serves applied tags under `TagItems` ([{Name,Id}]) and returns the legacy `Tags` string list as null on BOTH the item list and detail endpoints (probed live on greg 2026-07-15). Three sites read `item["Tags"]` directly: - cell.py:629 — the red ToDelete button computed `checked=False` for an already-tagged clip loaded from the library (confirmed: the two files just tagged came back Tags=None / TagItems=[ToDelete] via the exact fetch_items query, so the indicator lied). - cell.py _toggle_tag — read `item["Tags"]` to decide add-vs-remove; `setdefault("Tags", [])` returns the null, so `list(None)` would raise on a library-loaded tagged clip (and mis-toggle even if it didn't). - wall.py update_tags — built the POST body from `Tags`, so it could drop a tag it never saw. Fix: one pure helper `urls.tag_names(item)` that prefers TagItems and falls back to Tags (string list or dict list), used at all three sites. `_toggle_tag` now keeps both shapes in sync in the local dict so the helper reflects the new state on the next read. The WRITE path is unchanged — Emby rebuilds TagItems from the posted `Tags` string list (verified: the manual tag persisted). Tests: 6 in test_urls.py covering the real Emby shape (Tags=null + TagItems), precedence, both fallbacks, empty/missing safety, and the untagged-vs-tagged checked computation. Suite green at v10.13.2. Co-Authored-By: Claude Opus 4.8 --- hyperwall/__init__.py | 2 +- hyperwall/cell.py | 25 +++++++++++-------------- hyperwall/urls.py | 20 ++++++++++++++++++++ hyperwall/wall.py | 10 +++------- tests/run_repo_guards.py | 4 ++-- tests/test_urls.py | 38 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 75 insertions(+), 24 deletions(-) diff --git a/hyperwall/__init__.py b/hyperwall/__init__.py index caba338..57ba886 100644 --- a/hyperwall/__init__.py +++ b/hyperwall/__init__.py @@ -7,7 +7,7 @@ from __future__ import annotations -__version__ = "10.13.1" +__version__ = "10.13.2" # Short "major.minor" form, derived — used for User-Agent / Emby auth Version / # window titles so a version bump touches exactly ONE line (this file). VERSION_SHORT = ".".join(__version__.split(".")[:2]) diff --git a/hyperwall/cell.py b/hyperwall/cell.py index 36601dd..ade3ccb 100644 --- a/hyperwall/cell.py +++ b/hyperwall/cell.py @@ -78,6 +78,7 @@ ) from . import theme from .perftrace import traced +from .urls import tag_names logger = logging.getLogger("HyperWall") @@ -625,14 +626,10 @@ def _begin_track(self, item: dict[str, Any]) -> None: self.lbl_title.setText(item.get("Name", "Unknown")) - # Update tag/fav buttons - raw = item.get("Tags", []) - tag_names = ( - [t.get("Name", "") for t in raw] - if raw and isinstance(raw[0], dict) - else list(raw) - ) - self.btn_tag.setChecked("ToDelete" in tag_names) + # Update tag/fav buttons. Emby serves applied tags under TagItems and + # leaves Tags null, so read via the shared helper (item["Tags"] alone + # shows an already-tagged clip as untagged). + self.btn_tag.setChecked("ToDelete" in tag_names(item)) self.btn_fav.setChecked( item.get("UserData", {}).get("IsFavorite", False) ) @@ -1203,17 +1200,17 @@ def _toggle_tag(self) -> None: if not self.current_item: return self._nudge_pill() - raw = self.current_item.setdefault("Tags", []) - tags = ( - [t.get("Name", "") for t in raw] - if raw and isinstance(raw[0], dict) - else list(raw) - ) + # Read via the helper (Emby puts tags in TagItems, leaves Tags null — + # the old item["Tags"] read hit list(None) on a library-loaded clip). + tags = tag_names(self.current_item) if "ToDelete" in tags: tags.remove("ToDelete") else: tags.append("ToDelete") + # Keep BOTH shapes in the local dict in sync so the helper (which + # prefers TagItems) reflects the new state on the next read. self.current_item["Tags"] = tags + self.current_item["TagItems"] = [{"Name": t} for t in tags] self.btn_tag.setChecked("ToDelete" in tags) # :checked tints the glyph red self.controller.update_tags(self.current_item) diff --git a/hyperwall/urls.py b/hyperwall/urls.py index da35cd8..b8884b0 100644 --- a/hyperwall/urls.py +++ b/hyperwall/urls.py @@ -97,6 +97,26 @@ def needs_transcode( ) +def tag_names(item: dict[str, Any]) -> list[str]: + """Tag names for an Emby item, tolerant of Emby's tag shapes. + + Emby returns applied tags under ``TagItems`` (a list of ``{Name, Id}``) + and leaves the legacy ``Tags`` string list **null** on both the item + list and detail endpoints — so reading ``item["Tags"]`` alone sees an + already-tagged item as untagged (the wall's ToDelete indicator was wrong, + and toggling a library-loaded tagged clip hit ``list(None)``). Prefer + ``TagItems``; fall back to ``Tags`` (which may itself be a string list or, + on some shapes, a list of dicts). Always returns a fresh list of str. + """ + ti = item.get("TagItems") + if ti: + return [t.get("Name", "") for t in ti if isinstance(t, dict)] + raw = item.get("Tags") or [] + if raw and isinstance(raw[0], dict): + return [t.get("Name", "") for t in raw] + return list(raw) + + def build_stream_url( *, base: str, diff --git a/hyperwall/wall.py b/hyperwall/wall.py index 1391abb..28a601d 100644 --- a/hyperwall/wall.py +++ b/hyperwall/wall.py @@ -53,7 +53,7 @@ from .emby import EmbyClient, ContentLoader from .urls import needs_transcode as _needs_transcode_pure from .reliability import is_systemic_outage -from .urls import build_stream_url +from .urls import build_stream_url, tag_names from .playlist import PlaylistManager, DEFAULT_GROUP logger = logging.getLogger("HyperWall") @@ -525,12 +525,8 @@ def _set_filter(self, mode: str) -> None: def update_tags(self, item: dict[str, Any]) -> None: iid = item["Id"] name = item.get("Name", "Unknown") - raw = item.get("Tags", []) - tags = ( - [t.get("Name", "") for t in raw] - if raw and isinstance(raw[0], dict) - else list(raw) - ) + # Read via the helper (Emby serves tags under TagItems, Tags is null). + tags = tag_names(item) def _worker() -> None: try: diff --git a/tests/run_repo_guards.py b/tests/run_repo_guards.py index ee5204a..0a127cc 100644 --- a/tests/run_repo_guards.py +++ b/tests/run_repo_guards.py @@ -46,10 +46,10 @@ def test_01_entry_point_imports(): def test_02_package_identity(): """Package has version and banner.""" from hyperwall import __version__, runtime_banner - assert __version__ == "10.13.1" + assert __version__ == "10.13.2" banner = runtime_banner() assert "Hyperwall" in banner - assert "10.13.1" in banner + assert "10.13.2" in banner def test_03_config_loads(): diff --git a/tests/test_urls.py b/tests/test_urls.py index e1944ef..136afbe 100644 --- a/tests/test_urls.py +++ b/tests/test_urls.py @@ -21,6 +21,7 @@ exceeds_1080p, exceeds_direct_budget, needs_transcode, + tag_names, ) @@ -203,6 +204,43 @@ def test_urls_carry_item_and_key(): assert "api_key=tok" in url +# ── tag_names (Emby TagItems vs Tags shape) ─────────────────────────────────── + +def test_tag_names_reads_tagitems_when_tags_null(): + # The exact shape greg's Emby returns for a tagged item (probed 2026-07-15): + # Tags is null, the applied tag lives in TagItems. The old item["Tags"] + # read saw this as untagged → wrong ToDelete indicator + list(None) crash. + item = {"Id": "x", "Tags": None, + "TagItems": [{"Name": "ToDelete", "Id": 21516}]} + assert tag_names(item) == ["ToDelete"] + + +def test_tag_names_tagitems_takes_precedence_over_tags(): + item = {"TagItems": [{"Name": "ToDelete"}], "Tags": ["stale"]} + assert tag_names(item) == ["ToDelete"] + + +def test_tag_names_falls_back_to_string_list(): + assert tag_names({"Tags": ["ToDelete", "keep"]}) == ["ToDelete", "keep"] + + +def test_tag_names_falls_back_to_dict_list_tags(): + assert tag_names({"Tags": [{"Name": "ToDelete"}]}) == ["ToDelete"] + + +def test_tag_names_empty_and_missing_safe(): + assert tag_names({}) == [] + assert tag_names({"Tags": None, "TagItems": None}) == [] + assert tag_names({"Tags": [], "TagItems": []}) == [] + + +def test_tag_names_untagged_item_not_checked(): + # The bug's user-visible symptom: an untagged item must compute False, a + # ToDelete-tagged one True — regardless of which field Emby populates. + assert "ToDelete" not in tag_names({"Tags": None, "TagItems": []}) + assert "ToDelete" in tag_names({"TagItems": [{"Name": "ToDelete"}]}) + + def run_all() -> int: tests = [v for k, v in sorted(globals().items()) if k.startswith("test_")] passed = failed = 0