451801 - The word "Severity" is hard coded; not all pages use field_descs.bug_severity

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect, P3)

Description

[Craig Sebenik]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.10) Gecko/20070223 CentOS/1.5.0.10-0.1.el4.centos Firefox/1.5.0.10
Build Identifier: 

Several files have "Severity" hard coded in the HTML:

template/en/default/list/edit-multiple.html.tmpl
template/en/default/pages/fields.html.tmpl
template/en/default/pages/quicksearchhack.html.tmpl
template/en/default/bug/create/create.html.tmpl
template/en/default/bug/create/create-guided.html.tmpl
template/en/default/bug/edit.html.tmpl
template/en/default/search/form.html.tmp
template/en/default/reports/duplicates-table.html.tmpl


All references to "Severity" should be replaced with:
    [% field_descs.bug_severity FILTER html %]


Reproducible: Always

Steps to Reproduce:
1. Change Severity in template/en/default/global/field-descs.none.tmpl
2. Run checksetup.pl (to compile the templates)
3. Go to any page referenced above, eg. create a new bug.
      It will still say "Severity", not what ever you changed it to in field-descs.

Comment 1

[Guy Pyrzak]

excellent point! This should be looked into, however, I wouldn't call this a P1. 

Comment 2

[Nitish Bezzala]

Some of the files mentioned above have been fixed, with this code:

           [%+ INCLUDE bug/field.html.tmpl
                bug = bug, field = select_fields.bug_severity,
                no_tds = 1, value = bug.bug_severity
                editable = bug.check_can_change_field('bug_severity', 0, 1) %]

Below is the code mentioned in this bugs description:

    [% field_descs.bug_severity FILTER html %]

Which one should I use?

Comment 3

[Craig Sebenik]

The following files have been fixed:
  template/en/default/bug/create/create.html.tmpl
  template/en/default/bug/edit.html.tmpl

Which leaves the following to be updated:
  template/en/default/list/edit-multiple.html.tmpl
  template/en/default/pages/fields.html.tmpl
  template/en/default/pages/quicksearchhack.html.tmpl
  template/en/default/bug/create/create-guided.html.tmpl
  template/en/default/search/form.html.tmp
  template/en/default/reports/duplicates-table.html.tmpl

To answer the question about which code to use... It Depends[tm]...

Any file that uses just the name of the field, "Severity", (eg. the help files), then I am using the simpler "field_descs.bug_severity".

For the other files (that require a a pull-down list), then I am using the bug/field.html.tmpl code.

Comment 4

[Craig Sebenik]

Attachment: v1

There are 2 "types" of changes; simple text change and change with an associated list of values.
The latter would be list/edit-multiple.html.tmpl:
     [% INCLUDE bug/field.html.tmpl
       bug = default, field = select_fields.bug_severity, editable = 1,
       allow_dont_change = 1, value = dontchange %]

There is also an associated "set-up" at the top of the file. It sets up the "select_fields" hash.

The rest of the files use a simple statement:
  [% field_descs.bug_severity FILTER html %]
Note: some of the files needed the "field_descs" variable set up using the following statement near the top of the file:
  [% PROCESS "global/field-descs.none.tmpl" %]

Keep in mind that some of this may seem like overkill for one variable, however, I do plan on replacing the rest of the hard-coded titles, also.
(Eg. see bug 451804)

I was a little unsure about some of the spacing standards, so I did my best to do what made sense.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 355726 [details] [diff] [review]
v1

>Index: template/en/default/bug/create/create-guided.html.tmpl

  Why didn't you make this template use field.html.tmpl?

>Index: template/en/default/list/edit-multiple.html.tmpl
>+[% USE Bugzilla %]
>+[% SET select_fields = {} %]
>+[% FOREACH field = Bugzilla.get_fields(
>+  { type => constants.FIELD_TYPE_SINGLE_SELECT, custom => 0 })
>+%]
>+  [% select_fields.${field.name} = field %]
>+[% END %]

  This is not the correct way to go about things. select_fields exists in other files because we do special things with selects. It would be better to create some globally-accessible "fields" hash, perhaps by editing Bugzilla::Template, that represents a hash of all the fields in Bugzilla. Might want to call it bug_fields, though, and perhaps generate it in field_descs instead of putting it in Bugzilla::Template.


  Everything else looks fine! :-)

Comment 6

[Craig Sebenik]

(In reply to comment #5)
> (From update of attachment 355726 [details] [diff] [review])
> >Index: template/en/default/bug/create/create-guided.html.tmpl
> 
>   Why didn't you make this template use field.html.tmpl?

When we discussed this in IRC, you couldn't remember the reason you took the approach you did for create/create.html.tmpl. But, you said, there must be a good reason. So, I figured that the reason probably applies to "create-guided" as well. :)

That was the only reason I chose to do it that way. I can definitely use the "field.html.tmpl" approach. No biggie!

> files because we do special things with selects. It would be better to create
> some globally-accessible "fields" hash, perhaps by editing Bugzilla::Template,
> that represents a hash of all the fields in Bugzilla. Might want to call it
> bug_fields, though, and perhaps generate it in field_descs instead of putting
> it in Bugzilla::Template.

I did think about making more substantial changes, but I just wanted to fix the problem at hand, rather than creating new ones. :)

I was hoping to change all of the "similar" fields to use the same approach (i.e. using "field.html.tmpl") and then at some point in the future going back and seeing if there was some more work that could be done.

Basically, I was just trying to use the same approach that was used elsewhere... i.e. not "rock the boat" too much... also, "smaller changes (usually) introduce fewer bugs". :)

But, again, I am totally open to another approach.

Comment 7

[Craig Sebenik]

Attachment: v2

I have changed the files that used the more complicated format to just use field-descs.none.tmpl.

If those files need to, eventually, use the more complicated code, it can easily be added then. :)

Comment 8

[Max Kanat-Alexander]

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

>--- template/en/default/bug/create/create-guided.html.tmpl	29 Dec 2008 21:17:21 -0000	1.44
>+++ template/en/default/bug/create/create-guided.html.tmpl	12 Jan 2009 23:26:16 -0000
>@@ -24,6 +24,7 @@
>   #%]
> 
> [% PROCESS global/variables.none.tmpl %]
>+[% PROCESS "global/field-descs.none.tmpl" %]

  Remove variables.none.tmpl if you add field-descs--it imports variables.none.

>--- template/en/default/pages/quicksearchhack.html.tmpl	2 Dec 2007 23:12:10 -0000	1.7
>+++ template/en/default/pages/quicksearchhack.html.tmpl	12 Jan 2009 23:26:17 -0000
>@@ -15,6 +15,7 @@
>   #%]
> 
> [% PROCESS global/variables.none.tmpl %]
>+[% PROCESS "global/field-descs.none.tmpl" %]

  Same here.

>Index: template/en/default/reports/duplicates-table.html.tmpl
> [snip]
> [% PROCESS global/variables.none.tmpl %]
>+[% PROCESS "global/field-descs.none.tmpl" %]

  And here.


  All the rest of this looks great. :-) Just submit a new patch and we should be good to go. :-)

Comment 9

[Matt Selsky [:selsky]]

Attachment: v3

Patch updated as per comments.  I also replaced Severity/severity in the release notes.

Should I deal with "Sev" as part of this bug?  That abbreviation is hard-coded in template/en/default/list/table.html.tmpl and template/en/default/whine/mail.html.tmpl

Comment 10

[Max Kanat-Alexander]

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

Review of attachment 536241 [details] [diff] [review]:
-----------------------------------------------------------------

::: template/en/default/pages/release-notes.html.tmpl
@@ +1224,5 @@
> +  <li>Searching the [% field_descs.bug_severity FILTER html %] field if you
> +    type something that matches the first few characters of a [% 
> +    field_descs.bug_severity FILTER lower FILTER html %]. You can explicitly
> +    search the [% field_descs.bug_severity FILTER html %] field if you want to
> +    find [% terms.bugs %] by [% field_descs.bug_severity FILTER lower FILTER

You can remove all the "FILTER lower" actually (although that was clever). I'm fine with having it appear in Title Case. r+ with that fixed on checkin.

Comment 11

[Max Kanat-Alexander]

And thank you for the patch! :-)

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/bug/create/create-guided.html.tmpl
modified template/en/default/list/edit-multiple.html.tmpl
modified template/en/default/pages/release-notes.html.tmpl
modified template/en/default/whine/mail.txt.tmpl
Committed revision 7911.