From 2a38150cb2d156f61537192381e812aeac7b1093 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Wed, 1 Oct 2025 16:23:14 +0530 Subject: [PATCH 01/12] [fix] Dynamic updating of notification verb #334 Made verb field in notification nullable. Created a property verb under AbstractNotification in models that prioritizes verb from NOTIFICATION_TYPES and uses DB as a fallback. Fixes #334 --- openwisp_notifications/base/models.py | 12 ++++++++++++ openwisp_notifications/base/notifications.py | 2 +- openwisp_notifications/handlers.py | 3 +-- .../0012_alter_notification_verb_nullable.py | 18 ++++++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 002611fa..adef7258 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -101,6 +101,18 @@ class AbstractNotification(UUIDModel, BaseNotification): _action_object = BaseNotification.action_object _target = BaseNotification.target + @property + def verb(self): + try: + config = get_notification_configuration(self.type) + return config.get("verb") or self.__dict__.get("verb") or "unspecified" + except Exception: + return self.__dict__.get("verb") or "unspecified" + + @verb.setter + def verb(self, value): + self.__dict__["verb"] = value + class Meta(BaseNotification.Meta): abstract = True diff --git a/openwisp_notifications/base/notifications.py b/openwisp_notifications/base/notifications.py index 3674b276..2ba4a297 100644 --- a/openwisp_notifications/base/notifications.py +++ b/openwisp_notifications/base/notifications.py @@ -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( diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index b2aa04cf..bc1c785c 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -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) @@ -146,7 +145,7 @@ def notify_handler(**kwargs): notification = Notification( recipient=recipient, actor=actor, - verb=str(verb), + verb="unspecified", public=public, description=description, timestamp=timestamp, diff --git a/openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py b/openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py new file mode 100644 index 00000000..f54bb504 --- /dev/null +++ b/openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.6 on 2025-10-01 09:39 + +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(max_length=255, null=True, blank=True), + ), + ] From 84a46bf6a65440138e1da22a7228127092f5c31c Mon Sep 17 00:00:00 2001 From: Pranshu <153133494+pranshustuff@users.noreply.github.com> Date: Wed, 1 Oct 2025 18:10:37 +0530 Subject: [PATCH 02/12] [change] Remove verb field from Notifs #334 Same as above Fixes #334. --- openwisp_notifications/handlers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index bc1c785c..21feb451 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -145,7 +145,6 @@ def notify_handler(**kwargs): notification = Notification( recipient=recipient, actor=actor, - verb="unspecified", public=public, description=description, timestamp=timestamp, From d2cd2831882b597b1689a0181463513b885e71f0 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Thu, 2 Oct 2025 00:38:34 +0530 Subject: [PATCH 03/12] [fix] Dynamic update for notif verb #334 Added a test to verify correct behaviour and specified exceptions in the try-except in the verb property in models.py Fixes #334 --- openwisp_notifications/base/models.py | 9 ++++-- .../tests/test_notifications.py | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index adef7258..70184b6a 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -103,11 +103,14 @@ class AbstractNotification(UUIDModel, BaseNotification): @property def verb(self): + config = {} try: config = get_notification_configuration(self.type) - return config.get("verb") or self.__dict__.get("verb") or "unspecified" - except Exception: - return self.__dict__.get("verb") or "unspecified" + except (NotificationRenderException, TypeError) as e: + logger.warning( + "Could not get notification config for type %s: %s", self.type, e + ) + return self.__dict__.get("verb") or config.get("verb") @verb.setter def verb(self, value): diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index ddccc541..7cfc2e80 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -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.verb}" + default_config["verb"] = "initial verb" + + self._create_notification() + notification = notification_queryset.first() + + with self.subTest("Test initial verb from config"): + self.assertEqual(notification.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.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): From 5b8f80bf4dd60acb6ed01ec8ca7ab36e2e1e95a2 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Thu, 2 Oct 2025 01:08:24 +0530 Subject: [PATCH 04/12] [fix] Ran ./manage.py makemigrations #334 It should pass CI bulds now I think. Fixes #334 --- .../migrations/0013_remove_notification_verb.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 openwisp_notifications/migrations/0013_remove_notification_verb.py diff --git a/openwisp_notifications/migrations/0013_remove_notification_verb.py b/openwisp_notifications/migrations/0013_remove_notification_verb.py new file mode 100644 index 00000000..b12afcc3 --- /dev/null +++ b/openwisp_notifications/migrations/0013_remove_notification_verb.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.6 on 2025-10-01 19:28 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_notifications", "0012_alter_notification_verb_nullable"), + ] + + operations = [ + migrations.RemoveField( + model_name="notification", + name="verb", + ), + ] From 90202be9c1837920367dc4bb95e7bd409bc3b397 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Thu, 2 Oct 2025 01:36:26 +0530 Subject: [PATCH 05/12] [fix] Deleting current migration #334 This reverts commit 5b8f80bf4dd60acb6ed01ec8ca7ab36e2e1e95a2. --- .../migrations/0013_remove_notification_verb.py | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 openwisp_notifications/migrations/0013_remove_notification_verb.py diff --git a/openwisp_notifications/migrations/0013_remove_notification_verb.py b/openwisp_notifications/migrations/0013_remove_notification_verb.py deleted file mode 100644 index b12afcc3..00000000 --- a/openwisp_notifications/migrations/0013_remove_notification_verb.py +++ /dev/null @@ -1,17 +0,0 @@ -# Generated by Django 5.2.6 on 2025-10-01 19:28 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ("openwisp_notifications", "0012_alter_notification_verb_nullable"), - ] - - operations = [ - migrations.RemoveField( - model_name="notification", - name="verb", - ), - ] From 09f7333b0bdf15337786d0b6fa28bd1ff41b2422 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Thu, 2 Oct 2025 02:41:16 +0530 Subject: [PATCH 06/12] [fix] Removed verb field from DB #334 Removed verb field from DB, verb now updates from config from NOTIFICATION_TYPES. Fixes #334 --- openwisp_notifications/base/models.py | 6 +++--- ...on_verb_nullable.py => 0012_remove_notification_verb.py} | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) rename openwisp_notifications/migrations/{0012_alter_notification_verb_nullable.py => 0012_remove_notification_verb.py} (76%) diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 70184b6a..571cebbb 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -107,10 +107,10 @@ def verb(self): try: config = get_notification_configuration(self.type) except (NotificationRenderException, TypeError) as e: - logger.warning( - "Could not get notification config for type %s: %s", self.type, e + logger.error( + "Couldn't get notification config for type %s : %s", self.type, e ) - return self.__dict__.get("verb") or config.get("verb") + return config.get("verb") or self.__dict__.get("verb") @verb.setter def verb(self, value): diff --git a/openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py b/openwisp_notifications/migrations/0012_remove_notification_verb.py similarity index 76% rename from openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py rename to openwisp_notifications/migrations/0012_remove_notification_verb.py index f54bb504..db8dc781 100644 --- a/openwisp_notifications/migrations/0012_alter_notification_verb_nullable.py +++ b/openwisp_notifications/migrations/0012_remove_notification_verb.py @@ -10,9 +10,8 @@ class Migration(migrations.Migration): ] operations = [ - migrations.AlterField( + migrations.RemoveField( model_name="notification", name="verb", - field=models.CharField(max_length=255, null=True, blank=True), ), ] From 7faaa8b9f263e5dbc6e79a0bd28529ed00976580 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Thu, 2 Oct 2025 04:02:36 +0530 Subject: [PATCH 07/12] [fix] Removed verb field from Sample App #334 Same as above Fixes #334 --- .../migrations/0004_remove_notification_verb.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py diff --git a/tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py b/tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py new file mode 100644 index 00000000..2b1e32f0 --- /dev/null +++ b/tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.6 on 2025-10-01 22:24 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("sample_notifications", "0003_default_groups_permissions"), + ] + + operations = [ + migrations.RemoveField( + model_name="notification", + name="verb", + ), + ] From 722c6645698af795e61f9cba8ca5bd41d820d0c9 Mon Sep 17 00:00:00 2001 From: Pranshu Date: Tue, 14 Oct 2025 18:42:36 +0530 Subject: [PATCH 08/12] [fix] Removing unused models import in migration #334 Same as above, should pass QA checks. Fixes #334. --- .../migrations/0012_remove_notification_verb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_notifications/migrations/0012_remove_notification_verb.py b/openwisp_notifications/migrations/0012_remove_notification_verb.py index db8dc781..2e9a86cc 100644 --- a/openwisp_notifications/migrations/0012_remove_notification_verb.py +++ b/openwisp_notifications/migrations/0012_remove_notification_verb.py @@ -1,6 +1,6 @@ # Generated by Django 5.2.6 on 2025-10-01 09:39 -from django.db import migrations, models +from django.db import migrations class Migration(migrations.Migration): From 32f35b44662295973dc0a1019eda38138cd5b525 Mon Sep 17 00:00:00 2001 From: pranshustuff Date: Wed, 15 Oct 2025 04:57:37 +0530 Subject: [PATCH 09/12] [fix] Dynamic Updates of Notif Verb #334 Not deleting DB field verb, so it is backwards compatible. Changed instances of .verb to .resolved_verb. Resolved_verb property allows .verb as fallback. Fixes #334. --- openwisp_notifications/base/models.py | 10 +++------- ...erb.py => 0012_alter_notification_verb.py} | 9 ++++++--- .../openwisp_notifications/default_message.md | 4 ++-- .../tests/test_notifications.py | 16 +++++++-------- openwisp_notifications/types.py | 4 ++-- tests/openwisp2/sample_notifications/apps.py | 2 +- .../0004_alter_notification_verb.py | 20 +++++++++++++++++++ .../0004_remove_notification_verb.py | 17 ---------------- 8 files changed, 42 insertions(+), 40 deletions(-) rename openwisp_notifications/migrations/{0012_remove_notification_verb.py => 0012_alter_notification_verb.py} (51%) create mode 100644 tests/openwisp2/sample_notifications/migrations/0004_alter_notification_verb.py delete mode 100644 tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 571cebbb..9c7a4a5f 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -102,19 +102,15 @@ class AbstractNotification(UUIDModel, BaseNotification): _target = BaseNotification.target @property - def verb(self): + def resolved_verb(self): config = {} try: config = get_notification_configuration(self.type) - except (NotificationRenderException, TypeError) as e: + except NotificationRenderException as e: logger.error( "Couldn't get notification config for type %s : %s", self.type, e ) - return config.get("verb") or self.__dict__.get("verb") - - @verb.setter - def verb(self, value): - self.__dict__["verb"] = value + return config.get("verb") or self.verb class Meta(BaseNotification.Meta): abstract = True diff --git a/openwisp_notifications/migrations/0012_remove_notification_verb.py b/openwisp_notifications/migrations/0012_alter_notification_verb.py similarity index 51% rename from openwisp_notifications/migrations/0012_remove_notification_verb.py rename to openwisp_notifications/migrations/0012_alter_notification_verb.py index 2e9a86cc..3b049015 100644 --- a/openwisp_notifications/migrations/0012_remove_notification_verb.py +++ b/openwisp_notifications/migrations/0012_alter_notification_verb.py @@ -1,6 +1,6 @@ -# Generated by Django 5.2.6 on 2025-10-01 09:39 +# Generated by Django 5.2.6 on 2025-10-14 18:40 -from django.db import migrations +from django.db import migrations, models class Migration(migrations.Migration): @@ -10,8 +10,11 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RemoveField( + migrations.AlterField( model_name="notification", name="verb", + field=models.CharField( + blank=True, max_length=255, null=True, verbose_name="verb" + ), ), ] diff --git a/openwisp_notifications/templates/openwisp_notifications/default_message.md b/openwisp_notifications/templates/openwisp_notifications/default_message.md index e673186a..d38e35cc 100644 --- a/openwisp_notifications/templates/openwisp_notifications/default_message.md +++ b/openwisp_notifications/templates/openwisp_notifications/default_message.md @@ -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 %} diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index 7cfc2e80..e2af7840 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -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 ) @@ -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 = ( '

admin

' ).format( @@ -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"): @@ -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, "

testing initiated by admin since 0\xa0minutes

", @@ -1514,20 +1514,20 @@ def test_dynamic_verb_changed(self): 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.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.verb, "initial verb") + 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.verb, "updated verb") + self.assertEqual(notification.resolved_verb, "updated verb") self.assertIn("updated verb", notification.message) with self.subTest("Test fallback to database verb"): diff --git a/openwisp_notifications/types.py b/openwisp_notifications/types.py index 04ae7ace..a368b02b 100644 --- a/openwisp_notifications/types.py +++ b/openwisp_notifications/types.py @@ -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}" " by [{notification.target}]({notification.target_link})" ), "message_template": "openwisp_notifications/default_message.md", @@ -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}", diff --git a/tests/openwisp2/sample_notifications/apps.py b/tests/openwisp2/sample_notifications/apps.py index 6c53a08d..bb4df376 100644 --- a/tests/openwisp2/sample_notifications/apps.py +++ b/tests/openwisp2/sample_notifications/apps.py @@ -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", }, ) diff --git a/tests/openwisp2/sample_notifications/migrations/0004_alter_notification_verb.py b/tests/openwisp2/sample_notifications/migrations/0004_alter_notification_verb.py new file mode 100644 index 00000000..900e6ce5 --- /dev/null +++ b/tests/openwisp2/sample_notifications/migrations/0004_alter_notification_verb.py @@ -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" + ), + ), + ] diff --git a/tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py b/tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py deleted file mode 100644 index 2b1e32f0..00000000 --- a/tests/openwisp2/sample_notifications/migrations/0004_remove_notification_verb.py +++ /dev/null @@ -1,17 +0,0 @@ -# Generated by Django 5.2.6 on 2025-10-01 22:24 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ("sample_notifications", "0003_default_groups_permissions"), - ] - - operations = [ - migrations.RemoveField( - model_name="notification", - name="verb", - ), - ] From c17905757f372161568046c7a660add4220bf2ae Mon Sep 17 00:00:00 2001 From: pranshustuff Date: Sat, 7 Feb 2026 02:06:15 +0530 Subject: [PATCH 10/12] [fix] Dynamic verb updates via context manager #334 Removed resolved_verb property. Gave preference to db verb, if it exists. If it is None then config verb is given prefernce. Fixes #334. --- openwisp_notifications/base/models.py | 27 ++++++------ .../migrations/0013_merge_20260131_1927.py | 13 ++++++ .../openwisp_notifications/default_message.md | 4 +- .../tests/test_notifications.py | 42 ++++++++++++------- openwisp_notifications/types.py | 4 +- tests/openwisp2/sample_notifications/apps.py | 2 +- .../migrations/0005_merge_20260131_1929.py | 13 ++++++ 7 files changed, 72 insertions(+), 33 deletions(-) create mode 100644 openwisp_notifications/migrations/0013_merge_20260131_1927.py create mode 100644 tests/openwisp2/sample_notifications/migrations/0005_merge_20260131_1929.py diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 9c7a4a5f..a0380110 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -62,6 +62,18 @@ def notification_render_attributes(obj, **attrs): } defaults.update(attrs) + db_verb = obj.verb + + config = {} + if obj.type: + try: + config = get_notification_configuration(obj.type) + except NotificationRenderException as e: + logger.error( + "Couldn't get notification config for type %s : %s", obj.type, e + ) + obj.verb = db_verb if db_verb is not None else config.get("verb") + for target_attr, source_attr in defaults.items(): setattr(obj, target_attr, getattr(obj, source_attr)) @@ -75,9 +87,11 @@ def notification_render_attributes(obj, **attrs): setattr(obj, "target", obj._related_object("target")) yield obj + obj.verb = db_verb for attr in defaults.keys(): - delattr(obj, attr) + if hasattr(obj, attr): + delattr(obj, attr) class AbstractNotification(UUIDModel, BaseNotification): @@ -101,17 +115,6 @@ 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 diff --git a/openwisp_notifications/migrations/0013_merge_20260131_1927.py b/openwisp_notifications/migrations/0013_merge_20260131_1927.py new file mode 100644 index 00000000..dd592e27 --- /dev/null +++ b/openwisp_notifications/migrations/0013_merge_20260131_1927.py @@ -0,0 +1,13 @@ +# Generated by Django 5.2.6 on 2026-01-31 18:27 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_notifications", "0012_alter_notification_verb"), + ("openwisp_notifications", "0012_replace_jsonfield_with_django_builtin"), + ] + + operations = [] diff --git a/openwisp_notifications/templates/openwisp_notifications/default_message.md b/openwisp_notifications/templates/openwisp_notifications/default_message.md index d38e35cc..e673186a 100644 --- a/openwisp_notifications/templates/openwisp_notifications/default_message.md +++ b/openwisp_notifications/templates/openwisp_notifications/default_message.md @@ -1,7 +1,7 @@ -{% block head %} {{ notification.level }} : {{notification.target}} {{ notification.resolved_verb }} {% endblock head %} +{% block head %} {{ notification.level }} : {{notification.target}} {{ notification.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.resolved_verb }}. +{{ notification.verb }}. {% endblock body %} diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index e2af7840..16866391 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -22,6 +22,7 @@ from openwisp_notifications import settings as app_settings from openwisp_notifications import tasks, utils +from openwisp_notifications.base.models import notification_render_attributes from openwisp_notifications.exceptions import NotificationRenderException from openwisp_notifications.handlers import ( notify_handler, @@ -407,7 +408,7 @@ def test_default_notification_type(self): self._create_notification() n = notification_queryset.first() self.assertEqual(n.level, "info") - self.assertEqual(n.resolved_verb, "default verb") + self.assertEqual(n.verb, "default verb") self.assertIn( "Default notification with default verb and level info by", n.message ) @@ -470,7 +471,7 @@ def test_generic_notification_type(self): self._create_notification() n = notification_queryset.first() self.assertEqual(n.level, "info") - self.assertEqual(n.resolved_verb, "generic verb") + self.assertEqual(n.verb, "generic verb") expected_output = ( '

admin

' ).format( @@ -573,8 +574,8 @@ def test_register_unregister_notification_type(self): "verbose_name": "Test Notification Type", "level": "test", "verb": "testing", - "message": "{notification.resolved_verb} initiated by {notification.actor} since {notification}", - "email_subject": "[{site.name}] {notification.resolved_verb} reported by {notification.actor}", + "message": "{notification.verb} initiated by {notification.actor} since {notification}", + "email_subject": "[{site.name}] {notification.verb} reported by {notification.actor}", } with self.subTest("Registering new notification type"): @@ -583,7 +584,7 @@ def test_register_unregister_notification_type(self): self._create_notification() n = notification_queryset.first() self.assertEqual(n.level, "test") - self.assertEqual(n.resolved_verb, "testing") + self.assertEqual(n.verb, "testing") self.assertEqual( n.message, "

testing initiated by admin since 0\xa0minutes

", @@ -1514,26 +1515,35 @@ def test_dynamic_verb_changed(self): 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["message"] = "Notification with {notification.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") + with self.subTest("Test verb is saved as plain string"): + self.assertEqual(notification.verb, "initial verb") + self.assertIsInstance(notification.verb, str) + + with self.subTest("Test verb renders correctly in message"): self.assertIn("initial verb", notification.message) - with self.subTest("Test verb changes dynamically from config"): + with self.subTest("Test config verb is used as fallback when db verb is None"): default_config["verb"] = "updated verb" + notification.verb = None + notification.save() + notification.refresh_from_db() 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") + with notification_render_attributes(notification) as n: + self.assertEqual(n.verb, "updated verb") + self.assertIn("updated verb", n.message) + + with self.subTest("Test db verb takes precedence over config verb"): + notification.verb = "db verb" + notification.save() + notification.refresh_from_db() + with notification_render_attributes(notification) as n: + self.assertEqual(n.verb, "db verb") default_config["message"] = original_message default_config["verb"] = original_verb diff --git a/openwisp_notifications/types.py b/openwisp_notifications/types.py index a368b02b..04ae7ace 100644 --- a/openwisp_notifications/types.py +++ b/openwisp_notifications/types.py @@ -10,7 +10,7 @@ "verbose_name": "Default Type", "email_subject": "[{site.name}] Default Notification Subject", "message": ( - "Default notification with {notification.resolved_verb} and level {notification.level}" + "Default notification with {notification.verb} and level {notification.level}" " by [{notification.target}]({notification.target_link})" ), "message_template": "openwisp_notifications/default_message.md", @@ -23,7 +23,7 @@ "verbose_name": "Generic Type", "email_subject": "[{site.name}] Generic Notification Subject", "message": ( - "Generic notification with {notification.resolved_verb} and level {notification.level}" + "Generic notification with {notification.verb} and level {notification.level}" " by [{notification.actor}]({notification.actor_link})" ), "description": "{notification.description}", diff --git a/tests/openwisp2/sample_notifications/apps.py b/tests/openwisp2/sample_notifications/apps.py index bb4df376..6c53a08d 100644 --- a/tests/openwisp2/sample_notifications/apps.py +++ b/tests/openwisp2/sample_notifications/apps.py @@ -24,7 +24,7 @@ def register_notification_types(self): "verbose_name": "Object created", "verb": "created", "level": "info", - "message": "{notification.target} object {notification.resolved_verb}.", + "message": "{notification.target} object {notification.verb}.", "email_subject": "[{site.name}] INFO: {notification.target} created", }, ) diff --git a/tests/openwisp2/sample_notifications/migrations/0005_merge_20260131_1929.py b/tests/openwisp2/sample_notifications/migrations/0005_merge_20260131_1929.py new file mode 100644 index 00000000..12ab73cf --- /dev/null +++ b/tests/openwisp2/sample_notifications/migrations/0005_merge_20260131_1929.py @@ -0,0 +1,13 @@ +# Generated by Django 5.2.6 on 2026-01-31 18:29 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("sample_notifications", "0004_alter_notification_verb"), + ("sample_notifications", "0004_replace_jsonfield_with_django_builtin"), + ] + + operations = [] From 36c37752650b78f766f7c6f706eef2c7edcdf75f Mon Sep 17 00:00:00 2001 From: pranshustuff Date: Sat, 7 Feb 2026 02:52:20 +0530 Subject: [PATCH 11/12] [chores] Updated Tests #334 Fixes #334 --- .../tests/test_notifications.py | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index b78f44b4..15c61efe 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -432,7 +432,8 @@ def test_default_notification_type(self): self._create_notification() n = notification_queryset.first() self.assertEqual(n.level, "info") - self.assertEqual(n.verb, "default verb") + with notification_render_attributes(n) as rendered: + self.assertEqual(rendered.verb, "default verb") self.assertIn( "Default notification with default verb and level info by", n.message ) @@ -495,7 +496,8 @@ def test_generic_notification_type(self): self._create_notification() n = notification_queryset.first() self.assertEqual(n.level, "info") - self.assertEqual(n.verb, "generic verb") + with notification_render_attributes(n) as rendered: + self.assertEqual(rendered.verb, "generic verb") expected_output = ( '

admin

' ).format( @@ -608,7 +610,8 @@ def test_register_unregister_notification_type(self): self._create_notification() n = notification_queryset.first() self.assertEqual(n.level, "test") - self.assertEqual(n.verb, "testing") + with notification_render_attributes(n) as rendered: + self.assertEqual(rendered.verb, "testing") self.assertEqual( n.message, "

testing initiated by admin since 0\xa0minutes

", @@ -1545,29 +1548,31 @@ def test_dynamic_verb_changed(self): self._create_notification() notification = notification_queryset.first() - with self.subTest("Test verb is saved as plain string"): - self.assertEqual(notification.verb, "initial verb") - self.assertIsInstance(notification.verb, str) + with self.subTest("DB does not store default verb"): + self.assertIsNone(notification.verb) - with self.subTest("Test verb renders correctly in message"): - self.assertIn("initial verb", notification.message) + with self.subTest("Initial config verb is rendered"): + with notification_render_attributes(notification) as n: + self.assertEqual(n.verb, "initial verb") + self.assertIn("initial verb", n.message) + + default_config["verb"] = "updated verb" + del notification.message - with self.subTest("Test config verb is used as fallback when db verb is None"): - default_config["verb"] = "updated verb" - notification.verb = None - notification.save() - notification.refresh_from_db() - del notification.message + with self.subTest("Config change affects existing notification"): with notification_render_attributes(notification) as n: self.assertEqual(n.verb, "updated verb") self.assertIn("updated verb", n.message) - with self.subTest("Test db verb takes precedence over config verb"): - notification.verb = "db verb" - notification.save() - notification.refresh_from_db() + notification.verb = "db verb" + notification.save() + notification.refresh_from_db() + del notification.message + + with self.subTest("DB verb overrides config"): with notification_render_attributes(notification) as n: self.assertEqual(n.verb, "db verb") + self.assertIn("db verb", n.message) default_config["message"] = original_message default_config["verb"] = original_verb From 9f9ca07f6e78f0639189defe180c4274b7ca2079 Mon Sep 17 00:00:00 2001 From: pranshustuff Date: Sat, 7 Feb 2026 03:04:17 +0530 Subject: [PATCH 12/12] [fix] Generating email notif with context manager #334 Fixes #334. --- openwisp_notifications/base/models.py | 39 +++++++++++++++------------ 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index a0380110..9df10a1f 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -297,23 +297,28 @@ def get_message(self): @cached_property def email_subject(self): - if self.type: - try: - config = get_notification_configuration(self.type) - data = self.data or {} - return config["email_subject"].format( - site=Site.objects.get_current(), notification=self, **data - ) - except (AttributeError, KeyError, NotificationRenderException) as exception: - self._invalid_notification( - self.pk, - exception, - "Error encountered in generating notification email", - ) - elif self.data.get("email_subject", None): - return self.data.get("email_subject") - else: - return self.message + with notification_render_attributes(self): + if self.type: + try: + config = get_notification_configuration(self.type) + data = self.data or {} + return config["email_subject"].format( + site=Site.objects.get_current(), notification=self, **data + ) + except ( + AttributeError, + KeyError, + NotificationRenderException, + ) as exception: + self._invalid_notification( + self.pk, + exception, + "Error encountered in generating notification email", + ) + elif self.data.get("email_subject", None): + return self.data.get("email_subject") + else: + return self.message def _related_object(self, field): obj_id = getattr(self, f"{field}_object_id")