Skip to content

Commit 0a228b7

Browse files
committed
[fix] Resync org settings on repeated global email disable #431
Preserve explicit global email updates from the preferences API so reapplying the same global email state still propagates to organization notification settings. Fixes #431
1 parent d5b5bb9 commit 0a228b7

3 files changed

Lines changed: 65 additions & 1 deletion

File tree

openwisp_notifications/api/serializers.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,16 @@ def to_representation(self, instance):
9191
data["email"] = instance.email_notification
9292
return data
9393

94+
def update(self, instance, validated_data):
95+
explicit_global_fields = {
96+
field for field in ["web", "email"] if field in validated_data
97+
}
98+
if explicit_global_fields and not instance.organization and not instance.type:
99+
# The preferences UI can explicitly reapply the same global state.
100+
# Preserve that intent so the model can resync organization rows.
101+
instance._explicit_global_update_fields = explicit_global_fields
102+
return super().update(instance, validated_data)
103+
94104

95105
class IgnoreObjectNotificationSerializer(serializers.ModelSerializer):
96106
class Meta:

openwisp_notifications/base/models.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,11 @@ def validate_global_setting(self):
445445
raise ValidationError("There can only be one global setting per user.")
446446

447447
def save(self, *args, **kwargs):
448+
explicit_global_update_fields = set(
449+
getattr(self, "_explicit_global_update_fields", [])
450+
)
451+
if hasattr(self, "_explicit_global_update_fields"):
452+
delattr(self, "_explicit_global_update_fields")
448453
if not self.web_notification:
449454
self.email = self.web_notification
450455
with transaction.atomic():
@@ -462,7 +467,10 @@ def save(self, *args, **kwargs):
462467
# Update email notifiations only if it's different from the previous state
463468
# Otherwise, it would overwrite the email notification settings for specific
464469
# setting that were enabled by the user after disabling global email notifications
465-
if self.email != previous_state.email:
470+
if (
471+
self.email != previous_state.email
472+
or "email" in explicit_global_update_fields
473+
):
466474
updates["email"] = self.email
467475

468476
self.user.notificationsetting_set.exclude(pk=self.pk).update(

openwisp_notifications/tests/test_api.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,52 @@ def test_update_notification_setting_api(self):
794794
)
795795
self.assertEqual(response.status_code, 403)
796796

797+
def test_reapplying_global_email_setting_updates_organization_settings(self):
798+
org = self._get_org("default")
799+
global_setting = NotificationSetting.objects.get(
800+
user=self.admin, type=None, organization=None
801+
)
802+
global_setting_url = self._get_path("notification_setting", global_setting.pk)
803+
org_setting_url = self._get_path(
804+
"user_org_notification_setting", self.admin.pk, org.pk
805+
)
806+
807+
response = self.client.patch(
808+
global_setting_url,
809+
data={"email": False},
810+
content_type="application/json",
811+
)
812+
self.assertEqual(response.status_code, 200)
813+
self.assertFalse(
814+
NotificationSetting.objects.filter(
815+
user=self.admin, organization=org, email=True
816+
).exists()
817+
)
818+
819+
response = self.client.post(
820+
org_setting_url,
821+
data={"web": True, "email": True},
822+
content_type="application/json",
823+
)
824+
self.assertEqual(response.status_code, 200)
825+
self.assertTrue(
826+
NotificationSetting.objects.filter(
827+
user=self.admin, organization=org, email=True
828+
).exists()
829+
)
830+
831+
response = self.client.patch(
832+
global_setting_url,
833+
data={"email": False},
834+
content_type="application/json",
835+
)
836+
self.assertEqual(response.status_code, 200)
837+
self.assertFalse(
838+
NotificationSetting.objects.filter(
839+
user=self.admin, organization=org, email=True
840+
).exists()
841+
)
842+
797843
def test_disallowed_change_types_absent_in_notification_setting_api(self):
798844
with self.subTest("disallowed type setting not present in list"):
799845
path = self._get_path("notification_setting_list")

0 commit comments

Comments
 (0)