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

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")