Replace underscores with whitespace when creating action label from action name

[Ryan J Ollos]

Consider a workflow action such as the following:

may_reassign = new,assigned,accepted,reopened -> assigned
may_reassign.operations = may_set_owner
may_reassign.permissions = TICKET_MODIFY 
may_reassign.set_owner =

If I want the action name to look nice, I must add another attribute:

may_reassign.name = may reassign

What would be the downside to doing a simple replacement?

trac/ticket/default_workflow.py

@@ -67 +67 @@
     for action, attributes in actions.items():
         # Default the 'name' attribute to the name used in the ini file
         if 'name' not in attributes:
-            attributes['name'] = action
+            attributes['name'] = action.replace("_", " ")
         # If not specified, an action is not the default.
         attributes['default'] = int(attributes.get('default', 0))
         # If operations are not specified, that means no operations

The only downside I can think of is when the user really wants an underscore in the name, but I guess that to be a rather infrequent occurrence.

The same replacement could be applied to TracTicketsCustomFields field name.

Speaking of which, it's a bit confusing that the attribute label is used for TracTicketsCustomFields, whereas name is used for TracWorkflow, considering the attributes describe similar behavior.

Anyway, those are just small things and not too important.

[Christian Boos]

I hope I'm not making this too complicated than it needs to be, but to me, name and label should be two different things:

• 'name' names the thing, i.e. could be used as a way to identify it
• 'label' is how it's displayed, i.e. the user facing value (could be localized, even)

In the ticket fields case, that's pretty clear, e.g. {'name': 'owner', 'label': N_('Owner')}. We don't have yet localization for the values themselves, but if we had (TracDev/Proposals/ConfigEnumTranslation), I think that clearly differentiating between 'name' and 'label' would be needed as well.

In the Workflow situation, we could have followed the above scheme by saying there's always an implicit <action>.name = <action> and there could be an explicit <action>.label = displayed name. Thinking about it, if we say that <action>.label defaults to <action>.name, we could switch to using .label and still support existing configurations, i.e. the <action>.name = without space would be only used to provide a default to <action>.label, not to change the action's name in the code.

And then, normalizing attributes['label'] = name.replace("_", " ") makes sense.

[Ryan J Ollos]

Replying to cboos:

I hope I'm not making this too complicated than it needs to be, but to me, name and label should be two different things:

• 'name' names the thing, i.e. could be used as a way to identify it
• 'label' is how it's displayed, i.e. the user facing value (could be localized, even)

We appear to be thinking similarly. For the ticket workflow the name attribute currently behaves in the way that you suggest the label attribute should behave. That's why I felt the choice of name for the ticket workflow attribute was poor, and the code should have originally been written like this:

trac/ticket/default_workflow.py

@@ -66 +66 @@
 
     for action, attributes in actions.items():
-        # Default the 'name' attribute to the name used in the ini file
-        if 'name' not in attributes:
-            attributes['name'] = action
+        # Default the 'label' attribute to the action name
+        if 'label' not in attributes:
+            attributes['label'] = action
         # If not specified, an action is not the default.
         attributes['default'] = int(attributes.get('default', 0))
         # If operations are not specified, that means no operations

In the Workflow situation, we could have followed the above scheme by saying there's always an implicit <action>.name = <action> and there could be an explicit <action>.label = displayed name. Thinking about it, if we say that <action>.label defaults to <action>.name, we could switch to using .label and still support existing configurations, i.e. the <action>.name = without space would be only used to provide a default to <action>.label, not to change the action's name in the code.

And then, normalizing attributes['label'] = name.replace("_", " ") makes sense.

parse_workflow_config builds a dictionary with the action as the key.

{u'resolve': {u'operations': [u'set_resolution'], 'name': u'resolve', 'default': 0, 'newstate': u'closed', u'set_resolution': [u''], 'oldstates': [u'new', u'assigned', u'accepted', u'reopened'], u'permissions': [u'TICKET_MODIFY']},

Therefore I expect the following patch would preserve backwards-compatibility by reading the name attribute when present, but giving preference to the label attribute and generating the label from the action name when neither name or label is present.

trac/ticket/default_workflow.py

@@ -65 +65 @@
         return [item for item in (x.strip() for x in value.split(',')) if item]
 
     for action, attributes in actions.items():
-        # Default the 'label' attribute to the name used in the ini file
-        if 'name' not in attributes:
-            attributes['name'] = action
+        # Default the 'label' attribute to the action name
+        if 'label' not in attributes:
+            if 'name' in attributes:  # backwards-compatibility, #11828
+                attributes['label'] = attributes['name']
+            else:
+                attributes['label'] = action.replace("_", " ")
         # If not specified, an action is not the default.
         attributes['default'] = int(attributes.get('default', 0))
         # If operations are not specified, that means no operations
@@ -133 +136 @@
             # exists, as no other action can then change its state. (#5307)
             self.actions['_reset'] = {
                 'default': 0,
-                'name': 'reset',
+                'label': 'reset',
                 'newstate': 'new',
                 'oldstates': [],  # Will not be invoked unless needed
                 'operations': ['reset_workflow'],
@@ -357 +360 @@
             if status != '*':
                 hints.append(tag_("Next status will be '%(name)s'",
                                   name=status))
-        return (this_action['name'], tag(separated(control, ' ')),
+        return (this_action['label'], tag(separated(control, ' ')),
                 tag(separated(hints, '. ', '.') if hints else ''))
 
     def get_ticket_changes(self, req, ticket, action):

This would allow us to deprecate the name attribute. If we want to eventually remove the compatibility code an upgrade step could be written, but I don't see any harm in leaving it in place.

[Christian Boos]

Yes exactly, and that's even simpler than what I thought, as it looks like that the 'name' attribute unmodified is not needed.

[Ryan J Ollos]

DONE Modify sample workflows in trunk/contrib/workflow.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t11828. Documentation updated in 1.1/TracWorkflow@5 and 1.1/TracTicketsCustomFields@6.

[Ryan J Ollos]

Committed to trunk in [13431:13432].