Activiti: PUT Task Variable Admin API is missing

[mauriziovitale]

We should have an API that allows an admin to update a task variable. Right now this endpoint is missing

[CTI777]

@erdemedeiros @salaboy
We have in RB TaskVariableController, but do not have at all TaskVariableAdminController.
Should we consider what else may be needed?

[erdemedeiros]

@CTI777 yes, please, add it to TaskVariableAdminController.

[CTI777]

@erdemedeiros @salaboy
I have a question regarding RB changes
We have a POST method setVariables (in TaskVariableControllerImpl) which works like set and update...
Update task variables (PUT) should work only in admin area.
How should I continue:

1. should we allow update in non admin area?
2. If we will add update method to admin area can I check existing variable using getVariables method? It seems that approach used for admin update process variables is not applicable here…

[salaboy]

as we discussed in the standup I would do as follow:
1) User (the activiti API internally it should check that the user sending the request is the assignee of the task to perform these actions, if the user sending the request is not the assignee these methods should fail):

• POST newVariable (only creates a new variable if it doesn't exist, if it exist it should fail)
• PUT update Variable (only update the value of an existing variable)
  2) Admin (can change any task, no matter if he/she is the assignee or candidate)
  
  
  
  • POST newVariable (only creates a new variable if it doesn't exist, if it exist it should fail)
  
  
  • PUT updateVariable (only update the value of an existing variable)
  
  

We should remove the method setVariable to be absolutely clear on what the behaviour is.

@CTI777 is it clearer now?

[CTI777]

To conclude (for user and admin area):
get: variables
post: newVariables
put: updateVariables

Question: to check existing task variables can I use getVariables?

[CTI777]

@salaboy
If user want to update / create several task variables, and one variable fail (e.g. try to create already existing var): should operation for all variables fail?

[salaboy]

@CTI777 I am having problems with the plural.. I wonder if we just do newVariable and updateVariable.
Is there any case where we want to update multiple variables or create multiple variables at the same time?
@mauriziovitale can you please clarify?

[mauriziovitale]

@salaboy right now we don't have any specific requirements related to the multiple variables. So we can start with newVariable and updateVariable

[salaboy]

@mauriziovitale awesome! @CTI777 let's try to do that then.. ping me in gitter if you have questions.

[ryandawsonuk]

Just to confirm current status - there are currently two PRs for the core level on this and we're just starting to look at the cloud level? It looks to me like we're just starting on the cloud level because I don't see any branch on activiti-cloud-runtime-bundle-service and it seems it was just agreed in the last comments what should be done for cloud.

[CTI777]

@ryandawsonuk
I have made currently 2 PR's: API, Activiti. The next will be RB, which is almost ready... I will do it next hours...

[CTI777]

Have made PR for RB.
I still have several questions:

1. Who can get task variables (implementation in Activiti) - currently no checks for assignee.
   If we will add check for assignee some tests may fail...
2. It seems that Type of variable is not supported in set/update variable
3. I do not change task variables Payload: SetTaskVariablesPayload.
   Maybe it is better to call it
   NewTaskVariablePayload
UpdateTaskVariablePayload
   But it seems that it would be better to refactor it (the changes will affect tests, some other modules)
4. SetTaskVariableCmdExecutor this is used currently for New Task Variable. To add same for Update Task Variable - will cause problems, because they have same payload type

[erdemedeiros]

@salaboy, @ryandawsonuk and I have some concerns about

We should remove the method setVariable to be absolutely clear on what the behaviour is.

This means a breaking change in the API, so I don't think we should do it for 7.0.x. Probably we should keep existing APIs methods (both Java and REST) as they are and only introduce the two new entries createVariable and updateVariable. Note that in terms of naming I prefer createVariable to newVariable.

[salaboy]

@erdemedeiros totally agree.. so let's not remove the old payloads and methods.. just add new ones with the new payloads as suggested by @CTI777

[CTI777]

@salaboy @erdemedeiros
What about createVariable vs newVariable?
Should I make createVariable method?

[salaboy]

@CTI777 createVariable please.

[CTI777]

@salaboy Ok, will try to complete things today evening...