Tickets admin: “Remove selected items” yields mistranslated error message when no item selected, logged as HTTP 500

[Jun Omae]

In /admin/ticket/priority page, Remove selected items with no selection shows the following message in French:

Aucun priority sélectionné

priority text is untranslated…

2016-02-26 19:27:04,572 Trac[main] WARNING: [192.168.11.11] HTTPInternalError: 500 Erreur Trac (Aucun priority sélectionné)

It would be good to add localized message for each enum value of ticket.

trac/ticket/admin.py

@@ -599 +599 @@
 
     _type = 'unknown'
     _enum_cls = None
+    _no_item_selected = None
 
     # TicketAdminPanel methods
 
@@ -651 +652 @@
                 elif req.args.get('remove'):
                     sel = req.args.get('sel')
                     if not sel:
-                        raise TracError(_("No %s selected") % self._type)
+                        raise TracError(gettext(self._no_item_selected))
                     if not isinstance(sel, list):
                         sel = [sel]
                     with self.env.db_transaction:
@@ -789 +790 @@
     _type = 'priority'
     _enum_cls = model.Priority
     _label = (N_('Priority'), N_('Priorities'))
+    _no_item_selected = N_("No priority selected")
 
 
 class ResolutionAdminPanel(AbstractEnumAdminPanel):
     _type = 'resolution'
     _enum_cls = model.Resolution
     _label = (N_('Resolution'), N_('Resolutions'))
+    _no_item_selected = N_("No resolution selected")
 
 
 class SeverityAdminPanel(AbstractEnumAdminPanel):
     _type = 'severity'
     _enum_cls = model.Severity
     _label = (N_('Severity'), N_('Severities'))
+    _no_item_selected = N_("No severity selected")
 
 
 class TicketTypeAdminPanel(AbstractEnumAdminPanel):
     _type = 'type'
     _enum_cls = model.Type
     _label = (N_('Ticket Type'), N_('Ticket Types'))
+    _no_item_selected = N_("No ticket type selected")
 
     _command_type = 'ticket_type'
     _command_help = {

[Christian Boos]

It's probably a good intermediate step until we finally tackle the TracDev/Proposals/ConfigEnumTranslation issue.

[Ryan J Ollos]

This is an aside related to one of the lines that has been changed in the proposed patch. The line is:

_("No %s selected") % self._type

whereas usually we would write that as:

_("No %(type)s selected", type=self._type)

I imagine that %(type)s is a better hint to translations, as opposed to %s. Are there any other impacts of using the former rather than the latter?

One reason I ask is that I see the former pattern used at many places in SpamFilter. One such place I corrected in [14836], to follow the more typical pattern we see in Trac. However, I was unable to find any positive or negative functional effects of this change.

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[Ryan J Ollos]

Milestone renamed

[Philippe Cloutier <chealer@…>]

Replying to Ryan J Ollos:

[…] I imagine that %(type)s is a better hint to translations, as opposed to %s. Are there any other impacts of using the former rather than the latter?

In this case, no, but in general, the current form is much more fragile than the one you suggest. This is not inherent to the % operator, but rather to usage of positional parameters instead of named parameters. Positional parameters are much more prone to mixes when text changes or when the translation uses a different phrase structure.

[Philippe Cloutier <chealer@…>]

Thank you for reporting

There are 2 issues here:

1. The partial translation of the message.
2. Logging of a 500 error in such cases.

The proposed patch solves #1 the issue, but to the cost of significant burden for translators. I would rather suggest:

1. Making that error unlikely/abnormal, perhaps by making the button disabled until items are selected, or by switching to individual removals
2. Should the error happen nevertheless, use a generic message (“No item selected; nothing removed”)

As for the second problem, if we cannot prevent such requests, change the code to 400.

This comment is from Philippe "Chealer" Cloutier. I am subscribing to this ticket, but notifications seem to be broken, and I struggle to display my full email address or link to my contact information from this comment. My email address is available on Kune ni povos’s contact page. All of my comments and contributions in this ticket are offered under the terms of CC0 1.0 (unless otherwise noted).

[Jun Omae]

1. Making that error unlikely/abnormal, perhaps by making the button disabled until items are selected, or by switching to individual removals

Already fixed in [12450] (#11056, Trac 1.1.2).