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