camunda-bpm-platform
camunda-bpm-platform copied to clipboard
Java and Rest API supports update and delete of task and process instance comments
This issue was imported from JIRA:
Field | Value |
---|---|
JIRA Link | CAM-12761 |
Reporter | NYBzR2xWhat is this name?This pseudonym name was generated based on the user name in JIRA to protect the personal data of our JIRA users. You can use this identifier to search for issues by the same reporter. |
Has restricted visibility comments | false |
User story
When using ACTHICOMMENT to save user comments for tasks and process instances, I want to be able to update and delete comments.
Background
Currently, the rest API supports the creation of comments for tasks - to complete the functionality and user journey around comments, the ability to updated and delete comments via REST API is requested (also for process instances).
Acceptance criteria
- I can use the Rest API to update and delete comments added to tasks
- All Comment APIs are covered by authorization checks
- cover OpenAPI documentation
Links:
- is caused by https://jira.camunda.com/browse/SUPPORT-9095
- is related to https://jira.camunda.com/browse/CAMPLAN-2
### Pull requests
- [ ] https://github.com/camunda/camunda-bpm-platform/pull/4333
Let me work on it
@ThorbenLindhauer do we need to change UI as well or just the API?
@ThorbenLindhauer @yanavasileva
I am thinking about these APIs:
DELETE - comment by ID, task ID or ProcessId
What do you guys think?
Hi @prajwolbhandari1,
It's great to see you are onto your next contribution.
do we need to change UI as well or just the API?
The scope of the ticket is only REST and Java API, no need for changes in the UI.
DELETE - comment by ID, task ID or ProcessId
That's a good start. My tip is to have a look at delete attachment (Java/REST) API, it might be similar implementation.
I see the ticket scope targets an update comment API as well. If you consider covering that in your PR as well, you will need something like TaskService#saveComment(comment)
and then PUT request similar to the updateTask
REST API (link).
I hope that helps to get you out of the door.
Best, Yana
Thanks for your reply Yana.
Let me work on these two endpoints:
- Delete by commentID endpoint just what you have suggested above that is similar to attachment.
- Update a comment endpoint as you have stated above.
Do we need to have endpoints that deletes all comments associated to taskId/processInstanceId or implementing above two endpoints will work for now?
Thank you again, Yana.
Looking into further, looks like deleting comments associated by taskId/processInstanceId makes sense. Let me implement those as well.
@yanavasileva story says it needs to do this:
All Comment APIs are covered by authorization checks
Can you point me out an example where it does it checks authorization checks?
Here: https://github.com/camunda/camunda-bpm-platform/blob/master/engine/src/main/java/org/camunda/bpm/engine/impl/GetDeployedTaskFormCmd.java#L49-L51 More info: https://docs.camunda.org/manual/7.21/user-guide/process-engine/authorization-service/#permissions-by-resource I would expect for both delete and update comments
- by task id to check for
update
permission ontask
resource - by process instance id to check for
update
permission onprocess instance
resource
You might notice that authorization checks are not performed for the existing comment (and attachment) APIs. I think we should still add them for newly introduced APIs. Also I don't think we need to introduce a new resources (similar to what's suggested here: https://github.com/camunda/camunda-bpm-platform/issues/2090#issuecomment-1275801172)
Here: https://github.com/camunda/camunda-bpm-platform/blob/master/engine/src/main/java/org/camunda/bpm/engine/impl/GetDeployedTaskFormCmd.java#L49-L51 More info: https://docs.camunda.org/manual/7.21/user-guide/process-engine/authorization-service/#permissions-by-resource I would expect for both delete and update comments
- by task id to check for
update
permission ontask
resource- by process instance id to check for
update
permission onprocess instance
resourceYou might notice that authorization checks are not performed for the existing comment (and attachment) APIs. I think we should still add them for newly introduced APIs. Also I don't think we need to introduce a new resources (similar to what's suggested here: #2090 (comment)) -- You mean we do or don't need to introduce a new resource? Just wanted to make sure :)
-- You mean we do or don't need to introduce a new resource? Just wanted to make sure :)
We don't need to introduce a new resource. My bad for the wording.
@yanavasileva opened a PR. Can you please take a look at it when you get a chance?
https://github.com/camunda/camunda-bpm-platform/pull/4333
@yanavasileva Do you mind taking a look at this change?
Requested changes from the contributor last week.