Simone Pelosi has proposed merging ~pelpsi/launchpad:muted-subscription-bug into launchpad:master.
Commit message: Added new logic to filter BugNotificationRecipient filters_by_person (dictionary) is the new data structure introduced: key is the person_id and the value is a set of tuple with the following format (BugNotificationRecipe, filter_id). Thanks to that structure we can filter the muted notifications simply by removing them from the filters_by_person[person_id] set. The remaining notifications will be sent to the user. LP: #2019428 Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #2019428 in Launchpad itself: "Not receiving (proper) emails for packages I'm subscribed to" https://bugs.launchpad.net/launchpad/+bug/2019428 For more details, see: https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/442903 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:muted-subscription-bug into launchpad:master.
diff --git a/lib/lp/bugs/model/bugnotification.py b/lib/lp/bugs/model/bugnotification.py index 9634c6a..1500351 100644 --- a/lib/lp/bugs/model/bugnotification.py +++ b/lib/lp/bugs/model/bugnotification.py @@ -10,6 +10,7 @@ __all__ = [ "BugNotificationSet", ] +from collections import defaultdict from datetime import datetime, timedelta, timezone from storm.expr import In, Join, LeftJoin @@ -255,6 +256,8 @@ class BugNotificationSet: # First we get some intermediate data structures set up. source_person_id_map = {} recipient_id_map = {} + notifications_by_source = defaultdict(set) + for recipient, sources in recipient_to_sources.items(): if recipient.id in muted_person_ids: continue @@ -279,10 +282,11 @@ class BugNotificationSet: } source_person_id_map[person_id] = data data["sources"].add(source) + notifications_by_source[person_id].add(source) # Now we actually look for the filters. store = IStore(BugSubscriptionFilter) source = store.using( - BugSubscriptionFilter, + (BugSubscriptionFilter,), Join( BugNotificationFilter, BugSubscriptionFilter.id @@ -312,25 +316,49 @@ class BugNotificationSet: list(source_person_id_map), ), ) + filter_ids = [] + no_filter_marker = -1 + + # Associate filters to each notification to facilitate search + filters_by_person = defaultdict(set) # Record the filters for each source. + source_filter_ids = set() for source_person_id, filter_id, filter_description in filter_data: + source_filter_ids.add(source_person_id) source_person_id_map[source_person_id]["filters"][ filter_id ] = filter_description filter_ids.append(filter_id) + filters_by_person[source_person_id] = { + (i, filter_id) + for i in notifications_by_source[source_person_id] + } + + for id in source_filter_ids: + del notifications_by_source[id] + + # Remaining notifications have no filters + for key in notifications_by_source: + filters_by_person[key] = { + (i, no_filter_marker) for i in notifications_by_source[key] + } # This is only necessary while production and sample data have # structural subscriptions without filters. Assign the filters to # each recipient. - no_filter_marker = -1 - for recipient_data in recipient_id_map.values(): for source_person_id in recipient_data["source person ids"]: recipient_data["filters"].update( source_person_id_map[source_person_id]["filters"] or {no_filter_marker: None} ) + + filters_by_person[ + recipient_data["principal"].id + ] = filters_by_person[recipient_data["principal"].id].union( + filters_by_person[source_person_id] + ) if filter_ids: # Now we get the information about subscriptions that might be # filtered and take that into account. @@ -355,19 +383,37 @@ class BugNotificationSet: del recipient_id_map[person_id]["filters"][ no_filter_marker ] + + # Remove notification if it's muted + filtered_set = set() + for tuple in filters_by_person[person_id]: + if tuple[1] != filter_id: + filtered_set.add(tuple) + filters_by_person[person_id] = filtered_set + # Now recipient_id_map has all the information we need. Let's # build the final result and return it. result = {} for recipient_data in recipient_id_map.values(): + # Getting filtered sources + if recipient_data["filters"]: filter_descriptions = [ description for description in recipient_data["filters"].values() if description ] - filter_descriptions.sort() # This is good for tests. + filtered_source = [] + for sources in filters_by_person[ + recipient_data["principal"].id + ]: + filtered_source.append(sources[0]) + + # This is good for tests. + filtered_source.reverse() + filter_descriptions.sort() result[recipient_data["principal"]] = { - "sources": recipient_data["sources"], + "sources": filtered_source, "filter descriptions": filter_descriptions, } return result diff --git a/lib/lp/bugs/scripts/tests/test_bugnotification.py b/lib/lp/bugs/scripts/tests/test_bugnotification.py index 9dc39fb..cf65f83 100644 --- a/lib/lp/bugs/scripts/tests/test_bugnotification.py +++ b/lib/lp/bugs/scripts/tests/test_bugnotification.py @@ -1511,3 +1511,62 @@ class TestSendBugNotifications(TestCaseWithFactory): self.assertEqual( BugNotificationStatus.SENT, bug_notification.status ) + + def test_muted_team_subscription(self): + # If a user holds a mute on a Team subscription, + # having a personal subscription for that bug + # should receive only notifications related to that. + + team_owner = self.factory.makePerson( + name="team-owner", email="team-ow...@canonical.com" + ) + team_participant = self.factory.makePerson( + name="team-participant", email="team-particip...@canonical.com" + ) + + team = self.factory.makeTeam( + name="team-name", owner=team_owner, members=[team_participant] + ) + bug = self.factory.makeBug() + + subscription = bug.default_bugtask.target.addBugSubscription( + team, team_owner + ) + # Take team subscription's filter + subscription_filter = subscription.bug_filters.one() + bug.default_bugtask.target.addBugSubscription( + team_participant, team_participant + ) + + # Mute team subscription for team_participant + + message = getUtility(IMessageSet).fromText( + "subject", + "a comment.", + bug.owner, + datecreated=self.ten_minutes_ago, + ) + notification = bug.addCommentNotification(message) + + BugSubscriptionFilterMute( + person=team_participant, + filter=subscription_filter, + ) + + notification = self.notification_set.getNotificationsToSend() + + pending_notification = get_email_notifications(notification) + # Since team subscription is muted for team_participant + # we expect two messages: one for the team member (owner) + # and the other for the personal subscription. + # team-ow...@canonical.com team-name-100000 + # team-particip...@canonical.com team-participant + + _, _, messages = list(pending_notification)[0] + + self.assertEqual("team-ow...@canonical.com", messages[0]["To"]) + self.assertEqual("team-particip...@canonical.com", messages[1]["To"]) + self.assertEqual("team-name", messages[0]["X-Launchpad-Message-For"]) + self.assertEqual( + "team-participant", messages[1]["X-Launchpad-Message-For"] + )
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp