From fd4f6b342d110f61e31330c636e31c96dfbbb6d7 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 19 Mar 2026 15:39:19 +0530 Subject: [PATCH 1/2] [fix] Correct handling of deferred fields when tracking changed fields When a Device instance was loaded with deferred fields (e.g. using QuerySet.only()), `_check_changed_fields()` incorrectly populated the `_initial_` attributes. Instead of storing the original value retrieved from the database, it stored the field name itself. This happened because the code iterated over `_changed_checked_fields` and assigned the field name rather than the actual value coming from `present_values`. The logic has been updated to iterate over `present_values.items()`, store the current database value in `_initial_`, and then restore the updated value on the instance. This ensures change detection works correctly even when fields were initially deferred. Changelog (Bugfix): Fixed incorrect initialization of `_initial_` values when a Device instance is loaded with deferred fields, which could break change detection logic. --- openwisp_controller/config/base/device.py | 6 +++--- openwisp_controller/config/tests/test_device.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/base/device.py b/openwisp_controller/config/base/device.py index d985ccaab..bcfff44ca 100644 --- a/openwisp_controller/config/base/device.py +++ b/openwisp_controller/config/base/device.py @@ -338,9 +338,9 @@ def _get_initial_values_for_checked_fields(self): if not present_values: return self.refresh_from_db(fields=present_values.keys()) - for field in self._changed_checked_fields: - setattr(self, f"_initial_{field}", field) - setattr(self, field, present_values[field]) + for field, value in present_values.items(): + setattr(self, f"_initial_{field}", getattr(self, field)) + setattr(self, field, value) def _check_name_changed(self): if self._initial_name == models.DEFERRED: diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index 9603606d3..177e367ca 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -541,6 +541,21 @@ def test_changed_checked_fields_no_duplicates(self): device.__init__() self.assertEqual(device._changed_checked_fields.count("last_ip"), 1) + def test_deferred_fields_populated_correctly(self): + device = self._create_device( + name="deferred-test", + last_ip="172.16.0.1", + management_ip="10.0.0.1", + ) + # Load the instance with deferred fields omitted + device = Device.objects.only("id").get(pk=device.pk) + device.last_ip = "172.16.1.1" + # Populate initial values for checked fields + device._check_changed_fields() + # Ensure `_initial_` contains the actual value, not the field name + self.assertEqual(getattr(device, "_initial_last_ip"), "172.16.0.1") + self.assertNotEqual(getattr(device, "_initial_last_ip"), "last_ip") + def test_exceed_organization_device_limit(self): org = self._get_org() org.config_limits.device_limit = 1 From 1a5fb89273689119e0fd4761f51d4e4410f28f30 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 27 Mar 2026 12:54:02 +0530 Subject: [PATCH 2/2] [fix] Made test more readable --- openwisp_controller/config/tests/test_device.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index 177e367ca..d9bc3f59d 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -544,17 +544,16 @@ def test_changed_checked_fields_no_duplicates(self): def test_deferred_fields_populated_correctly(self): device = self._create_device( name="deferred-test", - last_ip="172.16.0.1", management_ip="10.0.0.1", ) # Load the instance with deferred fields omitted device = Device.objects.only("id").get(pk=device.pk) - device.last_ip = "172.16.1.1" - # Populate initial values for checked fields - device._check_changed_fields() + device.management_ip = "10.0.0.55" + # Saving the device object will populate the deferred fields + device.save() # Ensure `_initial_` contains the actual value, not the field name - self.assertEqual(getattr(device, "_initial_last_ip"), "172.16.0.1") - self.assertNotEqual(getattr(device, "_initial_last_ip"), "last_ip") + self.assertEqual(getattr(device, "_initial_management_ip"), "10.0.0.55") + self.assertNotEqual(getattr(device, "_initial_management_ip"), "management_ip") def test_exceed_organization_device_limit(self): org = self._get_org()