[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/improve-bug-commenter-search into lp:launchpad

[noreply]

The proposal to merge lp:~cjwatson/launchpad/improve-bug-commenter-search into 
lp:launchpad has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/improve-bug-commenter-search/+merge/369526
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
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

Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/improve-bug-commenter-search into lp:launchpad

[Colin Watson]

~cjwatson, old query for +commentedbugs with default options:

launchpad_staging=> EXPLAIN (ANALYZE, BUFFERS) SELECT BugTaskFlat.bugtask FROM 
BugTaskFlat LEFT JOIN Product ON BugTaskFlat.product = Product.id AND 
Product.active WHERE BugTaskFlat.status IN (25, 10, 20, 21, 22, 13, 14) AND 
BugTaskFlat.duplicateof IS NULL AND (BugTaskFlat.product IS NULL OR 
Product.active = true) AND BugTaskFlat.bug IN (SELECT BugMessage.bug FROM 
BugMessage WHERE BugMessage.index > 0 AND BugMessage.owner = 91) AND 
(BugTaskFlat.information_type IN (1, 2) OR COALESCE((BugTaskFlat.access_grants) 
&& (SELECT ARRAY_AGG(TeamParticipation.team) FROM TeamParticipation WHERE 
TeamParticipation.person = 91), false) OR 
COALESCE((BugTaskFlat.access_policies) && (SELECT 
ARRAY_AGG(AccessPolicyGrant.policy) FROM AccessPolicyGrant JOIN 
TeamParticipation ON TeamParticipation.team = AccessPolicyGrant.grantee WHERE 
TeamParticipation.person = 91), false)) ORDER BY BugTaskFlat.importance DESC, 
BugTaskFlat.bugtask LIMIT 76 OFFSET 0;

   QUERY PLAN   
  

 Limit  (cost=1768.60..7050.15 rows=76 width=8) (actual time=12.241..594.731 
rows=76 loops=1)
   Buffers: shared hit=122385
   InitPlan 1 (returns $0)
 ->  Aggregate  (cost=62.92..62.93 rows=1 width=32) (actual 
time=0.328..0.328 rows=1 loops=1)
   Buffers: shared hit=171
   ->  Index Scan using teamparticipation_person_idx on 
teamparticipation  (cost=0.43..62.18 rows=293 width=4) (actual 
time=0.022..0.255 rows=330 loops=1)
 Index Cond: (person = 91)
 Buffers: shared hit=171
   InitPlan 2 (returns $2)
 ->  Aggregate  (cost=1704.51..1704.52 rows=1 width=32) (actual 
time=11.016..11.016 rows=1 loops=1)
   Buffers: shared hit=5208
   ->  Nested Loop  (cost=0.85..1704.32 rows=75 width=4) (actual 
time=0.035..8.835 rows=13483 loops=1)
 Buffers: shared hit=5208
 ->  Index Scan using teamparticipation_person_idx on 
teamparticipation teamparticipation_1  (cost=0.43..62.18 rows=293 width=4) 
(actual time=0.014..0.167 rows=330 loops=1)
   Index Cond: (person = 91)
   Buffers: shared hit=171
 ->  Index Only Scan using 
accesspolicygrant__grantee__policy__key on accesspolicygrant  (cost=0.42..5.56 
rows=4 width=8) (actual time=0.003..0.021 rows=41 loops=330)
   Index Cond: (grantee = teamparticipation_1.team)
   Heap Fetches: 13483
   Buffers: shared hit=5037
   ->  Nested Loop Left Join  (cost=1.16..1079174.95 rows=15529 width=8) 
(actual time=12.240..594.708 rows=76 loops=1)
 Filter: ((bugtaskflat.product IS NULL) OR product.active)
 Buffers: shared hit=122385
 ->  Nested Loop Semi Join  (cost=0.86..1073265.94 rows=17761 width=12) 
(actual time=12.237..594.513 rows=76 loops=1)
   Buffers: shared hit=122313
   ->  Index Scan Backward using 
bugtaskflat__importance__bugtask__idx on bugtaskflat  (cost=0.43..490403.01 
rows=647482 width=16) (actual time=11.641..533.065 rows=4968 loops=1)
 Filter: ((duplicateof IS NULL) AND ((information_type = 
ANY ('{1,2}'::integer[])) OR COALESCE((access_grants && $0), false) OR 
COALESCE((access_policies && $2), false)) AND (status = ANY 
('{25,10,20,21,22,13,14}'::integer[])))
 Rows Removed by Filter: 56834
 Buffers: shared hit=66461
   ->  Index Scan using bugmessage__bug__index__key on bugmessage  
(cost=0.43..0.90 rows=1 width=4) (actual time=0.012..0.012 rows=0 loops=4968)
 Index Cond: ((bug = bugtaskflat.bug) AND (index > 0))
 Filter: (owner = 91)
 Rows Removed by Filter: 9
 Buffers: shared hit=55852
 ->  Index Scan using product_pkey on product  (cost=0.29..0.32 rows=1 
width=5) (actual time=0.002..0.002 rows=0 loops=76)
   Index Cond: (bugtaskflat.product = id)
   Filter: active
   Buffers: shared hit=72
 Planning time: 14.577 ms
 Execution time: 594.892 ms
(40 rows)

~cjwatson, new query for +commentedbugs with default options:

launchpad_staging=> EXPLAIN (ANALYZE, BUFFERS) SELECT BugTaskFlat.bugtask FROM 
BugTaskFlat LEFT JOIN Product ON BugTaskFlat.product = Product.id AND 
Product.active WHERE BugTaskFlat.status IN (25, 10, 20, 21, 22, 13, 14) AND 
BugTaskFlat.duplicateof IS NULL AND (BugTaskFlat.product IS NULL OR 
Product.active = true) AND BugTaskFlat.bug IN (SELECT 

Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/improve-bug-commenter-search into lp:launchpad

[William Grant]

Review: Approve code

I'd like to see a query plan from a couple of representative users, but it was 
already pretty bad and timed out for heavy users I suppose.
-- 
https://code.launchpad.net/~cjwatson/launchpad/improve-bug-commenter-search/+merge/369526
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
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

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/improve-bug-commenter-search into lp:launchpad

[Colin Watson]

Colin Watson has proposed merging 
lp:~cjwatson/launchpad/improve-bug-commenter-search into lp:launchpad.

Commit message:
Include metadata-only bug changes in Person:+commentedbugs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/improve-bug-commenter-search/+merge/369526

Some kinds of changes that users can make to bugs are currently invisible to 
all bug searches: if a user just changes some bug metadata (title, status, 
etc.) then that can't be searched for, because it isn't internally represented 
as a comment.  On the other hand, bug web pages render metadata changes in a 
way that looks somewhat similar to comments, so this is a bit confusing.  It's 
also a problem when dealing with certain kinds of abuse.

I think the simplest option is to just include these kinds of changes under 
"Commented bugs" (and hence also under "Related bugs").  There may be some 
cases where that's a little surprising, but I think they'll be outweighed by 
the added usefulness.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/improve-bug-commenter-search into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2017-06-14 02:56:41 +
+++ lib/lp/bugs/model/bugtasksearch.py	2019-07-01 15:00:27 +
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -57,6 +57,7 @@
 BugAffectsPerson,
 BugTag,
 )
+from lp.bugs.model.bugactivity import BugActivity
 from lp.bugs.model.bugattachment import BugAttachment
 from lp.bugs.model.bugbranch import BugBranch
 from lp.bugs.model.bugmessage import BugMessage
@@ -612,11 +613,22 @@
 
 if params.bug_commenter:
 extra_clauses.append(
-BugTaskFlat.bug_id.is_in(Select(
-BugMessage.bugID, tables=[BugMessage],
-where=And(
-BugMessage.index > 0,
-BugMessage.owner == params.bug_commenter
+BugTaskFlat.bug_id.is_in(Union(
+Select(
+BugMessage.bugID, tables=[BugMessage],
+where=And(
+BugMessage.index > 0,
+BugMessage.owner == params.bug_commenter)),
+Select(
+BugActivity.bugID, tables=[BugActivity],
+where=And(
+BugActivity.person == params.bug_commenter,
+# This is distressingly fragile, but BugActivity
+# doesn't really give us any better way to exclude
+# the bug creation event.
+Or(
+BugActivity.whatchanged != u'bug',
+BugActivity.message != u'added bug'))
 
 if params.affects_me:
 params.affected_user = params.user

=== modified file 'lib/lp/bugs/model/tests/test_bugtasksearch.py'
--- lib/lp/bugs/model/tests/test_bugtasksearch.py	2016-06-24 23:35:24 +
+++ lib/lp/bugs/model/tests/test_bugtasksearch.py	2019-07-01 15:00:27 +
@@ -1,4 +1,4 @@
-# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -73,6 +73,7 @@
 greater_than,
 not_equals,
 )
+from lp.services.webapp.snapshot import notify_modified
 from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
@@ -217,7 +218,7 @@
 # Note that this does not include the bug description (which is
 # stored as the first comment of a bug.) Hence, if we let the
 # reporter of our first test bug comment on the second test bug,
-# a search for bugs having comments from this person retruns only
+# a search for bugs having comments from this person returns only
 # the second bug.
 commenter = self.bugtasks[0].bug.owner
 expected = self.bugtasks[1]
@@ -227,6 +228,23 @@
 user=None, bug_commenter=commenter)
 self.assertSearchFinds(params, [expected])
 
+def test_search_by_bug_commenter_finds_activity(self):
+# Searching by bug commenter also includes bugs where the given
+# person only made a change to the bug's metadata but did not leave
+# an actual comment.
+# Note that this does not include the activity log entry created
+# when the bug is added. Hence, if we let the reporter of our first
+# test bug change the second