camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.03k stars 1.53k forks source link

Get user task query provides comment and attachment properties #2404

Open ThorbenLindhauer opened 3 years ago

ThorbenLindhauer commented 3 years ago

This issue was imported from JIRA:

Field Value
JIRA Link CAM-12513
Reporter NYBzR2x
What 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 fetching user tasks, I want to check if attachments and comments have been added so that I can inform the task user accordingly.

Background

Fetching task objects via API does not provide properties to identify if a comment or attachment has been added to the user task.

Without these fields, the customer has to perform an additional query per field to get the desired information - see attachment <^TaskClass_changes.docx>

Acceptance criteria

Links:

### Pull requests
- [ ] https://github.com/camunda/camunda-bpm-platform/pull/4306
sumankumarpani commented 3 months ago

@yanavasileva, I am interested to work on this feature, please help with any additional information.

yanavasileva commented 3 months ago

Hello @sumankumarpani,

Great, thank you for your interest!

Here are the entry points for the REST API:

This ticket is only for the JSON scenario, so no need to consider the HAL scenario below this: https://github.com/camunda/camunda-bpm-platform/blob/master/engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/sub/task/impl/TaskResourceImpl.java#L184-L186

As end result of the ticket, I would expect the TaskDto to be extended with two more fields: comment: true/false" and "attachment": true/false

You can explore the options how to populate the information. Maybe it will be sufficient to query for existing task comments/attachments as we won't need the complete entries.

One more thing, the OpenAPI should be adjusted accordingly with this ticket: https://github.com/camunda/camunda-bpm-platform/blob/master/engine-rest/engine-rest-openapi/src/main/templates/models/org/camunda/bpm/engine/rest/dto/runtime/task/TaskDto.ftl

I stay open for further questions.

Best, Yana

sumankumarpani commented 3 months ago

Thank you for the providing details around the issue and the expectations, I will start working considering these points and keep you posted on the progress and questions if any.

sumankumarpani commented 3 months ago

@yanavasileva, I wanted to understand what will be the best approach for us to check existence of comments and attachments. We have existing methods like getAttachments & getComments which we can reuse, but it might be expensive operation as it reads and returns the actual list of data for the taskid , or we can introduce queries to just check the counts and decide boolean results based on that. While I prefer the second approach, let me know what you think.

Update: After doing some changes I think we can reuse methods such as getAttachments & getComments , and I assume it wont be that expensive. Here's something I am planning to introduce: attachment = !getProcessEngine().getTaskService().getTaskAttachments(id).isEmpty()

sumankumarpani commented 3 months ago

@yanavasileva @danielkelemen, for providing data related to attachments and comments for a task below are the ideas I have. Option 1: Reusing existing getAttachments and getComments method to populate these boolean flags Option 2: Introducing queries that checks counts of comments and attachments and decide based on the returned count.

My initial impression was to go with option2 but later I feel we can reuse codes with option1 and achieve the goal with lesser code change.

Let me know what's your thoughts.

yanavasileva commented 3 months ago

@sumankumarpani, to reduce costs, we can introduce withCommnentAttachmentInfo filter criteria so we populate the data only if that parameter is passed. Then we can reuse the existing methods as suggested in Option 1.

sumankumarpani commented 3 months ago

Thanks for reviewing this @yanavasileva , this seems like a nice idea. Since we keep attachments and comments field in the TaskDto , when user doesn't pass true for the filter "withCommentAttachmentInfo" still the response will contain both the parameters as false. When user enables the filter our service will evaluate the actuals.

We will update the swagger with description about this, but thinking will this behaviour create any confusions for end users ?

yanavasileva commented 3 months ago

Valid concern. We do similar thing already for startProcessInstance API (REST docs). There we have ProcessInstanceDto and ProcessInstanceWithVariablesDto. You can check the REST API, OpenAPI, and Java API for the endpoint, for a reference.

sumankumarpani commented 2 months ago

Pr raised for this feature, please help to review.

https://github.com/camunda/camunda-bpm-platform/pull/4306