From 2a5889f5b2184f29b411ddcc4e6e827d33ed916c Mon Sep 17 00:00:00 2001 From: None <190783071+CodeBormen@users.noreply.github.com> Date: Mon, 1 Jun 2026 22:56:22 -0500 Subject: [PATCH] fix(channels): Channel Group Override interactions - Compact numbering: resolve the source relation when an override stores channels under the target group, so hiding releases the slot, unhiding assigns one, and repack sees the channels (no more spurious RANGE_EXHAUSTED). Type-safe override match; the bulk path stays single-query on the common path. - Channel form: keep the clear-override control available when an override's value equals the provider value (isFormFieldOverridden is now existence-aware). - Tests: compact override hide/unhide/repack, no-override fast-path guard, repack query-scaling guard, and existence-aware override detection. --- apps/channels/compact_numbering.py | 75 ++++++++- apps/m3u/tests/test_sync_correctness.py | 146 ++++++++++++++++++ frontend/src/utils/forms/ChannelUtils.js | 9 +- .../forms/__tests__/ChannelUtils.test.js | 42 +++++ 4 files changed, 262 insertions(+), 10 deletions(-) diff --git a/apps/channels/compact_numbering.py b/apps/channels/compact_numbering.py index f3d7183c..8cb62f53 100644 --- a/apps/channels/compact_numbering.py +++ b/apps/channels/compact_numbering.py @@ -44,20 +44,38 @@ def is_compact_group(group_relation): def get_group_relation_for_channel(channel): """Resolve the ChannelGroupM3UAccount that owns this auto-created channel. Returns None for manual channels, or when the relation has - been deleted.""" - if ( - not channel.auto_created - or not channel.auto_created_by_id - or not channel.channel_group_id - ): + been deleted. + + With a Channel Group Override active, sync stores the channel under + the override target group's id, not the source group's id recorded on + the relation. The direct lookup then misses, so fall back to scanning + the account's relations for one whose group_override points at the + channel's current group. The fallback runs only on a direct miss, so + the common no-override path keeps its single SELECT. + """ + if not channel.auto_created or not channel.auto_created_by_id: return None + if not channel.channel_group_id: + return None + try: return ChannelGroupM3UAccount.objects.get( m3u_account_id=channel.auto_created_by_id, channel_group_id=channel.channel_group_id, ) except ChannelGroupM3UAccount.DoesNotExist: - return None + pass + + # group_override may be stored as int or str; compare as strings so + # the match is type-agnostic. + target = str(channel.channel_group_id) + for rel in ChannelGroupM3UAccount.objects.filter( + m3u_account_id=channel.auto_created_by_id + ): + cp = _custom_properties_as_dict(rel.custom_properties) + if str(cp.get("group_override", "")) == target: + return rel + return None def build_reserved_set(exclude_channel_ids=None, range_start=None, range_end=None): @@ -151,6 +169,29 @@ def assign_compact_numbers_for_channels(channel_ids): for rel in relations_qs: relations_by_pair[(rel.m3u_account_id, rel.channel_group_id)] = rel + # Override fallback: pairs the direct lookup missed carry an override- + # target channel_group_id. Resolve them with one extra query over the + # unresolved accounts (not one per pair), so the common path keeps its + # single narrow query. + unresolved = [k for k in pair_keys if k not in relations_by_pair] + if unresolved: + override_relations = {} + for rel in ChannelGroupM3UAccount.objects.filter( + m3u_account_id__in={k[0] for k in unresolved} + ): + cp = _custom_properties_as_dict(rel.custom_properties) + target = cp.get("group_override") + if not target: + continue + try: + override_relations[(rel.m3u_account_id, int(target))] = rel + except (TypeError, ValueError): + continue + for key in unresolved: + rel = override_relations.get(key) + if rel is not None: + relations_by_pair[key] = rel + by_relation = {} for key, group_channels in by_pair.items(): rel = relations_by_pair.get(key) @@ -265,11 +306,29 @@ def _repack_inner(group_relation): else None ) + # Match the override target group too: channels created under an + # override live under the target's id, not the source group's. + group_ids = {group_id} + override_group_id = cp.get("group_override") + if override_group_id: + try: + group_ids.add(int(override_group_id)) + except (TypeError, ValueError): + logger.warning( + "Ignoring non-numeric group_override %r on relation %s", + override_group_id, + group_relation.id, + ) + + # Known limitation: if two source groups on the same account override + # into the SAME target group, their channels are indistinguishable + # here (channels carry no source-group back-reference), so each repack + # renumbers the shared target's channels into its own range. channels = list( Channel.objects.filter( auto_created=True, auto_created_by_id=account_id, - channel_group_id=group_id, + channel_group_id__in=group_ids, ).select_related("override") ) diff --git a/apps/m3u/tests/test_sync_correctness.py b/apps/m3u/tests/test_sync_correctness.py index 2c185ae0..509171c9 100644 --- a/apps/m3u/tests/test_sync_correctness.py +++ b/apps/m3u/tests/test_sync_correctness.py @@ -2049,3 +2049,149 @@ class Migration0037DemoteOrphansTests(TestCase): "auto_created=True or deleted", ) self.assertIsNone(ch.auto_created_by) + + +class CompactNumberingWithGroupOverrideTests(TestCase): + """ + Compact numbering must keep working when a Channel Group Override is + configured on the source ChannelGroupM3UAccount. With an override, + sync stores auto-created channels under the OVERRIDE TARGET group's id + rather than the source group's id recorded on the relation. The + compact paths resolve the relation from the channel's group id, so + without the override-aware fallback they all miss and slot accounting + silently breaks (hidden channels keep their numbers, unhides get none, + repack sees zero channels). + + Fix location: apps/channels/compact_numbering.py + (get_group_relation_for_channel fallback, _repack_inner group_ids, + assign_compact_numbers_for_channels bulk fallback). + """ + + def _override_setup(self, start=100, end=110): + account = _make_account() + source_group = _make_group(name="SourcePPV") + target_group = _make_group(name="TargetAll") + rel = _attach_group_to_account( + account, + source_group, + custom_properties={ + "compact_numbering": True, + "group_override": target_group.id, + }, + ) + rel.auto_sync_channel_start = start + rel.auto_sync_channel_end = end + rel.save() + return account, source_group, target_group, rel + + def _auto_channel(self, account, group, number=None, hidden=False, name="PPV"): + return Channel.objects.create( + name=name, + channel_number=number, + channel_group=group, + auto_created=True, + auto_created_by=account, + hidden_from_output=hidden, + ) + + def test_hide_releases_slot_under_group_override(self): + # Fail signature: channel_number stays populated after hide = + # release_compact_number_on_hide bailed because + # get_group_relation_for_channel returned None for the override + # target group. + account, source, target, rel = self._override_setup() + ch = self._auto_channel(account, target, number=100) + + ch.hidden_from_output = True + ch.save() + ch.refresh_from_db() + + self.assertIsNone( + ch.channel_number, + "Hiding an auto channel under a Channel Group Override must " + "release its compact slot (channel_number=None)", + ) + + def test_unhide_assigns_slot_under_group_override(self): + # Fail signature: channel_number stays None after unhide = + # assign_compact_number_on_unhide bailed on the override target. + account, source, target, rel = self._override_setup() + ch = self._auto_channel(account, target, number=None, hidden=True) + + ch.hidden_from_output = False + ch.save() + ch.refresh_from_db() + + self.assertEqual( + ch.channel_number, + 100, + "Unhiding an auto channel under a Channel Group Override must " + "assign a number from the compact range", + ) + + def test_repack_sees_channels_under_override_target(self): + # Fail signature: assigned=0 = _repack_inner filtered on the source + # group id and found none of the channels stored under the target. + from apps.channels.compact_numbering import repack_group + + account, source, target, rel = self._override_setup() + channels = [ + self._auto_channel(account, target, number=900 + i, name=f"C{i}") + for i in range(3) + ] + + result = repack_group(rel) + + self.assertEqual(result["assigned"], 3) + self.assertEqual(result["failed"], 0) + nums = sorted( + Channel.objects.filter( + id__in=[c.id for c in channels] + ).values_list("channel_number", flat=True) + ) + self.assertEqual(nums, [100, 101, 102]) + + def test_no_override_fast_path_still_resolves(self): + # Regression guard: the common no-override case must still resolve + # the relation via the direct lookup (channel.channel_group_id == + # source group id), unaffected by the override fallback. + from apps.channels.compact_numbering import ( + get_group_relation_for_channel, + ) + + account = _make_account() + group = _make_group(name="PlainSports") + rel = _attach_group_to_account( + account, group, custom_properties={"compact_numbering": True} + ) + ch = self._auto_channel(account, group, number=100) + + resolved = get_group_relation_for_channel(ch) + self.assertIsNotNone(resolved) + self.assertEqual(resolved.id, rel.id) + + def test_repack_under_override_query_count_does_not_scale(self): + # Perf guard: the override-aware repack widens the channel lookup's + # IN clause; it must not add a query per channel. Query count must + # be identical for N and 3*N channels. + from django.db import connection + from django.test.utils import CaptureQueriesContext + from apps.channels.compact_numbering import repack_group + + account, source, target, rel = self._override_setup(start=100, end=300) + + def measure(n): + Channel.objects.filter(auto_created_by=account).delete() + for i in range(n): + self._auto_channel(account, target, number=900 + i, name=f"C{i}") + with CaptureQueriesContext(connection) as ctx: + repack_group(rel) + return len(ctx.captured_queries) + + small = measure(5) + large = measure(15) + self.assertEqual( + small, + large, + f"repack query count scaled with channel count: {small} -> {large}", + ) diff --git a/frontend/src/utils/forms/ChannelUtils.js b/frontend/src/utils/forms/ChannelUtils.js index 467ff1e7..cc3743e3 100644 --- a/frontend/src/utils/forms/ChannelUtils.js +++ b/frontend/src/utils/forms/ChannelUtils.js @@ -94,10 +94,15 @@ export const getProviderFormValue = (channel, field) => { return channel?.[field] ?? ''; }; -// Form value differs from the channel's provider value. Manual -// channels always return false (no provider value to compare to). +// Whether a field carries an override. Manual channels return false (no +// provider value to override). True when a persisted override row holds a +// value for the field, OR the live form value diverges from provider. The +// persisted check keeps the reset control available when an override's +// value coincides with the provider value. export const isFormFieldOverridden = (channel, field, formValue) => { if (!channel?.auto_created) return false; + const persisted = channel.override?.[field]; + if (persisted !== null && persisted !== undefined) return true; const normalizedForm = normalizeFieldValue(field, formValue); const normalizedProvider = normalizeFieldValue(field, channel[field]); return normalizedForm !== normalizedProvider; diff --git a/frontend/src/utils/forms/__tests__/ChannelUtils.test.js b/frontend/src/utils/forms/__tests__/ChannelUtils.test.js index 73c550ee..88002c09 100644 --- a/frontend/src/utils/forms/__tests__/ChannelUtils.test.js +++ b/frontend/src/utils/forms/__tests__/ChannelUtils.test.js @@ -14,6 +14,7 @@ import { buildOverridePayload, listOverriddenFields, clearChannelOverrides, + isFormFieldOverridden, } from '../ChannelUtils.js'; // ── API mock ─────────────────────────────────────────────────────────────────── @@ -721,6 +722,47 @@ describe('ChannelUtils', () => { }); }); + // ── isFormFieldOverridden ──────────────────────────────────────────────────── + + describe('isFormFieldOverridden', () => { + it('returns false for manual channels', () => { + const ch = makeChannel({ auto_created: false }); + expect(isFormFieldOverridden(ch, 'name', 'Anything')).toBe(false); + }); + + it('returns true when the form value differs from the provider value', () => { + const ch = makeChannel({ auto_created: true, name: 'Provider Name' }); + expect(isFormFieldOverridden(ch, 'name', 'Custom Name')).toBe(true); + }); + + it('returns false when no override exists and form matches provider', () => { + const ch = makeChannel({ auto_created: true, name: 'Provider Name' }); + expect(isFormFieldOverridden(ch, 'name', 'Provider Name')).toBe(false); + }); + + // A persisted override whose value coincides with the provider value + // must still count as overridden, so the reset affordance stays + // available to clear it. Value-only detection wrongly returned false + // here (pencil showed, reset button vanished). + it('returns true when a persisted override exists even if its value equals the provider value', () => { + const ch = makeChannel({ + auto_created: true, + channel_group_id: 5, + override: { channel_group_id: 5 }, + }); + expect(isFormFieldOverridden(ch, 'channel_group_id', '5')).toBe(true); + }); + + it('returns false when the override field is explicitly null (cleared)', () => { + const ch = makeChannel({ + auto_created: true, + channel_group_id: 5, + override: { channel_group_id: null }, + }); + expect(isFormFieldOverridden(ch, 'channel_group_id', '5')).toBe(false); + }); + }); + // ── buildOverridePayload ───────────────────────────────────────────────────── describe('buildOverridePayload', () => {