launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2019-07-01 15:00:27 +0000
@@ -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 +0000
+++ lib/lp/bugs/model/tests/test_bugtasksearch.py	2019-07-01 15:00:27 +0000
@@ -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 test bug, a search for bugs having
+        # comments from this person returns only the second bug.
+        commenter = self.bugtasks[0].bug.owner
+        expected = self.bugtasks[1]
+        with person_logged_in(commenter):
+            with notify_modified(expected.bug, ['title']):
+                expected.bug.title = 'some modified title'
+        params = self.getBugTaskSearchParams(
+            user=None, bug_commenter=commenter)
+        self.assertSearchFinds(params, [expected])
+
     def test_search_by_person_affected_by_bug(self):
         # Search results can be limited to bugs which affect a given person.
         affected_user = self.factory.makePerson()