[req-changes] Improved code comment and test

Author: pandafy

openwisp_controller/config/base/config.py

@@ -330,26 +330,24 @@ def manage_vpn_clients(cls, action, instance, pk_set, **kwargs):
         else:
             templates = pk_set
 
+        # Check if all templates in pk_set are required templates. If they are,
+        # skip deletion of VpnClient objects at this point.
         if len(pk_set) != templates.filter(required=True).count():
             # Explanation:
-            # The SortedManyToManyField clears all templates before adding new ones.
-            # This operation emits an m2m_changed signal with the "post_clear" action,
-            # which triggers the "enforce_required_templates" receiver to add
-            # required templates back.
+            # SortedManyToManyField clears all existing templates before adding
+            # new ones. This triggers an m2m_changed signal with the "post_clear"
+            # action, which is handled by the "enforce_required_templates" signal
+            # receiver. That receiver re-adds the required templates.
             #
-            # Adding required templates emits another m2m_changed signal with the
-            #  "post_add" action, which executes this receiver. At this point, only
-            # required templates exist in the DB, hence we cannot decide which
-            # VpnClient objects to delete.
+            # Re-adding required templates triggers another m2m_changed signal
+            # with the "post_add" action. At this stage, only required templates
+            # exist in the DB, so we cannot yet determine which VpnClient objects
+            # should be deleted based on the new selection.
             #
-            # Therefore, we don't delete any VpnClient objects at this point
-            # that are not associated with the current templates. The receiver
-            # will be called again with the "post_add" action when all the templates
-            # are added back, including the required ones. And then, it will
-            # delete any VpnClient objects that are not associated with the
-            # current templates.
-
-            # Delete VPN clients that are not associated with current templates
+            # Therefore, we defer deletion of VpnClient objects until the "post_add"
+            # signal is triggered again—after all templates, including the required
+            # ones, have been fully added. At that point, we can identify and
+            # delete VpnClient objects not linked to the final template set.
             instance.vpnclient_set.exclude(
                 template_id__in=instance.templates.values_list("id", flat=True)
             ).delete()

openwisp_controller/config/tests/test_admin.py

@@ -2077,17 +2077,23 @@ def _update_template(templates):
 
         # Remove first VPN template via admin
         _update_template(templates=[required_template, vpn2_template])
-
         self.assertEqual(config.templates.count(), 2)
         self.assertEqual(config.vpnclient_set.count(), 1)
         # Verify only vpn2 client remains
-        self.assertFalse(config.vpnclient_set.filter(vpn=vpn1).exists())
-        self.assertTrue(config.vpnclient_set.filter(vpn=vpn2).exists())
+        self.assertEqual(config.vpnclient_set.filter(vpn=vpn1).exists(), False)
+        self.assertEqual(config.vpnclient_set.filter(vpn=vpn2).exists(), True)
         # Verify that the vpn2 client is the same as before (same PK)
         self.assertEqual(
             vpn2_client.pk, config.vpnclient_set.filter(vpn=vpn2).first().pk
         )
 
+        # Remove the second VPN template and ensure the required template remains.
+        # This verifies that VpnClient object should get deleted it there's only
+        # one required template applied.
+        _update_template(templates=[required_template])
+        self.assertEqual(config.templates.count(), 1)
+        self.assertEqual(config.vpnclient_set.count(), 0)
+
     def test_vpn_template_switch(self):
         """
         Test switching between two VPN templates that use the same VPN server