From 4c2793eba611c839b055e31c041ab276bf388ab0 Mon Sep 17 00:00:00 2001 From: Evgeny Fadeev Date: Thu, 9 Aug 2012 10:35:58 -0400 Subject: fixed filtering of notifications by the user groups --- askbot/models/post.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/askbot/models/post.py b/askbot/models/post.py index e72ff5e4..974d923b 100644 --- a/askbot/models/post.py +++ b/askbot/models/post.py @@ -25,6 +25,7 @@ import askbot from askbot.utils.slug import slugify from askbot import const from askbot.models.user import EmailFeedSetting +from askbot.models.user import GroupMembership from askbot.models.tag import Tag, MarkedTag from askbot.models.tag import get_groups, tags_match_some_wildcard from askbot.models.tag import get_global_group @@ -650,15 +651,6 @@ class Post(models.Model): raise NotImplementedError - def get_authorized_group_ids(self): - """returns values list query set for the group id's of the post""" - if self.is_comment(): - return self.parent.get_authorized_group_ids() - elif self.is_answer() or self.is_question(): - return self.groups.values_list('id', flat = True) - else: - raise NotImplementedError() - def delete(self, **kwargs): """deletes comment and concomitant response activity records, as well as mention records, while preserving @@ -726,21 +718,29 @@ class Post(models.Model): if askbot_settings.GROUPS_ENABLED == False: return candidates else: - #here we filter candidates against the post authorized groups - authorized_group_ids = list(self.get_authorized_group_ids()) - - if len(authorized_group_ids) == 0:#if there are no groups - all ok + if len(candidates) == 0: return candidates - - #and filter the users by those groups - filtered_candidates = list() + #get post groups + groups = list(self.groups.all()) + + if len(groups) == 0: + logging.critical('post %d is groupless' % self.id) + return list() + + #load group memberships for the candidates + memberships = GroupMembership.objects.filter( + user__in=candidates, + group__in=groups + ) + user_ids = set(memberships.values_list('user__id', flat=True)) + + #scan through the user ids and see which are group members + filtered_candidates = set() for candidate in candidates: - c_groups = candidate.get_groups() - if c_groups.filter(id__in = authorized_group_ids).count(): - filtered_candidates.append(candidate) + if candidate.id in user_ids: + filtered_candidates.add(candidate) return filtered_candidates - def format_for_email( self, quote_level = 0, is_leaf_post = False, format = None -- cgit v1.2.3-1-g7c22 From aeffec86e7cb4c65b26767afb6efc05bf4e261d5 Mon Sep 17 00:00:00 2001 From: Evgeny Fadeev Date: Thu, 9 Aug 2012 23:53:06 -0400 Subject: refactored the notification issuing function on the post --- askbot/conf/minimum_reputation.py | 14 ++++- askbot/models/post.py | 113 ++++++++++++++++++++++++++++++--- askbot/tasks.py | 128 ++++++-------------------------------- 3 files changed, 135 insertions(+), 120 deletions(-) diff --git a/askbot/conf/minimum_reputation.py b/askbot/conf/minimum_reputation.py index 54ba0f65..2c3a3496 100644 --- a/askbot/conf/minimum_reputation.py +++ b/askbot/conf/minimum_reputation.py @@ -181,7 +181,6 @@ settings.register( ) ) - settings.register( livesettings.IntegerValue( MIN_REP, @@ -190,3 +189,16 @@ settings.register( description=_('Post answers and comments by email') ) ) + +settings.register( + livesettings.IntegerValue( + MIN_REP, + 'MIN_REP_TO_TRIGGER_EMAIL', + default=15, + description=_('Trigger email notifications'), + help_text=_( + 'Reduces spam as notifications wont\'t be sent ' + 'to regular users for posts of low karma users' + ) + ) +) diff --git a/askbot/models/post.py b/askbot/models/post.py index 974d923b..d54f7092 100644 --- a/askbot/models/post.py +++ b/askbot/models/post.py @@ -24,6 +24,7 @@ import askbot from askbot.utils.slug import slugify from askbot import const +from askbot.models.user import Activity from askbot.models.user import EmailFeedSetting from askbot.models.user import GroupMembership from askbot.models.tag import Tag, MarkedTag @@ -577,6 +578,77 @@ class Post(models.Model): tag__in=groups ).delete() + + def issue_update_notifications( + self, + updated_by=None, + notify_sets=None, + timestamp=None, + created=False, + diff=None + ): + """Called when a post is updated. Arguments: + + * ``notify_sets`` - result of ``Post.get_notify_sets()`` method + * ``created`` - a boolean. True when ``post`` has just been created + * remaining arguments are self - explanatory + + The method does two things: + + * records "red envelope" recipients of the post + * sends email alerts to all subscribers to the post + """ + #todo: take into account created == True case + (activity_type, update_object) = self.get_updated_activity_data(created) + + if self.is_comment(): + #it's just a comment! + summary = self.text + else: + #summary = post.get_latest_revision().summary + summary = diff + + update_activity = Activity( + user = updated_by, + active_at = timestamp, + content_object = self, + activity_type = activity_type, + question = self.get_origin_post(), + summary = summary + ) + update_activity.save() + + update_activity.add_recipients(notify_sets['for_inbox']) + + #create new mentions (barring the double-adds) + for u in notify_sets['for_mentions'] - notify_sets['for_inbox']: + Activity.objects.create_new_mention( + mentioned_whom = u, + mentioned_in = self, + mentioned_by = updated_by, + mentioned_at = timestamp + ) + + for user in (notify_sets['for_inbox'] | notify_sets['for_mentions']): + user.update_response_counts() + + #shortcircuit if the email alerts are disabled + if askbot_settings.ENABLE_EMAIL_ALERTS == False: + return + #todo: fix this temporary spam protection plug + if created and askbot_settings.MIN_REP_TO_TRIGGER_EMAIL: + if not (updated_by.is_administrator() or updated_by.is_moderator()): + if updated_by.reputation < askbot_settings.MIN_REP_TO_TRIGGER_EMAIL: + notify_sets['for_email'] = \ + [u for u in notify_sets['for_email'] if u.is_administrator()] + + from askbot.models import send_instant_notifications_about_activity_in_post + send_instant_notifications_about_activity_in_post( + update_activity=update_activity, + post=self, + recipients=notify_sets['for_email'], + ) + def make_private(self, user, group_id = None): """makes post private within user's groups todo: this is a copy-paste in thread and post @@ -1166,6 +1238,34 @@ class Post(models.Model): # for subscriber in subscribers: return self.filter_authorized_users(subscribers) + def get_notify_sets(self, mentioned_users=None, exclude_list=None): + """returns three lists in a dictionary with keys: + * 'for_inbox' - users for which to add inbox items + * 'for_mentions' - for whom mentions are added + * 'for_email' - to whom email notifications should be sent + """ + result = dict() + result['for_mentions'] = set(mentioned_users) - set(exclude_list) + #what users are included depends on the post type + #for example for question - all Q&A contributors + #are included, for comments only authors of comments and parent + #post are included + result['for_inbox'] = self.get_response_receivers(exclude_list=exclude_list) + + if askbot_settings.ENABLE_EMAIL_ALERTS == False: + result['for_email'] = set() + else: + #todo: weird thing is that only comments need the recipients + #todo: debug these calls and then uncomment in the repo + #argument to this call + result['for_email'] = self.get_instant_notification_subscribers( + potential_subscribers=result['for_inbox'], + mentioned_users=result['for_mentions'], + exclude_list=exclude_list + ) + return result + + def get_latest_revision(self): return self.revisions.order_by('-revised_at')[0] @@ -1708,9 +1808,7 @@ class Post(models.Model): for answer in question.thread.posts.get_answers().all(): recipients.update(answer.get_author_list()) - recipients -= set(exclude_list) - - return list(recipients) + return recipients - set(exclude_list) def _question__get_response_receivers(self, exclude_list = None): """returns list of users who might be interested @@ -1731,8 +1829,7 @@ class Post(models.Model): for a in self.thread.posts.get_answers().all(): recipients.update(a.get_author_list()) - recipients -= set(exclude_list) - return recipients + return recipients - set(exclude_list) def _comment__get_response_receivers(self, exclude_list = None): """Response receivers are commenters of the @@ -1746,9 +1843,7 @@ class Post(models.Model): include_comments = True, ) ) - users -= set(exclude_list) - return list(users) - + return users - set(exclude_list) def get_response_receivers(self, exclude_list = None): """returns a list of response receiving users @@ -1761,7 +1856,7 @@ class Post(models.Model): elif self.is_comment(): receivers = self._comment__get_response_receivers(exclude_list) elif self.is_tag_wiki() or self.is_reject_reason(): - return list()#todo: who should get these? + return set()#todo: who should get these? else: raise NotImplementedError diff --git a/askbot/tasks.py b/askbot/tasks.py index da07477b..d4c3b676 100644 --- a/askbot/tasks.py +++ b/askbot/tasks.py @@ -27,8 +27,7 @@ from celery.decorators import task from askbot.conf import settings as askbot_settings from askbot import const from askbot import mail -from askbot.models import Activity, Post, Thread, User, ReplyAddress -from askbot.models import send_instant_notifications_about_activity_in_post +from askbot.models import Post, Thread, User, ReplyAddress from askbot.models.badges import award_badges_signal # TODO: Make exceptions raised inside record_post_update_celery_task() ... @@ -102,122 +101,31 @@ def record_post_update_celery_task( diff = None, ): #reconstitute objects from the database - updated_by = User.objects.get(id = updated_by_id) - post_content_type = ContentType.objects.get(id = post_content_type_id) - post = post_content_type.get_object_for_this_type(id = post_id) + updated_by = User.objects.get(id=updated_by_id) + post_content_type = ContentType.objects.get(id=post_content_type_id) + post = post_content_type.get_object_for_this_type(id=post_id) newly_mentioned_users = User.objects.filter( - id__in = newly_mentioned_user_id_list + id__in=newly_mentioned_user_id_list ) try: - record_post_update( - post = post, - updated_by = updated_by, - newly_mentioned_users = newly_mentioned_users, - timestamp = timestamp, - created = created, - diff = diff + notify_sets = post.get_notify_sets( + mentioned_users=newly_mentioned_users, + exclude_list=[updated_by,] + ) + post.issue_update_notifications( + updated_by=updated_by, + notify_sets=notify_sets, + timestamp=timestamp, + created=created, + diff=diff ) except Exception: - # HACK: exceptions from Celery job don;t propagate upwards to Django test runner - # so at least le't sprint tracebacks + # HACK: exceptions from Celery job don't propagate upwards + # to the Django test runner + # so at least let's print tracebacks print >>sys.stderr, traceback.format_exc() raise -def record_post_update( - post = None, - updated_by = None, - newly_mentioned_users = None, - timestamp = None, - created = False, - diff = None - ): - """Called when a post is updated. Arguments: - - * ``newly_mentioned_users`` - users who are mentioned in the - post for the first time - * ``created`` - a boolean. True when ``post`` has just been created - * remaining arguments are self - explanatory - - The method does two things: - - * records "red envelope" recipients of the post - * sends email alerts to all subscribers to the post - """ - #todo: take into account created == True case - (activity_type, update_object) = post.get_updated_activity_data(created) - - if post.is_comment(): - #it's just a comment! - summary = post.text - else: - #summary = post.get_latest_revision().summary - summary = diff - - update_activity = Activity( - user = updated_by, - active_at = timestamp, - content_object = post, - activity_type = activity_type, - question = post.get_origin_post(), - summary = summary - ) - update_activity.save() - - #what users are included depends on the post type - #for example for question - all Q&A contributors - #are included, for comments only authors of comments and parent - #post are included - recipients = post.get_response_receivers( - exclude_list = [updated_by, ] - ) - - update_activity.add_recipients(recipients) - - #create new mentions - for u in newly_mentioned_users: - #todo: a hack - some users will not have record of a mention - #may need to fix this in the future. Added this so that - #recipients of the response who are mentioned as well would - #not get two notifications in the inbox for the same post - if u in recipients: - continue - Activity.objects.create_new_mention( - mentioned_whom = u, - mentioned_in = post, - mentioned_by = updated_by, - mentioned_at = timestamp - ) - - assert(updated_by not in recipients) - - for user in (set(recipients) | set(newly_mentioned_users)): - user.update_response_counts() - - #shortcircuit if the email alerts are disabled - if askbot_settings.ENABLE_EMAIL_ALERTS == False: - return - - #todo: weird thing is that only comments need the recipients - #todo: debug these calls and then uncomment in the repo - #argument to this call - notification_subscribers = post.get_instant_notification_subscribers( - potential_subscribers = recipients, - mentioned_users = newly_mentioned_users, - exclude_list = [updated_by, ] - ) - #todo: fix this temporary spam protection plug - if created: - if not (updated_by.is_administrator() or updated_by.is_moderator()): - if updated_by.reputation < 15: - notification_subscribers = \ - [u for u in notification_subscribers if u.is_administrator()] - send_instant_notifications_about_activity_in_post( - update_activity = update_activity, - post = post, - recipients = notification_subscribers, - ) - - @task(ignore_result = True) def record_question_visit( question_post = None, -- cgit v1.2.3-1-g7c22 From a54c5f955b704f1c6b8c6a4ffc37c6307043dbfc Mon Sep 17 00:00:00 2001 From: Evgeny Fadeev Date: Fri, 10 Aug 2012 15:03:27 -0400 Subject: made notifications work for the thread sharing --- askbot/const/__init__.py | 20 ++++++----- askbot/models/__init__.py | 8 +++-- askbot/models/post.py | 30 ++++++---------- askbot/models/question.py | 4 +-- .../skins/default/templates/question/sidebar.html | 2 +- askbot/tasks.py | 7 +++- askbot/views/commands.py | 41 ++++++++++++++++++++++ 7 files changed, 78 insertions(+), 34 deletions(-) diff --git a/askbot/const/__init__.py b/askbot/const/__init__.py index 2633055f..037ee379 100644 --- a/askbot/const/__init__.py +++ b/askbot/const/__init__.py @@ -176,6 +176,7 @@ TYPE_ACTIVITY_MODERATED_POST_EDIT = 25 TYPE_ACTIVITY_CREATE_REJECT_REASON = 26 TYPE_ACTIVITY_UPDATE_REJECT_REASON = 27 TYPE_ACTIVITY_VALIDATION_EMAIL_SENT = 28 +TYPE_ACTIVITY_POST_SHARED = 29 #TYPE_ACTIVITY_EDIT_QUESTION = 17 #TYPE_ACTIVITY_EDIT_ANSWER = 18 @@ -199,6 +200,7 @@ TYPE_ACTIVITY = ( (TYPE_ACTIVITY_FAVORITE, _('selected favorite')), (TYPE_ACTIVITY_USER_FULL_UPDATED, _('completed user profile')), (TYPE_ACTIVITY_EMAIL_UPDATE_SENT, _('email update sent to user')), + (TYPE_ACTIVITY_POST_SHARED, _('a post was shared')), ( TYPE_ACTIVITY_UNANSWERED_REMINDER_SENT, _('reminder about unanswered questions sent'), @@ -244,6 +246,7 @@ RESPONSE_ACTIVITY_TYPES_FOR_INSTANT_NOTIFICATIONS = ( TYPE_ACTIVITY_UPDATE_QUESTION, TYPE_ACTIVITY_ANSWER, TYPE_ACTIVITY_ASK_QUESTION, + TYPE_ACTIVITY_POST_SHARED ) @@ -256,6 +259,7 @@ RESPONSE_ACTIVITY_TYPES_FOR_DISPLAY = ( TYPE_ACTIVITY_COMMENT_ANSWER, TYPE_ACTIVITY_UPDATE_ANSWER, TYPE_ACTIVITY_UPDATE_QUESTION, + TYPE_ACTIVITY_POST_SHARED, # TYPE_ACTIVITY_PRIZE, # TYPE_ACTIVITY_MARK_ANSWER, # TYPE_ACTIVITY_VOTE_UP, @@ -267,15 +271,15 @@ RESPONSE_ACTIVITY_TYPES_FOR_DISPLAY = ( # TYPE_ACTIVITY_FAVORITE, ) - RESPONSE_ACTIVITY_TYPE_MAP_FOR_TEMPLATES = { - TYPE_ACTIVITY_COMMENT_QUESTION: 'question_comment', - TYPE_ACTIVITY_COMMENT_ANSWER: 'answer_comment', - TYPE_ACTIVITY_UPDATE_ANSWER: 'answer_update', - TYPE_ACTIVITY_UPDATE_QUESTION: 'question_update', - TYPE_ACTIVITY_ANSWER: 'new_answer', - TYPE_ACTIVITY_ASK_QUESTION: 'new_question', - } + TYPE_ACTIVITY_COMMENT_QUESTION: 'question_comment', + TYPE_ACTIVITY_COMMENT_ANSWER: 'answer_comment', + TYPE_ACTIVITY_UPDATE_ANSWER: 'answer_update', + TYPE_ACTIVITY_UPDATE_QUESTION: 'question_update', + TYPE_ACTIVITY_ANSWER: 'new_answer', + TYPE_ACTIVITY_ASK_QUESTION: 'new_question', + TYPE_ACTIVITY_POST_SHARED: 'post_shared' +} assert( set(RESPONSE_ACTIVITY_TYPES_FOR_INSTANT_NOTIFICATIONS) \ diff --git a/askbot/models/__init__.py b/askbot/models/__init__.py index 5392fc30..27336576 100644 --- a/askbot/models/__init__.py +++ b/askbot/models/__init__.py @@ -2806,6 +2806,8 @@ def format_instant_notification_email( assert(isinstance(post, Post) and post.is_question()) elif update_type == 'new_question': assert(isinstance(post, Post) and post.is_question()) + elif update_type == 'post_shared': + pass else: raise ValueError('unexpected update_type %s' % update_type) @@ -2830,9 +2832,11 @@ def format_instant_notification_email( content_preview += '

======= Full thread summary =======

' - content_preview += post.thread.format_for_email() + content_preview += post.thread.format_for_email(user=to_user) - if post.is_comment(): + if update_type == 'post_shared': + user_action = _('%(user)s shared a %(post_link)s.') + elif post.is_comment(): if update_type.endswith('update'): user_action = _('%(user)s edited a %(post_link)s.') else: diff --git a/askbot/models/post.py b/askbot/models/post.py index d54f7092..8d2d05a4 100644 --- a/askbot/models/post.py +++ b/askbot/models/post.py @@ -583,30 +583,29 @@ class Post(models.Model): self, updated_by=None, notify_sets=None, + activity_type=None, timestamp=None, - created=False, diff=None ): """Called when a post is updated. Arguments: * ``notify_sets`` - result of ``Post.get_notify_sets()`` method - * ``created`` - a boolean. True when ``post`` has just been created - * remaining arguments are self - explanatory The method does two things: * records "red envelope" recipients of the post * sends email alerts to all subscribers to the post """ - #todo: take into account created == True case - (activity_type, update_object) = self.get_updated_activity_data(created) - + assert(activity_type is not None) if self.is_comment(): #it's just a comment! summary = self.text else: #summary = post.get_latest_revision().summary - summary = diff + if diff: + summary = diff + else: + summary = self.text update_activity = Activity( user = updated_by, @@ -636,7 +635,7 @@ class Post(models.Model): if askbot_settings.ENABLE_EMAIL_ALERTS == False: return #todo: fix this temporary spam protection plug - if created and askbot_settings.MIN_REP_TO_TRIGGER_EMAIL: + if askbot_settings.MIN_REP_TO_TRIGGER_EMAIL: if not (updated_by.is_administrator() or updated_by.is_moderator()): if updated_by.reputation < askbot_settings.MIN_REP_TO_TRIGGER_EMAIL: notify_sets['for_email'] = \ @@ -1136,10 +1135,7 @@ class Post(models.Model): #print 'answer subscribers: ', answer_subscribers #print 'exclude_list is ', exclude_list - subscriber_set -= set(exclude_list) - - #print 'final subscriber set is ', subscriber_set - return list(subscriber_set) + return subscriber_set - set(exclude_list) def _comment__get_instant_notification_subscribers( self, @@ -1205,13 +1201,7 @@ class Post(models.Model): subscriber_set.update(global_subscribers) - #print 'exclude list is: ', exclude_list - if exclude_list: - subscriber_set -= set(exclude_list) - - #print 'final list of subscribers:', subscriber_set - - return list(subscriber_set) + return subscriber_set - set(exclude_list) def get_instant_notification_subscribers( self, potential_subscribers = None, @@ -1230,7 +1220,7 @@ class Post(models.Model): exclude_list=exclude_list ) elif self.is_tag_wiki() or self.is_reject_reason(): - return list() + return set() else: raise NotImplementedError diff --git a/askbot/models/question.py b/askbot/models/question.py index 14515b8f..ab3e6959 100644 --- a/askbot/models/question.py +++ b/askbot/models/question.py @@ -560,9 +560,9 @@ class Thread(models.Model): else: return self.title - def format_for_email(self): + def format_for_email(self, user=None): """experimental function: output entire thread for email""" - question, answers, junk = self.get_cached_post_data() + question, answers, junk = self.get_cached_post_data(user=user) output = question.format_for_email_as_subthread() if answers: answer_heading = ungettext( diff --git a/askbot/skins/default/templates/question/sidebar.html b/askbot/skins/default/templates/question/sidebar.html index 6c4643e9..ef99e988 100644 --- a/askbot/skins/default/templates/question/sidebar.html +++ b/askbot/skins/default/templates/question/sidebar.html @@ -84,7 +84,7 @@
{% csrf_token %} diff --git a/askbot/tasks.py b/askbot/tasks.py index d4c3b676..4aa11798 100644 --- a/askbot/tasks.py +++ b/askbot/tasks.py @@ -112,13 +112,18 @@ def record_post_update_celery_task( mentioned_users=newly_mentioned_users, exclude_list=[updated_by,] ) + #todo: take into account created == True case + #update_object is not used + (activity_type, update_object) = post.get_updated_activity_data(created) + post.issue_update_notifications( updated_by=updated_by, notify_sets=notify_sets, + activity_type=activity_type, timestamp=timestamp, - created=created, diff=diff ) + except Exception: # HACK: exceptions from Celery job don't propagate upwards # to the Django test runner diff --git a/askbot/views/commands.py b/askbot/views/commands.py index 316895f7..1b6b21e8 100644 --- a/askbot/views/commands.py +++ b/askbot/views/commands.py @@ -1156,12 +1156,40 @@ def share_question_with_group(request): group_name = form.cleaned_data['recipient_name'] thread = models.Thread.objects.get(id=thread_id) + question_post = thread._question_post() + + #get notif set before + sets1 = question_post.get_notify_sets( + mentioned_users=list(), + exclude_list=[request.user,] + ) + + #share the post if group_name == askbot_settings.GLOBAL_GROUP_NAME: thread.make_public(recursive=True) else: group = models.Tag.group_tags.get(name=group_name) thread.add_to_groups((group,), recursive=True) + #get notif sets after + sets2 = question_post.get_notify_sets( + mentioned_users=list(), + exclude_list=[request.user,] + ) + + notify_sets = { + 'for_mentions': sets2['for_mentions'] - sets1['for_mentions'], + 'for_email': sets2['for_email'] - sets1['for_email'], + 'for_inbox': sets2['for_inbox'] - sets1['for_inbox'] + } + + question_post.issue_update_notifications( + updated_by=request.user, + notify_sets=notify_sets, + activity_type=const.TYPE_ACTIVITY_POST_SHARED, + timestamp=datetime.datetime.now() + ) + return HttpResponseRedirect(thread.get_absolute_url()) except Exception: error_message = _('Sorry, looks like sharing request was invalid') @@ -1181,6 +1209,19 @@ def share_question_with_user(request): user = models.User.objects.get(username=username) group = user.get_personal_group() thread.add_to_groups([group], recursive=True) + #notify the person + #todo: see if user could already see the post - b/f the sharing + notify_sets = { + 'for_inbox': set([user]), + 'for_mentions': set([user]), + 'for_email': set([user]) + } + thread._question_post().issue_update_notifications( + updated_by=request.user, + notify_sets=notify_sets, + activity_type=const.TYPE_ACTIVITY_POST_SHARED, + timestamp=datetime.datetime.now() + ) return HttpResponseRedirect(thread.get_absolute_url()) except Exception: -- cgit v1.2.3-1-g7c22