Add option "NOTIFICATIONS_SOFT_DELETE", fix issue #52

This commit is contained in:
zzy 2015-04-11 13:28:52 +08:00
parent c8647ce302
commit 191c1250df
5 changed files with 205 additions and 26 deletions

1
.gitignore vendored
View file

@ -4,3 +4,4 @@ dist
.DS_Store
MANIFEST
.coverage
htmlcov

View file

@ -117,7 +117,20 @@ You can attach arbitrary data to your notifications by doing the following:
* Add to your settings.py: ``NOTIFICATIONS_USE_JSONFIELD=True``
Then, any extra arguments you pass to ``notify.send(...)`` will be attached to the ``.data`` attribute of the notification object. These will be serialised using the JSONField's serialiser, so you may need to take that into account: using only objects that will be serialised is a good idea.
Then, any extra arguments you pass to ``notify.send(...)`` will be attached to the ``.data`` attribute of the notification object.
These will be serialised using the JSONField's serialiser, so you may need to take that into account: using only objects that will be serialised is a good idea.
Soft delete
-----------
By default, ``delete/(?P<slug>\d+)/`` deletes specified notification record from DB.
You can change this behaviour to "mark ``Notification.deleted`` field as ``True``" by:
* Add to your settings.py: ``NOTIFICATIONS_SOFT_DELETE=True``
With this option, QuerySet methods ``unread`` and ``read`` contain one more filter: ``deleted=False``.
Meanwhile, QuerySet methods ``deleted``, ``active``, ``mark_all_as_deleted``, ``mark_all_as_active`` are turned on.
See more details in QuerySet methods section.
API
====
@ -140,11 +153,13 @@ There are some other QuerySet methods, too.
~~~~~~~~~~~~~~~
Return all of the unread notifications, filtering the current queryset.
When ``NOTIFICATIONS_SOFT_DELETE=True``, this filter contains ``deleted=False``.
``qs.read()``
~~~~~~~~~~~~~~~
Return all of the read notifications, filtering the current queryset.
When ``NOTIFICATIONS_SOFT_DELETE=True``, this filter contains ``deleted=False``.
``qs.mark_all_as_read()`` | ``qs.mark_all_as_read(recipient)``
@ -158,6 +173,30 @@ Mark all of the unread notifications in the queryset (optionally also filtered b
Mark all of the read notifications in the queryset (optionally also filtered by ``recipient``) as unread.
``qs.deleted()``
~~~~~~~~~~~~~~~~
Return all notifications that have ``deleted=True``, filtering the current queryset.
Must be used with ``NOTIFICATIONS_SOFT_DELETE=True``.
``qs.active()``
~~~~~~~~~~~~~~~
Return all notifications that have ``deleted=False``, filtering the current queryset.
Must be used with ``NOTIFICATIONS_SOFT_DELETE=True``.
``qs.mark_all_as_deleted()`` | ``qs.mark_all_as_deleted(recipient)``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Mark all notifications in the queryset (optionally also filtered by ``recipient``) as ``deleted=True``.
Must be used with ``NOTIFICATIONS_SOFT_DELETE=True``.
``qs.mark_all_as_active()`` | ``qs.mark_all_as_active(recipient)``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Mark all notifications in the queryset (optionally also filtered by ``recipient``) as ``deleted=False``.
Must be used with ``NOTIFICATIONS_SOFT_DELETE=True``.
Model methods
-------------
@ -203,4 +242,3 @@ Storing the count in a variable for further processing is advised, such as::
:alt: Code coverage on coveralls
:scale: 100%
:target: https://coveralls.io/r/django-notifications/django-notifications?branch=master

View file

@ -3,6 +3,7 @@ from django.conf import settings
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic
from django.db import models
from django.core.exceptions import ImproperlyConfigured
from six import text_type
from .utils import id2slug
@ -11,6 +12,7 @@ from .signals import notify
from model_utils import managers, Choices
from jsonfield.fields import JSONField
def now():
# Needs to be be a function as USE_TZ can change based on if we are testing or not.
_now = datetime.datetime.now
@ -22,15 +24,43 @@ def now():
pass
return _now()
#SOFT_DELETE = getattr(settings, 'NOTIFICATIONS_SOFT_DELETE', False)
def is_soft_delete():
#TODO: SOFT_DELETE = getattr(settings, ...) doesn't work with "override_settings" decorator in unittest
# But is_soft_delete is neither a very elegant way. Should try to find better approach
return getattr(settings, 'NOTIFICATIONS_SOFT_DELETE', False)
def assert_soft_delete():
if not is_soft_delete():
msg = """To use 'deleted' field, please set 'NOTIFICATIONS_SOFT_DELETE'=True in settings.
Otherwise NotificationQuerySet.unread and NotificationQuerySet.read do NOT filter by 'deleted' field.
"""
raise ImproperlyConfigured(msg)
class NotificationQuerySet(models.query.QuerySet):
def unread(self):
"Return only unread items in the current queryset"
return self.filter(unread=True)
"""Return only unread items in the current queryset"""
if is_soft_delete():
return self.filter(unread=True, deleted=False)
else:
""" when SOFT_DELETE=False, developers are supposed NOT to touch 'deleted' field.
In this case, to improve query performance, don't filter by 'deleted' field
"""
return self.filter(unread=True)
def read(self):
"Return only read items in the current queryset"
return self.filter(unread=False)
"""Return only read items in the current queryset"""
if is_soft_delete():
return self.filter(unread=False, deleted=False)
else:
""" when SOFT_DELETE=False, developers are supposed NOT to touch 'deleted' field.
In this case, to improve query performance, don't filter by 'deleted' field
"""
return self.filter(unread=False)
def mark_all_as_read(self, recipient=None):
"""Mark as read any unread messages in the current queryset.
@ -57,6 +87,39 @@ class NotificationQuerySet(models.query.QuerySet):
qs.update(unread=True)
def deleted(self):
"""Return only deleted items in the current queryset"""
assert_soft_delete()
return self.filter(deleted=True)
def active(self):
"""Return only active(un-deleted) items in the current queryset"""
assert_soft_delete()
return self.filter(deleted=False)
def mark_all_as_deleted(self, recipient=None):
"""Mark current queryset as deleted.
Optionally, filter by recipient first.
"""
assert_soft_delete()
qs = self.active()
if recipient:
qs = qs.filter(recipient=recipient)
qs.update(deleted=True)
def mark_all_as_active(self, recipient=None):
"""Mark current queryset as active(un-deleted).
Optionally, filter by recipient first.
"""
assert_soft_delete()
qs = self.deleted()
if recipient:
qs = qs.filter(recipient=recipient)
qs.update(deleted=False)
class Notification(models.Model):
"""
Action model describing the actor acting out a verb (on an optional

View file

@ -13,6 +13,7 @@ except ImportError:
from django.conf import settings
from django.contrib.auth.models import User
from django.core.exceptions import ImproperlyConfigured
from django.core.urlresolvers import reverse
from django.utils.timezone import utc, localtime
from django.utils import timezone
@ -22,6 +23,7 @@ from notifications import notify
from notifications.models import Notification
from notifications.utils import id2slug
class NotificationTest(TestCase):
@override_settings(USE_TZ=True)
@ -46,24 +48,26 @@ class NotificationTest(TestCase):
delta = timezone.now() - notification.timestamp
self.assertTrue(delta.seconds < 60)
class NotificationManagersTest(TestCase):
def setUp(self):
class NotificationManagersTest(TestCase):
def setUp(self):
self.message_count = 10
self.from_user = User.objects.create(username="from2", password="pwd", email="example@example.com")
self.to_user = User.objects.create(username="to2", password="pwd", email="example@example.com")
for i in range(10):
for i in range(self.message_count):
notify.send(self.from_user, recipient=self.to_user, verb='commented', action_object=self.from_user)
def test_unread_manager(self):
self.assertEqual(Notification.objects.unread().count(),10)
self.assertEqual(Notification.objects.unread().count(), self.message_count)
n = Notification.objects.filter(recipient=self.to_user).first()
n.mark_as_read()
self.assertEqual(Notification.objects.unread().count(),9)
self.assertEqual(Notification.objects.unread().count(), self.message_count-1)
for n in Notification.objects.unread():
self.assertTrue(n.unread)
def test_read_manager(self):
self.assertEqual(Notification.objects.unread().count(),10)
self.assertEqual(Notification.objects.unread().count(), self.message_count)
n = Notification.objects.filter(recipient=self.to_user).first()
n.mark_as_read()
self.assertEqual(Notification.objects.read().count(),1)
@ -71,25 +75,54 @@ class NotificationManagersTest(TestCase):
self.assertFalse(n.unread)
def test_mark_all_as_read_manager(self):
self.assertEqual(Notification.objects.unread().count(),10)
self.assertEqual(Notification.objects.unread().count(), self.message_count)
Notification.objects.filter(recipient=self.to_user).mark_all_as_read()
self.assertEqual(Notification.objects.unread().count(),0)
def test_mark_all_as_unread_manager(self):
self.assertEqual(Notification.objects.unread().count(),10)
self.assertEqual(Notification.objects.unread().count(), self.message_count)
Notification.objects.filter(recipient=self.to_user).mark_all_as_read()
self.assertEqual(Notification.objects.unread().count(),0)
Notification.objects.filter(recipient=self.to_user).mark_all_as_unread()
self.assertEqual(Notification.objects.unread().count(),10)
self.assertEqual(Notification.objects.unread().count(), self.message_count)
def test_mark_all_deleted_manager_without_soft_delete(self):
self.assertRaises(ImproperlyConfigured, Notification.objects.active)
self.assertRaises(ImproperlyConfigured, Notification.objects.active)
self.assertRaises(ImproperlyConfigured, Notification.objects.mark_all_as_deleted)
self.assertRaises(ImproperlyConfigured, Notification.objects.mark_all_as_active)
@override_settings(NOTIFICATIONS_SOFT_DELETE=True)
def test_mark_all_deleted_manager(self):
n = Notification.objects.filter(recipient=self.to_user).first()
n.mark_as_read()
self.assertEqual(Notification.objects.read().count(), 1)
self.assertEqual(Notification.objects.unread().count(), self.message_count-1)
self.assertEqual(Notification.objects.active().count(), self.message_count)
self.assertEqual(Notification.objects.deleted().count(), 0)
Notification.objects.mark_all_as_deleted()
self.assertEqual(Notification.objects.read().count(), 0)
self.assertEqual(Notification.objects.unread().count(), 0)
self.assertEqual(Notification.objects.active().count(), 0)
self.assertEqual(Notification.objects.deleted().count(), self.message_count)
Notification.objects.mark_all_as_active()
self.assertEqual(Notification.objects.read().count(), 1)
self.assertEqual(Notification.objects.unread().count(), self.message_count-1)
self.assertEqual(Notification.objects.active().count(), self.message_count)
self.assertEqual(Notification.objects.deleted().count(), 0)
class NotificationTestPages(TestCase):
def setUp(self):
self.message_count = 10
self.from_user = User.objects.create_user(username="from", password="pwd", email="example@example.com")
self.to_user = User.objects.create_user(username="to", password="pwd", email="example@example.com")
self.to_user.is_staff = True
self.to_user.save()
for i in range(10):
for i in range(self.message_count):
notify.send(self.from_user, recipient=self.to_user, verb='commented', action_object=self.from_user)
def logout(self):
@ -113,7 +146,7 @@ class NotificationTestPages(TestCase):
response = self.client.get(reverse('notifications:unread'))
self.assertEqual(response.status_code,200)
self.assertEqual(len(response.context['notifications']),len(self.to_user.notifications.unread()))
self.assertEqual(len(response.context['notifications']),10)
self.assertEqual(len(response.context['notifications']), self.message_count)
for i,n in enumerate(self.to_user.notifications.all()):
if i%3 == 0:
@ -123,7 +156,7 @@ class NotificationTestPages(TestCase):
response = self.client.get(reverse('notifications:unread'))
self.assertEqual(response.status_code,200)
self.assertEqual(len(response.context['notifications']),len(self.to_user.notifications.unread()))
self.assertTrue(len(response.context['notifications'])<10)
self.assertTrue(len(response.context['notifications']) < self.message_count)
response = self.client.get(reverse('notifications:mark_all_as_read'))
self.assertRedirects(response,reverse('notifications:all'))
@ -143,3 +176,38 @@ class NotificationTestPages(TestCase):
slug = id2slug(self.to_user.notifications.first().id)
response = self.client.get(reverse('notifications:mark_as_unread',args=[slug])+"?next="+reverse('notifications:unread'))
self.assertRedirects(response,reverse('notifications:unread'))
def test_delete_messages_pages(self):
self.login('to', 'pwd')
slug = id2slug(self.to_user.notifications.first().id)
response = self.client.get(reverse('notifications:delete', args=[slug]))
self.assertRedirects(response, reverse('notifications:all'))
response = self.client.get(reverse('notifications:all'))
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.context['notifications']), len(self.to_user.notifications.all()))
self.assertEqual(len(response.context['notifications']), self.message_count-1)
response = self.client.get(reverse('notifications:unread'))
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.context['notifications']), len(self.to_user.notifications.unread()))
self.assertEqual(len(response.context['notifications']), self.message_count-1)
@override_settings(NOTIFICATIONS_SOFT_DELETE=True)
def test_soft_delete_messages_manager(self):
self.login('to', 'pwd')
slug = id2slug(self.to_user.notifications.first().id)
response = self.client.get(reverse('notifications:delete', args=[slug]))
self.assertRedirects(response, reverse('notifications:all'))
response = self.client.get(reverse('notifications:all'))
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.context['notifications']), len(self.to_user.notifications.active()))
self.assertEqual(len(response.context['notifications']), self.message_count-1)
response = self.client.get(reverse('notifications:unread'))
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.context['notifications']), len(self.to_user.notifications.unread()))
self.assertEqual(len(response.context['notifications']), self.message_count-1)

View file

@ -3,6 +3,7 @@ from django.contrib.auth.decorators import login_required
from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
from django.shortcuts import get_object_or_404, render, redirect
from django.template.context import RequestContext
from django.conf import settings
from .utils import slug2id
from .models import Notification
@ -11,8 +12,12 @@ def all(request):
"""
Index page for authenticated user
"""
if getattr(settings, 'NOTIFICATIONS_SOFT_DELETE', False):
qs = request.user.notifications.active()
else:
qs = request.user.notifications.all()
return render(request, 'notifications/list.html', {
'notifications': request.user.notifications.all()
'notifications': qs
})
actions = request.user.notifications.all()
@ -77,17 +82,21 @@ def mark_as_unread(request, slug=None):
return redirect('notifications:all')
@login_required
def delete(request, slug=None):
id = slug2id(slug)
_id = slug2id(slug)
notification = get_object_or_404(Notification, recipient=request.user, id=id)
notification.deleted = True
notification.save()
notification = get_object_or_404(Notification, recipient=request.user, id=_id)
if getattr(settings, 'NOTIFICATIONS_SOFT_DELETE', False):
notification.deleted = True
notification.save()
else:
notification.delete()
next = request.GET.get('next')
_next = request.GET.get('next')
if next:
return redirect(next)
if _next:
return redirect(_next)
return redirect('notifications:all')