Skip to content

Commit

Permalink
[fix] Fixed changed signals emitting on the creation of new device op…
Browse files Browse the repository at this point in the history
…enwisp#649

Creating a new device object from the admin dashboard
emitted "device_group_changed" and "device_name_changed"
signals. The code for checking the values of changed fields
didn't take creating a device from admin into consideration.

Fixed the code that checks changed fields and added an
automated test for the same.

Closes openwisp#649
  • Loading branch information
pandafy authored May 17, 2022
1 parent 86caa4b commit 6d07b05
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
7 changes: 6 additions & 1 deletion openwisp_controller/config/base/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,13 @@ def save(self, *args, **kwargs):
self.key = KeyField.default_callable()
else:
self.key = self.generate_key(shared_secret)
state_adding = self._state.adding
super().save(*args, **kwargs)
self._check_changed_fields()
# The value of "self._state.adding" will always be "False"
# after performing the save operation. Hence, the actual value
# is stored in the "state_adding" variable.
if not state_adding:
self._check_changed_fields()

def _check_changed_fields(self):
self._get_initial_values_for_checked_fields()
Expand Down
22 changes: 22 additions & 0 deletions openwisp_controller/config/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
from swapper import load_model

from openwisp_users.tests.utils import TestOrganizationMixin
from openwisp_utils.tests import catch_signal

from ...geo.tests.utils import TestGeoMixin
from ...tests.utils import TestAdminMixin
from .. import settings as app_settings
from ..signals import device_group_changed, device_name_changed, management_ip_changed
from .utils import (
CreateConfigTemplateMixin,
CreateDeviceGroupMixin,
Expand All @@ -36,6 +38,7 @@

class TestAdmin(
TestGeoMixin,
CreateDeviceGroupMixin,
CreateConfigTemplateMixin,
TestVpnX509Mixin,
TestAdminMixin,
Expand Down Expand Up @@ -140,6 +143,25 @@ def test_add_device(self):
device.config.templates.filter(name__in=['t1', 't2']).count(), 2
)

def test_add_device_does_not_emit_changed_signals(self):
org1 = self._get_org()
path = reverse(f'admin:{self.app_label}_device_add')
data = self._get_device_params(org=org1)
data.update({'group': str(self._create_device_group().pk)})
self._login()
with catch_signal(
device_group_changed
) as mocked_device_group_changed, catch_signal(
device_name_changed
) as mocked_device_name_changed, catch_signal(
management_ip_changed
) as mocked_management_ip_changed:
self.client.post(path, data)

mocked_device_group_changed.assert_not_called()
mocked_device_name_changed.assert_not_called()
mocked_management_ip_changed.assert_not_called()

def test_preview_device(self):
org = self._get_org()
self._create_template(organization=org)
Expand Down
4 changes: 3 additions & 1 deletion openwisp_controller/config/tests/test_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,10 @@ def test_device_group_changed_emitted(self):
)

def test_device_group_changed_not_emitted_on_creation(self):
org = self._get_org()
device_group = self._create_device_group(organization=org)
with catch_signal(device_group_changed) as handler:
self._create_device(organization=self._get_org())
self._create_device(name='test', organization=org, group=device_group)
handler.assert_not_called()

def test_device_field_changed_checks(self):
Expand Down

0 comments on commit 6d07b05

Please sign in to comment.