From 183b744a8eb751dbcb4e752167eddc28c294de3c Mon Sep 17 00:00:00 2001 From: sbcrumb Date: Sun, 27 Sep 2026 14:30:40 -0400 Subject: [PATCH] fix: single-group Category Detail no longer appends ": All" to the prefix Category Detail = "None - single group (prefix only, no categories)" built its group name through the same path as every other category, giving ": All" instead of the prefix alone the setting's description promises. Fixed in _make_group_name, the one helper all callers share, so cleanup, stream re-sort and group lookup all agree on the new name too. Two edge cases handled per the issue discussion: a prefix with no channels still needs a name ("All", unchanged), and a prefix already ending in a separator (e.g. "DTV-") must not leave it dangling ("DTV", not "DTV-All"). Existing ": All" groups from before this fix are renamed in place on the next sync (preserving the group's id and its channels) rather than left behind as an orphaned duplicate next to a freshly created "" group. If a "" group already exists too, both are left alone and a warning is logged instead of merging channels silently. That migration runs from both places that resolve a channel's group: _sync_groups (used by Full Sync) and, as of this fix, _do_sync_channels as well - there is no Sync Groups button in the UI, so Sync Channels is, in practice, the only path some installs ever take. Before this it built its own group lookup independently and never called the migration, so pressing Sync Channels against an existing ": All" group created a second "" group instead of renaming the first, orphaning the channels already on the old one. Fixes #30 --- Lineuparr/plugin.py | 82 ++++++++++- tests/test_group_prefix_only.py | 235 ++++++++++++++++++++++++++++++++ 2 files changed, 315 insertions(+), 2 deletions(-) create mode 100644 tests/test_group_prefix_only.py diff --git a/Lineuparr/plugin.py b/Lineuparr/plugin.py index 232cecb..0d70af6 100644 --- a/Lineuparr/plugin.py +++ b/Lineuparr/plugin.py @@ -156,6 +156,13 @@ class PluginConfig: DEFAULT_RATE_LIMITING = "none" DEFAULT_CHANNEL_NUMBERING = "lineup" + # Sentinel category key _apply_category_detail uses for "None - single + # group" (Category Detail = "none"): every channel is merged under this one + # key, and _make_group_name special-cases it to mean "prefix only, no + # category suffix". Shared here so the two places that must agree on the + # exact string cannot drift apart. + SINGLE_GROUP_CATEGORY = "All" + RATE_LIMIT_NONE = 0.0 RATE_LIMIT_LOW = 0.1 RATE_LIMIT_MEDIUM = 0.5 @@ -845,7 +852,7 @@ def _apply_category_detail(self, data, detail): all_channels = [] for channels in original_cats.values(): all_channels.extend(channels) - result["categories"] = {"All": all_channels} + result["categories"] = {PluginConfig.SINGLE_GROUP_CATEGORY: all_channels} return result if detail in ("simple", "refined"): @@ -1037,7 +1044,22 @@ def _make_group_name(self, prefix, category): """Build full group name from prefix and category. If prefix ends with a separator character, append category directly. Otherwise add ': ' between prefix and category. - Examples: 'DTV-' + 'News' = 'DTV-News', 'DIRECTV' + 'News' = 'DIRECTV: News'""" + Examples: 'DTV-' + 'News' = 'DTV-News', 'DIRECTV' + 'News' = 'DIRECTV: News' + + Category Detail = "none" ("single group (prefix only, no categories)") + is the one exception: it merges every channel under the sentinel + category PluginConfig.SINGLE_GROUP_CATEGORY, and the resulting group + name must be the prefix ALONE, not ": All" - the setting's own + description promises "no categories". Every caller reaches this + sentinel the same way (it flows out of _apply_category_detail via + lineup["categories"]), so handling it here is the single place that + keeps every one of this method's callers in agreement. A real lineup + category that happens to be named exactly "All" under Normal/Simple/ + Refined detail collapses the same way; that is an accepted, harmless + edge case rather than something worth a second sentinel value.""" + if category == PluginConfig.SINGLE_GROUP_CATEGORY: + stripped_prefix = prefix.rstrip(" :_-/") if prefix else "" + return stripped_prefix or PluginConfig.SINGLE_GROUP_CATEGORY if prefix: if prefix[-1] in ":_-/ ": return f"{prefix}{category}" @@ -2727,6 +2749,49 @@ def _do_preview_stream_match(self, settings, logger): # DESTRUCTIVE ACTIONS # ======================================================================== + def _migrate_single_group_name(self, prefix, dry_run, logger): + """Rename a pre-fix ": All" group to the new prefix-only name. + + Before this fix, Category Detail = "none" built its group name the + same way every other category did, giving ": All" instead of + the "prefix only" the setting promises. Someone already using this + mode has that old group in Dispatcharr; without this step, the next + sync would create a second, new "" group under the fixed + name and leave the old one behind as an orphaned duplicate holding + their existing channels. + + Renaming (not recreating) preserves the group's id, so every channel + already assigned to it stays assigned - only its name changes. If a + "" group already exists too, merging the two would mean + moving channels between groups, which is a bigger and riskier + decision than this fix should make silently, so both are left alone + and a warning is logged instead. + """ + old_name = f"{prefix}: All" if prefix and prefix[-1] not in ":_-/ " else f"{prefix}All" if prefix else "All" + new_name = self._make_group_name(prefix, PluginConfig.SINGLE_GROUP_CATEGORY) + if old_name == new_name: + return # no prefix - "All" both before and after the fix + + old_group = ChannelGroup.objects.filter(name=old_name).first() + if not old_group: + return # nothing to migrate + + if ChannelGroup.objects.filter(name=new_name).exists(): + logger.warning( + f"{LOG_PREFIX} Both '{old_name}' and '{new_name}' channel groups exist; " + f"leaving both as-is. Merge them manually if '{old_name}' is a leftover " + f"from before this fix." + ) + return + + if dry_run: + logger.info(f"{LOG_PREFIX} [DRY RUN] Would rename group '{old_name}' to '{new_name}'") + return + + old_group.name = new_name + old_group.save(update_fields=["name"]) + logger.info(f"{LOG_PREFIX} Renamed group '{old_name}' to '{new_name}' (ID: {old_group.id})") + def _sync_groups(self, settings, logger): """Create/update ChannelGroups from lineup categories.""" dry_run = settings.get("dry_run_mode", False) @@ -2734,6 +2799,9 @@ def _sync_groups(self, settings, logger): prefix = self._get_group_prefix(settings, lineup) rate_limiter = SmartRateLimiter(settings.get("rate_limiting", PluginConfig.DEFAULT_RATE_LIMITING)) + if PluginConfig.SINGLE_GROUP_CATEGORY in lineup["categories"]: + self._migrate_single_group_name(prefix, dry_run, logger) + created = 0 existed = 0 @@ -2817,6 +2885,16 @@ def _do_sync_channels(self, settings, logger): rate_limiter = SmartRateLimiter(settings.get("rate_limiting", PluginConfig.DEFAULT_RATE_LIMITING)) assigner = self._init_assigner_state(settings) + # Sync Channels builds its own group lookup below rather than going + # through _sync_groups, so it needs this same migration call: without + # it, a lineup using Category Detail = "none" whose only prior sync + # was ever via this button (there is no Sync Groups button in the + # UI) would never see its pre-fix ": All" group renamed, and + # get_or_create just below would create a second "" group + # instead, orphaning the channels already on the old one. + if PluginConfig.SINGLE_GROUP_CATEGORY in lineup["categories"]: + self._migrate_single_group_name(prefix, dry_run, logger) + # Ensure groups exist existing_groups = {g['name']: g['id'] for g in ChannelGroup.objects.all().values('id', 'name')} for category in lineup["categories"]: diff --git a/tests/test_group_prefix_only.py b/tests/test_group_prefix_only.py new file mode 100644 index 0000000..9c56c10 --- /dev/null +++ b/tests/test_group_prefix_only.py @@ -0,0 +1,235 @@ +"""Regression for GitHub issue #30: Category Detail "None - single group +(prefix only, no categories)" created ": All" instead of a +prefix-only group name. + +Root cause: _apply_category_detail("none") merges every channel under the +sentinel category "All", and _make_group_name treated "All" like any other +category, appending it after the prefix the same way it appends "News" or +"Movies". The setting's own description promises "prefix only, no +categories", so the "All" suffix was a bug. + +The fix (in _make_group_name, the single place every caller goes through) +special-cases the sentinel category so the result is the prefix alone. It +also has to handle two things without a setting or a code change from the +maintainer's review of issue #30: + + 1. Someone already running this mode has a ": All" group in + Dispatcharr. _migrate_single_group_name renames that existing group + (preserving its id and channel assignments) instead of leaving it as an + orphaned duplicate next to a freshly created "" group - unless a + "" group already exists too, in which case both are left alone + and a warning is logged (merging channels between groups is out of + scope for a silent migration). + 2. A prefix that already ends in a separator (e.g. "DTV-") must not leave + that separator dangling ("DTV-" not "DTV-All" -> "DTV", not "DTV-"). +""" +import logging + +import pytest + +from Lineuparr import plugin as plugin_module +from Lineuparr.plugin import Plugin, PluginConfig + + +@pytest.fixture +def plugin(): + return Plugin.__new__(Plugin) + + +@pytest.fixture +def logger(): + return logging.getLogger("test") + + +class TestApplyCategoryDetailUsesSharedSentinel: + def test_none_detail_merges_under_the_shared_sentinel(self, plugin): + data = {"categories": {"News": [{"name": "CNN"}], "Sports": [{"name": "ESPN"}]}} + result = plugin._apply_category_detail(data, "none") + assert list(result["categories"].keys()) == [PluginConfig.SINGLE_GROUP_CATEGORY] + assert len(result["categories"][PluginConfig.SINGLE_GROUP_CATEGORY]) == 2 + + +class TestMakeGroupNameSingleGroupMode: + @pytest.mark.parametrize("prefix,expected", [ + ("Spectrum", "Spectrum"), + ("DIRECTV", "DIRECTV"), + ("", "All"), + ("DTV-", "DTV"), + ("DTV: ", "DTV"), + ("DTV ", "DTV"), + ("DTV_", "DTV"), + ("DTV/", "DTV"), + ("---", "All"), # prefix is nothing but separators - falls back + ]) + def test_single_group_category_is_prefix_only(self, plugin, prefix, expected): + assert plugin._make_group_name(prefix, PluginConfig.SINGLE_GROUP_CATEGORY) == expected + + +class TestMakeGroupNameOtherDetailModesUnchanged: + """The fix must not touch Normal/Simple/Refined category naming.""" + + @pytest.mark.parametrize("prefix,category,expected", [ + ("Spectrum", "Movies", "Spectrum: Movies"), + ("Spectrum", "News", "Spectrum: News"), + ("DTV-", "News", "DTV-News"), + ("", "News", "News"), + ]) + def test_normal_categories_unchanged(self, plugin, prefix, category, expected): + assert plugin._make_group_name(prefix, category) == expected + + def test_a_real_category_literally_named_all_also_collapses(self, plugin): + # Accepted edge case, documented on _make_group_name: a genuine + # lineup category named exactly "All" collapses the same way the + # sentinel does. Harmless (it already reads as one merged group). + assert plugin._make_group_name("Spectrum", "All") == "Spectrum" + + +# --- Migration: renaming a pre-fix ": All" group ------------------- + +class _FakeGroup: + def __init__(self, name, id): + self.name = name + self.id = id + self.saved = False + + def save(self, update_fields=None): + self.saved = True + + +class _FakeQuerySet: + def __init__(self, items): + self._items = items + + def first(self): + return self._items[0] if self._items else None + + def exists(self): + return bool(self._items) + + def values(self, *fields): + return [{f: getattr(g, f) for f in fields} for g in self._items] + + +class _FakeManager: + def __init__(self, groups): + self._groups = groups + + def filter(self, name=None): + return _FakeQuerySet([g for g in self._groups if g.name == name]) + + def all(self): + return _FakeQuerySet(list(self._groups)) + + def get_or_create(self, name, defaults=None): + for g in self._groups: + if g.name == name: + return g, False + new_group = _FakeGroup(name, id=max((g.id for g in self._groups), default=0) + 1) + self._groups.append(new_group) + return new_group, True + + +class _FakeChannelGroup: + def __init__(self, groups): + self.objects = _FakeManager(groups) + + +class TestMigrateSingleGroupName: + def test_renames_old_group_when_new_name_is_free(self, plugin, logger, monkeypatch): + old = _FakeGroup("Spectrum: All", id=7) + fake = _FakeChannelGroup([old]) + monkeypatch.setattr(plugin_module, "ChannelGroup", fake) + + plugin._migrate_single_group_name("Spectrum", dry_run=False, logger=logger) + + assert old.name == "Spectrum" + assert old.saved is True + + def test_leaves_both_alone_and_warns_when_new_name_already_exists(self, plugin, caplog, monkeypatch): + old = _FakeGroup("Spectrum: All", id=7) + new = _FakeGroup("Spectrum", id=9) + fake = _FakeChannelGroup([old, new]) + monkeypatch.setattr(plugin_module, "ChannelGroup", fake) + + with caplog.at_level(logging.WARNING): + plugin._migrate_single_group_name("Spectrum", dry_run=False, logger=logging.getLogger("test")) + + assert old.name == "Spectrum: All" # unchanged + assert old.saved is False + assert any("Spectrum: All" in r.message and "Spectrum" in r.message for r in caplog.records) + + def test_dry_run_does_not_rename(self, plugin, logger, monkeypatch): + old = _FakeGroup("Spectrum: All", id=7) + fake = _FakeChannelGroup([old]) + monkeypatch.setattr(plugin_module, "ChannelGroup", fake) + + plugin._migrate_single_group_name("Spectrum", dry_run=True, logger=logger) + + assert old.name == "Spectrum: All" + assert old.saved is False + + def test_no_old_group_is_a_silent_no_op(self, plugin, logger, monkeypatch): + fake = _FakeChannelGroup([]) + monkeypatch.setattr(plugin_module, "ChannelGroup", fake) + + # Must not raise. + plugin._migrate_single_group_name("Spectrum", dry_run=False, logger=logger) + + def test_no_prefix_is_a_no_op_without_touching_the_database(self, plugin, logger, monkeypatch): + # Both old and new names are "All" with no prefix - nothing to migrate, + # and the method should return before ever calling ChannelGroup.objects. + class _ExplodingManager: + def filter(self, **kwargs): + raise AssertionError("should not query the database") + + class _ExplodingChannelGroup: + objects = _ExplodingManager() + + monkeypatch.setattr(plugin_module, "ChannelGroup", _ExplodingChannelGroup) + + plugin._migrate_single_group_name("", dry_run=False, logger=logger) + + def test_renames_with_a_separator_terminated_prefix(self, plugin, logger, monkeypatch): + old = _FakeGroup("DTV-All", id=3) + fake = _FakeChannelGroup([old]) + monkeypatch.setattr(plugin_module, "ChannelGroup", fake) + + plugin._migrate_single_group_name("DTV-", dry_run=False, logger=logger) + + assert old.name == "DTV" + assert old.saved is True + + +class TestSyncChannelsAlsoMigrates: + """Regression from code review on this PR: the migration must run from + the Sync Channels button too, not only Full Sync. There is no Sync + Groups button in the UI, so Sync Channels (_do_sync_channels) is, in + practice, the only path some installs ever take. Before this fix it + built its own group lookup independently of _sync_groups and never + called the migration, so pressing Sync Channels against an existing + ": All" group created a second "" group instead of + renaming the first - leaving the old group's channels orphaned. + """ + + def test_sync_channels_renames_the_old_group_instead_of_duplicating_it( + self, plugin, logger, monkeypatch + ): + old = _FakeGroup("Spectrum: All", id=7) + fake = _FakeChannelGroup([old]) + monkeypatch.setattr(plugin_module, "ChannelGroup", fake) + + # No channels in the lineup: this test is about which group exists + # afterward, not channel assignment, so Channel.objects is never + # touched and does not need its own fake. + monkeypatch.setattr( + plugin, "_load_lineup", + lambda settings, logger: {"categories": {PluginConfig.SINGLE_GROUP_CATEGORY: []}}, + ) + monkeypatch.setattr(plugin, "_get_group_prefix", lambda settings, lineup: "Spectrum") + plugin._suppress_refresh = False # Plugin.__new__ skips __init__ + + plugin._do_sync_channels({}, logger) + + names = [g.name for g in fake.objects._groups] + assert names == ["Spectrum"], f"expected the old group renamed in place, got {names}" + assert old.saved is True