[fix] Fixed reverting template doesn't send config_changed signal #836

Fixes #836

Author: pandafy

openwisp_controller/config/apps.py

@@ -55,6 +55,7 @@ def ready(self, *args, **kwargs):
 
     def __setmodels__(self):
         self.device_model = load_model('config', 'Device')
+        self.template_model = load_model('config', 'Template')
         self.devicegroup_model = load_model('config', 'DeviceGroup')
         self.config_model = load_model('config', 'Config')
         self.vpn_model = load_model('config', 'Vpn')
@@ -135,11 +136,21 @@ def connect_signals(self):
             sender=self.config_model,
             dispatch_uid='devicegroup_templates_change_handler.backend_changed',
         )
+        pre_save.connect(
+            self.template_model.pre_save_handler,
+            sender=self.template_model,
+            dispatch_uid='template_pre_save_handler',
+        )
         pre_save.connect(
             handlers.organization_disabled_handler,
             sender=self.org_model,
             dispatch_uid='organization_disabled_pre_save_clear_device_checksum_cache',
         )
+        post_save.connect(
+            self.template_model.post_save_handler,
+            sender=self.template_model,
+            dispatch_uid='template_post_save_handler',
+        )
         post_save.connect(
             self.org_limits.post_save_handler,
             sender=self.org_model,

openwisp_controller/config/base/template.py

@@ -106,30 +106,31 @@ class Meta:
         verbose_name_plural = _('templates')
         unique_together = (('organization', 'name'),)
 
-    def save(self, *args, **kwargs):
+    @classmethod
+    def pre_save_handler(cls, instance, *args, **kwargs):
         """
-        modifies status of related configs
-        if key attributes have changed (queries the database)
+        Modifies status of related configs
         """
-        update_related_config_status = False
-        if not self._state.adding:
-            if hasattr(self, 'backend_instance'):
-                del self.backend_instance
-            current = self.__class__.objects.get(pk=self.pk)
-            try:
-                current_checksum = current.checksum
-            except NetjsonconfigValidationError:
-                # If the Netjsonconfig library upgrade changes the schema,
-                # the old configuration may become invalid, raising an exception.
-                # Setting the checksum to None forces related configurations to update.
-                current_checksum = None
-            update_related_config_status = self.checksum != current_checksum
-        # save current changes
-        super().save(*args, **kwargs)
-        # update relations
-        if update_related_config_status:
+        try:
+            current = cls.objects.get(id=instance.id)
+        except cls.DoesNotExist:
+            return
+        if hasattr(instance, 'backend_instance'):
+            del instance.backend_instance
+        try:
+            current_checksum = current.checksum
+        except NetjsonconfigValidationError:
+            # If the Netjsonconfig library upgrade changes the schema,
+            # the old configuration may become invalid, raising an exception.
+            # Setting the checksum to None forces related configurations to update.
+            current_checksum = None
+        instance._update_related_config_status = instance.checksum != current_checksum
+
+    @classmethod
+    def post_save_handler(cls, instance, created, *args, **kwargs):
+        if not created and getattr(instance, '_update_related_config_status', False):
             transaction.on_commit(
-                lambda: update_template_related_config_status.delay(self.pk)
+                lambda: update_template_related_config_status.delay(instance.pk)
             )
 
     def _update_related_config_status(self):

openwisp_controller/config/tests/test_admin.py

@@ -8,13 +8,19 @@
 from django.contrib.auth import get_user_model
 from django.core.exceptions import ValidationError
 from django.core.files.base import ContentFile
+from django.core.management import call_command
 from django.db import IntegrityError
 from django.db.models.signals import post_save
 from django.test import TestCase, TransactionTestCase
 from django.urls import reverse
+from reversion.models import Version
 from swapper import load_model
 
-from openwisp_utils.tests import AdminActionPermTestMixin, catch_signal
+from openwisp_utils.tests import (
+    AdminActionPermTestMixin,
+    capture_any_output,
+    catch_signal,
+)
 
 from ...geo.tests.utils import TestGeoMixin
 from ...tests.utils import TestAdminMixin
@@ -2446,6 +2452,31 @@ def test_device_changelist_deactivate_admin_action(self):
             is_initially_deactivated=False,
         )
 
+    @capture_any_output()
+    def test_restoring_template_sends_config_modified(self):
+        template = self._create_template(default=True)
+        call_command('createinitialrevisions')
+        # Make changes to the template and create revision
+        template.config['interfaces'][0]['name'] = 'eth1'
+        template.full_clean()
+        template.save()
+        call_command('createinitialrevisions')
+
+        config = self._create_config(organization=self._get_org())
+        config.set_status_applied()
+        config_checksum = config.checksum
+
+        # Revert the oldest version for the template
+        version = Version.objects.get_for_model(Template).last()
+        version.revert()
+        template.refresh_from_db()
+        config = Config.objects.get(id=config.id)
+        # Verify the template is restored to the previous version
+        self.assertEqual(template.config['interfaces'][0]['name'], 'eth0')
+        # Verify config status is changed to modified.
+        self.assertEqual(config.status, 'modified')
+        self.assertNotEqual(config.checksum, config_checksum)
+
 
 class TestDeviceGroupAdmin(
     CreateDeviceGroupMixin,