[tests] Fix on_commit test failures and add regression tests

- Wrap device creation and HTTP requests with captureOnCommitCallbacks(execute=True)
  in TestCase-based tests so transaction.on_commit callbacks fire correctly
  (TestCase wraps tests in a never-committed transaction so on_commit never fires)
- Affected tests: test_config_backend_changed, test_device_create_with_group,
  test_device_api_change_config_backend, test_add_device_with_group_templates,
  test_device_import_with_group_apply_templates,
  test_unassigning_group_removes_old_templates, test_group_templates_are_not_forced
- Add test_handlers_transaction.py with TransactionTestCase to verify
  change_devices_templates.delay is called after commit and not on rollback
  for all three model handlers (Device, DeviceGroup, Config) and the list branch

Author: prakash-kalwaniya

openwisp_controller/config/tests/test_admin.py

@@ -671,9 +671,12 @@ def test_device_import_with_group_apply_templates(self):
         self.assertIn("confirm_form", response.context)
         confirm_form = response.context["confirm_form"]
         data = confirm_form.initial
-        response = self.client.post(
-            reverse(f"admin:{self.app_label}_device_process_import"), data, follow=True
-        )
+        with self.captureOnCommitCallbacks(execute=True):
+            response = self.client.post(
+                reverse(f"admin:{self.app_label}_device_process_import"),
+                data,
+                follow=True,
+            )
         self.assertEqual(response.status_code, 200)
         device = Device.objects.first()
         self.assertIsNotNone(device)
@@ -746,9 +749,10 @@ def test_add_device_with_group_templates(self):
         with catch_signal(post_save) as mock_post_save, catch_signal(
             device_group_changed
         ) as device_group_changed_mock:
-            self.client.post(
-                reverse(f"admin:{self.app_label}_device_add"), data, follow=True
-            )
+            with self.captureOnCommitCallbacks(execute=True):
+                self.client.post(
+                    reverse(f"admin:{self.app_label}_device_add"), data, follow=True
+                )
         device = Device.objects.first()
         mock_post_save.assert_any_call(
             signal=post_save,
@@ -767,7 +771,8 @@ def test_unassigning_group_removes_old_templates(self):
         template = self._create_template(name="template")
         dg = self._create_device_group(name="test-group", organization=org)
         dg.templates.add(template)
-        device = self._create_device(organization=org, group=dg)
+        with self.captureOnCommitCallbacks(execute=True):
+            device = self._create_device(organization=org, group=dg)
         self.assertIn(template, device.config.templates.all())
         path = reverse(f"admin:{self.app_label}_device_change", args=[device.pk])
         params = self._get_device_params(org=org)
@@ -780,7 +785,8 @@ def test_unassigning_group_removes_old_templates(self):
                 "group": "",
             }
         )
-        response = self.client.post(path, params, follow=True)
+        with self.captureOnCommitCallbacks(execute=True):
+            response = self.client.post(path, params, follow=True)
         self.assertNotContains(response, "errors", status_code=200)
         device.refresh_from_db()
         self.assertIsNone(device.group)
@@ -791,7 +797,8 @@ def test_group_templates_are_not_forced(self):
         t = self._create_template(name="t")
         dg = self._create_device_group(name="test-group", organization=o)
         dg.templates.add(t)
-        d = self._create_device(organization=o, group=dg)
+        with self.captureOnCommitCallbacks(execute=True):
+            d = self._create_device(organization=o, group=dg)
         self.assertIn(t, d.config.templates.all())
         path = reverse(f"admin:{self.app_label}_device_change", args=[d.pk])
         params = self._get_device_params(org=o)

openwisp_controller/config/tests/test_api.py

@@ -172,7 +172,8 @@ def test_device_create_with_group(self):
         org = self._get_org()
         data["organization"] = org.pk
         data["group"] = dg.pk
-        r = self.client.post(path, data, content_type="application/json")
+        with self.captureOnCommitCallbacks(execute=True):
+            r = self.client.post(path, data, content_type="application/json")
         self.assertEqual(r.status_code, 201)
         self.assertEqual(Device.objects.count(), 1)
         self.assertIn(template, Device.objects.first().config.templates.all())
@@ -474,7 +475,8 @@ def test_device_api_change_config_backend(self):
         t2 = self._create_template(name="t2", backend="netjsonconfig.OpenWisp")
         dg1 = self._create_device_group(name="dg-1")
         dg1.templates.add(t1, t2)
-        d1 = self._create_device(name="test-device", group=dg1)
+        with self.captureOnCommitCallbacks(execute=True):
+            d1 = self._create_device(name="test-device", group=dg1)
         self.assertIn(t1, d1.config.templates.all())
         path = reverse("config_api:device_detail", args=[d1.pk])
         data = {
@@ -490,7 +492,8 @@ def test_device_api_change_config_backend(self):
                 "config": {},
             },
         }
-        r = self.client.put(path, data, content_type="application/json")
+        with self.captureOnCommitCallbacks(execute=True):
+            r = self.client.put(path, data, content_type="application/json")
         self.assertEqual(r.status_code, 200)
         self.assertEqual(r.data["group"], dg1.pk)
         self.assertEqual(r.data["config"]["backend"], "netjsonconfig.OpenWisp")

openwisp_controller/config/tests/test_config.py

@@ -878,7 +878,8 @@ def test_config_backend_changed(self):
         group.templates.add(*[t1, t2])
         with self.subTest("config_backend_changed signal must not be sent on creation"):
             with catch_signal(config_backend_changed) as handler:
-                d = self._create_device(group=group, organization=org)
+                with self.captureOnCommitCallbacks(execute=True):
+                    d = self._create_device(group=group, organization=org)
                 handler.assert_not_called()
                 self.assertTrue(d.config.templates.filter(pk=t1.pk).exists())
                 self.assertFalse(d.config.templates.filter(pk=t2.pk).exists())
@@ -896,7 +897,8 @@ def test_config_backend_changed(self):
             with catch_signal(config_backend_changed) as handler:
                 c = d.config
                 c.backend = backend
-                c.save(update_fields=["backend"])
+                with self.captureOnCommitCallbacks(execute=True):
+                    c.save(update_fields=["backend"])
                 handler.assert_called_once_with(
                     sender=Config,
                     signal=config_backend_changed,

openwisp_controller/config/tests/test_handlers_transaction.py

@@ -0,0 +1,109 @@
+from unittest.mock import patch
+
+from django.db import transaction
+from django.test import TransactionTestCase
+from swapper import load_model
+
+from openwisp_controller.config.handlers import devicegroup_templates_change_handler
+
+Device = load_model("config", "Device")
+DeviceGroup = load_model("config", "DeviceGroup")
+Config = load_model("config", "Config")
+
+
+class ForceRollback(Exception):
+    """Sentinel exception used to deliberately roll back a transaction in tests."""
+
+
+class TestHandlersTransaction(TransactionTestCase):
+    @patch("openwisp_controller.config.tasks.change_devices_templates.delay")
+    def test_devicegroup_templates_change_handler_on_commit(self, mock_delay):
+        """
+        Test that change_devices_templates is called via delay and only after
+        transaction.on_commit block commits successfully.
+        """
+
+        class MockMeta:
+            def __init__(self, model_name):
+                self.model_name = model_name
+
+        class MockState:
+            def __init__(self, adding):
+                self.adding = adding
+
+        class MockInstance:
+            def __init__(self, id, model_name):
+                self.id = id
+                self._meta = MockMeta(model_name)
+                self._state = MockState(adding=False)
+
+        # Test cases for each model caller
+        model_names = [
+            Device._meta.model_name,
+            DeviceGroup._meta.model_name,
+            Config._meta.model_name,
+        ]
+
+        for model_name in model_names:
+            with self.subTest(model_name=model_name):
+                mock_instance = MockInstance(id="test-id", model_name=model_name)
+                kwargs = {}
+                if model_name == Config._meta.model_name:
+                    kwargs["backend"] = "test-backend"
+                    kwargs["old_backend"] = "old-backend"
+                elif model_name == DeviceGroup._meta.model_name:
+                    kwargs["templates"] = []
+                    kwargs["old_templates"] = []
+                elif model_name == Device._meta.model_name:
+                    kwargs["group_id"] = "test-group"
+                    kwargs["old_group_id"] = "old-group"
+
+                # Case 1: Transaction commits
+                mock_delay.reset_mock()
+                with transaction.atomic():
+                    devicegroup_templates_change_handler(mock_instance, **kwargs)
+                    # Should not be called inside the transaction
+                    mock_delay.assert_not_called()
+
+                mock_delay.assert_called_once()  # Should be called after commit
+
+                # Case 2: Transaction rolls back
+                mock_delay.reset_mock()
+                with self.assertRaises(ForceRollback):
+                    with transaction.atomic():
+                        devicegroup_templates_change_handler(mock_instance, **kwargs)
+                        mock_delay.assert_not_called()
+                        raise ForceRollback
+
+                # Should not be called because it rolled back
+                mock_delay.assert_not_called()
+
+    @patch("openwisp_controller.config.tasks.change_devices_templates.delay")
+    def test_devicegroup_templates_change_handler_list_branch(self, mock_delay):
+        """
+        Test that the list branch of devicegroup_templates_change_handler
+        (type(instance) is list) correctly dispatches change_devices_templates.delay
+        after commit and not on rollback.
+        """
+        instance_list = ["device-id-1", "device-id-2"]
+        kwargs = {"group_id": "test-group", "old_group_id": "old-group"}
+
+        with self.subTest("list branch: delay called after commit"):
+            mock_delay.reset_mock()
+            with transaction.atomic():
+                devicegroup_templates_change_handler(instance_list, **kwargs)
+                # Should not be called inside the transaction
+                mock_delay.assert_not_called()
+
+            mock_delay.assert_called_once()  # Should be called after commit
+
+        with self.subTest("list branch: delay not called on rollback"):
+            mock_delay.reset_mock()
+            with self.assertRaises(ForceRollback):
+                with transaction.atomic():
+                    devicegroup_templates_change_handler(instance_list, **kwargs)
+                    mock_delay.assert_not_called()
+                    raise ForceRollback
+
+            # Should not be called because it rolled back
+            mock_delay.assert_not_called()