changeset 301:ee451ad46af1

Fixing #140; limit topic notification emails to at most 1 per day, or more if the user visits the topic.
author Brian Neal <bgneal@gmail.com>
date Thu, 13 Jan 2011 03:27:42 +0000
parents 58e9b8b2965f
children 94bd52dada89
files gpp/forums/models.py gpp/forums/views/subscriptions.py
diffstat 2 files changed, 56 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/gpp/forums/models.py	Tue Jan 11 04:04:44 2011 +0000
+++ b/gpp/forums/models.py	Thu Jan 13 03:27:42 2011 +0000
@@ -183,7 +183,7 @@
     sticky = models.BooleanField(blank=True, default=False)
     locked = models.BooleanField(blank=True, default=False)
     subscribers = models.ManyToManyField(User, related_name='subscriptions',
-            verbose_name='subscribers', blank=True)
+            verbose_name='subscribers', blank=True, through='Subscription')
     bookmarkers = models.ManyToManyField(User, related_name='favorite_topics',
             verbose_name='bookmarkers', blank=True)
 
@@ -391,3 +391,13 @@
 
     def __unicode__(self):
         return u'Post %d, %s' % (self.post.pk, self.embed.title)
+
+
+class Subscription(models.Model):
+    """
+    This model is a "through" table for the M2M relationshiop between forum
+    topics and users (subscribers).
+    """
+    topic = models.ForeignKey(Topic)
+    user = models.ForeignKey(User)
+    notify_date = models.DateTimeField(null=True)
--- a/gpp/forums/views/subscriptions.py	Tue Jan 11 04:04:44 2011 +0000
+++ b/gpp/forums/views/subscriptions.py	Thu Jan 13 03:27:42 2011 +0000
@@ -1,4 +1,6 @@
 """This module handles the subscriptions of users to forum topics."""
+import datetime
+
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.contrib.sites.models import Site
@@ -12,18 +14,48 @@
 from django.template import RequestContext
 from django.views.decorators.http import require_POST
 
-from forums.models import Topic
+from forums.models import Topic, TopicLastVisit, Subscription
 from core.functions import send_mail
 from core.paginator import DiggPaginator
 
 
+# This constant is the minimum amount of time a user will be
+# notified of new posts in a topic. Currently it is set to 1 day.
+# Thus a user will be notified at most once per day of new posts.
+TOPIC_NOTIFY_DELAY = datetime.timedelta(days=1)
+
+
 def notify_topic_subscribers(post):
-    """The argument post is a newly created post. Send out an email
-    notification to all subscribers of the post's parent Topic."""
+    """
+    The argument post is a newly created post. Send out an email
+    notification to all subscribers of the post's parent Topic.
+    The emails are throttled such that unless the user visits the topic,
+    only 1 email will be sent per TOPIC_NOTIFY_DELAY period.
+    Visiting the topic will reset this delay.
+
+    """
+    #TODO: consider moving this function of the HTTP request/response cycle.
 
     topic = post.topic
-    recipients = topic.subscribers.exclude(
-            id=post.user.id).values_list('email', flat=True)
+    subscriptions = Subscription.objects.filter(topic=post.topic).exclude(
+            user=post.user).select_related()
+
+    if not subscriptions:
+        return
+
+    subscriber_ids = [sub.user.id for sub in subscriptions]
+    tlvs = dict(TopicLastVisit.objects.filter(topic=topic,
+            user__in=subscriber_ids).values_list('user', 'last_visit'))
+
+    recipients = []
+    for sub in subscriptions:
+        if (sub.notify_date is None or
+            post.creation_date - sub.notify_date > TOPIC_NOTIFY_DELAY or
+            tlvs.get(sub.user.id, datetime.datetime.min) > sub.notify_date):
+
+            recipients.append(sub.user.email)
+            sub.notify_date = post.creation_date
+            sub.save()
 
     if recipients:
         site = Site.objects.get_current()
@@ -48,7 +80,8 @@
     """Subscribe the user to the requested topic."""
     topic = get_object_or_404(Topic.objects.select_related(), id=topic_id)
     if topic.forum.category.can_access(request.user):
-        topic.subscribers.add(request.user)
+        sub = Subscription(topic=topic, user=request.user)
+        sub.save()
         return HttpResponseRedirect(
             reverse("forums-subscription_status", args=[topic.id]))
     raise Http404   # TODO return HttpResponseForbidden instead
@@ -59,7 +92,12 @@
 def unsubscribe_topic(request, topic_id):
     """Unsubscribe the user to the requested topic."""
     topic = get_object_or_404(Topic, id=topic_id)
-    topic.subscribers.remove(request.user)
+    try:
+        sub = Subscription.objects.get(topic=topic, user=request.user)
+    except Subscriptions.DoesNotExist:
+        pass
+    else:
+        sub.delete()
     return HttpResponseRedirect(
         reverse("forums-subscription_status", args=[topic.id]))