Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -657,5 +657,12 @@
<@lib.parameter name = "parentTaskId"
location = "query"
type = "string"
desc = "Restrict query to all tasks that are sub tasks of the given task. Takes a task id." />

<@lib.parameter name = "withCommentAttachmentInfo"
location = "query"
type = "boolean"
defaultValue = "false"
last = last
desc = "Restrict query to all tasks that are sub tasks of the given task. Takes a task id." />
desc = "Check if task has attachments and/or comments. Value may only be `true`, as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Please add performance considerations to the description. Adding the filter will do additional attachment and comments queries to the database, that might slow down the query in case of big size of the tables.

`false` is the default behavior." />
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,18 @@
<@lib.property
name = "tenantId"
type = "string"
last = true
desc = "If not `null`, the tenant id of the task." />

<@lib.property
name = "attachment"
type = "boolean"
desc = "Specifies if an attachment exists for the task." />

<@lib.property
name = "comment"
type = "boolean"
last = true
desc = "Specifies if an comment exists for the task." />

</@lib.dto>
</#macro>
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@
"binding": "version",
"version": 2
},
"tenantId": "aTenantId"
"tenantId": "aTenantId",
"attachment":false,
"comment":false
}
]
}'] />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@
"binding": "version",
"version": 2
},
"tenantId":"aTenantId"
"tenantId":"aTenantId",
"attachments":false,
"comments":false
}
]
}',
Expand Down Expand Up @@ -148,7 +150,9 @@
"caseDefinitionId": null,
"suspended": false,
"formKey": "embedded:app:develop/invoice-forms/approve-invoice.html",
"tenantId": null
"tenantId": null,
"attachment":false,
"comment":false
}
]
}'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@
"binding": "version",
"version": 2
},
"tenantId":"aTenantId"
"tenantId":"aTenantId",
"attachment":false,
"comment":false
}
}'] />

Expand Down
yanavasileva marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.Date;

import com.fasterxml.jackson.annotation.JsonProperty;
import org.camunda.bpm.engine.BadUserRequestException;
import org.camunda.bpm.engine.form.CamundaFormRef;
import org.camunda.bpm.engine.rest.dto.converter.DelegationStateConverter;
Expand Down Expand Up @@ -49,7 +50,9 @@ public class TaskDto {
private String formKey;
private CamundaFormRef camundaFormRef;
private String tenantId;

@JsonProperty("attachment")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ For consistency, let's remove the annotation:

Suggested change
@JsonProperty("attachment")

Copy link
Author

@sumankumarpani sumankumarpani May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reluctant to add this, I am facing an weird issue where hasAttachment is translated to attachments instead of attachment in the json response, hence decided to add. Let me know your thoughts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this. I will try to see from where it's coming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also try to debug it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured it , jackson considers the setter method to decide the boolean var name.

private boolean hasAttachment;
private boolean hasComment;
public String getId() {
return id;
}
Expand Down Expand Up @@ -194,6 +197,20 @@ public void setTenantId(String tenantId) {
this.tenantId = tenantId;
}

public boolean getAttachments() {
return hasAttachment;
}
public void setAttachments(boolean hasAttachment) {
this.hasAttachment = hasAttachment;
}

public boolean getComment() {
return hasComment;
}

public void setComments(boolean hasComment) {
this.hasComment = hasComment;
}
public static TaskDto fromEntity(Task task) {
TaskDto dto = new TaskDto();
dto.id = task.getId();
Expand Down Expand Up @@ -221,6 +238,8 @@ public static TaskDto fromEntity(Task task) {
dto.caseInstanceId = task.getCaseInstanceId();
dto.suspended = task.isSuspended();
dto.tenantId = task.getTenantId();
dto.hasAttachment = task.hasAttachment();
dto.hasComment = task.hasComment();

try {
dto.formKey = task.getFormKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ public class TaskQueryDto extends AbstractQueryDto<TaskQuery> {

private List<TaskQueryDto> orQueries;

private Boolean withCommentAttachmentInfo;

public TaskQueryDto() {

}
Expand Down Expand Up @@ -708,6 +710,11 @@ public void setVariableValuesIgnoreCase(Boolean variableValuesCaseInsensitive) {
this.variableValuesIgnoreCase = variableValuesCaseInsensitive;
}

@CamundaQueryParam(value = "withCommentAttachmentInfo", converter = BooleanConverter.class)
public void setWithCommentAttachmentInfo(Boolean withCommentAttachmentInfo) {
this.withCommentAttachmentInfo = withCommentAttachmentInfo;
}

@Override
protected boolean isValidSortByValue(String value) {
return VALID_SORT_BY_VALUES.contains(value);
Expand Down Expand Up @@ -1078,6 +1085,8 @@ public Boolean isVariableValuesIgnoreCase() {
return variableValuesIgnoreCase;
}

public Boolean getWithCommentAttachmentInfo() { return withCommentAttachmentInfo;}

@Override
protected void applyFilters(TaskQuery query) {
if (orQueries != null) {
Expand Down Expand Up @@ -1442,6 +1451,9 @@ protected void applyFilters(TaskQuery query) {
}
}
}
if (withCommentAttachmentInfo != null && withCommentAttachmentInfo) {
query.withCommentAttachmentInfo();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ public void testGetSingleTask() {
.body("caseDefinitionId", equalTo(MockProvider.EXAMPLE_CASE_DEFINITION_ID))
.body("tenantId", equalTo(MockProvider.EXAMPLE_TENANT_ID))
.body("lastUpdated", equalTo(MockProvider.EXAMPLE_TASK_LAST_UPDATED))
.body("attachment", equalTo(MockProvider.EXAMPLE_TASK_ATTACHMENT_STATE))
.body("comment", equalTo(MockProvider.EXAMPLE_TASK_COMMENT_STATE))
.when().get(SINGLE_TASK_URL);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ public void testSimpleTaskQuery() {
boolean returnedSuspensionState = from(content).getBoolean("[0].suspended");
String returnedFormKey = from(content).getString("[0].formKey");
String returnedTenantId = from(content).getString("[0].tenantId");
boolean returnedAttachmentsInfo = from(content).getBoolean("[0].attachment");
boolean returnedCommentsInfo = from(content).getBoolean("[0].comment");

assertThat(MockProvider.EXAMPLE_TASK_NAME).isEqualTo(returnedTaskName);
assertThat(MockProvider.EXAMPLE_TASK_ID).isEqualTo(returnedId);
Expand All @@ -210,6 +212,8 @@ public void testSimpleTaskQuery() {
assertThat(MockProvider.EXAMPLE_TASK_SUSPENSION_STATE).isEqualTo(returnedSuspensionState);
assertThat(MockProvider.EXAMPLE_FORM_KEY).isEqualTo(returnedFormKey);
assertThat(MockProvider.EXAMPLE_TENANT_ID).isEqualTo(returnedTenantId);
assertThat(MockProvider.EXAMPLE_TASK_ATTACHMENT_STATE).isEqualTo(returnedAttachmentsInfo);
assertThat(MockProvider.EXAMPLE_TASK_COMMENT_STATE).isEqualTo(returnedCommentsInfo);

}

Expand Down Expand Up @@ -521,6 +525,7 @@ private Map<String, Boolean> getCompleteBooleanQueryParameters() {
parameters.put("withCandidateUsers", true);
parameters.put("withoutCandidateUsers", true);
parameters.put("withoutDueDate", true);
parameters.put("withCommentAttachmentInfo", true);

return parameters;
}
Expand Down Expand Up @@ -586,6 +591,7 @@ private void verifyBooleanParameterQueryInvocation() {
verify(mockQuery).withCandidateUsers();
verify(mockQuery).withoutCandidateUsers();
verify(mockQuery).withoutDueDate();
verify(mockQuery).withCommentAttachmentInfo();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ public abstract class MockProvider {
public static final String EXAMPLE_TASK_DEFINITION_KEY = "aTaskDefinitionKey";
public static final boolean EXAMPLE_TASK_SUSPENSION_STATE = false;

public static final boolean EXAMPLE_TASK_ATTACHMENT_STATE = false;

public static final boolean EXAMPLE_TASK_COMMENT_STATE = false;

// task comment
public static final String EXAMPLE_TASK_COMMENT_ID = "aTaskCommentId";
public static final String EXAMPLE_TASK_COMMENT_FULL_MESSAGE = "aTaskCommentFullMessage";
Expand Down Expand Up @@ -1055,7 +1059,9 @@ public static MockTaskBuilder mockTask() {
.caseExecutionId(EXAMPLE_CASE_EXECUTION_ID)
.formKey(EXAMPLE_FORM_KEY)
.camundaFormRef(EXAMPLE_FORM_KEY, EXAMPLE_FORM_REF_BINDING, EXAMPLE_FORM_REF_VERSION)
.tenantId(EXAMPLE_TENANT_ID);
.tenantId(EXAMPLE_TENANT_ID)
.hasAttachment(EXAMPLE_TASK_ATTACHMENT_STATE)
.hasComment(EXAMPLE_TASK_COMMENT_STATE);
}

public static List<Task> createMockTasks() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ public class MockTaskBuilder {
private String formKey;
private CamundaFormRef camundaFormRef;
private String tenantId;
private boolean hasAttachment;

private boolean hasComment;



public MockTaskBuilder id(String id) {
this.id = id;
Expand Down Expand Up @@ -163,6 +168,16 @@ public MockTaskBuilder tenantId(String tenantId) {
return this;
}

public MockTaskBuilder hasAttachment(boolean hasAttachment) {
this.hasAttachment = hasAttachment;
return this;
}

public MockTaskBuilder hasComment(boolean hasComment) {
this.hasComment = hasComment;
return this;
}

public Task build() {
Task mockTask = mock(Task.class);
when(mockTask.getId()).thenReturn(id);
Expand All @@ -187,6 +202,8 @@ public Task build() {
when(mockTask.getFormKey()).thenReturn(formKey);
when(mockTask.getCamundaFormRef()).thenReturn(camundaFormRef);
when(mockTask.getTenantId()).thenReturn(tenantId);
when(mockTask.hasAttachment()).thenReturn(hasAttachment);
when(mockTask.hasComment()).thenReturn(hasComment);
return mockTask;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.camunda.bpm.engine.task.Task;
import org.camunda.bpm.engine.task.TaskQuery;
import org.camunda.bpm.engine.variable.type.ValueType;
import org.camunda.bpm.engine.impl.history.HistoryLevel;

/**
* @author Joram Barrez
Expand Down Expand Up @@ -175,6 +176,7 @@ public class TaskQueryImpl extends AbstractQuery<TaskQuery, Task> implements Tas
// or query /////////////////////////////
protected List<TaskQueryImpl> queries = new ArrayList<>(Arrays.asList(this));
protected boolean isOrQueryActive = false;
protected boolean withCommentAttachmentInfo = false;

public TaskQueryImpl() {
}
Expand Down Expand Up @@ -1088,6 +1090,12 @@ protected boolean hasExcludingConditions() {
|| CompareUtil.elementIsNotContainedInArray(processInstanceBusinessKey, processInstanceBusinessKeys);
}

@Override
public TaskQuery withCommentAttachmentInfo() {
this.withCommentAttachmentInfo = true;
return this;
}

public List<String> getCandidateGroups() {
if (cachedCandidateGroups != null) {
return cachedCandidateGroups;
Expand Down Expand Up @@ -1441,6 +1449,13 @@ public List<Task> executeList(CommandContext commandContext, Page page) {
}
}

if (withCommentAttachmentInfo && Context.getProcessEngineConfiguration().getHistoryLevel()!= HistoryLevel.HISTORY_LEVEL_NONE) {
yanavasileva marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Use equals to compare:

Suggested change
if (withCommentAttachmentInfo && Context.getProcessEngineConfiguration().getHistoryLevel()!= HistoryLevel.HISTORY_LEVEL_NONE) {
if (withCommentAttachmentInfo && !Context.getProcessEngineConfiguration().getHistoryLevel().equals(HistoryLevel.HISTORY_LEVEL_NONE)) {

for (Task task : taskList) {
// verify attachment and comments exists for the task
((TaskEntity) task).initializeAttachmentAndComments();
}
}

return taskList;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ public class TaskEntity extends AbstractVariableScope implements Task, DelegateT
protected boolean isFormKeyInitialized = false;
protected String formKey;
protected CamundaFormRef camundaFormRef;
protected boolean attachmentExists;
protected boolean commentExists;

@SuppressWarnings({ "unchecked" })
protected transient VariableStore<VariableInstanceEntity> variableStore
Expand Down Expand Up @@ -1451,6 +1453,10 @@ public void initializeFormKey() {
}
}
}
public void initializeAttachmentAndComments(){
this.attachmentExists = !getProcessEngine().getTaskService().getTaskAttachments(id).isEmpty();
this.commentExists = !getProcessEngine().getTaskService().getTaskComments(id).isEmpty();
Comment on lines +1457 to +1458
Copy link
Member

@yanavasileva yanavasileva May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Please re-use the command context instead of executing command (get attachment/comments) in the command (task query):

Suggested change
this.attachmentExists = !getProcessEngine().getTaskService().getTaskAttachments(id).isEmpty();
this.commentExists = !getProcessEngine().getTaskService().getTaskComments(id).isEmpty();
this.attachmentExists = !Context.getCommandContext().getAttachmentManager().findAttachmentsByTaskId(id).isEmpty();
this.commentExists = !Context.getCommandContext().getCommentManager().findCommentsByTaskId(id).isEmpty();

}
yanavasileva marked this conversation as resolved.
Show resolved Hide resolved

@Override
public String getFormKey() {
Expand Down Expand Up @@ -1758,7 +1764,14 @@ public void bpmnError(String errorCode, String errorMessage, Map<String, Object>
throw ProcessEngineLogger.CMD_LOGGER.exceptionBpmnErrorPropagationFailed(errorCode, ex);
}
}

@Override
public boolean hasAttachment() {
return attachmentExists;
}
@Override
public boolean hasComment() {
return commentExists;
}
public void escalation(String escalationCode, Map<String, Object> variables) {
ensureTaskActive();
ActivityExecution activityExecution = getExecution();
Expand Down
5 changes: 5 additions & 0 deletions engine/src/main/java/org/camunda/bpm/engine/task/Task.java
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,9 @@ public interface Task {
*/
void setTenantId(String tenantId);

/** Returns if an attachment exists for the task */
boolean hasAttachment();
/** Signifies if a comment exists for the task */
boolean hasComment();

}
yanavasileva marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -1104,4 +1104,9 @@ public interface TaskQuery extends Query<TaskQuery, Task> {
* this exception, {@link #or()} must be invoked first.
*/
TaskQuery endOr();

/**
* Evaluates existence of attachment and comments associated with the task, defaults to false.
*/
Comment on lines +1108 to +1110
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Please add performance considerations to the javadoc. Adding the filter will do additional attachment and comments queries to the database, that might slow down the query in case of big size of the tables.

TaskQuery withCommentAttachmentInfo();
yanavasileva marked this conversation as resolved.
Show resolved Hide resolved
}
Loading