323905 - "Group" "isn't equal to" boolean chart does not work correctly

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Frédéric Buclin]

I tried:

"Group" "isn't equal to" "Foo"  <--- doesn't work

I also tried:

NOT ("Group" "is equal to" "Foo")  <--- doesn't work


That's pretty annoying in general, and also because I wanted to use them during QA tests to check group restrictions on bugs.

Comment 1

[Wayne Mery (:wsmwk)]

I encountered this several months ago and thought it was a fluke (or me).

Comment 2

[Max Kanat-Alexander]

Now that we're basically in the RC cycle, I don't particularly want to block on anything that isn't "major" or a regression.

But I think that this would be something that would even be good to fix on the 2.20 branch.

Comment 3

[Joel Peshkin]

Attachment: Patch to join properly

Comment 4

[Frédéric Buclin]

Comment on attachment 209057 [details] [diff] [review]
Patch to join properly

>             push(@supptables,
>                     "LEFT JOIN groups AS groups_$chartid " .
>+                    "ON groups_$chartid.id = bug_group_map_$chartid.group_id " .
>+                    "AND $term");
>+            $term = "$ff IS NOT NULL";

I don't understand this code but... is it expected to use $term in push() *before* defining $term?

Comment 5

[Joel Peshkin]

(In reply to comment #4)
> (From update of attachment 209057 [details] [diff] [review] [edit])
> >             push(@supptables,
> >                     "LEFT JOIN groups AS groups_$chartid " .
> >+                    "ON groups_$chartid.id = bug_group_map_$chartid.group_id " .
> >+                    "AND $term");
> >+            $term = "$ff IS NOT NULL";
> 
> I don't understand this code but... is it expected to use $term in push()
> *before* defining $term?
> 

No... funcsbykey() sets $term, we use it, then we add a new $term (since we, after all, are a funcsbykey() function ourselves) so that there is something about which to be boolean.

Comment 6

[Vlad Dascalu]

Comment on attachment 209057 [details] [diff] [review]
Patch to join properly

+            $ff = $f = "groups_$chartid.name";

Why are we setting $f? AFAIK, we don't have a field in the bugs table for group, and anyway, it could be used only by change* afterwards, which we explicitely forbid due to ?!.

But I guess it doesn't hurt.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

This seems reasonable to get into the most recent release, but do we really want to push fixes like this into older releases?  After all, it's not a critical fix.

Comment 8

[Dave Miller [:justdave]]

Yeah, let's take it.  We haven't switched to security-only yet, and it is a rather blatent bug to the few folks who do run into it.

Comment 9

[Frédéric Buclin]

joel, can you make sure your patch really works correctly on 2.20? ;)

tip:

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

2.22rc1:

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

2.20.1:

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

Comment 10

[Frédéric Buclin]

Reopening! Our QA installations still report incorrect results. When a group is not mandatory, and there are some bugs which are not restricted to this group, "Group" "isn't equal to" "Foo" still reports no bug, which is wrong.

Comment 11

[Frédéric Buclin]

(In reply to comment #10)
> "Group" "isn't equal to" "Foo" still reports no bug, which is wrong.

Note that NOT("group" "is equal to" "Foo") now works, which wasn't the case before applying joel's patch. So his patch only fixed half of the problem.

joel, could you fix the second half of the problem, as described in comment 10?

Comment 12

[Ingolf Steinbach]

*** Bug 337670 has been marked as a duplicate of this bug. ***

Comment 13

[Joel Peshkin]

(In reply to comment #10)
> Reopening! Our QA installations still report incorrect results. When a group is
> not mandatory, and there are some bugs which are not restricted to this group,
> "Group" "isn't equal to" "Foo" still reports no bug, which is wrong.
> 

"Group" "isn't equal to" "Foo" should report bug that ARE in a group whose name is not "Foo"  

It should not report bugs that are in no group.

Comment 14

[Ingolf Steinbach]

(In reply to comment #13)
> "Group" "isn't equal to" "Foo" should report bug that ARE in a group whose name
> is not "Foo"  
> 
> It should not report bugs that are in no group.

That's what is currently implemented. (There seem to be several possibilities to do *this* kind of search.)

What is missing is to find those bugs which are not restricted to a certain group.

Comment 15

[Joel Peshkin]

That is what 
Negate ("Group" "is equal to" "Foo") 
does

Comment 16

[Ingolf Steinbach]

It does not in 2.20.2.

Comment 17

[Ingolf Steinbach]

(In reply to comment #11)
> Note that NOT("group" "is equal to" "Foo") now works, which wasn't the case
> before applying joel's patch. So his patch only fixed half of the problem.

Frédéric, does it really work for you with 2.20.2?

In our 2.20.2 installation the following links (note the difference at the end) both show bug #14 (which is restricted to group JOP):

http://our-bugzilla/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&product=xxx&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=bug_group&type0-0-0=equals&value0-0-0=JOP
http://our-bugzilla/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&product=xxx&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&negate0=1&field0-0-0=bug_group&type0-0-0=equals&value0-0-0=JOP

Comment 18

[Frédéric Buclin]

I did some tests on 2.23.1 in a product which has a mandatory and an optional groups. First, it completely ignores the mandatory group (the group_control_map table is not included in the SQL query). Secondly, NOT("group" "is equal to" "Foo") where Foo is the optional group indeed returns incorrect results as bugs being in the Foo group are also reported. I wonder how I tested this thing previously. ;) joel, could you investigate?

Comment 19

[Joel Peshkin]

Sure, but I m not sure if it makes sense to have this bug open.  It has already landed a long time ago.  I doubt if we want to automatically land any new patch just because the a+ is already set for this bug.

Please open a new bug.

Comment 20

[Marc Schumann [:Wurblzap]]

Ok, so what's the new bug?

Comment 21

[Frédéric Buclin]

(In reply to comment #20)
> Ok, so what's the new bug?
> 

bug 337670