mirror of
https://github.com/Dispatcharr/Dispatcharr.git
synced 2026-07-17 16:50:53 +00:00
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.
This commit is contained in:
None
parent
36896d7a45
commit
2a5889f5b2
4 changed files with 262 additions and 10 deletions
|
|
@ -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")
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -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}",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue