Mercurial > public > sg101
diff gpp/forums/views/subscriptions.py @ 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 | a46788862737 |
children | b2b37cdd020a |
line wrap: on
line diff
--- 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]))