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

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
User Interface
P3
trivial
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Craig Sebenik, Assigned: selsky)

Tracking

(Blocks: 1 bug)

unspecified
Bugzilla 4.4
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

v3
3.65 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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.

Updated

10 years ago
Assignee: ui → craig5
Blocks: 218746
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

10 years ago
Priority: -- → P1
Summary: Severity is hard coded; not all pages use field_descs.bug_severity → The word "Severity" is hard coded; not all pages use field_descs.bug_severity
Target Milestone: --- → Bugzilla 4.0

Comment 1

10 years ago
excellent point! This should be looked into, however, I wouldn't call this a P1. 
Priority: P1 → P3
Whiteboard: [Good Intro Bug]

Comment 2

9 years ago
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?
(Reporter)

Comment 3

9 years ago
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.
(Reporter)

Comment 4

9 years ago
Created attachment 355726 [details] [diff] [review]
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.
Attachment #355726 - Flags: review?(bugzilla-mozilla)

Updated

9 years ago
Attachment #355726 - Flags: review?(bugzilla-mozilla) → review?(mkanat)

Comment 5

9 years ago
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! :-)
Attachment #355726 - Flags: review?(mkanat) → review-
(Reporter)

Comment 6

9 years ago
(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.
(Reporter)

Comment 7

9 years ago
Created attachment 356604 [details] [diff] [review]
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. :)
Attachment #356604 - Flags: review?(mkanat)

Comment 8

9 years ago
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. :-)
Attachment #356604 - Flags: review?(mkanat) → review-
(Assignee)

Comment 9

7 years ago
Created attachment 536241 [details] [diff] [review]
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
Assignee: craig5 → selsky
Status: NEW → ASSIGNED
Attachment #536241 - Flags: review?(mkanat)

Updated

7 years ago
Attachment #355726 - Attachment is obsolete: true

Updated

7 years ago
Attachment #356604 - Attachment is obsolete: true

Comment 10

7 years ago
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.
Attachment #536241 - Flags: review?(mkanat) → review+

Updated

7 years ago
Flags: approval?

Updated

7 years ago
Flags: approval? → approval+

Comment 11

7 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

6 years ago
Whiteboard: [Good Intro Bug]
You need to log in before you can comment on or make changes to this bug.