Add option "NOTIFICATIONS_SOFT_DELETE", fix issue #52

.gitignore

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

README.rst

@@ -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
-

notifications/models.py

@@ -3,6 +3,7 @@
 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 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 @@ def mark_all_as_unread(self, recipient=None):
 
         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

notifications/tests/tests.py

@@ -13,6 +13,7 @@
 
 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.models import Notification
 from notifications.utils import id2slug
 
+
 class NotificationTest(TestCase):
 
     @override_settings(USE_TZ=True)
@@ -46,50 +48,81 @@ def test_disable_timezone(self):
         delta = timezone.now() - notification.timestamp
         self.assertTrue(delta.seconds < 60)
 
+
 class NotificationManagersTest(TestCase):
-    def setUp(self):
 
+    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)
         for n in Notification.objects.read():
             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 @@ def test_unread_messages_pages(self):
         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 @@ def test_unread_messages_pages(self):
         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 @@ def test_next_pages(self):
         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)

notifications/views.py

@@ -3,6 +3,7 @@
 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')