Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions openwisp_notifications/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,17 @@ class AbstractNotification(UUIDModel, BaseNotification):
_action_object = BaseNotification.action_object
_target = BaseNotification.target

@property
def resolved_verb(self):
config = {}
try:
config = get_notification_configuration(self.type)
except NotificationRenderException as e:
logger.error(
"Couldn't get notification config for type %s : %s", self.type, e
)
return config.get("verb") or self.verb

class Meta(BaseNotification.Meta):
abstract = True

Expand Down
2 changes: 1 addition & 1 deletion openwisp_notifications/base/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class AbstractNotification(models.Model):
actor = GenericForeignKey("actor_content_type", "actor_object_id")
actor.short_description = _("actor")

verb = models.CharField(_("verb"), max_length=255)
verb = models.CharField(_("verb"), max_length=255, null=True, blank=True)
description = models.TextField(_("description"), blank=True, null=True)

target_content_type = models.ForeignKey(
Expand Down
2 changes: 0 additions & 2 deletions openwisp_notifications/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ def notify_handler(**kwargs):
level = kwargs.pop(
"level", notification_template.get("level", Notification.LEVELS.info)
)
verb = notification_template.get("verb", kwargs.pop("verb", None))
user_app_name = User._meta.app_label

where = Q(is_superuser=True)
Expand Down Expand Up @@ -146,7 +145,6 @@ def notify_handler(**kwargs):
notification = Notification(
recipient=recipient,
actor=actor,
verb=str(verb),
public=public,
description=description,
timestamp=timestamp,
Expand Down
20 changes: 20 additions & 0 deletions openwisp_notifications/migrations/0012_alter_notification_verb.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 5.2.6 on 2025-10-14 18:40

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("openwisp_notifications", "0011_populate_organizationnotificationsettings"),
]

operations = [
migrations.AlterField(
model_name="notification",
name="verb",
field=models.CharField(
blank=True, max_length=255, null=True, verbose_name="verb"
),
),
]
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{% block head %} {{ notification.level }} : {{notification.target}} {{ notification.verb }} {% endblock head %}
{% block head %} {{ notification.level }} : {{notification.target}} {{ notification.resolved_verb }} {% endblock head %}
{% block body %}
{% if notification.actor_link %}[{{notification.actor}}]({{notification.actor_link}}){% else %}{{notification.actor}}{% endif %}
reports
{% if notification.target_link %}[{{notification.target}}]({{notification.target_link}}){% else %}{{notification.target}}{% endif %}
{{ notification.verb }}.
{{ notification.resolved_verb }}.
{% endblock body %}
42 changes: 37 additions & 5 deletions openwisp_notifications/tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ def test_default_notification_type(self):
self._create_notification()
n = notification_queryset.first()
self.assertEqual(n.level, "info")
self.assertEqual(n.verb, "default verb")
self.assertEqual(n.resolved_verb, "default verb")
self.assertIn(
"Default notification with default verb and level info by", n.message
)
Expand Down Expand Up @@ -470,7 +470,7 @@ def test_generic_notification_type(self):
self._create_notification()
n = notification_queryset.first()
self.assertEqual(n.level, "info")
self.assertEqual(n.verb, "generic verb")
self.assertEqual(n.resolved_verb, "generic verb")
expected_output = (
'<p><a href="https://example.com{user_path}">admin</a></p>'
).format(
Expand Down Expand Up @@ -573,8 +573,8 @@ def test_register_unregister_notification_type(self):
"verbose_name": "Test Notification Type",
"level": "test",
"verb": "testing",
"message": "{notification.verb} initiated by {notification.actor} since {notification}",
"email_subject": "[{site.name}] {notification.verb} reported by {notification.actor}",
"message": "{notification.resolved_verb} initiated by {notification.actor} since {notification}",
"email_subject": "[{site.name}] {notification.resolved_verb} reported by {notification.actor}",
}

with self.subTest("Registering new notification type"):
Expand All @@ -583,7 +583,7 @@ def test_register_unregister_notification_type(self):
self._create_notification()
n = notification_queryset.first()
self.assertEqual(n.level, "test")
self.assertEqual(n.verb, "testing")
self.assertEqual(n.resolved_verb, "testing")
self.assertEqual(
n.message,
"<p>testing initiated by admin since 0\xa0minutes</p>",
Expand Down Expand Up @@ -1506,6 +1506,38 @@ def test_notification_preference_page(self):
response = self.client.get(reverse(preference_page, args=(uuid4(),)))
self.assertEqual(response.status_code, 404)

@mock_notification_types
def test_dynamic_verb_changed(self):
self.notification_options.update(
{"type": "default", "target": self._get_org_user()}
)
default_config = get_notification_configuration("default")
original_message = default_config["message"]
original_verb = default_config.get("verb", "default verb")
default_config["message"] = "Notification with {notification.resolved_verb}"
default_config["verb"] = "initial verb"

self._create_notification()
notification = notification_queryset.first()

with self.subTest("Test initial verb from config"):
self.assertEqual(notification.resolved_verb, "initial verb")
self.assertIn("initial verb", notification.message)

with self.subTest("Test verb changes dynamically from config"):
default_config["verb"] = "updated verb"
del notification.message
self.assertEqual(notification.resolved_verb, "updated verb")
self.assertIn("updated verb", notification.message)

with self.subTest("Test fallback to database verb"):
unregister_notification_type("default")
notification.__dict__["verb"] = "db verb"
self.assertEqual(notification.verb, "db verb")

default_config["message"] = original_message
default_config["verb"] = original_verb


class TestTransactionNotifications(TestOrganizationMixin, TransactionTestCase):
def setUp(self):
Expand Down
4 changes: 2 additions & 2 deletions openwisp_notifications/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"verbose_name": "Default Type",
"email_subject": "[{site.name}] Default Notification Subject",
"message": (
"Default notification with {notification.verb} and level {notification.level}"
"Default notification with {notification.resolved_verb} and level {notification.level}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break the existing integration with other OpenWISP apps.

We have a context manager notification_render_attributes , Can we set the verb using notification type in the context manage and then clear the field (set to None) in cleanup process?

@contextmanager
def notification_render_attributes(obj, **attrs):
"""
This context manager sets temporary attributes on
the notification object to allowing rendering of
notification.
It can only be used to set aliases of the existing attributes.
By default, it will set the following aliases:
- actor_link -> actor_url
- action_link -> action_url
- target_link -> target_url
"""
defaults = {
"actor_link": "actor_url",
"action_link": "action_url",
"target_link": "target_url",
}
defaults.update(attrs)
for target_attr, source_attr in defaults.items():
setattr(obj, target_attr, getattr(obj, source_attr))
# In Django 5.1+, GenericForeignKey fields defined in parent models can no longer
# be overridden in child models (https://code.djangoproject.com/ticket/36295).
# To avoid multiple database queries, we explicitly set these attributes here
# using our cached _related_object method instead of relying on the default
# GenericForeignKey accessor which would bypass our caching mechanism.
setattr(obj, "actor", obj._related_object("actor"))
setattr(obj, "action_object", obj._related_object("action_object"))
setattr(obj, "target", obj._related_object("target"))
yield obj
for attr in defaults.keys():
delattr(obj, attr)

It should behave similarly to the resolved_verb property.

db_verb = obj.verb
# we give preference to the verb stored in the database
verb = obj.verb or config.get('verb')

# In cleanup 
obj.verb = db_verb

Do you think a solution like this will solve our problem?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not tested it yet. But I think it's a plausible solution.

I am a little busy at the moment. I'll get back to you sometime around next week after investigating this approach.

Thanks again for the review!

" by [{notification.target}]({notification.target_link})"
),
"message_template": "openwisp_notifications/default_message.md",
Expand All @@ -23,7 +23,7 @@
"verbose_name": "Generic Type",
"email_subject": "[{site.name}] Generic Notification Subject",
"message": (
"Generic notification with {notification.verb} and level {notification.level}"
"Generic notification with {notification.resolved_verb} and level {notification.level}"
" by [{notification.actor}]({notification.actor_link})"
),
"description": "{notification.description}",
Expand Down
2 changes: 1 addition & 1 deletion tests/openwisp2/sample_notifications/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def register_notification_types(self):
"verbose_name": "Object created",
"verb": "created",
"level": "info",
"message": "{notification.target} object {notification.verb}.",
"message": "{notification.target} object {notification.resolved_verb}.",
"email_subject": "[{site.name}] INFO: {notification.target} created",
},
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 5.2.6 on 2025-10-14 23:01

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("sample_notifications", "0003_default_groups_permissions"),
]

operations = [
migrations.AlterField(
model_name="notification",
name="verb",
field=models.CharField(
blank=True, max_length=255, null=True, verbose_name="verb"
),
),
]
Loading