[req-changes] Improved code

Author: pandafy

openwisp_controller/config/controller/views.py

@@ -402,16 +402,13 @@ def post(self, request, *args, **kwargs):
         new = False
         try:
             device = self.model.objects.get(key=key)
-            # update hw info
+            # update device info
             for attr in self.UPDATABLE_FIELDS:
                 if attr in request.POST:
                     if attr == "name":
-                        if self._should_update_hostname(request, device):
-                            device.skip_push_update_on_save()
-                        else:
-                            # Don't update Device.name
-                            continue
-                    setattr(device, attr, request.POST.get(attr))
+                        self._update_device_name(request, device)
+                    else:
+                        setattr(device, attr, request.POST.get(attr))
             config = device.config
         # if get queryset fails, instantiate a new Device and Config
         except self.model.DoesNotExist:
@@ -463,14 +460,14 @@ def post(self, request, *args, **kwargs):
             s.format(**attributes), content_type="text/plain", status=201
         )
 
-    def _should_update_hostname(self, request, device):
+    def _update_device_name(self, request, device):
         """
-        Determines whether the hostname should be updated during re-registration.
+        Updates the device name during re-registration if the name in the
+        payload is not equal to the device's MAC address.
 
-        Returns False if the hostname equals the MAC address (agents send MAC address
-        as hostname when hostname is OpenWrt or if default_hostname is set to *).
-
-        This prevents overwriting user configured hostnames with default/generic values.
+        Agents send the MAC address as hostname when the hostname is "OpenWrt"
+        or if default_hostname is set to *. This method prevents overwriting
+        user-configured hostnames with default/generic MAC address values.
         """
         name = request.POST.get("name")
         mac_address = device.mac_address
@@ -480,7 +477,9 @@ def _should_update_hostname(self, request, device):
             else None
         )
         normalized_name = name.replace(":", "").replace("-", "").lower()
-        return normalized_name != normalized_mac
+        if normalized_name != normalized_mac:
+            device.name = name
+            device.skip_push_update_on_save()
 
 
 class GetVpnView(SingleObjectMixin, View):

openwisp_controller/config/tests/test_controller.py

@@ -38,7 +38,7 @@
 Organization = load_model("openwisp_users", "Organization")
 
 
-class TestControllerMixin:
+class TestRegistrationMixin:
     register_url = reverse_lazy("controller:device_register")
 
     def _create_org(self, shared_secret=TEST_ORG_SHARED_SECRET, **kwargs):
@@ -63,7 +63,7 @@ def _get_reregistration_payload(self, device, **kwargs):
 
 
 class TestController(
-    TestControllerMixin, CreateConfigTemplateMixin, TestVpnX509Mixin, TestCase
+    TestRegistrationMixin, CreateConfigTemplateMixin, TestVpnX509Mixin, TestCase
 ):
     """
     tests for config.controller

openwisp_controller/connection/tests/test_tasks.py

@@ -7,7 +7,7 @@
 from django.test import TestCase, TransactionTestCase
 from swapper import load_model
 
-from ...config.tests.test_controller import TestControllerMixin
+from ...config.tests.test_controller import TestRegistrationMixin
 from .. import tasks
 from .utils import CreateConnectionsMixin
 
@@ -90,7 +90,7 @@ def test_launch_command_exception(self, *args):
 
 
 class TestTransactionTasks(
-    TestControllerMixin, CreateConnectionsMixin, TransactionTestCase
+    TestRegistrationMixin, CreateConnectionsMixin, TransactionTestCase
 ):
     @mock.patch.object(tasks.update_config, "delay")
     def test_update_config_hostname_changed_on_reregister(self, mocked_update_config):