From f575dacc0fecf9e2924e374b01df49d5bec988b5 Mon Sep 17 00:00:00 2001 From: AviGawande <2041024@gcoej.ac.in> Date: Sat, 15 Feb 2025 12:32:44 +0530 Subject: [PATCH 1/2] [fix] Store notification verb in configuration --- openwisp_notifications/base/models.py | 21 ++++++++-- openwisp_notifications/handlers.py | 10 ++++- .../0008_alter_notification_verb.py | 17 ++++++++ openwisp_notifications/models.py | 12 ++++++ .../openwisp_notifications/default_message.md | 4 +- .../tests/test_notifications.py | 42 +++++++++++++++++-- 6 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 openwisp_notifications/migrations/0008_alter_notification_verb.py diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 5925920d..1550f394 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -65,6 +65,7 @@ class AbstractNotification(UUIDModel, BaseNotification): _actor = BaseNotification.actor _action_object = BaseNotification.action_object _target = BaseNotification.target + verb = models.CharField(max_length=128, null=True) class Meta(BaseNotification.Meta): abstract = True @@ -156,13 +157,16 @@ def get_message(self): try: config = get_notification_configuration(self.type) data = self.data or {} + # Create a context with notification_verb + context = dict(notification=self, **data) if 'message' in data: - md_text = data['message'].format(notification=self, **data) + md_text = data['message'].format(**context) elif 'message' in config: - md_text = config['message'].format(notification=self, **data) + md_text = config['message'].format(**context) else: md_text = render_to_string( - config['message_template'], context=dict(notification=self, **data) + config['message_template'], + context=context ).strip() except (AttributeError, KeyError, NotificationRenderException) as exception: self._invalid_notification( @@ -235,6 +239,17 @@ def redirect_view_url(self): reverse('notifications:notification_read_redirect', args=(self.pk,)) ) + @property + def notification_verb(self): + """ + Returns notification verb from type configuration if verb is None, + otherwise returns the stored verb + """ + if self.verb is None and self.type: + config = get_notification_configuration(self.type) + return config.get('verb', '') + return self.verb or '' + class AbstractNotificationSetting(UUIDModel): _RECEIVE_HELP = ( diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index c364513d..2ed57c64 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -57,10 +57,16 @@ def notify_handler(**kwargs): except NotificationRenderException as error: logger.error(f'Error encountered while creating notification: {error}') return + + # Only store verb in database if it's explicitly provided in kwargs + verb = kwargs.pop('verb', None) + if verb is None and notification_type: + # Don't store the type's default verb in database + verb = None + 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) @@ -139,7 +145,7 @@ def notify_handler(**kwargs): notification = Notification( recipient=recipient, actor=actor, - verb=str(verb), + verb=verb, public=public, description=description, timestamp=timestamp, diff --git a/openwisp_notifications/migrations/0008_alter_notification_verb.py b/openwisp_notifications/migrations/0008_alter_notification_verb.py new file mode 100644 index 00000000..3058a88e --- /dev/null +++ b/openwisp_notifications/migrations/0008_alter_notification_verb.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.19 on 2025-02-15 06:11 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("openwisp_notifications", "0007_notificationsetting_deleted"), + ] + + operations = [ + migrations.AlterField( + model_name="notification", + name="verb", + field=models.CharField(max_length=128, null=True), + ), + ] diff --git a/openwisp_notifications/models.py b/openwisp_notifications/models.py index ae7bd9fe..c2601b6e 100644 --- a/openwisp_notifications/models.py +++ b/openwisp_notifications/models.py @@ -5,6 +5,7 @@ AbstractNotification, AbstractNotificationSetting, ) +from .types import get_notification_configuration class Notification(AbstractNotification): @@ -13,6 +14,17 @@ class Meta(AbstractNotification.Meta): app_label = 'openwisp_notifications' swappable = swappable_setting('openwisp_notifications', 'Notification') + @property + def notification_verb(self): + """ + Returns notification verb from type configuration if verb is None, + otherwise returns the stored verb + """ + if self.verb is None and self.type: + config = get_notification_configuration(self.type) + return config.get('verb', '') + return self.verb + class NotificationSetting(AbstractNotificationSetting): class Meta(AbstractNotificationSetting.Meta): diff --git a/openwisp_notifications/templates/openwisp_notifications/default_message.md b/openwisp_notifications/templates/openwisp_notifications/default_message.md index e673186a..14b1b071 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.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.verb }}. +{{ notification.notification_verb }}. {% endblock body %} diff --git a/openwisp_notifications/tests/test_notifications.py b/openwisp_notifications/tests/test_notifications.py index 0b510d7d..07c60b10 100644 --- a/openwisp_notifications/tests/test_notifications.py +++ b/openwisp_notifications/tests/test_notifications.py @@ -11,7 +11,7 @@ from django.core.exceptions import ImproperlyConfigured from django.db.models.signals import post_migrate, post_save from django.template import TemplateDoesNotExist -from django.test import TransactionTestCase +from django.test import TransactionTestCase, TestCase from django.urls import reverse from django.utils import timezone from django.utils.timesince import timesince @@ -33,6 +33,7 @@ from openwisp_notifications.types import ( _unregister_notification_choice, get_notification_configuration, + NOTIFICATION_TYPES, ) from openwisp_notifications.utils import _get_absolute_url from openwisp_users.tests.utils import TestOrganizationMixin @@ -319,7 +320,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') + self.assertIsNone(n.verb) + self.assertEqual(n.notification_verb, 'default verb') self.assertIn( 'Default notification with default verb and level info by', n.message ) @@ -356,7 +358,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') + self.assertIsNone(n.verb) + self.assertEqual(n.notification_verb, 'generic verb') expected_output = ( '

admin

' ).format( @@ -999,3 +1002,36 @@ def test_notification_cache_update(self): self.assertEqual(notification.target.username, 'new operator name') # Done for populating cache self.assertEqual(operator_cache.username, 'new operator name') + + +class TestNotificationVerb(TestCase): + def setUp(self): + self.admin = User.objects.create_superuser( + username='admin', + password='admin', + email='admin@admin.com' + ) + + def test_notification_verb_update(self): + # Create notification with default type + notify.send( + sender=self.admin, + type='default', + recipient=self.admin, + ) + + notification = self.admin.notifications.first() + # Verify verb is None in database + self.assertIsNone(notification.verb) + # Verify notification shows verb from configuration + self.assertEqual(notification.notification_verb, 'default verb') + + # Change verb in notification type + old_config = NOTIFICATION_TYPES['default'].copy() + NOTIFICATION_TYPES['default']['verb'] = 'updated verb' + + # Verify notification shows updated verb + self.assertEqual(notification.notification_verb, 'updated verb') + + # Restore original configuration + NOTIFICATION_TYPES['default'] = old_config From 8889a32f341953a40da990d6e27af90542fed0d2 Mon Sep 17 00:00:00 2001 From: AviGawande <2041024@gcoej.ac.in> Date: Sat, 1 Mar 2025 16:21:05 +0530 Subject: [PATCH 2/2] revert models verb --- openwisp_notifications/base/models.py | 1 - openwisp_notifications/handlers.py | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/openwisp_notifications/base/models.py b/openwisp_notifications/base/models.py index 1550f394..88e1c052 100644 --- a/openwisp_notifications/base/models.py +++ b/openwisp_notifications/base/models.py @@ -65,7 +65,6 @@ class AbstractNotification(UUIDModel, BaseNotification): _actor = BaseNotification.actor _action_object = BaseNotification.action_object _target = BaseNotification.target - verb = models.CharField(max_length=128, null=True) class Meta(BaseNotification.Meta): abstract = True diff --git a/openwisp_notifications/handlers.py b/openwisp_notifications/handlers.py index 2ed57c64..dc142010 100644 --- a/openwisp_notifications/handlers.py +++ b/openwisp_notifications/handlers.py @@ -57,11 +57,9 @@ def notify_handler(**kwargs): except NotificationRenderException as error: logger.error(f'Error encountered while creating notification: {error}') return - - # Only store verb in database if it's explicitly provided in kwargs + verb = kwargs.pop('verb', None) if verb is None and notification_type: - # Don't store the type's default verb in database verb = None level = kwargs.pop(