418068 - No way to search for "date of last comment" before/after a given date

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Currently, there is no way to search for "date of last comment" before/after a given date in bugzilla.


E.g.: Under advanced search with boolean charts, (Unconfirmed, new, assigned, reopened Calendar bugs)

NOT "Comment" -> "changed after" -> 2007-10-25

throws up a search with bug 401034 (at the bottom, correct as of writing) ("is greater than" didn't work as well)

That bug still has comments added after 2007-10-25.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The query would prove useful when searching for old bugs without any recent comment in them; the other option which works is "Last changed date" but that also includes insignificant changes like changes in the CC list.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Similar SQL query requiring a subselect for the required search above (as noted via IRC):

mysql> select bugid from (select bugs.bug_id as bugid, max(longdescs.bug_when) as lastcomment from bugs left join longdescs on bugs.bug_id = longdescs.bug_id where product_id=2 and resolution=” group by bugs.bug_id) as thetable where lastcomment < '2007-10-27';

It would be nice to be able to search this via Bugzilla itself, through the advanced search function.

Comment 3

[Max Kanat-Alexander]

Your query below actually did the right thing, but sometimes NOT is confusing. What happened is that there was *a* comment added before that date, so that matched the NOT condition. At least, I'm pretty sure that's the intended behavior.

If you try it with 2007-10-23 and that bug still shows up, then that would be a problem, and this should be reopened.

Comment 4

[Dave Miller [:justdave]]

The problem isn't that the query didn't do what he expected, the problem is that there is no way to compose a query that does what he expects.

Comment 5

[Dave Miller [:justdave]]

I spent an hour last night trying to help him figure out how to make that query, and gave up and ended up running SQL for him to get it.

here's what he wanted to look for:

Any bugs which were still open in the Calendar product which have not had any comments added to them since 2007-10-25.

He doesn't care if there were any other changes made to them (CC list, whiteboard, whatever) only if there were comments or not.  Because the query conditions are checked separately for every comment on the bug the way it's currently set up, looking for "not comment changed after 10/25" apparently finds you bugs that have comments made before that date, whether or not they actually have any after it.

Comment 6

[Jesse Clark]

Attachment: v1

A big problem here is that the SQL for searching comment change date is inconsistent with other fields. This patch will give them the same behavior, which appears to negate properly.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Is this bug affecting only comment change date? In other words, does this bug also affect searching for "date of last flag change" or "date of last keyword added/removed" or "date of last status whiteboard change" or "date of last bug blocks/depends change", before/after a given date?

Comment 8

[Jesse Clark]

Attachment: v2 - fix work_time searches too

> Is this bug affecting only comment change date? In other words, does this bug
> also affect searching for "date of last flag change" or "date of last keyword
> added/removed" or "date of last status whiteboard change" or "date of last bug
> blocks/depends change", before/after a given date?

Change searches usually refer to the activity log, but comment change searches refer to the date of comment creation, so this bug only affects comment changed before/after and work time changed before/after (since work time is stored in comments).

By putting all change date conditions in the JOIN clause rather than the WHERE cause, we get the same behavior as existing change filters, including reasonable negation behavior. However, since JOIN conditions are always ANDed, it is hard to express conditions like "work time changed before 2000 OR comment included 'Y2K'" which sound simple but actually must span multiple charts in order to refer to different events.

Comment 9

[Max Kanat-Alexander]

Comment on attachment 304816 [details] [diff] [review]
v2 - fix work_time searches too

Yes, this is fine, although formatting-wise, I usually prefer to see things formatted like:

LEFT JOIN table AS $table
       ON blah = blah
          AND X = Y
          AND Z = W

Sometimes that formatting isn't practical. In that case, I still like to see the SQL keywords all right-aligned to each other. Makes for easier reading and understanding of what's going on.

Comment 10

[Frédéric Buclin]

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.156; previous revision: 1.155
done

Comment 11

[Jesse Clark]

Attachment: v3 - fix crash in v2

v2 somehow got accepted despite a misnamed argument that was causing it to crash. Here is a patch against v2 to correct that and fix the formatting issue mentioned by mkanat.

Comment 12

[Max Kanat-Alexander]

Comment on attachment 306184 [details] [diff] [review]
v3 - fix crash in v2

Yes, looks fine.

It got accepted because I was under the impression that you personally rigorously test all of these patches before posting them.

Comment 13

[Frédéric Buclin]

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.157; previous revision: 1.156
done

Comment 14

[Dave Miller [:justdave]]

Comment on attachment 304816 [details] [diff] [review]
v2 - fix work_time searches too

since v3 was a patch against this and not a replacement, this patch is not obsolete, you need both.

Comment 15

[Dave Miller [:justdave]]

Comment on attachment 306184 [details] [diff] [review]
v3 - fix crash in v2

Was just working on backporting this to 3.0 for bmo, and this is still broken:

>+                                 "AND $table.work_time <> 0" .
>+                                 "AND $table.bug_when $operator " .
>+                                  $dbh->quote(SqlifyDate($$v)) );

You're missing a space after the <> 0 up there, so that's going to wind up looking like "<> 0AND" when it gets stitched back together and I somehow doubt that works.

Comment 16

[Dave Miller [:justdave]]

Attachment: Backport to 3.0 branch

Here's my backport to the 3.0 branch for use on b.m.o.  I don't know if this is worthy of taking on the branch upstream or not, guess mkanat or LpSolit can make the call on that, but I'm posting the patch anyway in case anyone else gets use out of it. :)  I'd appreciate a look-see from jjclark just for completeness, since he wrote both the patch this is backported from and the one that caused the major code differences between 3.0 and trunk in this file.

Comment 17

[Dave Miller [:justdave]]

Attachment: correction to v2+v3 patches

one-character patch to fix the spacing error on trunk.  So for a complete patch for the trunk you need "v2" + "v3" + this patch.

Comment 18

[Dave Miller [:justdave]]

trunk:

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.159; previous revision: 1.158
done

Comment 19

[Wayne Mery (:wsmwk)]

(In reply to comment #16)
> Created an attachment (id=312011) [details]
> Backport to 3.0 branch
> 
> Here's my backport to the 3.0 branch for use on b.m.o.  I don't know if this is
> worthy of taking on the branch upstream or not, guess mkanat or LpSolit can
> make the call on that

Thanks Dave for looking at this.

I don't know the time frame for 3.2 coming to bmo, so my wish is for this patch to land on bmo if 3.2 is some weeks or months away. The improved functionality is much needed for bugdays.

Comment 20

[Dave Miller [:justdave]]

it'll be live on bmo later tonight, that's why I backported it. :)

Comment 21

[Frédéric Buclin]

Comment on attachment 312011 [details] [diff] [review]
Backport to 3.0 branch

Now that Bugzilla 3.0 is restricted to security fixes only, we are not going to take it upstream. And b.m.o now uses 3.2.