26074 - Ability to limit search by number of Comments

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement, P3)

Description

[Sean Richardson]

Requested: a way to limit searches to just those bug reports to which no
additional comments have been made since they were reported.

The bugs that most need attention on the "Browser-General" list are those
that have been in the database for some time but nobody has added any comments
to yet. Among the bugs one or more weeks old, most have had some comment added,
so at present there is no good way to zero in on these bugs.

Now that boolean charts have been added, it would always be possible to
AND a search with the bugs that do not contain the string
"-- Additional Comments From", but that would be brutally expensive.

Since it probably isn't going to ever be necessary to know the number of
comments added, this would amount to adding a boolean field, set to 0 when a 
bug is created, and set to 1 when a comment is added.

Enabling this would allow both those prescreening older "Browser-General" bugs 
and those prescreening recent bugs across all components to concentrate on
those not already touched, reducing duplication of effort.

Comment 1

[Terry Weissman]

> Now that boolean charts have been added, it would always be possible to
> AND a search with the bugs that do not contain the string
> "-- Additional Comments From", but that would be brutally expensive.

Actually, that won't work.  (It would have at one time, but no longer!)  That
string is not actually in the database; it is created by the web pages on the
fly.

Determining the number of comments is now easy.  I could add an entry in the
boolean chart that read "number of comments on the bug", and you could query for
that being <= 1.  (The initial description counts as a comment, as far as the
underlying DB is concerned.)

Note that there are also ways of querying for "bugs that haven't been touched
for a week".

Comment 2

[Sean Richardson]

Your proposed fix sounds great, and would definitely handle the case of
"bugs that haven't been commented upon ever".

I don't suppose the reporter's original comment ("Description" on enter_bug.cgi)
could be number 0 instead of number 1? Looking at a show_bug.cgi page, the 
reporter's original comments are under "Description", but the rest of the 
comments are under an "------- Additional Comments From" line, which gives the
prima facie appearance that they are in a different category, especially
since the reporter's comment was labelled "Description" on enter_bug.cgi too. 
And I know you wouldn't want to change Description to Comments in both of those
cgi scripts for consistency with the underlying data model ;-0

This would allow a more natural sounding query like "number of comments
equal to zero". I'll understand if you don't want to do this, though.

Comment 3

[Sean Richardson]

Reading bug 21080, "Need query for activity on bugs", FIXED by the boolean
charts, I was about to say that this bug report is almost superfluous, 
but since Jan Leger took all of the bugs assigned to "[e-mail]"
today there are no bugs on the "Browser-General" list that would show as
inactive for more than a day now. 

This type of search would still be useful for the task of finding bugs
with no comments, the number would just have to be upped by one to adjust for
the new comment explaining the change of ownership on all those bugs, while
the "bugs that haven't been touched for a week" query would now fail utterly.

Comment 4

[Terry Weissman]

[e-mail] is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Comment 5

[Dave Miller [:justdave]]

moving to real milestones...

Comment 6

[Andreas Franke (gone)]

-> Bugzilla product, Query component, reassigning.

Comment 7

[Frédéric Buclin]

The request is very simple. For example:

SELECT bug_id, count(*) FROM longdescs GROUP BY bug_id HAVING count(*)<2;

gives all bugs having less than two comments.

But implementing this in Bugzilla/Search.pm is by far less easy. Is someone
familiar with this file?

Comment 8

[Joel Peshkin]

It is now possible to search for a boolean chart like...
Add a boolean chart for "Commenter" "is not equal to" "%reporter%"
which would restrict a search to bugs where someone other than the reporter has
commented.

Does that impact the need for this feature?

Comment 9

[Frédéric Buclin]

I don't really care about this bug anymore... reassigning to default owner.

Comment 10

[Max Kanat-Alexander]

Attachment: Work In Progress

Here's a patch that adds comment counts to both search criteria and the buglist. However, it currently has really miserable performance, and I'm not quite sure how to get around that. We can't really use a HAVING clause in the main SQL, because that breaks boolean charts. The SQL I have in this patch is logically sound and does not break boolean charts, but it is miserably slow on a large installation (so slow that your searches will effectively never return).

Comment 11

[Wayne Mery (:wsmwk)]

i'd be happy if it could special case detecting just 0 comments or no comments other than reporters in a performant manner.

Comment 12

[Max Kanat-Alexander]

(In reply to comment #11)
> i'd be happy if it could special case detecting just 0 comments or no comments
> other than reporters in a performant manner.

  Yeah, that'd be useful, but even if we do that I need some framework to make it clear that certain types of searches can only be done under certain circumstances, or we technically are just adding more bugs to the search interface (and believe me, it doesn't need any more of those).

Comment 13

[Max Kanat-Alexander]

Okay, I found something that may work. This SQL returns very quickly:

SELECT totals.bug_id FROM (SELECT bug_id, COUNT(*) AS comment_count FROM longdescs GROUP BY bug_id) AS totals WHERE totals.comment_count > 100

So either we can put that into a view or we can use it as a table in the query.

Comment 14

[Max Kanat-Alexander]

Attachment: v1

Okay, that SQL seems to work pretty nicely! A lot of this patch involves updating the xt/search.t tests to also test the longdescs.count field, which is our first count field.

This is breaking the percentage_complete xt/search.t tests, though, and I think that it has something more to do with the tests than with the actual functionality (although I do believe it is exposing a bug with percentage_complete...which is totally unsurprising).

Comment 15

[Max Kanat-Alexander]

Attachment: v2

Okay, upon testing, this is still relatively slow on my local copy of the bmo dataset, but not so slow that it will never return. (Also, my local copy of the bmo dataset is generally slow because my machine's hardware is not as nice as bmo's.)

I'm not aware of any way that this could get faster, other than us deciding to start caching comment counts in the bugs table, and I don't want to start adding database-side caches back to Bugzilla.

This patch fixes an error with the isprivate stuff in the last patch.

Comment 16

[Max Kanat-Alexander]

Attachment: v3

Here we go! This one now fully passes all of the xt/search.t tests. (Mostly the difference between this one and v2 is just fixes for the tests.)

Comment 17

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Comment.pm
modified Bugzilla/Field.pm
modified Bugzilla/Search.pm
modified template/en/default/global/field-descs.none.tmpl
modified template/en/default/list/table.html.tmpl
modified xt/lib/Bugzilla/Test/Search.pm
modified xt/lib/Bugzilla/Test/Search/Constants.pm
modified xt/lib/Bugzilla/Test/Search/FieldTest.pm
modified xt/lib/Bugzilla/Test/Search/InjectionTest.pm
Committed revision 7521.

Comment 18

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/15424de08cad
[fenix] For https://github.com/mozilla-mobile/fenix/issues/26074 fix failing settingsTest screen shot UI test