diff --git a/openwisp_notifications/api/serializers.py b/openwisp_notifications/api/serializers.py index 42d91348..cd799e9c 100644 --- a/openwisp_notifications/api/serializers.py +++ b/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: diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 002611fa..3bf1f016 100644 --- a/openwisp_notifications/base/models.py +++ b/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( diff --git a/openwisp_notifications/tests/test_api.py b/openwisp_notifications/tests/test_api.py index 34a514be..1de366a9 100644 --- a/openwisp_notifications/tests/test_api.py +++ b/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")