863745 - Enable multi-select fields in reports

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, enhancement)

Description

[Pami Ketolainen]

Attachment: Patch to enable multi-select fields in reports

To make using (custom) multi-select fields more usable for categorizing bugs, they should be available as report axis options. The fact that with multi-select field a bug can belong to multiple "categories" makes the number crunching in reports a bit difficult.

How ever, I managed to patch the reports system so that multi-select fields can be used as report axes, and the numbers still make sense for the most parts.

The patch is against trunk.

Comment 1

[Pami Ketolainen]

Attachment: v2

Few fixes to the patch:
- warnings about shared variables in report.cgi
- null values shown in total row/column in some cases

Comment 2

[Frédéric Buclin]

Comment on attachment 740757 [details] [diff] [review]
v2

>=== modified file 'report.cgi'

>@@ -352,7 +393,8 @@
>         foreach my $value (@{$field->legal_values}) {
>             push(@sorted, $value->name) if $names->{$value->name};
>         }
>-        unshift(@sorted, ' ') if $field_name eq 'resolution';
>+        unshift(@sorted, ' ') if ($field_name eq 'resolution' || (
>+                                  $field->type == FIELD_TYPE_MULTI_SELECT));

This part of the code is conflicting with the already committed bug 212471. I suppose it's easy enough to fix.

I tried replacing it by --- manually, but no number is displayed for bugs with no values for the selected multi-select field. Maybe another place needs to be fixed too. I will review the patch more carefully once this problem is fixed.

Comment 3

[Pami Ketolainen]

Attachment: v3

While fixing the conflict I also noticed that the buglist links in tabular report for the bugs with no selected value returned wrong results.

To fix this I changed the search code to treat '---' value for multiselect fields as 'isempty' operator (Requires fix for the bug 901589). I'm not sure if this is a good solution, but so far I couldn't figure out a better or cleaner one.

Comment 4

[Frédéric Buclin]

(In reply to Pami Ketolainen from comment #3)
> fields as 'isempty' operator (Requires fix for the bug 901589)

FYI, I didn't look at your patch yet as you say bug 901589 must be fixed first.

Comment 5

[Frédéric Buclin]

Comment on attachment 786261 [details] [diff] [review]
v3

>=== modified file 'report.cgi'

>+        unshift(@sorted, '---') if ($field_name eq 'resolution' ||
>+                                    $field->type == FIELD_TYPE_MULTI_SELECT);

Nit: || at the beginning of the 2nd line.


>+        # On SQLite GROUP_CONCAT with DISTINCT always uses ',' as separator

Maybe this comment should go away. It's more confusing than helpful. Or maybe it should be reworded to something like "Some DB servers have a space after the comma, some others don't".



>=== modified file 'template/en/default/filterexceptions.pl'

>   'row_total',
>-  'col_totals.$col',
>+  'col_total',
>   'grand_total', 

All the *_total should go away as you don't need them anymore, see below.



>=== modified file 'template/en/default/reports/report-table.html.tmpl'

>+        [% row_total = data.$tbl.$total_key.$row OR 0 %]
>         [% row_total %]</a>

You could merge both lines as you don't need row_total anymore.


>+          [% col_total = data.$tbl.$col.$total_key OR 0 %]
>+          [% col_total %]</a>

Same here.


>+        [% grand_total = data.$tbl.$total_key.$total_key OR 0 %]

Same here.


>+[% IF note_multi_select %]
>+<i style="font-size: smaller;">NOTE: Axes contain multi-value fields, so the
>+  total numbers might not add up, as single [% terms.bug %] can match several
>+  rows or columns.</i>
>+[% END %]

You should enclose this text in <p></p>


Your patch is working fine, and the patch looks good. Thanks for implementing this feature! :) r=LpSolit

Comment 6

[Frédéric Buclin]

I will do the fixes on checkin myself (tiny conflict with one of the security patches, so I will commit this patch myself, *after* the security patch is committed).

Comment 7

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified report.cgi
modified Bugzilla/Search.pm
modified skins/standard/reports.css
modified template/en/default/filterexceptions.pl
modified template/en/default/reports/report-table.html.tmpl
Committed revision 8792.

Comment 8

[Frédéric Buclin]

Added to relnotes for 5.0rc1.