Edgewall Software
Modify

Opened 17 years ago

Closed 15 years ago

Last modified 15 years ago

#3919 closed defect (fixed)

[PATCH] Notifications use field name instead of label for custom fields

Reported by: jflorian@… Owned by: ebray
Priority: normal Milestone: 0.12
Component: notification Version: 0.9.3
Severity: minor Keywords: custom fields i18n labels
Cc: trac.dev@…, hju@…, dalemiller@…, dale.miller@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

While it is very nice to have the custom fields appear in the notification mail, it would be better if they used the label given to them instead.

Often the field name is an abbreviation that can be quite confusing for most of our users whereas the labels would be something they'd recognize immediately.

Attachments (2)

field-labels-ticket-3919-r8022.patch (9.2 KB ) - added by ebray 15 years ago.
Patch to use field labels when displaying field names in the ticket changelog, ticket diffs, the query page, the report page, and e-mail notifications.
field-labels-ticket-3919-r8515.patch (11.4 KB ) - added by ebray 15 years ago.
Sorry for the delay on this. Updated patch with cboos's suggestions (there were a few spots in trac.ticket.api where _() needed to be replaced with N_() so that translated strings aren't put in the cache). I also updated a couple tests so that they work with these changes.

Download all attachments as: .zip

Change History (20)

comment:1 by jflorian@…, 17 years ago

Version: 0.100.9.3

Sorry, forgot to set the version to what we're actually using.

comment:2 by trac_dev at mail.ru, 17 years ago

Cc: trac.dev@… added

add to cc

comment:3 by Christian Boos, 17 years ago

Component: ticket systemnotification
Keywords: custom fields added
Milestone: 0.12
Owner: changed from Jonas Borgström to Emmanuel Blot
Severity: normalminor

comment:4 by osimons, 16 years ago

Keywords: i18n labels added

It is exactly as we do it on the regular fields also for web ui, so either we need to change the current practice for all fields, or else this won't make sense.

However, with current i18n effort underway, I suppose this is also related to that as it for many will make the default labels and fieldnames very different. Currently all labels and field names are the same (except capitalization) - that won't be the case later.

comment:5 by Christian Boos, 16 years ago

… not to mention the fact that currently it's the same mail sent to everyone, and that with i18n we probably need to partition the recipient list according to their language and send as many differently translated mails as needed.

comment:6 by Christian Boos, 16 years ago

#6854 was closed as duplicate

comment:7 by Christian Boos, 16 years ago

Milestone: 0.130.12

comment:8 by hju@…, 16 years ago

Cc: hju@… added

while localization is an very interesting feature, I'm much more interested on the basic defect Notifications use field name instead of label for custom fields at the moment. It's exactly what I was looking for at the moment.

comment:9 by hju@…, 16 years ago

Taking a brief look in the code, I implemented a little patch to change to label instead of names for all fields in notification.

Its not the overall solution, just a "hard change" to label.

The patch concerns notification.py:

  • trac/ticket/notification.py

     
    156          for f in [f['name'] for f in fields if f['type'] != 'textarea']:
    157              if not tkt.values.has_key(f):
     156         for f in [f for f in fields if f['type'] != 'textarea']:
     157             if not tkt.values.has_key(f['name']):
    158158                 continue
    159              fval = tkt[f]
     159             fval = tkt[f['name']]
    160160             if fval.find('\n') != -1:
    161161                 continue
    162162             idx = 2 * (i % 2)
    163              if len(f) > width[idx]:
    164                  width[idx] = len(f)
     163             if len(f['label']) > width[idx]:
     164                 width[idx] = len(f['label'])
    165165             if len(fval) > width[idx + 1]:
    166166                 width[idx + 1] = len(fval)
    167167             i += 1
     
    175176         for f in [f for f in fields if f['name'] != 'description']:
    176177             fname = f['name']
     178             flabel = f['label']
     179             if not flabel:
     180                 flabel = fname
    177181             if not tkt.values.has_key(fname):
    178182                 continue
    179183             fval = tkt[fname]
    180184             if f['type'] == 'textarea' or '\n' in unicode(fval):
    181                  big.append((fname.capitalize(), CRLF.join(fval.splitlines())))
     185                 big.append((flabel.capitalize(), CRLF.join(fval.splitlines())))
    182186             else:
    183                  txt += format[i % 2] % (fname.capitalize(), fval)
     187                 txt += format[i % 2] % (flabel.capitalize(), fval)
    184188                 i += 1

The patch ist not overall tested, but implemented in our environment (0.11) an running properly (till now).

comment:10 by dalemiller@…, 15 years ago

Cc: dalemiller@… added

This change should not be limited to the email notifications. The label for custom fields should be used in the comments section when displaying information from the ticket_change table. Trac should also use the custom label in the timeline displays. A good example of this is with the Master Tickets Plugin. If one prefers to change the labels of "blockedby" and "blocking" to something else it can be confusing when the timeline and ticket comments use the names and not the labels.

comment:11 by dale.miller@…, 15 years ago

Cc: dale.miller@… added

correcting username in cc field. (it would not let me remove my incorrect entry)

comment:12 by ebray, 15 years ago

Summary: Notifications use field name instead of label for custom fields[PATCH] Notifications use field name instead of label for custom fields

I'm attaching a somewhat more complete patch for this. It uses the field labels (if available) in the ticket changelog, ticket diffs, reports, and queries. Didn't touch RSS or any of the export formats—the export formats in particular should be left alone for what I think should be obvious reasons.

The patch involves creating these silly little field_labels dicts everywhere, but I don't feel too bad about it now that the fields are cached.

I also refactored TicketSystem._get_ticket_fields() so that the labels for all of the built in ticket fields can be more easily translated. I generated a new messages.pot, but I figure there's no reason to include that in the patch.

by ebray, 15 years ago

Patch to use field labels when displaying field names in the ticket changelog, ticket diffs, the query page, the report page, and e-mail notifications.

comment:13 by Christian Boos, 15 years ago

Owner: changed from Emmanuel Blot to ebray

It's a good first step, but the patch needs to be fixed to use N_ when storing the label field names into the fields (a no-op, we're just marking the string for translation). Otherwise you'd store the translation for whatever language was the one of the first request which triggered the caching. Then those label keys have to be translated dynamically when processing a request, with gettext(field_labels[f]) (not _ here, as there's nothing to extract).

comment:14 by Christian Boos, 15 years ago

Ping. Any chance to get an update on the patch?

by ebray, 15 years ago

Sorry for the delay on this. Updated patch with cboos's suggestions (there were a few spots in trac.ticket.api where _() needed to be replaced with N_() so that translated strings aren't put in the cache). I also updated a couple tests so that they work with these changes.

comment:15 by Christian Boos, 15 years ago

The gettext call is missing, and having that many field_labels dicts look suboptimal, I'll have a more detailed look.

Still, thanks for the update!

comment:16 by Christian Boos, 15 years ago

Applied Erik's patch in r8523, refactored it a bit to call gettext appropriately in r8524.

Further related i18n tweaks in r8525 and r8527.

Follow-up needed for the Custom Queries.

comment:17 by Christian Boos, 15 years ago

Resolution: fixed
Status: newclosed

Labels translated in the Custom Query as well with [8534].

There's still an issue with the Javascript there, adding a new filter still adds the name of the filter instead of the label, but I didn't want to touch the Javascript for now (see #7111).

If there's another place where the (translated) labels should have used, please reopen.

comment:18 by Remy Blank, 15 years ago

The translation of ticket field labels has been fixed in [8604].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain ebray.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from ebray to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.