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

[hypnoastic]

What does this PR do?

This fixes a notification preferences bug where reapplying the global email disable state did not resync the user’s organization-level notification settings.

What changed and why?

• Preserve explicit global preference updates coming from the preferences API.
• Resync organization-level notification settings even when global email is set to False again and the stored global value was already False.
• Add regression coverage for the exact API flow used by the notification preferences page.

UI Verification

Screen.Recording.2026-03-21.at.8.20.33.PM.mov

Self Review Checklist

• Kept the fix minimal and limited to the affected notification preferences flow
• Added regression coverage for the issue
• Verified focused and broader relevant tests pass
• Avoided unrelated refactors

Fixes #431

[coderabbitai]

📝 Walkthrough

Walkthrough

The patch ensures explicitly provided global notification fields propagate to organization-level settings even if their values don’t change. The serializer records which global fields (web and/or email) were present on update by setting _explicit_global_update_fields on the instance. The model’s save() reads this hint to include those fields in the bulk update payload for organization-scoped settings, removing the hint afterward. Tests were added to verify reapplying global disables correctly updates organization settings.

Sequence Diagram(s)

This section is skipped because the changes are primarily a bug fix that modifies existing internal logic flow rather than introducing a new feature or significantly restructuring control flow across multiple components in a way that would benefit from visualization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Bug Fixes	❌ Error	Root cause fix correctly implements explicit field tracking; API regression tests are comprehensive and deterministic. However, no Selenium test was added to verify the UI bug fix despite the issue showing a user-facing notification preferences page bug.	Add a Selenium test reproducing the issue #431 scenario: disable global email, enable org email, re-disable global email, and verify UI correctly reflects synchronized settings after refresh.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title follows the required [fix] prefix format and clearly describes the core fix: resyncing organization settings when disabling global email notifications.
Description check	✅ Passed	Description covers what changed, why it changed, includes UI verification, self-review checklist, and references issue #431, though the template checklist items are not all explicitly checked.
Linked Issues check	✅ Passed	The code changes directly address issue #431 by preserving explicit global updates and resyncing organization settings even when disabling an already-disabled global notification.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the notification preferences bug: serializer override, model save logic, API tests, and a minor Selenium test reformatting for consistency.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.

Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@openwisp_notifications/base/models.py`:
- Around line 470-474: The update-building logic applies the
explicit_global_update_fields check for email but not for web; make web
symmetric by only adding updates["web"] when self.web != previous_state.web or
"web" in explicit_global_update_fields (mirroring the email check using
self.email, previous_state.email, explicit_global_update_fields and
updates["email"]) so future conditional logic for web behaves consistently.

In `@openwisp_notifications/tests/test_api.py`:
- Around line 819-824: Add a symmetric test that verifies reapplying the global
"web" setting behaves like the "email" case: create a new test method (e.g.,
test_reapplying_global_web_setting_updates_organization_settings) that follows
the same setup and flow as the existing email resync test but toggles web (use
web=False then reapply web=True) and asserts the org setting changes and any
side effects are triggered; reference the same request flow that posts to
org_setting_url and the same explicit_global_update_fields handling to ensure
the code path for "web" is exercised.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4f1f1349-0c79-418a-bd24-3239df5c1e02

📥 Commits

Reviewing files that changed from the base of the PR and between d5b5bb9 and 0a228b7.

📒 Files selected for processing (3)

• openwisp_notifications/api/serializers.py
• openwisp_notifications/base/models.py
• openwisp_notifications/tests/test_api.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

• GitHub Check: Python==3.11 | django~=5.1.0
• GitHub Check: Python==3.12 | django~=5.0.0
• GitHub Check: Python==3.11 | django~=5.2.0
• GitHub Check: Python==3.13 | django~=5.2.0
• GitHub Check: Python==3.12 | django~=4.2.0
• GitHub Check: Python==3.12 | django~=5.2.0
• GitHub Check: Python==3.12 | django~=5.1.0
• GitHub Check: Python==3.10 | django~=5.1.0
• GitHub Check: Python==3.11 | django~=4.2.0
• GitHub Check: Python==3.10 | django~=5.0.0
• GitHub Check: Python==3.10 | django~=4.2.0
• GitHub Check: Python==3.10 | django~=5.2.0
• GitHub Check: Python==3.11 | django~=5.0.0

🔇 Additional comments (3)

openwisp_notifications/base/models.py (1)

448-453: LGTM: Clean extraction and cleanup of the hint attribute.

The pattern of reading _explicit_global_update_fields into a local variable and immediately deleting the transient attribute is sound. This prevents stale state if the instance is saved multiple times.

openwisp_notifications/api/serializers.py (1)

94-102: LGTM: Clean implementation of the explicit field detection.

The logic correctly:

1. Detects which fields (web/email) are explicitly provided in the request payload via validated_data.
2. Only applies this to global settings (where both organization and type are None).
3. Passes the hint to the model via a transient instance attribute before delegating to super().update().

This approach keeps the serializer and model properly decoupled while enabling the model to know the caller's intent.

openwisp_notifications/tests/test_api.py (1)

797-841: LGTM: Comprehensive regression test for the bug fix.

This test accurately reproduces the bug scenario from issue #431:

1. Disables global email notifications
2. Re-enables email for a specific organization
3. Re-applies the global email disable
4. Verifies that organization-level email settings are properly synced

The assertions correctly validate the fix by checking that no email=True settings remain for the organization after step 3.

[hypnoastic]

Forced Push to keep the commit history clean before review.

[hypnoastic]

I have addressed all the review feedback given by CodeRabbit and the PR is ready for a review.