Merge lp:~wgrant/launchpad/bug-869631 into lp:launchpad

Proposed by William Grant on 2012-03-07
Status: Merged
Approved by: William Grant on 2012-03-07
Approved revision: no longer in the source branch.
Merged at revision: 14913
Proposed branch: lp:~wgrant/launchpad/bug-869631
Merge into: lp:launchpad
Diff against target: 44 lines (+17/-3)
2 files modified
lib/lp/bugs/model/bug.py (+5/-2)
lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py (+12/-1)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-869631
Reviewer Review Type Date Requested Status
Steve Kowalik (community) code 2012-03-07 Approve on 2012-03-07
Review via email: mp+96280@code.launchpad.net

Commit Message

[r=stevenk][bug=869631] Fix Bug.duplicate_subscriptions to restrict the mute search to the dupe bug.

Description of the Change

This branch fixes a bug subscription query to be correct and a thousand times faster. Bug.duplicate_subscriptions has a BugMute subquery to attempt to filter out muted subscriptions, but the query ends up uncorrelated, so a single subscription to any bug erroneously causes any of the user's dupe subscriptions to be muted. This also causes seq scans over a few tables, making things terribly slow.

Adding tables=[BugMute] changes it to the intended correlated subquery -- fast and correct. I added a test for this behaviour, and changed an existing test which relied on the broken one.

To post a comment you must log in.
Steve Kowalik (stevenk) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/model/bug.py'
2--- lib/lp/bugs/model/bug.py 2012-02-20 22:21:36 +0000
3+++ lib/lp/bugs/model/bug.py 2012-03-07 02:08:26 +0000
4@@ -2613,8 +2613,11 @@
5 BugSubscription.bug_notification_level >= self.level,
6 BugSubscription.bug_id == Bug.id,
7 Bug.duplicateof == self.bug,
8- Not(In(BugSubscription.person_id,
9- Select(BugMute.person_id, BugMute.bug_id == Bug.id))))
10+ Not(In(
11+ BugSubscription.person_id,
12+ Select(
13+ BugMute.person_id, BugMute.bug_id == Bug.id,
14+ tables=[BugMute]))))
15
16 @property
17 def duplicate_subscribers(self):
18
19=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py'
20--- lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py 2012-01-04 17:21:01 +0000
21+++ lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py 2012-03-07 02:08:26 +0000
22@@ -258,10 +258,21 @@
23 duplicate_bug, duplicate_bug_subscription = (
24 self._create_duplicate_subscription())
25 with person_logged_in(duplicate_bug.owner):
26- self.bug.mute(duplicate_bug.owner, duplicate_bug.owner)
27+ duplicate_bug.mute(duplicate_bug.owner, duplicate_bug.owner)
28 found_subscriptions = self.getInfo().duplicate_subscriptions
29 self.assertContentEqual([], found_subscriptions)
30
31+ def test_duplicate_other_mute(self):
32+ # If some other bug is muted, the dupe is still listed.
33+ duplicate_bug, duplicate_bug_subscription = (
34+ self._create_duplicate_subscription())
35+ with person_logged_in(duplicate_bug.owner):
36+ self.factory.makeBug().mute(
37+ duplicate_bug.owner, duplicate_bug.owner)
38+ found_subscriptions = self.getInfo().duplicate_subscriptions
39+ self.assertContentEqual(
40+ [duplicate_bug_subscription], found_subscriptions)
41+
42 def test_duplicate_for_private_bug(self):
43 # The set of subscribers from duplicate bugs is always empty when the
44 # master bug is private.