Skip to content

Commit d9f342c

Browse files
committed
[fix] Use continue instead of return in manage_devices_group_templates
The loop in manage_devices_group_templates used `return` when a device had no config, which aborted processing of all remaining devices in the batch. Changed to `continue` so that devices without a config are skipped individually while the rest of the batch is still processed. Added a regression test to verify no early termination occurs when the first device in a batch has no config.
1 parent f3c99c4 commit d9f342c

File tree

2 files changed

+56
-3
lines changed

2 files changed

+56
-3
lines changed

openwisp_controller/config/base/device.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ def manage_devices_group_templates(cls, device_ids, old_group_ids, group_id):
504504
config_created = hasattr(device, "config")
505505
if not config_created:
506506
# device has no config (device group has no templates)
507-
return
507+
continue
508508
group_templates = Template.objects.none()
509509
if group_id:
510510
group = DeviceGroup.objects.get(pk=group_id)

openwisp_controller/config/tests/test_device_group.py

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@
99

1010
from .. import settings as app_settings
1111
from ..signals import group_templates_changed
12-
from .utils import CreateDeviceGroupMixin, CreateTemplateMixin
12+
from .utils import CreateDeviceGroupMixin, CreateDeviceMixin, CreateTemplateMixin
1313

14+
Config = load_model("config", "Config")
15+
Device = load_model("config", "Device")
1416
DeviceGroup = load_model("config", "DeviceGroup")
17+
Template = load_model("config", "Template")
1518

1619

17-
class TestDeviceGroup(CreateDeviceGroupMixin, CreateTemplateMixin, TestCase):
20+
class TestDeviceGroup(
21+
CreateDeviceMixin, CreateDeviceGroupMixin, CreateTemplateMixin, TestCase
22+
):
1823
def test_device_group(self):
1924
self._create_device_group(
2025
meta_data={"captive_portal_url": "https//example.com"}
@@ -61,3 +66,51 @@ def test_device_group_signals(self):
6166
raw=False,
6267
using="default",
6368
)
69+
70+
def test_manage_devices_group_templates_no_early_termination(self):
71+
"""
72+
Regression test: when the first device in the batch has no config
73+
(because its group has no templates), the loop must continue and
74+
still apply group templates to the second device.
75+
"""
76+
org = self._get_org()
77+
t1 = self._create_template(name="t1")
78+
# group_with_templates has a template; group_without does not
79+
group_with_templates = self._create_device_group(
80+
name="group-with-tpl", organization=org
81+
)
82+
group_with_templates.templates.add(t1)
83+
group_without_templates = self._create_device_group(
84+
name="group-without-tpl", organization=org
85+
)
86+
# device1: belongs to a group without templates → no config created
87+
device1 = self._create_device(
88+
name="device-no-config",
89+
organization=org,
90+
group=group_without_templates,
91+
mac_address="00:00:00:00:00:01",
92+
)
93+
self.assertFalse(hasattr(device1, "config"))
94+
# device2: has a config object
95+
device2 = self._create_device(
96+
name="device-with-config",
97+
organization=org,
98+
mac_address="00:00:00:00:00:02",
99+
)
100+
Config.objects.create(
101+
device=device2,
102+
backend="netjsonconfig.OpenWrt",
103+
)
104+
device2.refresh_from_db()
105+
self.assertTrue(hasattr(device2, "config"))
106+
self.assertEqual(device2.config.templates.count(), 0)
107+
# Call manage_devices_group_templates with device1 (no config) first,
108+
# then device2 (has config). device1 should be skipped, device2 must
109+
# still get group_with_templates's template applied.
110+
Device.manage_devices_group_templates(
111+
device_ids=[device1.pk, device2.pk],
112+
old_group_ids=[None, None],
113+
group_id=group_with_templates.pk,
114+
)
115+
device2.refresh_from_db()
116+
self.assertIn(t1, device2.config.templates.all())

0 commit comments

Comments
 (0)