[fix] Fixed handling of lazy translation objects #438

[nemesifier]

Lazy translation objects are now evaluated properly.

Fixes #438

Checklist

• I have read the OpenWISP Contributing Guidelines.
• I have manually tested the changes proposed in this pull request.
• I have written new test cases for new code and/or updated existing tests for changes to existing code.
• N/A I have updated the documentation.

[coderabbitai]

Walkthrough

The handler now performs a targeted runtime coercion: when kwargs contains message and it's not a plain string, it is cast to str before assigning to notification.data, preventing JSON serialization errors from lazy translation objects. Tests were updated: OrganizationUser and Group are declared earlier to avoid duplication, and a new test asserts that providing gettext_lazy in notification data does not raise and that the stored message is a plain string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: handling of lazy translation objects in notifications, and references issue #438.
Description check	✅ Passed	The description covers the key points: what was fixed (lazy translation objects), which issue it closes, and confirms the checklist items completed.
Linked Issues check	✅ Passed	The code changes directly address issue #438: the handler coerces lazy message objects to strings before JSON serialization, and tests verify lazy translation objects are properly handled.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing lazy translation handling: modifying the handler to coerce messages to strings and adding test coverage for the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch issues/438-gettext_lazy-not-serializable

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a26828a and fbf1a9f.

📒 Files selected for processing (2)

• openwisp_notifications/handlers.py
• openwisp_notifications/tests/test_notifications.py

🚧 Files skipped from review as they are similar to previous changes (1)

• openwisp_notifications/handlers.py

🧰 Additional context used

🧬 Code graph analysis (1)

openwisp_notifications/tests/test_notifications.py (2)

openwisp_notifications/swapper.py (1)

• load_model (6-7)

openwisp_notifications/base/models.py (2)

• email_subject (285-302)
• message (244-246)

⏰ 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). (14)

• GitHub Check: Python==3.13 | django~=5.1.0
• GitHub Check: Python==3.11 | django~=5.1.0
• GitHub Check: Python==3.11 | django~=4.2.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~=5.0.0
• GitHub Check: Python==3.12 | django~=5.1.0
• GitHub Check: Python==3.10 | django~=5.2.0
• GitHub Check: Python==3.10 | django~=5.0.0
• GitHub Check: Python==3.11 | django~=5.0.0
• GitHub Check: Python==3.10 | django~=5.1.0
• GitHub Check: Python==3.12 | django~=4.2.0
• GitHub Check: Python==3.12 | django~=5.2.0
• GitHub Check: Python==3.10 | django~=4.2.0

🔇 Additional comments (3)

openwisp_notifications/tests/test_notifications.py (3)

20-22: LGTM — import updates are minimal and align with new test usage.

---

54-60: LGTM — centralized model loading keeps test setup consistent.

---

114-137: LGTM — solid regression coverage for non‑serializable message payloads.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@openwisp_notifications/handlers.py`:
- Around line 46-61: The _coerce_lazy_strings function is over-eagerly
converting all non-str types to strings; change it so it only coerces Django
lazy translation objects (instances of django.utils.functional.Promise) and
leaves other JSON-serializable types (None, bool, int, float, etc.) intact.
Update _coerce_lazy_strings to import Promise, recursively process dicts/lists
as before, return str(value) only when isinstance(value, Promise), return values
unchanged for str and for all other types (including None, bool, int, float),
and keep the recursion logic for nested structures in functions like
_coerce_lazy_strings itself.

🧹 Nitpick comments (1)

openwisp_notifications/tests/test_notifications.py (1)

114-136: Good regression test, but consider adding coverage for mixed data types.

The test correctly verifies that lazy translation objects are coerced to plain strings. However, given the implementation in _coerce_lazy_strings, it would be valuable to add a sub-test verifying that non-string JSON-serializable values (integers, booleans, None) are preserved as their original types, not converted to strings.

💡 Suggested additional test coverage

def test_lazy_translation(self):
    """
    Regression test for issue `#438`.
    Test that notifications with lazy translation objects in data
    can be saved without raising a TypeError.
    """
    # Using gettext_lazy in notification data should not fail
    notification_options = dict(
        sender=self.admin,
        description="Test Notification",
        verb="Test Notification",
        email_subject="Test Email subject",
        url="https://localhost:8000/admin",
        message=_("Translated message"),
        count=42,  # numeric value should be preserved
        enabled=True,  # boolean should be preserved
    )
    # Must not raise TypeError: Object of type __proxy__ is not JSON serializable
    notify.send(**notification_options)
    self.assertEqual(notification_queryset.count(), 1)
    n = notification_queryset.first()
    # Verify the message was stored as a plain string, not a proxy object
    self.assertEqual(n.data.get("message"), "Translated message")
    # Verify the stored value is not a lazy proxy object
    self.assertNotIsInstance(n.data.get("message"), Promise)
    # Verify non-string types are preserved
    self.assertEqual(n.data.get("count"), 42)
    self.assertIsInstance(n.data.get("count"), int)
    self.assertEqual(n.data.get("enabled"), True)
    self.assertIsInstance(n.data.get("enabled"), bool)

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec47735 and a26828a.

📒 Files selected for processing (2)

• openwisp_notifications/handlers.py
• openwisp_notifications/tests/test_notifications.py

🧰 Additional context used

🧬 Code graph analysis (1)

openwisp_notifications/tests/test_notifications.py (3)

openwisp_notifications/base/notifications.py (1)

• timesince (107-114)

openwisp_notifications/base/models.py (2)

• email_subject (285-302)
• message (244-246)

openwisp_notifications/api/serializers.py (1)

• data (51-56)

🔇 Additional comments (2)

openwisp_notifications/tests/test_notifications.py (1)

20-22: LGTM!

The imports for Promise and gettext_lazy are appropriate for testing the lazy translation coercion functionality.

openwisp_notifications/handlers.py (1)

183-183: Usage is correct, but depends on fixing the helper function.

The call site is appropriate—processing kwargs before assignment ensures lazy strings are coerced. However, this will only work correctly once _coerce_lazy_strings is fixed to preserve non-string JSON-serializable types.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}