329701 - Add flag support to quicksearch

Status: RESOLVED FIXED

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

Description

[Daniel Veditz [:dveditz]]

https://bugzilla.mozilla.org/quicksearch.js does not support groups or flags, because it was created before flags came about and back when groups were numeric.

An easy patch (tested locally) will make life much nicer for the handful of us in the security group or triaging releases

Comment 1

[Daniel Veditz [:dveditz]]

Attachment: add group and flag support

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: update the page, too

I suppose we ought to give people a clue this new feature exists. Add to the quicksearchhack.html advanced help page.

Comment 3

[Daniel Veditz [:dveditz]]

Any news on this?

Comment 4

[Gervase Markham [:gerv]]

Dan: is this bug b.m.o.-specific, other than that you'd particularly like it on b.m.o.? If not, it might get more attention in the main Bugzilla product.

Gerv

Comment 5

[Reed Loden [:reed]]

Fixed by bmo upgrade.

Comment 6

[Jesse Ruderman]

Not fixed by the bmo upgrade, afaict.  "group:security" and "flag:blocking" don't affect searches and it seems like they're just ignored (bug 365082).

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 214390 [details] [diff] [review]
add group and flag support

This patch was for the client-side quicksearch, doesn't apply to the perl-based server-side quicksearch.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 214394 [details] [diff] [review]
update the page, too

This page update is still valid though once the support is there.

Comment 10

[Daniel Veditz [:dveditz]]

The dupe bug 366254 was requesting slightly more functionality from the flag: query -- LpSolit wanted to be able to query "flag:review?lpsolit" to find not just every bug with the review flag, but specifically the ones requesting his review (or possibly multiple reviewers).

I was mainly interested in the blocking flags so my initial request didn't include that scenario. Perhaps there should be separate syntax for that, "request:"

examples:
I want the list of bugs nominated for the upcoming FF security releases

    flag:blocking1.8.0.10?,blocking1.8.1.2?

(note: I believe Jesse has filed a bug saying the comma "OR" isn't working anymore)

LpSolit wants a list of review requests waiting on his team:

    request:review?lpsolit,wicked

A shortcut to my review queue might be nice:

    request:review,superreview?dveditz

The last one over-complicates the parsing, feel free to drop it. I do need multiple items in the blocking "flag" query though.

Comment 11

[Max Kanat-Alexander]

We already have bug 174085 for groups--please use that and limit this bug to flags.

Comment 12

[Daniel Veditz [:dveditz]]

Er, OK -- but aren't they basically the same change with just different token names and bugzilla fields? Is it really easier to double the brain print on the developer and reviewers to manage similar patches in different places at different times?

Comment 13

[Teemu Mannermaa (:wicked)]

Attachment: Add flag support, V1

Adds ability to search for flags and their statuses as well as requestee and setter for flags. It was easiest to map these to three different fields as that's the way boolean charts work. That's were name and status (+, - or ?) combination came too. Normal QuickSearch OR rules apply.

Comment 14

[Frédéric Buclin]

I would still like to see the |flag:review?lpsolit| syntax being implemented. All you have to do when detecting /^flag:/ is to split the remaining string |review?lpsolit| on the "?" character and assumes the left part to be the flag name and the right part the requestee name. Or isn't it as simple as it seems to be?

Comment 15

[Jesse Ruderman]

I guess that would work for ?, but not for -, since - appears in many flag names.

Comment 16

[Frédéric Buclin]

Sure, but if a flag is set to "-", we already know there is no requestee, so there is no need to parse the string. We would only parse it on "?".

Comment 17

[Jesse Ruderman]

I can imagine someone wanting to search for "bugs minused by dveditz" or "bugs with patches rejected by roc".  But I think you're right that searching for "bugs with patches waiting for review by roc" would be a more common kind of search.

Comment 18

[Frédéric Buclin]

Comment on attachment 258950 [details] [diff] [review]
Add flag support, V1

>Index: Bugzilla/Search/Quicksearch.pm

>+                "request" => "requestees.login_name",

I don't think we need 'request'. It's too vague and may be confused with 'flag:'. Having 'req' as a shortcut is enough.

As discussed on IRC, review?lpsolit is a common string visible in the UI and so should also be supported as a shortcut for 'flag:review? requestee:lpsolit'. Split on "?" if you find one in the string and call AddChart() twice.

Comment 19

[Frédéric Buclin]

Comment on attachment 258950 [details] [diff] [review]
Add flag support, V1

Canceling review per our discussion on IRC (summarized in previous comments).

Comment 20

[Teemu Mannermaa (:wicked)]

Attachment: Add flag support, V2

Removed request alias and added support for |flag?requestee| shortcut syntax that gets treated same as |flag:flag? req:requestee| search. No comma operators supported on either side. Also added protection plusses so that they don't need quoting (helps especially with queries like |flag:review+|).

Comment 21

[Frédéric Buclin]

Comment on attachment 272923 [details] [diff] [review]
Add flag support, V2

>Index: Bugzilla/Search/Quicksearch.pm

>+                    elsif ($or_operand =~ /^([^\?]+\?)([^\?]+)$/) {

We should allow |review?| (even if flag:review? would do it), and so replace your 2nd + by a *. I tested your patch with this change, and it works fine.



>Index: template/en/default/pages/quicksearchhack.html.tmpl

>+  <td><i>flag</i><b>?</b><i>requestee</i></td>
    ...
>+  <td><i>flag</i><b>?</b><i>requestee</i></td>

Instead of writing these lines twice, we should use the rowspan="2" attribute in the first <td> above, as it's done for :area already. This makes it clearer that both lines are related.


Your patch seems to work fine, and both changes above can be done on checkin, so r=LpSolit. Thanks a lot for this useful patch. :)

Comment 22

[Frédéric Buclin]

Checking in Bugzilla/Search/Quicksearch.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v  <--  Quicksearch.pm
new revision: 1.16; previous revision: 1.15
done
Checking in template/en/default/pages/quicksearchhack.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/quicksearchhack.html.tmpl,v  <--  quicksearchhack.html.tmpl
new revision: 1.5; previous revision: 1.4
done

Comment 23

[Frédéric Buclin]

Attachment: same patch, with nits fixed

Here is the patch I committed, in case we want it for b.m.o (which currently runs 3.0, not 3.2).

Comment 24

[Max Kanat-Alexander]

Added to the release notes for Bugzilla 3.2 in a patch on bug 432331.