diff --git a/django_nyt/conf.py b/django_nyt/conf.py index 797a5d5c..db0c4561 100644 --- a/django_nyt/conf.py +++ b/django_nyt/conf.py @@ -35,10 +35,6 @@ class AppSettings: NYT_DB_TABLE_PREFIX: str = "nyt" """The table prefix for tables in the database. Do not change this unless you know what you are doing.""" - NYT_ENABLED: bool = True - """Global setting to force-fully disable all propagation and creation of - notifications.""" - NYT_ENABLE_ADMIN: bool = False """Enable django-admin registration for django-nyt's ModelAdmin classes.""" diff --git a/django_nyt/decorators.py b/django_nyt/decorators.py index 889f877b..fbc29b09 100644 --- a/django_nyt/decorators.py +++ b/django_nyt/decorators.py @@ -8,17 +8,21 @@ def disable_notify(f): - """Disable notifications. Example: + """Disable notifications. - @disable_notify - def your_function(): - notify("no one will be notified", ...) + Does not work for async stuff, only disables notify in the same process. + + Example:: + + @disable_notify + def your_function(): + notify("no one will be notified", ...) """ @wraps(f) - def wrapper(request, *args, **kwargs): + def wrapper(*args, **kwargs): django_nyt._disable_notifications = True - response = f(request, *args, **kwargs) + response = f(*args, **kwargs) django_nyt._disable_notifications = False return response diff --git a/django_nyt/models.py b/django_nyt/models.py index e1979f79..1807ea98 100644 --- a/django_nyt/models.py +++ b/django_nyt/models.py @@ -335,7 +335,15 @@ def save(self, *args, **kwargs): super(Notification, self).save(*args, **kwargs) @classmethod - def create_notifications(cls, key, **kwargs): + def create_notifications( + cls, + key, + object_id=None, + content_type=None, + filter_exclude=None, + recipient_users=None, + **kwargs + ): """ Creates notifications directly in database -- do not call directly, use django_nyt.notify(...) @@ -346,14 +354,15 @@ def create_notifications(cls, key, **kwargs): if not key or not isinstance(key, str): raise KeyError("No notification key (string) specified.") - object_id = kwargs.pop("object_id", None) - filter_exclude = kwargs.pop("filter_exclude", None) or {} - recipient_users = kwargs.pop("recipient_users", None) + if filter_exclude is None: + filter_exclude = {} objects_created = [] subscriptions = Subscription.objects.filter(notification_type__key=key).exclude( **filter_exclude ) + + # TODO: This query should include notification_type__content_type? Why was this not added? if object_id: subscriptions = subscriptions.filter( Q(object_id=object_id) | Q(object_id=None) diff --git a/django_nyt/utils.py b/django_nyt/utils.py index aaf35521..f35b34ea 100644 --- a/django_nyt/utils.py +++ b/django_nyt/utils.py @@ -1,11 +1,12 @@ from typing import Any +from typing import List from typing import Union from django.contrib.contenttypes.models import ContentType from django.db.models import Model from django.utils.translation import gettext as _ -from . import _disable_notifications +import django_nyt from . import models from .conf import app_settings @@ -17,7 +18,7 @@ def notify( url: str = None, filter_exclude: dict = None, recipient_users: list = None, -) -> int: +) -> List[models.Notification]: """ Notify subscribing users of a new event. Key can be any kind of string, just make sure to reuse it where applicable. @@ -51,14 +52,15 @@ def notify( instead of notifying all subscribers of the event. Notice that users still have to be actually subscribed to the event key! - :param target_object: Any django model instance that this notification - relates to. Use django content types. + :param target_object: Any Django model instance that this notification + relates to. Uses Django content types. + Subscriptions with a matching content_type and object_id will be notified. :param filter_exclude: Keyword arguments passed to filter out Subscriptions. Will be handed to ``Subscription.objects.exclude(**filter_exclude)``. """ - if _disable_notifications: - return 0 + if django_nyt._disable_notifications: + return [] if target_object: if not isinstance(target_object, Model): @@ -71,7 +73,7 @@ def notify( else: object_id = None - objects = models.Notification.create_notifications( + notifications = models.Notification.create_notifications( key, object_id=object_id, message=message, @@ -84,9 +86,9 @@ def notify( if app_settings.NYT_ENABLE_CHANNELS: from django_nyt import subscribers - subscribers.notify_subscribers(objects, key) + subscribers.notify_subscribers(notifications, key) - return len(objects) + return notifications def subscribe( diff --git a/docs/howto/index.rst b/docs/howto/index.rst index b603fc70..cb608474 100644 --- a/docs/howto/index.rst +++ b/docs/howto/index.rst @@ -29,6 +29,18 @@ users can also be subscribed to specific object (i.e. "*your* comment was update notify(_("OMG! Something happened"), EVENT_KEY, target_object=my_model_instance) +In order to subscribe a user to receive notifications only for a specific object, +you can create the subscription like this: + +.. code-block:: python + + subscribe( + user_setting, + EVENT_KEY, + content_type=ContentType.objects.get_for_model(MyModel), + object_id=my_model_instance.id, + ) + Excluding certain recipients ---------------------------- diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 5dc7e317..8943e925 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -13,16 +13,19 @@ Release Notes so it's now encouraged to use ``/`` separators in notification keys, for instance ``comments/new`` is matched by ``comments/**``. #125 (Benjamin Balder Bach) * Django 5 support #128 (Benjamin Balder Bach) -* New arguments ``--domain`` and ``--http-only`` for management command ``notifymail`` #130 (Benjamin Balder Bach) +* New arguments ``--domain`` and ``--http-only`` for management command ``notifymail``. #130 (Benjamin Balder Bach) +* Documentation reorganized with Diataxis structure and more how-to guides have been added. (Benjamin Balder Bach) **Changed** * Tests migrated to pytest #123 (Benjamin Balder Bach) * Default email notification paths are changed to ``notifications/emails/default.txt`` and ``notifications/emails/default_subject.txt`` #125 (Benjamin Balder Bach) * Notification URLs added to emails have a hard-coded `https://` (before, this was `http://`) #125 (Benjamin Balder Bach) -* New test-friendly settings pattern changes internal names of settings, but has no effects on Django settings #127 (Benjamin Balder Bach). +* New test-friendly settings pattern changes internal names of settings, but has no effects on Django settings #127 (Benjamin Balder Bach) * Corrected name of method to ``Settings.get_default_settings`` #129 (Benjamin Balder Bach) -* Improvements to docstrings of main methods ``notify()`` and ``subscribe()``. #129 (Benjamin Balder Bach) +* Improvements to docstrings of main methods ``notify()`` and ``subscribe()`` #129 (Benjamin Balder Bach) +* Return value of ``notify()`` was changed - it no longer returns an `int` (number of created notifications), instead it returns a list of created notifications. + This is very useful, see :doc:`howto/object_relations` #134 (Benjamin Balder Bach) **Fixed** @@ -33,6 +36,7 @@ Release Notes **Removed** * Python 3.7 support removed #129 (Benjamin Balder Bach) +* Unused (!) setting ``NYT_ENABLED`` was removed #134 (Benjamin Balder Bach) 1.3 --- diff --git a/tests/core/test_basic.py b/tests/core/test_basic.py index 22799da6..64c39553 100644 --- a/tests/core/test_basic.py +++ b/tests/core/test_basic.py @@ -1,8 +1,11 @@ from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType from django.test import TestCase from django_nyt import models from django_nyt import utils +from django_nyt.decorators import disable_notify +from tests.testapp.models import TestModel User = get_user_model() @@ -38,6 +41,19 @@ def test_notify(self): # Check that there is exactly 1 notification self.assertEqual(models.Notification.objects.all().count(), 1) + def test_disable_notify(self): + # Subscribe User 1 to test key + utils.subscribe(self.user1_settings, self.TEST_KEY) + + @disable_notify + def inner(): + utils.notify("Test is a test", self.TEST_KEY) + + inner() + + # Check that there is exactly 1 notification + self.assertEqual(models.Notification.objects.all().count(), 0) + def test_notify_two_users(self): # Subscribe User 2 to test key @@ -52,3 +68,31 @@ def test_notify_two_users(self): utils.notify("Another test", self.TEST_KEY) self.assertEqual(models.Notification.objects.filter(occurrences=2).count(), 2) + + def test_failure_target_object_not_model(self): + + # Subscribe User 1 to test key + utils.subscribe(self.user1_settings, self.TEST_KEY) + with self.assertRaises(TypeError): + utils.notify("Another test", self.TEST_KEY, target_object=object()) + + def test_with_target_object(self): + + related_object = TestModel.objects.create(name="test_with_target_object") + content_type = ContentType.objects.get_for_model(TestModel) + # Subscribe User 1 to test key + utils.subscribe( + self.user1_settings, + self.TEST_KEY, + content_type=content_type, + object_id=related_object.id, + ) + utils.notify("Test related object", self.TEST_KEY, target_object=related_object) + + self.assertEqual( + models.Notification.objects.filter( + subscription__object_id=related_object.id, + subscription__notification_type__content_type=content_type, + ).count(), + 1, + )