[req-changes] Updated README and fixed precedence of variables

README.rst

@@ -536,10 +536,20 @@ the order (high to low) of their precedence:
 
 1. `User defined device variables <#user-defined-device-variables>`_
 2. `Predefined device variables <#predefined-device-variables>`_
-3. `Global variables <#global-variables>`_
-4. `Group variables <#group-variables>`_
+3. `Group variables <#group-variables>`_
+4. `Global variables <#global-variables>`_
 5. `Template default values <#template-default-values>`_
 
+User defined device variables
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+In the device configuration section you can find a section named
+"Configuration variables" where it is possible to define the configuration
+variables and their values, as shown in the example below:
+
+.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/device-context.png
+   :alt: context
+
 Predefined device variables
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -550,15 +560,19 @@ Each device gets the following attributes passed as configuration variables:
 * ``name``
 * ``mac_address``
 
-User defined device variables
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Group variables
+~~~~~~~~~~~~~~~
 
-In the device configuration section you can find a section named
-"Configuration variables" where it is possible to define the configuration
-variables and their values, as shown in the example below:
+Variables can also be defined in `Device groups <#device-groups>`__.
 
-.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/device-context.png
-   :alt: context
+Refer the `Group configuration variables <group-configuration-variables>`_
+section for detailed information.
+
+Global variables
+~~~~~~~~~~~~~~~~
+
+Variables can also be defined globally using the
+`OPENWISP_CONTROLLER_CONTEXT <#openwisp-controller-context>`_ setting.
 
 Template default values
 ~~~~~~~~~~~~~~~~~~~~~~~
@@ -581,20 +595,6 @@ The default values of variables can be manipulated from the section
 .. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/template-default-values.png
   :alt: default values
 
-Group variables
-~~~~~~~~~~~~~~~
-
-Variables can also be defined in `Device groups <#device-groups>`__.
-
-Refer the `Group configuration variables <group-configuration-variables>`_
-section for detailed information.
-
-Global variables
-~~~~~~~~~~~~~~~~
-
-Variables can also be defined globally using the
-`OPENWISP_CONTROLLER_CONTEXT <#openwisp-controller-context>`_ setting.
-
 System defined variables
 ~~~~~~~~~~~~~~~~~~~~~~~~
 

openwisp_controller/config/base/config.py

@@ -558,7 +558,7 @@ def _has_device(self):
         return hasattr(self, 'device')
 
     def get_vpn_context(self):
-        context = super().get_context()
+        context = {}
         for vpnclient in self.vpnclient_set.all().select_related('vpn', 'cert'):
             vpn = vpnclient.vpn
             vpn_id = vpn.pk.hex
@@ -599,15 +599,11 @@ def get_context(self, system=False):
         additional context passed to netjsonconfig
         """
         c = collections.OrderedDict()
-        extra = {}
-        if hasattr(self, 'device') and self.device._get_group():
-            extra.update(self.device._get_group().get_context())
-        extra.update(self.get_vpn_context())
-        for func in self._config_context_functions:
-            extra.update(func(config=self))
-        if app_settings.HARDWARE_ID_ENABLED and self._has_device():
-            extra.update({'hardware_id': str(self.device.hardware_id)})
+        # Add global variables
+        context = super().get_context()
         if self._has_device():
+            # These pre-defined variables are needed at the start of OrderedDict.
+            # Hence, they are added separately.
             c.update(
                 [
                     ('name', self.name),
@@ -616,9 +612,20 @@ def get_context(self, system=False):
                     ('key', self.key),
                 ]
             )
-            if self.context and not system:
-                extra.update(self.context)
-        c.update(sorted(extra.items()))
+            if self.device._get_group():
+                # Add device group variables
+                context.update(self.device._get_group().get_context())
+            # Add predefined variables
+            context.update(self.get_vpn_context())
+            for func in self._config_context_functions:
+                context.update(func(config=self))
+            if app_settings.HARDWARE_ID_ENABLED:
+                context.update({'hardware_id': str(self.device.hardware_id)})
+
+        if self.context and not system:
+            context.update(self.context)
+
+        c.update(sorted(context.items()))
         return c
 
     def get_system_context(self):

openwisp_controller/config/migrations/0048_devicegroup_context.py → openwisp_controller/config/migrations/0049_devicegroup_context.py

@@ -9,7 +9,7 @@
 class Migration(migrations.Migration):
 
     dependencies = [
-        ('config', '0047_add_organizationlimits'),
+        ('config', '0048_wifi_radio_band_migration'),
     ]
 
     operations = [

openwisp_controller/config/tests/test_admin.py

@@ -1519,16 +1519,14 @@ def test_system_context(self):
             path = reverse(f'admin:{self.app_label}_device_change', args=[d.pk])
             self._test_system_context_field_helper(path)
 
-    def test_no_system_context(self):
+    @patch.dict(app_settings.CONTEXT, {})
+    def test_no_system_context(self, *args):
         self._create_template()
-        old_context = app_settings.CONTEXT.copy()
-        app_settings.CONTEXT = {}
         path = reverse(f'admin:{self.app_label}_template_add')
         r = self.client.get(path)
         self.assertContains(
             r, 'There are no system defined variables available right now'
         )
-        app_settings.CONTEXT = old_context
 
     def test_config_form_old_templates(self):
         config = self._create_config(organization=self._get_org())

openwisp_controller/config/tests/test_device.py

@@ -321,6 +321,40 @@ def test_device_get_system_context(self):
         self.assertEqual(system_context['SSID'], 'OpenWISP')
         self.assertNotIn('test', system_context.keys())
 
+    @mock.patch.dict(app_settings.CONTEXT, {'variable_type': 'global-context-variable'})
+    def test_configuration_variable_priority(self, *args):
+        org = self._get_org()
+        device_group = self._create_device_group(
+            organization=org, context={'variable_type': 'device-group-variable'}
+        )
+        device = self._create_device(organization=org, group=device_group)
+        config = self._create_config(
+            device=device,
+            context={
+                'id': 'user-defined-variable',
+                'variable_type': 'user-defined-variable',
+            },
+        )
+
+        # User defined variable has highest precedence
+        self.assertEqual(config.get_context()['id'], 'user-defined-variable')
+        self.assertEqual(config.get_context()['variable_type'], 'user-defined-variable')
+        # Second precedence is given to predefined device variables
+        config.context = {}
+        self.assertEqual(
+            config.get_context()['id'],
+            str(device.pk),
+        )
+        # It is not possible to overwrite the pre-defined device variables.
+        # Therefore, for further tests, "tesT" variable is used.
+        # Third precedence is given to the device group variable'
+        self.assertEqual(config.get_context()['variable_type'], 'device-group-variable')
+        # Fourth precedence is given to global variables
+        device.group = None
+        self.assertEqual(
+            config.get_context()['variable_type'], 'global-context-variable'
+        )
+
     def test_management_ip_changed_not_emitted_on_creation(self):
         with catch_signal(management_ip_changed) as handler:
             self._create_device(organization=self._get_org())