Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

#7181 closed defect (fixed)

r6919 breaks custom workflow set_owner operation!

Reported by: robert.nadler@… Owned by: Eli Carter
Priority: normal Milestone: 0.11
Component: ticket system Version: 0.11rc1
Severity: critical Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

We use a custom workflow that has worked fine through r6912. After installing rc1, actions that have a set_owner operation always fail to commit the ticket change and kick you into preview mode with this banner:

Warning: 'user' is not a valid value for the owner field.

This happens no matter which user is selected.

Attachments (1)

CdicWorkflow.py (3.6 KB ) - added by robert.nadler@… 12 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by Eli Carter, 12 years ago

Can you test before and after r6919? That is the changeset I would first suspect. Also, if you can post your custom workflow, that may prove useful. I'll try to reproduce this locally regardless.

comment:2 by robert.nadler@…, 12 years ago

I switched back to the r6912 version of web_ui.py (previously changed at r6907) and everything works fine.

Our workflow uses a custom permissions provider (CDIC_*) and some custom actions. All of the set_owner operations below fail with the warning when the r6919 version of web_ui.py is used.

assign = open -> assigned
assign.name = Assign: Set owner
assign.operations = set_owner
assign.permissions = CDIC_MANAGER
assign.default = 1
assigntester = fixed -> fixed
assigntester.name = AssignTester
assigntester.operations = set_owner
assigntester.permissions = CDIC_MANAGER
close = new,reviewed -> closed
close.name = Close
close.operations = set_resolution
close.permissions = CDIC_DBA
leave = * -> *
leave.default = 2
leave.name = Leave
leave.operations = leave_status
open = new -> open
open.name = Open
open.permissions = CDIC_DBA
openagain = recommend_close,reviewed -> open
openagain.name = OpenAgain
openagain.operations = del_owner
openagain.permissions = CDIC_MANAGER
reassign = assigned -> assigned
reassign.name = ReAssign
reassign.operations = set_owner
reassign.permissions = CDIC_MANAGER
recommendclose = tested,open,fixed,assigned -> recommend_close
recommendclose.name = RecommendClose
recommendclose.operations = del_owner
recommendclose.permissions = CDIC_TESTER
reopen = closed -> open
reopen.name = ReOpen
reopen.operations = del_resolution,del_owner
reopen.permissions = CDIC_DBA
reviewed = recommend_close,tested -> reviewed
reviewed.name = Reviewed
reviewed.operations = leave_status
reviewed.permissions = CDIC_DBA
testpassed = fixed -> tested
testpassed.name = TestPassed
testpassed.operations = del_owner
testpassed.permissions = CDIC_TESTER
unassign = assigned -> open
unassign.name = UnAssign
unassign.operations = del_owner
unassign.permissions = CDIC_DEVELOPER

comment:3 by Eli Carter, 12 years ago

Owner: changed from Christian Boos to Eli Carter
Status: newassigned

comment:4 by Eli Carter, 12 years ago

Using the given configuration without the permission settings, I'm not able to reproduce the problem you're seeing. (tested with r6941)

r6919 changed the ITicketManipulator interface to match the behavior it had in 0.10.*. At this point, I suspect the error you are getting is from a plugin that uses that interface. What plugins are you using? Can you disable all the plugins, and verify that the issue goes away? Assuming it does, then reenable them one by one to determine which is the culprit.

From there we should be able to get your system running again.

comment:5 by robert.nadler@…, 12 years ago

Disabling these did not solve the problem.

BatchModify 0.2.0
CDICProjects (WikiMacroBase)
TracSVNHooks 0.1
TracTicketDelete 2.0
excel-report-plugin

I couldn't disable our CdicWorkflow plugin because it's the permissions provider for our workflow. It only implements ITicketActionController and IPermissionRequestor.

I enabled the DEBUG log and didn't see anything informative.

Our system is running fine with the older version of web_ui.py, but I'm willing to help diagnosis this problem.

comment:6 by Christian Boos, 12 years ago

Milestone: 0.11

comment:7 by Eli Carter, 12 years ago

Can you comment out the permissions in the config, disable that plugin, and try reproducing with that?

Are you able to post your ITicketActionController part of that plugin? I'm not seeing any custom operations in your config; is your ITicketActionController handling all actions?

Hm. Part of r6919 also changed when the get_ticket_changes was called relative to populating the changes made to the ticket. If you are using ITicketActionController to try to validate that changes are OK, you'll want to use the ITicketManipulator interface instead. Alternatively, make an argument why the _populate() call should be done before the get_ticket_changes() calls.

by robert.nadler@…, 12 years ago

Attachment: CdicWorkflow.py added

comment:8 by robert.nadler@…, 12 years ago

I've attached our workflow plugin (CdicWorkflow.py). Most of the workflow is done through the standard trac.ini defined actions. The plugin is used to copy developer and tester custom fields to the owner for status changes between assigned/fixed/tested.

As you'll see, the plugin does not validate changes from the standard workflow.

Now that you have everything, you'll hopefully be able to reproduce the problem.

comment:9 by Eli Carter, 12 years ago

Looking at your plugin, I'd suggest a different approach. If you make a set_owner_to_field operation, and had it understand a .owner_to_field option, the workflow you have implemented in this plugin can then be described in the .ini, and other workflows can as well.

The change in the location of the _populate() call may still be an issue; you wouldn't be able to set the developer field in the ticket and use the operation in the same step. If you move the _populate() call to just after the elif req.method == 'POST' line, does your problem go away?

comment:10 by robert.nadler@…, 12 years ago

I've never heard of the set_owner_to_field operation. It's not in the documentation or default_workflow.py. In any case, we needed the action labels and behavior to change based on the custom field contents.

Our workflow plugin does not have a _populate() call or a elif req.method == 'POST' line, so I'm not sure what you're referring to.

Were you able to reproduce the problem with this workflow and plugin?

comment:11 by Eli Carter, 12 years ago

Apparently my explanation was confusing; you weren't the only one confused.

The _populate() call I was talking about is in trac/ticket/web_ui.py in the Trac core. The call to that method used to occur before the call to all the get_ticket_changes() methods, but when fixing #6879, I moved the _populate() call to after that point [6919]. This means that the workflow plugins do not currently see the changes the user made, but they used to. I expect I will need to change it back to calling _populate() first.

As for my suggestion about a different approach, that was a suggestion about a different way to write your plugin. It would have required implementing a set_owner_to_field operation, and then using that. However, I went ahead and implemented that and a couple other operations as a plugin, AdvancedTicketWorkflowPlugin

Using that, you should be able to move your business logic out of the plugin, and into your trac.ini.

I have not tried reproducing the problem with your plugin yet.

comment:12 by Eli Carter, 12 years ago

I convinced myself the _populate() call needs to move back to where it was before [6919], so that is back to the way it was. Done in 0.11-stable [7014] and trunk [7016]. Please verify that fixes the behavior of your current plugin. Then rework your workflow using the set_owner_to_field operation from the AdvancedTicketWorkflowPlugin.

comment:13 by Eli Carter, 12 years ago

Status: assignednew

comment:14 by Eli Carter, 12 years ago

Status: newassigned

comment:15 by Eli Carter, 12 years ago

I made an attempt to reproduce the reported problem, but without success.

  • Updated to trunk@6919
  • Replaced tab characters in CdicWorkflow.py with 8 spaces.
  • Replaced [ticket-workflow] section with provided config.
  • Add CDICActionController to [ticket] workflow.
  • Added custom fields:
    [ticket-custom]
    developer = text
    developer.label = CDIC developer
    tester = text
    tester.label = CDIC tester
    
  • Logged in as user with TRAC_ADMIN.
  • Set developer and tester fields to other users (alice and bob)
  • Tried various actions such as AssignTester, TestFailed, etc.
  • Did not reproduce the problem.

comment:16 by Eli Carter, 12 years ago

At this point, the workflow you want to implement should be doable with wiki:AdvancedTicketWorkflowPlugin using its set_owner_to_field operation, and the IPermissionRequestor portion of your plugin.

If I don't hear anything on this, I'll close this ticket next week.

comment:17 by robert.nadler@…, 12 years ago

It's too bad you weren't able to reproduce the problem. Your tests were probably complete, but just to be sure: The problem I had was with workflow actions (like assign) that had a set_owner operation, not the custom actions.

The new AdvancedTicketWorkflowPlugin looks great. Unfortunately we just completed a full process validation with our custom workflow and went live last week. Changes to the workflow implementation of this magnitude would require a significant re-validation. If I even suggested that to my team this early-on I'm sure they'd string me up. "If it ain't broke, don't fix it" will have to do for now.

Thanks for all your help on this!

comment:18 by Eli Carter, 12 years ago

Resolution: fixed
Status: assignedclosed

I'm pretty sure I did cover that in my testing; so I'm going to close this.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Eli Carter.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Eli Carter 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.