Closed Bug 863745 Opened 12 years ago Closed 11 years ago

Enable multi-select fields in reports

Categories

(Bugzilla :: Reporting/Charting, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 5.0

People

(Reporter: pami.ketolainen, Assigned: pami.ketolainen)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch Patch to enable multi-select fields in reports (obsolete) (deleted) — Splinter Review
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.
Attachment #739618 - Flags: review?
Attached patch v2 (obsolete) (deleted) — Splinter Review
Few fixes to the patch: - warnings about shared variables in report.cgi - null values shown in total row/column in some cases
Attachment #739618 - Attachment is obsolete: true
Attachment #739618 - Flags: review?
Attachment #740757 - Flags: review?(LpSolit)
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.
Attachment #740757 - Flags: review?(LpSolit) → review-
Assignee: charting → pami.ketolainen
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → Bugzilla 5.0
Attached patch v3 (deleted) — Splinter Review
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.
Attachment #740757 - Attachment is obsolete: true
Attachment #786261 - Flags: review?(LpSolit)
(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.
Depends on: 901589
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
Attachment #786261 - Flags: review?(LpSolit) → review+
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).
Flags: approval?
Keywords: relnote
Flags: approval? → approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/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.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Blocks: 179581
Added to relnotes for 5.0rc1.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: