Making Workflows more predictable

After setting up a new client I was playing around with the requisition workflow only to find out, that I could not reliably determine which user a newly created workflow activity was going to be routed to. This was true for simple workflow steps and even more so for approval steps. Partly this was surely due to my limited experience with adempiere workflows in general, on the other hand it was easy to create situations which should never happen, like workflow activities being visible to each and every user in an adempiere installation.

The patch set in Tracker item 1734897
(http://sourceforge.net/tracker/index.php?func=detail&aid=1734897&group_id=176962&atid=879334)
tries to improve the situation and is proposed for rewiew to be included in adempiere trunk. I tried to include extensive documentation for every modification in the patches themselves; this should of course be shortened to a sensible amount of code comments.

What was observed/changed specifically:

1.
--
Routing of workflow activities is done by assigning an 'AD_User_ID' to an activity. The system tried to get this 'AD_User_ID' from the responsible of the node underlying the activity by directly calling getAD_User_ID(). This yielded unexpected effects in situations when the responsible type was not 'Human' but 'Role' or 'Org' because the workflow administrator could not see or influence what was stored in the 'AD_User_ID'-Field of a responsible in these cases. Even worse: When selecting the responsible type 'Org' the fields AD_User_ID and AD_Role_ID were filled with default values from the currently logged on user

Proposed solution: Make sure that the fields AD_Org_ID, AD_Role_ID and AD_User_ID of the workflow responsible have predictable values, i.e

Responsible type 'Org' => AD_Role_ID = null, AD_User_ID = null; routing is intended to the supervisor of the Org.

Responsible type 'Role' => AD_User_ID = null; routing is intended to the supervisor of the role; qualifying a specific, not null, role is mandatory.

Responsible type 'Human' => AD_Role_ID = null; routing is intended to the specified user, or if the user is not specified, routing is delegated to the process level.

The following changes implement the desired behaviour:
a) Make the AD_Role_ID in AD_WF_Responsible not generally mandatory. This requires a change in the database an a new generation of X_AD_WF_Responsible.java.
b) Set AD_User_ID to null when the responsible type is not 'Human'; set AD_Role_ID to null when the responsible type is not 'Role' in the beforeSave(...) method of MWFResponsible.java.
c) Create '<NULL>' as newly allowed tag for field default values in GridField.java and use this tag in AD_WF_Responsible.AD_User_ID and AD_WF_Responsible.AD_Role_ID.

2.
--
It seems wrong to determine the AD_User_ID for an Activity by simply getting the AD_User_ID of the responsible of the underlying node, irrespective of the responsible type.

Proposed solution: Implement a new method getRespAD_User_ID() (and helper method getOrg()) in MWF_Responsible.java which returns the user id in the case of 'Human' as the responsible type and tries to return the supervisor user id in the case of 'Role' or 'Org' as the responsible type, and use this method instead of getAD_User_ID() in MWFActivity.java nd MWFProcess.java.

3.
--
Make a better effort to find a matching routing target (user id) in MWFActivity.setResponsible(...) and create a fallback mechanism: No routing target found => send activity to the org supervisor or the SuperUser.

4.
--
Make sure that the 'ownDocument' flag is set correctly for every checked user in MFWActivity.getApprovalUser(...).

5.
--
Make sure that the right roles are found in MFWActivity.getApprovalUser(...) by calling 'user.getRoles(AD_Org_ID)'. ALL roles for the user should be returned which allow him/her access to the given org, not only the the roles with direct org access, but also those which delegate org access to the user level. This is implemented by an extended sql query in MUser.getRoles(...).

6.
--
Send a notification to the document owner in case an exception is thrown in the course of a workflow (see MFWActivity.run()).

7.
--
Make trying to find an approval user not dependent on the fact that the underlying node has an 'invoker' responsible; terminate the workflow (by throwing an exception) and set the document status accordingly if no approval user can be found (see MFWActivity.performWork(...) /* User Choice*/).

8.
--
Remove the check if a user is in a role, which allows him to approve his own document, from the beginning of the MWFActivity.setUserChoice(...)-method. IMHO this is against the meaning of 'Approve own document': Why should a user who is faced with the task of approving documents generally be required to have the ability to approve his OWN documents? If the document to approve really IS his own document this will be respected when trying to find an approval user in the call to getApprovalUser(...).

9.
--
Add record information to the note sent to a document owner if a document was not approved (see MWFActivity.setUserChoice(...)).

Regards

Matthias