set_owner_to_self should use author field of form

[Ryan J Ollos]

When changing a ticket the author field is populated from prefs, that author field value can be replaced by the user, and the value of the author field is used as the ticket change author. However, when accepting a ticket, the set_owner_to_self action does not use the author field when consider who the ticket should be assigned to.

The following patch has been lightly tested, but may be a starting point in aligning the behavior of the workflow with the rest of the ticket form.

trac/ticket/default_workflow.py

@@ -29 +29 @@
 from trac.perm import PermissionSystem
 from trac.ticket.api import ITicketActionController, TicketSystem
 from trac.ticket.model import Resolution
+from trac.util import get_reporter_id
 from trac.util.text import obfuscate_email_address
 from trac.util.translation import _, tag_, cleandoc_
 from trac.web.chrome import Chrome, add_script, add_script_data
@@ -285 +286 @@
                                "%(current_owner)s to the selected user",
                                current_owner=current_owner))
         elif 'set_owner_to_self' in operations and \
-                ticket._old.get('owner', ticket['owner']) != req.authname:
+                ticket._old.get('owner', ticket['owner']) != \
+                        get_reporter_id(req, 'author'):
             hints.append(_("The owner will be changed from %(current_owner)s "
                            "to %(authname)s", current_owner=current_owner,
-                           authname=req.authname))
+                           authname=get_reporter_id(req, 'author')))
         if 'set_resolution' in operations:
             if 'set_resolution' in this_action:
                 resolutions = [x.strip() for x in
@@ -364 +366 @@
                     newowner = newowner[0]
                 updated['owner'] = newowner
             elif operation == 'set_owner_to_self':
-                updated['owner'] = req.authname
+                updated['owner'] = get_reporter_id(req, 'author')
             elif operation == 'del_resolution':
                 updated['resolution'] = ''
             elif operation == 'set_resolution':

[Ryan J Ollos]

The *args, **kwargs in the initializer signature trunk/trac/ticket/default_workflow.py@13056:119#L101​ seems to be an artifact of earlier implementation in which Component.__init__ was executed in the constructor: [5441]. I propose to remove the arguments on the trunk.

The only case I see on trac-hacks.org in which ConfigurableTicketWorkflow is instantiated in a plugin is ​th:AdvancedTicketWorkflowPlugin - ​th:browser:/advancedticketworkflowplugin/0.12/advancedworkflow/controller.py@9962:35#L22. The proposed change would not affect ​th:AdvancedWorkflowPlugin.

[Ryan J Ollos]

Replying to rjollos:

The *args, **kwargs in the initializer signature trunk/trac/ticket/default_workflow.py@13056:119#L101​ seems to be an artifact of earlier implementation in which Component.__init__ was executed in the constructor: [5441]. I propose to remove the arguments on the trunk.

Removed in [13172].

[Ryan J Ollos]

One additional issue can be fixed in this ticket: when [ticket] restrict_owner = false and an unauthenticated user with name and email set for the session, the name and email should be used to populate the input in the set_owner operation, rather than anonymous.

[Ryan J Ollos]

Committed to 1.0-stable in [13215:13216], merged to trunk in [13217:13219].

In log:rjollos.git:t11418 I tried adding some unit testing, but they seemed rather klunky and contrived. Testing with functional tests continues to prove challenging because of the shared state of the environment between tests. The ideal fix could be a new functional test framework that makes the tests easier to write and allows tests to be written so that they don't share state (though the latter could possibly be done independent of the former).

[Ryan J Ollos]

Stupid error from [13218] was corrected in [13313]. I should pay better attention and use the debugger rather than being lazy with print statements.