Task api enhancement to include attachment and comments fields. #4306

sumankumarpani · 2024-04-29T05:42:29Z

Feat(engine) : User task api provide comment and attachment properties

Enhance the get tasks api to include "attachments" and "comments" boolean field which return existence of associated attachment and comments with the task.
Include boolean filter 'withCommentAttachmentInfo' and evaluate existence of attachment and comments when true.
Enhance Swagger specs

related to GIT Issue : #2404

yanavasileva · 2024-05-13T06:23:50Z

Hi @sumankumarpani,

Thank you for raising this.
We will have a look and get back to you.

Best,
Yana

sumankumarpani · 2024-05-16T04:56:30Z

Thanks @yanavasileva , let me know what are your thoughts/ suggestions.

yanavasileva

@sumankumarpani, I haven't finish completely the review yet but I want to share early feedback and store my notes.
👍 Overall expression is that your approach is good, I just want to have a second look.

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/TaskEntity.java

engine/src/main/java/org/camunda/bpm/engine/impl/TaskQueryImpl.java

engine/src/main/java/org/camunda/bpm/engine/task/Task.java

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/TaskEntity.java

engine/src/main/java/org/camunda/bpm/engine/task/TaskQuery.java

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/dto/task/TaskDto.java

CLAassistant · 2024-05-24T19:48:37Z

All committers have signed the CLA.

yanavasileva

Thank you for considering my feedback so far, I managed to complete the review now.
Please see the comments below.

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/dto/task/TaskDto.java

engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/TaskEntity.java

engine/src/main/java/org/camunda/bpm/engine/task/TaskQuery.java

engine/src/test/java/org/camunda/bpm/engine/test/api/task/TaskQueryTest.java

engine/src/main/java/org/camunda/bpm/engine/impl/TaskQueryImpl.java

engine/src/test/java/org/camunda/bpm/engine/test/api/task/TaskQueryTest.java

yanavasileva · 2024-05-28T10:12:45Z

...t-openapi/src/main/templates/models/org/camunda/bpm/engine/rest/dto/runtime/task/TaskDto.ftl

❌ I don't see an action on this comment (in OpenAPI, REST API, Java API):
#2404 (comment)
One option is the described here:
#2404 (comment)
Other option will be to add comment to the APIs that can be true only if withCommentAttachmentInfo filter is passed. That might create confusion in users.

@yanavasileva , I am exploring option 1 of extending the dto class and adding additional variables.

I got a doubt @yanavasileva , our changes should be applicable to GET & POST /task apis only ,and not /task/
{id} , let me know if my understanding is correct?

@sumankumarpani, the ticket was initially reported for GET /task REST API but covering GET /task/{id} will add consistency to the endpoints. The TaskDto is used as a return type for all of those endpoints so the made changes so far make sense.

Got it, Thanks.

yanavasileva · 2024-05-28T10:14:03Z

...t-openapi/src/main/templates/models/org/camunda/bpm/engine/rest/dto/runtime/task/TaskDto.ftl

❌ In all OpenAPI files, hasAttachment/hasComment changes are not reflected.

github-actions · 2024-06-25T04:05:40Z

Closing due to lack of requested feedback. If you would like us to look at this, please provide the requested information to re-open the PR.

sumankumarpani · 2024-06-25T14:36:05Z

@yanavasileva Please help to reopen the pr, I have worked the changes but facing issues to sync it , requesting some time to sync my changes and update the pr.

sumankumarpani · 2024-06-25T14:43:57Z

Hi, Please help to reopen the pr, I have worked on the changes requested, but facing some issues with syncing them. Requesting few days of time to sync my changes to this pr.

…

On Tue, 25 Jun, 2024, 9:36 am github-actions[bot], ***@***.***> wrote: Closed #4306 <#4306>. — Reply to this email directly, view it on GitHub <#4306 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJBYVNYNTGMGBIOANZ3Z3OLZJDUCVAVCNFSM6AAAAABG5VJFSGVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJTGI3TIOBQGI3DAOA> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

github-actions · 2024-07-05T04:06:16Z

Closing due to lack of requested feedback. If you would like us to look at this, please provide the requested information to re-open the PR.

Signed-off-by: Kumar Pani, Suman <Suman.KumarPani@fmr.com>

sumankumarpani · 2024-07-30T09:58:24Z

@yanavasileva I have added some changes, please help to review them again.

yanavasileva

Hi @sumankumarpani,

Thank you for your patience with this.
I am almost done with the review. I need to run some tests and clarify open points before we can merge the feature.
I have added my notes so they are documented for everyone to get access to them for reference. I will let you know in case any further input/action is needed from your side.

Best,
Yana

engine/src/main/java/org/camunda/bpm/engine/impl/TaskQueryImpl.java

yanavasileva · 2024-08-17T07:57:06Z

...st/engine-rest/src/test/java/org/camunda/bpm/engine/rest/TaskRestServiceInteractionTest.java

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;


❌ Not needed as far as I can see.

Suggested change

import org.slf4j.Logger;

import org.slf4j.LoggerFactory;

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/dto/task/TaskDto.java

yanavasileva · 2024-08-17T08:06:54Z

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/TaskRestService.java


 @Produces(MediaType.APPLICATION_JSON)
 public interface TaskRestService {

  public static final String PATH = "/task";

  @Path("/{id}")
-  TaskResource getTask(@PathParam("id") String id);
+  TaskResource getTask(@PathParam("id") String id, @QueryParam("withCommentAttachmentInfo") Optional<Boolean> withCommentAttachmentInfo);


Note to myself: check CI and if there are cons against using Optional.

@sumankumarpani, the engine-rest changes are breaking the tests for one of our supported environments. I am checking how is best to adjust the new parameter and preserve backwards compatibility. I will contact you further with my findings.

If you want to see the failures for yourself, you can execute the following (with JDK 11):
mvn clean install -Pwildfly-compatibility -f engine-rest/engine-rest/pom.xml

@yanavasileva I am seeing this error, are you seeing the same.
Response body doesn't match expectation.
Expected: is ""
Actual: {"type":"RuntimeException","message":"RESTEASY003875: Unable to find a constructor that takes a String param or a valueOf() or fromString() method for javax.ws.rs.QueryParam("withCommentAttachmentInfo") on public org.camunda.bpm.engine.rest.sub.task.TaskResource org.camunda.bpm.engine.rest.impl.TaskRestServiceImpl.getTask(java.lang.String,java.util.Optional) for basetype: java.util.Optional","code":null}

[ERROR] TaskVariableRestResourceInteractionTest.testGetObjectVariables:162 5 expectations failed.
Expected status code <200> but was <500>.

I couldn't find the exact reason yet, will update here if I get any clue. Meanwhile let me know if you recall handling similar in the past.

Yes, I observe similar errors.

@yanavasileva I modified the the param Optional withCommentAttachmentInfo to Boolean withCommentAttachmentInfo and given it a default value false, to make it work. Let me know if we can proceed with the solution.

Boolean withCommentAttachmentInfo and given it a default value false, to make it work.

It seems like a better fit. We use similar approach in other places too.
Please adjust the code, I will run then the tests again.

@yanavasileva I have pushed changes , please take a look.

engine-rest/engine-rest-openapi/src/main/templates/lib/commons/task-query-params.ftl

yanavasileva · 2024-08-17T08:13:02Z

engine-rest/engine-rest-openapi/src/main/templates/paths/task/get.ftl

the response body can be TaskWithA..Dto, do we need to adjust the response dto in the OpenAPI?

…enhancements

yanavasileva

@sumankumarpani, I checked the changes. Overall it looks good, I need to perform some final testing.

Have a look at my suggestions.

Heads up, we approach the code freeze of the minor release.
❗ If you want the feature to be included in 7.22.0, please apply the feedback by 17th of September.

yanavasileva · 2024-09-09T15:47:06Z

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/TaskRestService.java

@@ -29,14 +29,15 @@
 import javax.ws.rs.core.Request;
 import javax.ws.rs.core.UriInfo;
 import java.util.List;
+import java.util.Optional;


❌ Not needed anymore:

Suggested change

import java.util.Optional;

yanavasileva · 2024-09-09T15:55:39Z

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/impl/TaskRestServiceImpl.java

@@ -19,6 +19,8 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;


❌ Not needed:

Suggested change

import java.util.Optional;

import java.util.Optional;

yanavasileva · 2024-09-09T15:58:34Z

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/impl/TaskRestServiceImpl.java

+    queryDto.setObjectMapper(getObjectMapper());
+
+    ProcessEngine engine = getProcessEngine();
+    TaskQuery query = queryDto.toQuery(engine);
+
+    List<Task> matchingTasks = executeTaskQuery(firstResult, maxResults, query);
+
+    List<TaskDto> result;
+    if (Boolean.TRUE.equals(queryDto.getWithCommentAttachmentInfo())) {
+      result = matchingTasks.stream().map(TaskWithAttachmentAndCommentDto::fromEntity).collect(Collectors.toList());
+    }
+    else {
+      result = matchingTasks.stream().map(TaskDto::fromEntity).collect(Collectors.toList());
+    }
+    return result;


❓ Do we need this change? Now the code is duplicated.

Suggested change

queryDto.setObjectMapper(getObjectMapper());

ProcessEngine engine = getProcessEngine();

TaskQuery query = queryDto.toQuery(engine);

List<Task> matchingTasks = executeTaskQuery(firstResult, maxResults, query);

List<TaskDto> result;

if (Boolean.TRUE.equals(queryDto.getWithCommentAttachmentInfo())) {

result = matchingTasks.stream().map(TaskWithAttachmentAndCommentDto::fromEntity).collect(Collectors.toList());

}

else {

result = matchingTasks.stream().map(TaskDto::fromEntity).collect(Collectors.toList());

}

return result;

return queryTasks(queryDto, firstResult, maxResults);

yanavasileva · 2024-09-09T16:00:51Z

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/impl/TaskRestServiceImpl.java

+    else {
+      result = matchingTasks.stream().map(TaskDto::fromEntity).collect(Collectors.toList());
+    }
+    return result;
  }

  public HalTaskList getHalTasks(UriInfo uriInfo, Integer firstResult, Integer maxResults) {


📓 No action, just comment. The param is not considered here. As Hal queries are used in Tasklist where we need performance.

Thanks @yanavasileva for your reviews. Certainly I am willing these changes to be part of 7.22 release. I am done with addressing the comments and unit/integrations tests are running fine. Please let me know if the platform/e2e tests ran well or need something to be addressed, I will push the changes accordingly at a single instance and we can wrap it up.

@yanavasileva I have addressed the pending comments.

engine-rest/engine-rest-openapi/src/main/templates/paths/task/get.ftl

engine-rest/engine-rest/src/main/java/org/camunda/bpm/engine/rest/dto/task/TaskDto.java

engine-rest/engine-rest-openapi/src/main/templates/lib/commons/task-query-params.ftl

yanavasileva

👍
Thank you for the work on this! Changes now look good to me.
I will merge the PR later today and it will be part of 7.22.0 scheduled for next month.

sumankumarpani · 2024-09-16T14:29:12Z

👍 Thank you for the work on this! Changes now look good to me. I will merge the PR later today and it will be part of 7.22.0 scheduled for next month.

Thanks for the update Yana :)

sumankumarpani mentioned this pull request Apr 29, 2024

Get user task query provides comment and attachment properties #2404

Closed

danielkelemen requested a review from yanavasileva May 3, 2024 12:48

danielkelemen assigned sumankumarpani May 3, 2024

yanavasileva reviewed May 22, 2024

View reviewed changes

yanavasileva requested changes May 28, 2024

View reviewed changes

yanavasileva added group:requested-info DRI: Yana group:stale DRI: Yana labels Jun 12, 2024

github-actions bot added group:stale DRI: Yana and removed group:stale DRI: Yana labels Jun 15, 2024

github-actions bot added the group:closed-as-inactive DRI: Yana label Jun 25, 2024

github-actions bot closed this Jun 25, 2024

yanavasileva reopened this Jun 26, 2024

github-actions bot added group:stale DRI: Yana and removed group:closed-as-inactive DRI: Yana group:stale DRI: Yana labels Jun 27, 2024

github-actions bot added the group:closed-as-inactive DRI: Yana label Jul 5, 2024

github-actions bot closed this Jul 5, 2024

yanavasileva removed group:requested-info DRI: Yana group:closed-as-inactive DRI: Yana group:stale DRI: Yana labels Jul 5, 2024

yanavasileva reopened this Jul 5, 2024

task api enhancements to include attachments and comments

b03a241

Signed-off-by: Kumar Pani, Suman <Suman.KumarPani@fmr.com>

brianwarner force-pushed the feature/ENTDAPCOE-126-task-api-enhancements branch from 3ea772d to b03a241 Compare July 22, 2024 18:56

yanavasileva self-requested a review August 13, 2024 06:42

yanavasileva reviewed Aug 17, 2024

View reviewed changes

sumankumarpani added 2 commits August 27, 2024 20:19

Merge branch 'refs/heads/master' into feature/ENTDAPCOE-126-task-api-…

7bfb2bc

…enhancements

update query param type to fix wildfly compatibility

3e6f2e4

yanavasileva self-requested a review September 7, 2024 10:34

yanavasileva requested changes Sep 9, 2024

View reviewed changes

Cleanup unused imports, code cleanup

6c7e383

yanavasileva approved these changes Sep 16, 2024

View reviewed changes

yanavasileva merged commit ddf039c into camunda:master Sep 16, 2024
1 check passed

Task api enhancement to include attachment and comments fields. #4306

Task api enhancement to include attachment and comments fields. #4306

Conversation

sumankumarpani commented Apr 29, 2024 • edited Loading

yanavasileva commented May 13, 2024

sumankumarpani commented May 16, 2024

yanavasileva left a comment • edited Loading

Choose a reason for hiding this comment

CLAassistant commented May 24, 2024 • edited Loading

yanavasileva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanavasileva Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 25, 2024

sumankumarpani commented Jun 25, 2024

sumankumarpani commented Jun 25, 2024 via email

github-actions bot commented Jul 5, 2024

sumankumarpani commented Jul 30, 2024

yanavasileva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanavasileva Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

sumankumarpani Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanavasileva Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanavasileva Aug 17, 2024 • edited Loading

Choose a reason for hiding this comment

yanavasileva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanavasileva left a comment

Choose a reason for hiding this comment

sumankumarpani commented Sep 16, 2024

sumankumarpani commented Apr 29, 2024 •

edited

Loading

yanavasileva left a comment •

edited

Loading

CLAassistant commented May 24, 2024 •

edited

Loading

yanavasileva Jun 12, 2024 •

edited

Loading

yanavasileva Aug 20, 2024 •

edited

Loading

sumankumarpani Aug 21, 2024 •

edited

Loading

yanavasileva Aug 28, 2024 •

edited

Loading

yanavasileva Aug 17, 2024 •

edited

Loading