[fix] Resync org settings on repeated global email disable #431

Preserve explicit global preference updates from the preferences API so reapplying the same global state still propagates to organization notification settings, add API regression coverage for repeated global email and web updates, and include the CI-requested formatting fix.

Fixes #431

Author: hypnoastic

openwisp_notifications/api/serializers.py

@@ -91,6 +91,16 @@ def to_representation(self, instance):
         data["email"] = instance.email_notification
         return data
 
+    def update(self, instance, validated_data):
+        explicit_global_fields = {
+            field for field in ["web", "email"] if field in validated_data
+        }
+        if explicit_global_fields and not instance.organization and not instance.type:
+            # The preferences UI can explicitly reapply the same global state.
+            # Preserve that intent so the model can resync organization rows.
+            instance._explicit_global_update_fields = explicit_global_fields
+        return super().update(instance, validated_data)
+
 
 class IgnoreObjectNotificationSerializer(serializers.ModelSerializer):
     class Meta:

openwisp_notifications/base/models.py

@@ -445,15 +445,26 @@ def validate_global_setting(self):
                 raise ValidationError("There can only be one global setting per user.")
 
     def save(self, *args, **kwargs):
+        explicit_global_update_fields = set(
+            getattr(self, "_explicit_global_update_fields", [])
+        )
+        if hasattr(self, "_explicit_global_update_fields"):
+            delattr(self, "_explicit_global_update_fields")
         if not self.web_notification:
             self.email = self.web_notification
         with transaction.atomic():
             if not self.organization and not self.type:
                 try:
-                    previous_state = self.__class__.objects.only("email").get(
+                    previous_state = self.__class__.objects.only("web", "email").get(
                         pk=self.pk
                     )
-                    updates = {"web": self.web}
+                    updates = {}
+
+                    if (
+                        self.web != previous_state.web
+                        or "web" in explicit_global_update_fields
+                    ):
+                        updates["web"] = self.web
 
                     # If global web notifications are disabled, then disable email notifications as well
                     if not self.web:
@@ -462,7 +473,10 @@ def save(self, *args, **kwargs):
                     # Update email notifiations only if it's different from the previous state
                     # Otherwise, it would overwrite the email notification settings for specific
                     # setting that were enabled by the user after disabling global email notifications
-                    if self.email != previous_state.email:
+                    if (
+                        self.email != previous_state.email
+                        or "email" in explicit_global_update_fields
+                    ):
                         updates["email"] = self.email
 
                     self.user.notificationsetting_set.exclude(pk=self.pk).update(

openwisp_notifications/tests/test_api.py

@@ -794,6 +794,98 @@ def test_update_notification_setting_api(self):
             )
             self.assertEqual(response.status_code, 403)
 
+    def test_reapplying_global_email_setting_updates_organization_settings(self):
+        org = self._get_org("default")
+        global_setting = NotificationSetting.objects.get(
+            user=self.admin, type=None, organization=None
+        )
+        global_setting_url = self._get_path("notification_setting", global_setting.pk)
+        org_setting_url = self._get_path(
+            "user_org_notification_setting", self.admin.pk, org.pk
+        )
+
+        response = self.client.patch(
+            global_setting_url,
+            data={"email": False},
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(
+            NotificationSetting.objects.filter(
+                user=self.admin, organization=org, email=True
+            ).exists()
+        )
+
+        response = self.client.post(
+            org_setting_url,
+            data={"web": True, "email": True},
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertTrue(
+            NotificationSetting.objects.filter(
+                user=self.admin, organization=org, email=True
+            ).exists()
+        )
+
+        response = self.client.patch(
+            global_setting_url,
+            data={"email": False},
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(
+            NotificationSetting.objects.filter(
+                user=self.admin, organization=org, email=True
+            ).exists()
+        )
+
+    def test_reapplying_global_web_setting_updates_organization_settings(self):
+        org = self._get_org("default")
+        global_setting = NotificationSetting.objects.get(
+            user=self.admin, type=None, organization=None
+        )
+        global_setting_url = self._get_path("notification_setting", global_setting.pk)
+        org_setting_url = self._get_path(
+            "user_org_notification_setting", self.admin.pk, org.pk
+        )
+
+        response = self.client.patch(
+            global_setting_url,
+            data={"web": False},
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(
+            NotificationSetting.objects.filter(
+                user=self.admin, organization=org, web=True
+            ).exists()
+        )
+
+        response = self.client.post(
+            org_setting_url,
+            data={"web": True, "email": False},
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertTrue(
+            NotificationSetting.objects.filter(
+                user=self.admin, organization=org, web=True
+            ).exists()
+        )
+
+        response = self.client.patch(
+            global_setting_url,
+            data={"web": False},
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(
+            NotificationSetting.objects.filter(
+                user=self.admin, organization=org, web=True
+            ).exists()
+        )
+
     def test_disallowed_change_types_absent_in_notification_setting_api(self):
         with self.subTest("disallowed type setting not present in list"):
             path = self._get_path("notification_setting_list")

openwisp_notifications/tests/test_selenium.py

@@ -171,13 +171,11 @@ def test_email_unsubscribe_page(self):
 
         with self.subTest("Network request fails"):
             self.open(unsubscribe_link)
-            self.web_driver.execute_script(
-                """
+            self.web_driver.execute_script("""
                 window.fetch = function() {
                     return Promise.reject(new Error('Simulated fetch failure'));
                 };
-            """
-            )
+            """)
             self.web_driver.find_element(By.ID, "toggle-btn").click()
             self.wait_for_visibility(By.ID, "error-msg")
             browser_logs = self.get_browser_logs()