CSV headers should be the field description, not the field name

RESOLVED FIXED in Bugzilla 4.2

Status

()

--
enhancement
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: michael.j.tosh, Assigned: michael.j.tosh)

Tracking

3.1.3
Bugzilla 4.2
Bug Flags:
approval +

Details

(URL)

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Bugzilla 3.1.3

Default template for CSV search output should respect the customized field descriptions.  This minimized the customizations needed to change the field names.  Especially when the recommendations are to use an unused field instead of adding a custom field.

Obviously there are many other places, most notably the search form and the bug edit page, where things like 'Summary' are spelled out, but even with that, this csv export feature would display short_desc, which for users used to seeing Summary may cause more confusion.

Reproducible: Always

Steps to Reproduce:
1. Click on URL specified above
2. Look at field names
3. Notice how they don't match what is displayed in the buglist window
Actual Results:  
bug table column names are used

Expected Results:  
corresponding column names from field-descs.none.tmpl should be used
(Assignee)

Comment 1

11 years ago
Created attachment 303330 [details] [diff] [review]
Code Patch Ver 1

Comment 2

11 years ago
Comment on attachment 303330 [details] [diff] [review]
Code Patch Ver 1

>+[% PROCESS "global/variables.none.tmpl"   %]

This line is useless. field-descs.none.tmpl already calls this file.

Updated

11 years ago
Version: unspecified → 3.1.3

Comment 3

11 years ago
If you want this patch to be actually formally reviewed, please follow our process:

  http://wiki.mozilla.org/Bugzilla:Developers

Updated

11 years ago
Assignee: query-and-buglist → michael.j.tosh
(Assignee)

Comment 4

11 years ago
Created attachment 303527 [details] [diff] [review]
Code Patch Ver 2
Attachment #303330 - Attachment is obsolete: true
Attachment #303527 - Flags: review?(mkanat)

Comment 5

11 years ago
The problem with this is that CSV is a machine-readable format, and people might be depending upon the names of those headers.

However, CSV is also our "Spreadsheet output format", and to people using and reading spreadsheets, those column headers are meaningless.

So *probably* what we should do is output the field descriptions, and have a URL-parameter option to output the old field names instead. However, before we add such an option, I'd need some evidence that those old field names are actually important or used, which they might not be.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Default template for CSV search output should respect the customized field descriptions → CSV headers should be the field description, not the field name
Target Milestone: --- → Bugzilla 4.0

Updated

11 years ago
Severity: normal → enhancement

Comment 6

11 years ago
Both are fine for me. Maybe asking in developers@ what people use would be a good idea.
Status: NEW → ASSIGNED

Comment 7

11 years ago
Comment on attachment 303527 [details] [diff] [review]
Code Patch Ver 2

Hey Michael. Do what we said in developers@.
Attachment #303527 - Flags: review?(mkanat) → review-
(Assignee)

Comment 8

11 years ago
Created attachment 310065 [details] [diff] [review]
Code Patch Ver 3

Made human=1 the default for the link.
Attachment #303527 - Attachment is obsolete: true
Attachment #310065 - Flags: review?(mkanat)
(Assignee)

Updated

11 years ago
Attachment #303527 - Flags: review-

Comment 9

11 years ago
Comment on attachment 303527 [details] [diff] [review]
Code Patch Ver 2

Restoring Max's r-. Please don't remove reviews.
Attachment #303527 - Flags: review-

Comment 10

10 years ago
Comment on attachment 310065 [details] [diff] [review]
Code Patch Ver 3

>diff -Nru template/en/default/list/list.csv.tmpl template/en/default/list/list.csv.tmpl.new
>+    [% colsepchar %][% field_descs.$column || column FILTER csv %]

  Does that work? It looks like field_descs.$column isn't actually getting filtered.

  Also, this patch is malformed (according to my patch command--I can't apply this patch). Make sure you make your patches with "cvs diff -Nru".
Attachment #310065 - Flags: review?(mkanat) → review-

Comment 11

10 years ago
Also if we care about readability, we should also process field values described in field-descs.none.tmpl.

Comment 12

10 years ago
(In reply to comment #11)
> Also if we care about readability, we should also process field values
> described in field-descs.none.tmpl.

  Sure. We should do that in a separate bug, though.
(Assignee)

Comment 13

10 years ago
Created attachment 357384 [details] [diff] [review]
Code Patch Ver 4

Updated patch to newest version.  Also moved field's "||" out to its own line to assure FILTER happens.
Attachment #310065 - Attachment is obsolete: true
Attachment #357384 - Flags: review?(mkanat)
(Assignee)

Updated

10 years ago
Flags: approval?
Flags: approval3.2?

Updated

10 years ago
Flags: approval?
Flags: approval3.2?

Comment 14

9 years ago
Comment on attachment 357384 [details] [diff] [review]
Code Patch Ver 4

>Index: bugzilla/template/en/default/list/list.csv.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.csv.tmpl,v
>retrieving revision 1.8
>diff -u -r1.8 list.csv.tmpl
>--- bugzilla/template/en/default/list/list.csv.tmpl	27 Aug 2008 23:26:24 -0000	1.8
>+++ bugzilla/template/en/default/list/list.csv.tmpl	16 Jan 2009 18:29:41 -0000
>@@ -17,15 +17,25 @@
>   #
>   # Contributor(s): Myk Melez <myk@mozilla.org>
>   #                 Gervase Markham <gerv@gerv.net>
>+  #                 Mike Tosh <oreomike@gmail.com>
>   #%]
> 
> [% PROCESS "global/field-descs.none.tmpl" %]
>+[% USE Bugzilla %]
> 
> [% colsepchar = user.settings.csv_colsepchar.value %]
> 
>-bug_id
>-[% FOREACH column = displaycolumns %]
>-  [% colsepchar %][% column FILTER csv %]
>+[% IF bugzilla.cgi('human') %]

  You should be passing a "human" template variable here instead of checking Bugzilla.cgi (in case somebody wants to use this template somewhere where there is no CGI object, in the future).
Attachment #357384 - Flags: review?(mkanat) → review-
(Assignee)

Comment 15

8 years ago
Created attachment 507455 [details] [diff] [review]
Code Patch Ver 5

I know it has been 18 months since the last time I've touched this, but here is the patch again with the suggestion from comment 14 in it.

> You should be passing a "human" template variable here instead of checking
> Bugzilla.cgi (in case somebody wants to use this template somewhere where 
> there is no CGI object, in the future).
Attachment #357384 - Attachment is obsolete: true
Attachment #507455 - Flags: review?(mkanat)

Comment 16

8 years ago
Comment on attachment 507455 [details] [diff] [review]
Code Patch Ver 5

Looks good to me! Thanks for the patch, Mike! :-)
Attachment #507455 - Flags: review?(mkanat) → review+

Updated

8 years ago
Flags: approval+
Target Milestone: Bugzilla 5.0 → Bugzilla 4.2

Comment 17

8 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified buglist.cgi
modified template/en/default/list/list.csv.tmpl
modified template/en/default/list/list.html.tmpl
Committed revision 7709.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.