diff --git a/HISTORY.rst b/HISTORY.rst index 1fb4b72..5682458 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -6,6 +6,7 @@ not released yet ---------------- * Fix documentation +* Allow rendering to work without saving the notification into DB 0.2.2 (2020-02-11) ------------------ diff --git a/example/tests/test_handlers.py b/example/tests/test_handlers.py index 1c8a010..ed39caa 100644 --- a/example/tests/test_handlers.py +++ b/example/tests/test_handlers.py @@ -9,7 +9,7 @@ from pynotify.dispatchers import BaseDispatcher from pynotify.handlers import BaseHandler from pynotify.helpers import signal_map -from pynotify.models import AdminNotificationTemplate +from pynotify.models import AdminNotificationTemplate, Notification # MOCK OBJECTS ------------------------------------------------------------------------------------ @@ -54,14 +54,20 @@ def get_related_objects(self): def get_extra_data(self): return {'some_value': 123} + def _can_handle(self): + return super()._can_handle() and self.signal_kwargs.get('can_handle', True) + def _can_create_notification(self, recipient): - return recipient.username != 'James' + return super()._can_create_notification(recipient) and self.signal_kwargs.get('can_create', True) - def _can_dispatch_notification(self, notification, dispatcher): - return notification.recipient.username != 'John' + def _can_save_notification(self, notification): + return super()._can_save_notification(notification) and self.signal_kwargs.get('can_save', True) - def _can_handle(self): - return super()._can_handle() and self.signal_kwargs.get('can_handle', True) + def _can_dispatch_notification(self, notification, dispatcher): + return ( + super()._can_dispatch_notification(notification, dispatcher) + and self.signal_kwargs.get('can_dispatch', True) + ) class Meta: signal = test_signal_data @@ -87,7 +93,6 @@ class HandlerTestCase(TestCase): def setUp(self): self.user1 = User.objects.create_user('Jack') self.user2 = User.objects.create_user('John') - self.user3 = User.objects.create_user('James') self.template = AdminNotificationTemplate.objects.create(title='Hello slug!', slug='test_slug') def test_handler_should_be_automatically_registered(self): @@ -123,7 +128,10 @@ class Meta: def test_handler_should_create_notification_using_template_data(self): users = [self.user1, self.user2] - test_signal_data.send(sender=None, recipients=[self.user1, self.user2, self.user3]) + test_signal_data.send(sender=None, recipients=[self.user1, self.user2]) + + self.assertEqual(Notification.objects.all().count(), 2) + self.assertEqual(len(MockDispatcher.dispatched_notifications), 2) for user in users: notification = user.notifications.get() @@ -134,23 +142,36 @@ def test_handler_should_create_notification_using_template_data(self): self.assertEqual(notification.get_extra_data(), {'some_value': 123}) self.assertEqual(related_object.name, 'first_recipient') self.assertEqual(related_object.content_object, self.user1) - if user.username == 'Jack': - self.assertIn(notification, MockDispatcher.dispatched_notifications) - - self.assertEqual(len(MockDispatcher.dispatched_notifications), 1) + self.assertIn(notification, MockDispatcher.dispatched_notifications) # Repeated notification should use the same template test_signal_data.send(sender=None, recipients=[self.user1]) notifications = self.user1.notifications.all() self.assertEqual(notifications[0].template, notifications[1].template) - # Test _can_handle() method is used - self.user1.notifications.all().delete() - test_signal_data.send(sender=None, recipients=[self.user1], can_handle=False) - self.assertEqual(self.user1.notifications.count(), 0) - def test_handler_should_create_notification_using_template_slug(self): test_signal_slug.send(sender=MockSender, recipients=[self.user1]) notification = self.user1.notifications.get() self.assertEqual(notification.template.admin_template, self.template) self.assertEqual(notification.title, 'Hello slug!') + + def test_can_handle_method_should_be_used(self): + test_signal_data.send(sender=None, recipients=[self.user1], can_handle=False) + self.assertEqual(self.user1.notifications.count(), 0) + self.assertEqual(len(MockDispatcher.dispatched_notifications), 0) + + def test_can_create_notification_method_should_be_used(self): + test_signal_data.send(sender=None, recipients=[self.user1], can_create=False) + self.assertEqual(self.user1.notifications.count(), 0) + self.assertEqual(len(MockDispatcher.dispatched_notifications), 0) + + def test_can_save_notification_method_should_be_used(self): + test_signal_data.send(sender=None, recipients=[self.user1], can_save=False) + self.assertEqual(self.user1.notifications.count(), 0) + self.assertEqual(len(MockDispatcher.dispatched_notifications), 1) + self.assertIsNone(MockDispatcher.dispatched_notifications[0].pk) + + def test_can_dispatch_notification_method_should_be_used(self): + test_signal_data.send(sender=None, recipients=[self.user1], can_dispatch=False) + self.assertEqual(self.user1.notifications.count(), 1) + self.assertEqual(len(MockDispatcher.dispatched_notifications), 0) diff --git a/example/tests/test_models.py b/example/tests/test_models.py index b0952c9..31bbe68 100644 --- a/example/tests/test_models.py +++ b/example/tests/test_models.py @@ -103,6 +103,7 @@ def setUp(self): self.author = User.objects.create_user('John') self.article = Article.objects.create(title='The Old Witch', author=self.author) self.random_user = User.objects.create_user('Mr.Random') + self.random_user2 = User.objects.create_user('Mr.Random 2') self.template = NotificationTemplate.objects.create( title='{{article}}', @@ -110,19 +111,20 @@ def setUp(self): trigger_action='{{article.get_absolute_url}}', ) - self.notification = Notification.objects.create( + self.notification = Notification( recipient=self.recipient, template=self.template, - related_objects={ - 'article': self.article, - 'author': self.article.author, - 'random_user': self.random_user, - }, - extra_data={ - 'some_value': 123, - 'decimal_value': Decimal('1.55'), - } ) + self.notification.set_local_related_objects({ + 'article': self.article, + 'author': self.article.author, + 'random_user': self.random_user, + }) + self.notification.set_extra_data({ + 'some_value': 123, + 'decimal_value': Decimal('1.55'), + }) + self.notification.save() def test_generated_fields_should_use_template_for_rendering(self): self.assertEqual(self.notification.title, 'The Old Witch') @@ -148,11 +150,27 @@ def test_extra_data_should_be_dictionary(self): self.notification.set_extra_data(1000) def test_related_objects_and_extra_data_should_not_contain_same_keys(self): + self.notification.set_local_related_objects({'random_user': self.random_user}) + with self.assertRaises(ValueError): + self.notification.context + with self.assertRaises(ValueError): + self.notification.save() + + self.notification.set_local_related_objects({'some_value': self.random_user}) + with self.assertRaises(ValueError): + self.notification.context + with self.assertRaises(ValueError): + self.notification.save() + + self.notification.set_local_related_objects({}) self.notification.set_extra_data({'article': 123}) + with self.assertRaises(ValueError): + self.notification.context with self.assertRaises(ValueError): self.notification.save() def test_context_should_contain_related_objects_as_proxies_and_extra_data(self): + self.notification.set_local_related_objects({'local_user': self.random_user2}) ctx = self.notification.context self.assertTrue(isinstance(ctx['article'], SecureRelatedObject)) @@ -164,6 +182,9 @@ def test_context_should_contain_related_objects_as_proxies_and_extra_data(self): self.assertTrue(isinstance(ctx['random_user'], SecureRelatedObject)) self.assertEqual(ctx['random_user']._object, self.random_user) + self.assertTrue(isinstance(ctx['local_user'], SecureRelatedObject)) + self.assertEqual(ctx['local_user']._object, self.random_user2) + self.assertEqual(ctx['some_value'], 123) self.assertEqual(ctx['decimal_value'], '1.55') @@ -186,20 +207,17 @@ def test_creating_notification_should_not_be_possible_with_related_objects_in_in ['abc', 'abc'], {'abc': 'abc'}, ) + notification = Notification.objects.create(recipient=self.recipient, template=self.template) for related_objects in INVALID_RELATED_OBJECTS: with self.assertRaises(TypeError): - Notification.objects.create( - recipient=self.recipient, - template=self.template, - related_objects=related_objects - ) + notification.set_local_related_objects(related_objects) + notification.save() + notification.set_local_related_objects({}) def test_creating_notification_should_allow_list_of_related_objects(self): - notification = Notification.objects.create( - recipient=self.recipient, - template=self.template, - related_objects=[self.random_user], - ) + notification = Notification(recipient=self.recipient, template=self.template) + notification.set_local_related_objects([self.random_user]) + notification.save() self.assertEqual(notification.related_objects.count(), 1) related_object = notification.related_objects.get() @@ -207,5 +225,37 @@ def test_creating_notification_should_allow_list_of_related_objects(self): self.assertEqual(related_object.content_object, self.random_user) self.assertEqual(notification.context, {}) + def test_saving_notification_should_save_list_of_local_related_objects_into_db(self): + notification = Notification(recipient=self.recipient, template=self.template) + + notification.set_local_related_objects([self.random_user2]) + self.assertEqual(len(notification._local_related_objects_list), 1) + self.assertEqual(notification._local_related_objects_list[0], self.random_user2) + self.assertEqual(notification.related_objects.count(), 0) + + notification.save() + self.assertEqual(len(notification._local_related_objects_list), 0) + self.assertEqual(notification.related_objects.count(), 1) + + related_object = notification.related_objects.get() + self.assertEqual(related_object.name, None) + self.assertEqual(related_object.content_object, self.random_user2) + + def test_saving_notification_should_save_dictionary_of_local_related_objects_into_db(self): + notification = Notification(recipient=self.recipient, template=self.template) + + notification.set_local_related_objects({'random_user2': self.random_user2}) + self.assertEqual(len(notification._local_related_objects_dict), 1) + self.assertEqual(notification._local_related_objects_dict['random_user2'], self.random_user2) + self.assertEqual(notification.related_objects.count(), 0) + + notification.save() + self.assertEqual(len(notification._local_related_objects_dict), 0) + self.assertEqual(notification.related_objects.count(), 1) + + related_object = notification.related_objects.get() + self.assertEqual(related_object.name, 'random_user2') + self.assertEqual(related_object.content_object, self.random_user2) + def test_notification_should_have_string_representation(self): self.assertEqual(str(self.notification), 'notification #{}'.format(self.notification.pk)) diff --git a/pynotify/handlers.py b/pynotify/handlers.py index b62776a..39e000b 100644 --- a/pynotify/handlers.py +++ b/pynotify/handlers.py @@ -61,6 +61,9 @@ class BaseHandler(metaclass=HandlerMeta): @cached_property def _template(self): + """ + Returns notification template that will be used for creation of notification(s). + """ template_slug = self.get_template_slug() if template_slug: admin_template = AdminNotificationTemplate.objects.get(slug=template_slug) @@ -75,50 +78,62 @@ def _template(self): return template + def _create_notification(self, recipient): + """ + Creates notification for ``recipient``. + """ + notification = Notification(recipient=recipient, template=self._template) + + extra_data = self.get_extra_data() + if extra_data: + notification.set_extra_data(extra_data) + + related_objects = self.get_related_objects() + if related_objects: + notification.set_local_related_objects(related_objects) + + if self._can_save_notification(notification): + notification.save() + + return notification + def _init_dispatchers(self): - self.dispatchers = [] + """ + Initializes dipatchers that will be used for sending of notification(s). + """ + self._dispatchers = [] dispatcher_classes = self.get_dispatcher_classes() if dispatcher_classes: for dispatcher_class in dispatcher_classes: - self.dispatchers.append(self._init_dispatcher(dispatcher_class)) + self._dispatchers.append(self._init_dispatcher(dispatcher_class)) def _init_dispatcher(self, dispatcher_class): - return dispatcher_class() - - def _can_create_notification(self, recipient): """ - Returns ``True`` if notification can be created for ``recipient``. + Initializes a single dispatcher. Override this method if you need specific initialization procedure. """ - return True + return dispatcher_class() - def _create_notification(self, recipient): + def _can_handle(self): """ - Creates notification for ``recipient``. + Returns ``True`` if handler can handle creating of notification(s). """ - if self._can_create_notification(recipient): - return Notification.objects.create( - recipient=recipient, - template=self._template, - related_objects=self.get_related_objects(), - extra_data=self.get_extra_data(), - ) + return True - def _can_dispatch_notification(self, notification, dispatcher): + def _can_create_notification(self, recipient): """ - Returns ``True`` if ``notification`` can be dispatched using ``dispatcher``. + Returns ``True`` if notification can be created for ``recipient``. """ return True - def _dispatch_notification(self, notification, dispatcher): + def _can_save_notification(self, notification): """ - Dispatches ``notification`` using ``dispatcher``. + Returns ``True`` if ``notification`` can be saved into DB. """ - if self._can_dispatch_notification(notification, dispatcher): - dispatcher.dispatch(notification) + return True - def _can_handle(self): + def _can_dispatch_notification(self, notification, dispatcher): """ - Returns ``True`` if handler should handle creating of notification(s). + Returns ``True`` if ``notification`` can be dispatched using ``dispatcher``. """ return True @@ -127,14 +142,19 @@ def handle(self, signal_kwargs): Handles creation of notifications from ``signal_kwargs``. """ self.signal_kwargs = signal_kwargs - if self._can_handle(): - self._init_dispatchers() - for recipient in self.get_recipients(): - notification = self._create_notification(recipient) - - if notification: - for dispatcher in self.dispatchers: - self._dispatch_notification(notification, dispatcher) + + if not self._can_handle(): + return + + self._init_dispatchers() + for recipient in self.get_recipients(): + if not self._can_create_notification(recipient): + continue + + notification = self._create_notification(recipient) + for dispatcher in self._dispatchers: + if self._can_dispatch_notification(notification, dispatcher): + dispatcher.dispatch(notification) def get_recipients(self): """ diff --git a/pynotify/models.py b/pynotify/models.py index fc080b0..acd0143 100644 --- a/pynotify/models.py +++ b/pynotify/models.py @@ -114,33 +114,6 @@ def render(self, field, context): return Template('{}{}'.format(settings.TEMPLATE_PREFIX, template_string)).render(Context(context)) -class NotificationManager(models.Manager): - - def _create_related_object(self, notification, obj, name=None): - if not isinstance(obj, models.Model): - raise TypeError('Related object must be an instance of model.') - NotificationRelatedObject.objects.create(name=name, notification=notification, content_object=obj) - - def create(self, recipient, template, related_objects=None, extra_data=None, **kwargs): - notification = super().create(recipient=recipient, template=template, **kwargs) - - if related_objects is not None: - if isinstance(related_objects, dict): - for name, obj in related_objects.items(): - self._create_related_object(notification, obj, name) - elif isinstance(related_objects, list): - for obj in related_objects: - self._create_related_object(notification, obj) - else: - raise TypeError('Related objects must be a list or dictionary in form {"name": object}.') - - if extra_data is not None: - notification.set_extra_data(extra_data) - notification.save() - - return notification - - class NotificationMeta(SmartModelBase, type): """ Creates property for each template field. The property returns rendered template. @@ -189,38 +162,78 @@ class Notification(BaseModel, metaclass=NotificationMeta): is_triggered = models.BooleanField(default=False, verbose_name=_l('is triggered')) extra_data = models.TextField(null=True, blank=True, verbose_name=_l('extra data')) - objects = NotificationManager() - class Meta: verbose_name = _l('notification') verbose_name_plural = _l('notifications') ordering = ('-created_at',) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._local_related_objects_dict = {} + self._local_related_objects_list = [] + + def _post_save(self, *args, **kwargs): + """ + Saves local related objects into DB. + """ + for name, obj in self._local_related_objects_dict.items(): + NotificationRelatedObject.objects.create(name=name, notification=self, content_object=obj) + self._local_related_objects_dict = {} + + for obj in self._local_related_objects_list: + NotificationRelatedObject.objects.create(notification=self, content_object=obj) + self._local_related_objects_list = [] + + def _check_related_objects_and_extra_data(self): + """ + Checks type of local related objects and uniqueness of all related object's names compared to extra data. + """ + local_related_objects = self._local_related_objects_list + [ + value for key, value in self._local_related_objects_dict.items() + ] + for obj in local_related_objects: + if not isinstance(obj, models.Model): + raise TypeError('Related object must be an instance of model.') + + db_related_object_names = [obj.name for obj in self.related_objects.filter(name__isnull=False)] + common_keys = ( + set(self.get_extra_data()) & set(self._local_related_objects_dict) + or set(self.get_extra_data()) & set(db_related_object_names) + or set(self._local_related_objects_dict) & set(db_related_object_names) + ) + if common_keys: + raise ValueError('Conflicting keys found for related objects and/or extra data: {}'.format( + ', '.join(common_keys) + )) + def _render(self, field): return self.template.render(field, self.context) - def _pre_save(self, *args, **kwargs): - keys = set(self.get_extra_data()) & set(obj.name for obj in self.related_objects.all() if obj.name) - if keys: - raise ValueError('Related objects and extra data contain same key(s): {}'.format(', '.join(keys))) + def clean(self): + self._check_related_objects_and_extra_data() - @cached_property - def related_objects_dict(self): + @property + def _get_secure_related_objects(self): """ - Returns named related objects as a dictionary where key is name of the related object and value is the object - itself. Related objects without name are skipped. + Returns dictionary of related objects for use in template context: + * key is name of the related object and value is the object itself, wrapped in a security class. + * local related objects and related objects from DB are combined together + * related objects without name are skipped. """ output = {} for obj in self.related_objects.filter(name__isnull=False): output[obj.name] = SecureRelatedObject(obj.content_object) if obj.content_object else DeletedRelatedObject() + for name, obj in self._local_related_objects_dict.items(): + output[name] = SecureRelatedObject(obj) return output - @property + @cached_property def context(self): """ Returns context dictionary used for rendering the template. """ - return {**self.related_objects_dict, **self.get_extra_data()} + self._check_related_objects_and_extra_data() + return {**self._get_secure_related_objects, **self.get_extra_data()} def set_extra_data(self, extra_data): """ @@ -242,6 +255,18 @@ def get_extra_data(self): """ return json.loads(self.extra_data) if self.extra_data is not None else {} + def set_local_related_objects(self, related_objects): + """ + Sets related objects locally. That means they can be used for rendering, but are not saved automatically. + You must call `save()` in order to save them. + """ + if isinstance(related_objects, dict): + self._local_related_objects_dict = related_objects + elif isinstance(related_objects, list): + self._local_related_objects_list = related_objects + else: + raise TypeError('Related objects must be a list or dictionary in form {"name": object}.') + class NotificationRelatedObject(BaseModel): """ diff --git a/requirements_dev.txt b/requirements_dev.txt index 296d46b..d5c6863 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -1,8 +1,8 @@ bumpversion==0.5.3 -coverage==4.5.1 +coverage==4.5.4 flake8==3.6.0 pip==18.1 -python-coveralls==2.9.1 +python-coveralls==2.9.3 sphinx==1.8.1 sphinx_rtd_theme==0.4.3 tox==3.5.2