fixed email sender job so it works as advertised

2 files changed, 198 insertions, 71 deletions

diff --git a/forum/const.py b/forum/const.py
index 39db5ad4..4c107572 100755
--- a/forum/const.py
+++ b/forum/const.py
@@ -90,3 +90,4 @@ CONST = {
 
 #how to filter questions by tags in email digests?
 TAG_EMAIL_FILTER_CHOICES = (('ignored', _('exclude ignored tags')),('interesting',_('allow only selected tags')))
+MAX_ALERTS_PER_EMAIL = 7
diff --git a/forum/management/commands/send_email_alerts.py b/forum/management/commands/send_email_alerts.py
index 761c604a..caeac5e8 100755
--- a/forum/management/commands/send_email_alerts.py
+++ b/forum/management/commands/send_email_alerts.py
@@ -11,6 +11,27 @@ from django.conf import settings
 import logging
 from forum.utils.odict import OrderedDict
 from django.contrib.contenttypes.models import ContentType
+from forum import const
+
+def extend_question_list(src, dst, limit=False):
+    """src is a query set with questions
+       or None
+       dst - is an ordered dictionary
+    """
+    if limit and len(dst.keys()) >= const.MAX_ALERTS_PER_EMAIL:
+        return
+    if src is None:#is not QuerySet
+        return #will not do anything if subscription of this type is not used
+    cutoff_time = src.cutoff_time
+    for q in src:
+        if q in dst:
+            #the latest cutoff time wins for a given question
+            #if the question falls into several subscription groups
+            if cutoff_time > dst[q]['cutoff_time']:
+                dst[q]['cutoff_time'] = cutoff_time
+        else:
+            #initialise a questions metadata dictionary to use for email reporting
+            dst[q] = {'cutoff_time':cutoff_time}
 
 class Command(NoArgsCommand):
     def handle_noargs(self,**options):
@@ -24,84 +45,123 @@ class Command(NoArgsCommand):
 
     def get_updated_questions_for_user(self,user):
 
-        q_sel = None 
-        q_ask = None 
-        q_ans = None 
-        q_all = None 
+        #these are placeholders for separate query sets per question group
+        #there are four groups - one for each EmailFeedSetting.feed_type
+        #and each group has subtypes A and B
+        #that's because of the strange thing commented below
+        #see note on Q and F objects marked with todo tag
+        q_sel_A = None
+        q_sel_B = None
+
+        q_ask_A = None
+        q_ask_B = None
+
+        q_ans_A = None
+        q_ans_B = None
+
+        q_all_A = None
+        q_all_B = None
+
         now = datetime.datetime.now()
         #Q_set1 - base questionquery set for this user
         Q_set1 = Question.objects.exclude(
-                                        last_activity_by=user,#exclude his/her own edits
-                                  ).exclude(
-                                        last_activity_at__lt=user.date_joined, #exclude old stuff
-                                  ).filter(
-                                        Q(viewed__who=user,viewed__when__lt=F('last_activity_at')) | \
-                                        ~Q(viewed__who=user)
-                                  ).exclude(
-                                        deleted=True
-                                  ).exclude(
-                                        closed=True
-                                  )
+                                last_activity_by=user
+                            ).exclude(
+                                last_activity_at__lt=user.date_joined#exclude old stuff
+                            ).exclude(
+                                deleted=True
+                            ).exclude(
+                                closed=True
+                            ).order_by('-last_activity_at')
+        #todo: for some reason filter on did not work as expected ~Q(viewed__who=user) | 
+        #      Q(viewed__who=user,viewed__when__lt=F('last_activity_at'))
+        #returns way more questions than you might think it should
+        #so because of that I've created separate query sets Q_set2 and Q_set3
+        #plus two separate queries run faster!
 
+        #questions that are not seen by the user
+        Q_set2 = Q_set1.filter(~Q(viewed__who=user))
+        #questions seen before the last modification
+        Q_set3 = Q_set1.filter(Q(viewed__who=user,viewed__when__lt=F('last_activity_at')))
+
+        #todo may shortcirquit here is len(user_feeds) == 0
         user_feeds = EmailFeedSetting.objects.filter(subscriber=user).exclude(frequency='n')
+        if len(user_feeds) == 0:
+            return {};#short cirquit
         for feed in user_feeds:
+            #each group of updates has it's own cutoff time
+            #to be saved as a new parameter for each query set
+            #won't send email for a given question if it has been done
+            #after the cutoff_time
             cutoff_time = now - EmailFeedSetting.DELTA_TABLE[feed.frequency]
             if feed.reported_at == None or feed.reported_at <= cutoff_time:
-                Q_set = Q_set1.exclude(last_activity_at__gt=cutoff_time)#report these excluded later
+                Q_set_A = Q_set2#.exclude(last_activity_at__gt=cutoff_time)#report these excluded later
+                Q_set_B = Q_set3#.exclude(last_activity_at__gt=cutoff_time)
                 feed.reported_at = now
                 feed.save()#may not actually report anything, depending on filters below
                 if feed.feed_type == 'q_sel':
-                    q_sel = Q_set.filter(followed_by=user)
-                    q_sel.cutoff_time = cutoff_time #store cutoff time per query set
+                    q_sel_A = Q_set_A.filter(followed_by=user)
+                    q_sel_A.cutoff_time = cutoff_time #store cutoff time per query set
+                    q_sel_B = Q_set_B.filter(followed_by=user)
+                    q_sel_B.cutoff_time = cutoff_time #store cutoff time per query set
                 elif feed.feed_type == 'q_ask':
-                    q_ask = Q_set.filter(author=user)
-                    q_ask.cutoff_time = cutoff_time
+                    q_ask_A = Q_set_A.filter(author=user)
+                    q_ask_A.cutoff_time = cutoff_time
+                    q_ask_B = Q_set_B.filter(author=user)
+                    q_ask_B.cutoff_time = cutoff_time
                 elif feed.feed_type == 'q_ans':
-                    q_ans = Q_set.filter(answers__author=user)
-                    q_ans.cutoff_time = cutoff_time
+                    q_ans_A = Q_set_A.filter(answers__author=user)[:const.MAX_ALERTS_PER_EMAIL]
+                    q_ans_A.cutoff_time = cutoff_time
+                    q_ans_B = Q_set_B.filter(answers__author=user)[:const.MAX_ALERTS_PER_EMAIL]
+                    q_ans_B.cutoff_time = cutoff_time
                 elif feed.feed_type == 'q_all':
                     if user.tag_filter_setting == 'ignored':
-                        ignored_tags = Tag.objects.filter(user_selections__reason='bad',user_selections__user=user)
-                        q_all = Q_set.exclude( tags__in=ignored_tags )
+                        ignored_tags = Tag.objects.filter(user_selections__reason='bad', \
+                                                            user_selections__user=user)
+                        q_all_A = Q_set_A.exclude( tags__in=ignored_tags )[:const.MAX_ALERTS_PER_EMAIL]
+                        q_all_B = Q_set_B.exclude( tags__in=ignored_tags )[:const.MAX_ALERTS_PER_EMAIL]
                     else:
-                        selected_tags = Tag.objects.filter(user_selections__reason='good',user_selections__user=user)
-                        q_all = Q_set.filter( tags__in=selected_tags )
-                    q_all.cutoff_time = cutoff_time
+                        selected_tags = Tag.objects.filter(user_selections__reason='good', \
+                                                            user_selections__user=user)
+                        q_all_A = Q_set_A.filter( tags__in=selected_tags )
+                        q_all_B = Q_set_B.filter( tags__in=selected_tags )
+                    q_all_A.cutoff_time = cutoff_time
+                    q_all_B.cutoff_time = cutoff_time
         #build list in this order
         q_list = OrderedDict()
-        def extend_question_list(src, dst):
-            """src is a query set with questions
-               or an empty list
-                dst - is an ordered dictionary
-            """
-            if src is None:
-                return #will not do anything if subscription of this type is not used
-            cutoff_time = src.cutoff_time
-            for q in src:
-                if q in dst:
-                    if cutoff_time < dst[q]['cutoff_time']:
-                        dst[q]['cutoff_time'] = cutoff_time
-                else:
-                    #initialise a questions metadata dictionary to use for email reporting
-                    dst[q] = {'cutoff_time':cutoff_time}
 
-        extend_question_list(q_sel, q_list)
-        extend_question_list(q_ask, q_list)
-        extend_question_list(q_ans, q_list)
-        extend_question_list(q_all, q_list)
+        extend_question_list(q_sel_A, q_list)
+        extend_question_list(q_sel_B, q_list)
+
+        if user.tag_filter_setting == 'interesting':
+            extend_question_list(q_all_A, q_list)
+            extend_question_list(q_all_B, q_list)
+
+        extend_question_list(q_ask_A, q_list, limit=True)
+        extend_question_list(q_ask_B, q_list, limit=True)
+
+        extend_question_list(q_ans_A, q_list, limit=True)
+        extend_question_list(q_ans_B, q_list, limit=True)
+
+        if user.tag_filter_setting == 'ignored':
+            extend_question_list(q_all_A, q_list, limit=True)
+            extend_question_list(q_all_B, q_list, limit=True)
 
         ctype = ContentType.objects.get_for_model(Question)
         EMAIL_UPDATE_ACTIVITY = const.TYPE_ACTIVITY_QUESTION_EMAIL_UPDATE_SENT
         for q, meta_data in q_list.items():
-            #todo use Activity, but first start keeping more Activity records
-            #act = Activity.objects.filter(content_type=ctype, object_id=q.id)
-            #because currently activity is not fully recorded to through
-            #revision records to see what kind modifications were done on
-            #the questions and answers
+            #this loop edits meta_data for each question
+            #so that user will receive counts on new edits new answers, etc
+            #maybe not so important actually??
+
+            #keeps email activity per question per user
             try:
-                update_info = Activity.objects.get(content_type=ctype, 
+                update_info = Activity.objects.get(
+                                                    user=user,
+                                                    content_type=ctype,
                                                     object_id=q.id,
-                                                    activity_type=EMAIL_UPDATE_ACTIVITY)
+                                                    activity_type=EMAIL_UPDATE_ACTIVITY
+                                                    )
                 emailed_at = update_info.active_at
             except Activity.DoesNotExist:
                 update_info = Activity(user=user, content_object=q, activity_type=EMAIL_UPDATE_ACTIVITY)
@@ -109,10 +169,22 @@ class Command(NoArgsCommand):
             except Activity.MultipleObjectsReturned:
                 raise Exception('server error - multiple question email activities found per user-question pair')
 
+            cutoff_time = meta_data['cutoff_time']#cutoff time for the question
+
+            #wait some more time before emailing about this question
+            if emailed_at > cutoff_time:
+                #here we are maybe losing opportunity to record the finding
+                #of yet unseen version of a question
+                meta_data['skip'] = True
+                continue
+
+            #collect info on all sorts of news that happened after
+            #the most recent emailing to the user about this question
             q_rev = QuestionRevision.objects.filter(question=q,\
-                                                    revised_at__lt=cutoff_time,\
                                                     revised_at__gt=emailed_at)
             q_rev = q_rev.exclude(author=user)
+
+            #now update all sorts of metadata per question
             meta_data['q_rev'] = len(q_rev)
             if len(q_rev) > 0 and q.added_at == q_rev[0].revised_at:
                 meta_data['q_rev'] = 0
@@ -121,21 +193,23 @@ class Command(NoArgsCommand):
                 meta_data['new_q'] = False
                 
             new_ans = Answer.objects.filter(question=q,\
-                                            added_at__lt=cutoff_time,\
                                             added_at__gt=emailed_at)
             new_ans = new_ans.exclude(author=user)
             meta_data['new_ans'] = len(new_ans)
             ans_rev = AnswerRevision.objects.filter(answer__question=q,\
-                                            revised_at__lt=cutoff_time,\
                                             revised_at__gt=emailed_at)
             ans_rev = ans_rev.exclude(author=user)
             meta_data['ans_rev'] = len(ans_rev)
-            if len(q_rev) == 0 and len(new_ans) == 0 and len(ans_rev) == 0:
-                meta_data['nothing_new'] = True
+
+            if 0 in (len(q_rev), len(new_ans), len(ans_rev)):
+                meta_data['skip'] = True
             else:
-                meta_data['nothing_new'] = False
+                meta_data['skip'] = False
                 update_info.active_at = now
                 update_info.save() #save question email update activity 
+        #q_list is actually a ordered dictionary
+        #print 'user %s gets %d' % (user.username, len(q_list.keys()))
+        #todo: sort question list by update time
         return q_list 
 
     def __action_count(self,string,number,output):
@@ -143,14 +217,18 @@ class Command(NoArgsCommand):
             output.append(_(string) % {'num':number})
 
     def send_email_alerts(self):
+        #does not change the database, only sends the email
         #todo: move this to template
         for user in User.objects.all():
+            #todo: q_list is a dictionary, not a list
             q_list = self.get_updated_questions_for_user(user)
+            if len(q_list.keys()) == 0:
+                continue
             num_q = 0
             num_moot = 0
             for meta_data in q_list.values():
-                if meta_data['nothing_new']:
-                    num_moot += 1
+                if meta_data['skip']:
+                    num_moot = True
                 else:
                     num_q += 1
             if num_q > 0:
@@ -162,11 +240,17 @@ class Command(NoArgsCommand):
                                 % {'num':num_q, 'name':user.username}
 
                 text += '<ul>'
+                items_added = 0
+                items_unreported = 0
                 for q, meta_data in q_list.items():
                     act_list = []
-                    if meta_data['nothing_new']:
+                    if meta_data['skip']:
                         continue
+                    if items_added >= const.MAX_ALERTS_PER_EMAIL:
+                        items_unreported = num_q - items_added #may be inaccurate actually, but it's ok
+                        
                     else:
+                        items_added += 1
                         if meta_data['new_q']:
                             act_list.append(_('new question'))
                         self.__action_count('%(num)d rev', meta_data['q_rev'],act_list)
@@ -176,15 +260,52 @@ class Command(NoArgsCommand):
                         text += '<li><a href="%s?sort=latest">%s</a> <font color="#777777">(%s)</font></li>' \
                                     % (url_prefix + q.get_absolute_url(), q.title, act_token)
                 text += '</ul>'
-                if num_moot > 0:
-                    text += '<p></p>'
-                    text += ungettext('There is also one question which was recently '\
-                                +'updated but you might not have seen its latest version.',
-                            'There are also %(num)d more questions which were recently updated '\
-                            +'but you might not have seen their latest version.',num_moot) \
-                                % {'num':num_moot,}
-                    text += _('Perhaps you could look up previously sent forum reminders in your mailbox.')
-                    text += '</p>'
+                text += '<p></p>'
+                #if len(q_list.keys()) >= const.MAX_ALERTS_PER_EMAIL:
+                #    text += _('There may be more questions updated since '
+                #                'you have logged in last time as this list is '
+                #                'abridged for your convinience. Please visit '
+                #                'the forum and see what\'s new!<br>'
+                #              )
+
+                text += _(
+                            'Please visit the forum and see what\'s new! '
+                            'Could you spread the word about it - '
+                            'can somebody you know help answering those questions or '
+                            'benefit from posting one?'
+                        )
+
+                feeds = EmailFeedSetting.objects.filter(
+                                                        subscriber=user,
+                                                    )
+                feed_freq = [feed.frequency for feed in feeds]
+                text += '<p></p>'
+                if 'd' in feed_freq:
+                    text += _('Your most frequent subscription setting is \'daily\' '
+                               'on selected questions. If you are receiving more than one '
+                               'email per day'
+                               'please tell about this issue to the forum administrator.'
+                               )
+                elif 'w' in feed_freq:
+                    text += _('Your most frequent subscription setting is \'weekly\' '
+                               'if you are receiving this email more than once a week '
+                               'please report this issue to the forum administrator.'
+                               )
+                text += ' '
+                text += _(
+                            'There is a chance that you may be receiving links seen '
+                            'before - due to a technicality that will eventually go away. '
+                        )
+                #    text += '</p>'
+                #if num_moot > 0:
+                #    text += '<p></p>'
+                #    text += ungettext('There is also one question which was recently '\
+                #                +'updated but you might not have seen its latest version.',
+                #            'There are also %(num)d more questions which were recently updated '\
+                #            +'but you might not have seen their latest version.',num_moot) \
+                #                % {'num':num_moot,}
+                #    text += _('Perhaps you could look up previously sent forum reminders in your mailbox.')
+                #    text += '</p>'
 
                 link = url_prefix + user.get_profile_url() + '?sort=email_subscriptions'
                 text += _('go to %(link)s to change frequency of email updates or %(email)s administrator') \
@@ -192,3 +313,8 @@ class Command(NoArgsCommand):
                 msg = EmailMessage(subject, text, settings.DEFAULT_FROM_EMAIL, [user.email])
                 msg.content_subtype = 'html'
                 msg.send()
+                #uncomment lines below to get copies of emails sent to others
+                #todo: maybe some debug setting would be appropriate here
+                #msg2 = EmailMessage(subject, text, settings.DEFAULT_FROM_EMAIL, ['your@email.com'])
+                #msg2.content_subtype = 'html'
+                #msg2.send()