ADempiere ERP Business Suite

Here is a summary of the descriptions for the two concerns of the attached
patch set: "Atomic Workflow Activities" and "Making Workflows more
Predictable". This patch set supersedes tracker item [ 1734897 ] "Making
Workflows more predictable".

Atomic Workflow Activities
--------------------------

1.
--
The most important change, which triggers most of the other changes,
happens in the 'run()' method of 'MWFActivity.java'. It is changed to that
setting the workflow state of the activity via 'setWFState(...)' and
performing the workload of the activity via 'performWork(...)' happens in a
closed transaction. Should an exception occur (or no commit) then
everything is rolled back.

To make this possible it was necessary to change the methods
'setWFState(...)' and 'setVariable(...)' to take the current transaction
into account.

In order to avoid a DB lockup it was also necessary to set the current
process of an activity prior to calling 'MWFActivity.setWFState(...)'
because the process was not set in the constructor method used (see
'MWFProcess.setWFState(...)'). For this a new helper method
'setProcess(...)' is introduced.

2.
--
In 'MWFProcess.checkActivities(...)' closed activities trigger the start of
new activities. This must not happen for uncommitted activities because
those might still be rolled back. As a consequence 'checkActivities(...)'
should read the activities via 'getActivities(...)' outside of the current
transaction context. For this a new method signature for
'getActivities(...)' taking the current transaction into account has to be
created. A wrapper method with the same signature as the old method is
created to stay compatible.

Additionally the workflow state should be set inside of a transaction
context if transaction is active. For this the method signature of
'MWFProcess.setWFState(...)' is extended to take a given transaction into
account. Just as above a wrapper method with the same signature as the old
method is created to stay compatible.

3.
--
My attempts to manage workflow processes or activities via the buttons in
the corresponding windows/tabs were not really successfull. As a first try
to improve this situation it seems sensible to explicitly set the
'Processed' flag of an activity in 'WFActivityManage.doIt()'. Otherwise the
activity is left in an "unmanageable" state which lead to the situation
that a new workflow for a requisition can not be started and the whole
document has be entered again.


Making Workflows more Predictable
---------------------------------

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(...)).