281503 - Using QA Contact for tabular report axis generates incorrect URLs

Status: RESOLVED WORKSFORME

(Bugzilla :: Reporting/Charting, defect)

Description

[Glanville, Jay Dickon]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

If you generate a tabular report and use QA Contact as the Virtical axis, and
then click the URL for a cell in the table, the resulting bug report isn't for
that cell, but for the totals column.

For example, look at this tabular report:
https://bugzilla.mozilla.org/report.cgi?x_axis_field=&y_axis_field=qa_contact&z_axis_field=&query_format=report-table&short_desc_type=allwordssubstr&short_desc=&product=Bugzilla&target_milestone=Bugzilla+2.18&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=RESOLVED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&format=table&action=wrap&field0-0-0=noop&type0-0-0=noop&value0-0-0=
it is a tabular report against all target milestones of Bugzilla 2.18 that are
resolved, and have the QC contact as the virtical axis.  If you follow the URL
for the first cell (for default-qa @ bugzilla.bugs), you should go to a bug list
showing only 4 bugs (currently).  Instead, you go to a bug list showing 1095
bugs, which happens to be the total bugs applicable to the original query.

Here's another example:
https://bugzilla.mozilla.org/report.cgi?x_axis_field=qa_contact&y_axis_field=bug_severity&z_axis_field=&query_format=report-table&short_desc_type=allwordssubstr&short_desc=&product=Bugzilla&target_milestone=Bugzilla+2.18&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=RESOLVED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&format=table&action=wrap&field0-0-0=noop&type0-0-0=noop&value0-0-0=
This is a similar report to the previous one, the only difference being that the
virtical axis is now Severity, and QA Contact has been moved to the horizontal
axis.  Again, when you click on a cell's URL, you don't go to the bug list for
that cell, but for the totals of that row.


This bug might be related to a similar bug I've raised: bug 279590.

Reproducible: Always

Comment 1

[Glanville, Jay Dickon]

Forgot to mention that the bug was found in Bugzilla 2.18rc3, and was also found
in 2.19.1+.

Comment 2

[Gervase Markham [:gerv]]

Good catch. The reason for this is that the reports system is adding
"&qa_contact=default-qa%40bugzilla.bugs" 
to the URL in order to restrict it, but the search code doesn't accept email
addresses that way. It wants something like:
"emailqa_contact1=1&email1=default-qa%40bugzilla.bugs"

So the question is: do we fix buglists, or do we fix reporting?

Reporting is written in a generic manner - it would be easiest if buglist.cgi
accepted "&qa_contact=<email>" as a synonym for an exact match search on QA
Contact (and the same for all the other email types, for that matter).

Gerv

Comment 3

[Gervase Markham [:gerv]]

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

Comment 4

[Matthias Kienle]

I have the same problem on a linux system with the Mozilla browser. The problem
exist since BZ Version 2.17.6 (2.17.6 was my oldest BZ) and the problem also
still exists with your web version 2.19.1+

Comment 5

[Marc Schumann [:Wurblzap]]

Attachment: Patch

Fixing buglists.

Unless I'm very much mistaken, this turns out to be a one-liner. (Well, a
two-liner actually.)

Comment 6

[Joel Peshkin]

Comment on attachment 192507 [details] [diff] [review]
Patch

Nope.  That's the wrong fix.  While it works as a band-aid, you cannot then
"edit this search" and, if you try for the bugs that have no qa_contact, it
returns all bugs.

This needs to be fixed in the code that writes the URL in the first place.

Comment 7

[Marc Schumann [:Wurblzap]]

Ok... Seems I'm not up to it then, unfortunately.

Comment 8

[Max Kanat-Alexander]

Not severe enough to block, but should still be fixed in 2.20 branch.

Comment 9

[Frédéric Buclin]

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

Comment 10

[Frédéric Buclin]

Only security and dataloss fixes will be accepted on the 2.20 branch.

Comment 11

[timeless]

Comment on attachment 192507 [details] [diff] [review]
Patch

justdave: we don't roundtrip any of these. I claim that roundtripping them is a separate bug;
https://bugzilla.mozilla.org/buglist.cgi?product=Bugzilla&resolution=---&assigned_to=[e-mail]&reporter=[e-mail]

One that I'm willing to work on. But I don't think that this patch is "wrong", it at least fixes precisely what it claims to fix and results in things no more/less broken than their siblings.

Comment 12

[Frédéric Buclin]

Comment on attachment 192507 [details] [diff] [review]
Patch

joel denied review for this patch saying this was the wrong fix. If you want to r+ it, do it with a separate flag, but don't overwrite another reviewer's review. Only justdave is allowed to do so!

Comment 13

[timeless]

Attachment: other half

Note: I explicitly set justdave?. That patch is correct for what it does.

The patch gives the user exact parity with the other broken flavors.

I also said I was going to work on the other half.

This patch (if it works) should let all three (assuming the other patch is applied) work with query.cgi. But they aren't related to the ability to click on links in charts. They're related to the fact that query.cgi can't roundtrip certain preexisting buglist.cgi urls.

Comment 14

[Marc Schumann [:Wurblzap]]

Comment on attachment 274885 [details] [diff] [review]
other half

query.cgi doesn't compile with this patch.
Please put both parts of the whole patch into one next round.

Comment 15

[timeless]

Attachment: combined

Comment 16

[Marc Schumann [:Wurblzap]]

Comment on attachment 276750 [details] [diff] [review]
combined

On a plain call to query.cgi (plain meaning no GET or POST parameters), the |++$chartn while| never exits.

Comment 17

[timeless]

Attachment: with fewer logic errors

Comment 18

[Greg Hendricks]

Comment on attachment 276776 [details] [diff] [review]
with fewer logic errors

This is concise and I have tested it. Seems to work perfectly. Good job.

Comment 19

[Marc Schumann [:Wurblzap]]

Retargeting to 3.2 so that this may go in.

Comment 20

[Frédéric Buclin]

Comment on attachment 276776 [details] [diff] [review]
with fewer logic errors

>Index: query.cgi
>-                      "emaillongdesc", "content",
>+                      "emaillongdesc", "content", "reporter",

reporter is already in the list. Maybe you meant qa_contact?


>+    # reporter, qa_contact, assigned_to
>+    my @avail;
>+    # look for free Any one of rows
>+    foreach my $anyoneof (1..2) { 
>+        next unless $default{'email'}->[$anyoneof];
>+        foreach my $kind (qw(emailassigned_to emailreporter emailqa_contact)) {
>+            next unless $default{$kind}->[$anyoneof];
>+            push @avail, $anyoneof;
>+            last;
>+        }
>+    } 
>+    my $chartn = 0;
>+    foreach my $kind (qw(reporter qa_contact assigned_to)) {
>+        next unless $default{$kind};
>+        if (@avail) {
>+            my $anyoneof = shift @avail;
>+            # If there's an available slot in "Any one of"
>+            $default{'email'}->[$anyoneof] = $default{$kind};
>+            $default{"email$kind"}->[$anyoneof] = 1;
>+        } else {
>+            # Add another boolean chart.
>+            # Look for the next free chart.
>+            ++$chartn while $default{"field$chartn-1-0"}
>+                         || $default{"field$chartn-0-1"}
>+                         || ($default{"field$chartn-0-0"}
>+                          && $default{"field$chartn-0-0"} ne 'noop');
>+            $default{"field$chartn-0-0"} = $kind;
>+            $default{"type$chartn-0-0"} = 'equals';
>+            $default{"value$chartn-0-0"} = $default{$kind};
>+        }
>+    }

Sorry, but this still doesn't address comment 6. This block has no effect on the buglist. All bugs are returned.

Comment 21

[Lisa Henry]

I applied the latest patch for bugzilla bug 281503 to my Bugzilla 3.0 installation, and while it fixed the tabular report, it seems to have caused a problem when editing a search.  If any email addresses were used in the search in the “Email Addresses and Bug Numbers” box, and the search was edited, the email text fields would be prefilled with ARRAY(xxx).  

The fix was to change this line of the patch:

35c35
< +        next unless $default{$kind};
---
> +        next if $default{$kind};

Otherwise, the pre-existing email address gets overwritten.  

Comment 22

[Lisa Henry]

Ah, that's not the right fix, either.  It breaks the default query.  I'm not 
sure what the correct fix is, but the current patch breaks 'Edit Query' for me.  

Comment 25

[Erik Magana]

hello all. I was able to resolve this issue. It seems that the core of the issue was in the Search.PM file, located under the Bugzilla folder. There is an array named @legal_fields. This array holds a list of non-custom fields that are usuable on a chart. I was able to resolve my issue by making the following modification:

From:
my @legal_fields = ("product", "version", "rep_platform", "op_sys",
         "bug_status", "resolution", "priority", "bug_severity",
         "assigned_to", "reporter", "component", "classification",
         "target_milestone", "bug_group");

To:

my @legal_fields = ("product", "version", "rep_platform", "op_sys",
         "bug_status", "resolution", "priority", "bug_severity",
         "assigned_to", "reporter", "component", "classification",
         "target_milestone", "bug_group", "qa_contact");

The value "qa_contact" was missing from the array and so it was not being supplied as an available paramter further down in the code.

Comment 26

[Erik Magana]

Doing further research, I believe that this bug itself is fixed by adding the "qa_contact" to the @legal_fields, as was suggested back in august of 2005. The issue described as not being able to edit the search is a seperate bug and is one that applies to the "reporter", "assigned_to" and "qa_contact" and as suggested is in how the URL is written. 

(In reply to comment #6)
> (From update of attachment 192507 [details] [diff] [review])
> Nope.  That's the wrong fix.  While it works as a band-aid, you cannot then
> "edit this search" and, if you try for the bugs that have no qa_contact, it
> returns all bugs.
> This needs to be fixed in the code that writes the URL in the first place.

Comment 27

[Erik Magana]

filed bug 516278 to address the seperate issue the edit search functionality being broken.

Comment 29

[Frédéric Buclin]

Bugzilla 3.2 is restricted to security bugs only. Moreover, this bug is either assigned to nobody or got no traction for several months now. Rather than retargetting it at each new release, I'm clearing the target milestone and the bug will be retargetted to some sensible release when someone starts fixing this bug for real (Bugzilla 3.8 more likely).

Comment 30

[Frédéric Buclin]

I just tested the URL given in comment 0, and it's working fine, so this problem no longer exists in 4.0. It also works fine with Bugzilla 4.2.