Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,273 @@ void patch_reassignTaskToBotUser_400(TestNamespace ns) throws Exception {
deleteThread(thread.getId());
}

@Test
void post_tagTaskWithValidJsonArrays_200(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about =
String.format("<#E::table::%s::columns::%s::tags>", table.getFullyQualifiedName(), "id");

User assigneeUser = SdkClients.adminClient().users().getByName("admin");
EntityReference assignee = assigneeUser.getEntityReference();

String validTagJson =
"[{\"tagFQN\":\"PersonalData.Personal\",\"source\":\"Classification\","
+ "\"labelType\":\"Manual\",\"state\":\"Confirmed\"}]";

CreateTaskDetails taskDetails =
new CreateTaskDetails()
.withType(TaskType.RequestTag)
.withAssignees(List.of(assignee))
.withOldValue("[]")
.withSuggestion(validTagJson);

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Add tags to column")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(taskDetails);

Thread taskThread = createThread(createThread);

assertNotNull(taskThread);
assertNotNull(taskThread.getTask());
assertEquals(TaskType.RequestTag, taskThread.getTask().getType());
assertEquals(validTagJson, taskThread.getTask().getSuggestion());

deleteThread(taskThread.getId());
}

@Test
void post_conversationThreadWithTaskDetails_400(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s>", table.getFullyQualifiedName());

User assigneeUser = SdkClients.adminClient().users().getByName("admin");
EntityReference assignee = assigneeUser.getEntityReference();

CreateTaskDetails taskDetails =
new CreateTaskDetails()
.withType(TaskType.RequestDescription)
.withAssignees(List.of(assignee))
.withOldValue("old")
.withSuggestion("new");

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Conversation with task payload")
.withAbout(about)
.withType(ThreadType.Conversation)
.withTaskDetails(taskDetails);

assertThrows(
Exception.class,
() -> createThread(createThread),
"taskDetails can only be provided for threads of type Task");
}

@Test
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

The new integration coverage validates that Conversation threads with non-null taskDetails are rejected, but it doesn’t cover Announcement threads carrying taskDetails. Since createThread has a dedicated Announcement branch, adding a similar 400 test for ThreadType.Announcement would help prevent regressions where announcements accidentally accept and persist task payloads.

Suggested change
@Test
@Test
void post_announcementThreadWithTaskDetails_400(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s>", table.getFullyQualifiedName());
User assigneeUser = SdkClients.adminClient().users().getByName("admin");
EntityReference assignee = assigneeUser.getEntityReference();
CreateTaskDetails taskDetails =
new CreateTaskDetails()
.withType(TaskType.RequestDescription)
.withAssignees(List.of(assignee))
.withOldValue("old")
.withSuggestion("new");
AnnouncementDetails announcementDetails =
new AnnouncementDetails()
.withStartTime(LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1))
.withEndTime(LocalDateTime.now(ZoneOffset.UTC).plusMinutes(2));
CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Announcement with task payload")
.withAbout(about)
.withType(ThreadType.Announcement)
.withAnnouncementDetails(announcementDetails)
.withTaskDetails(taskDetails);
assertThrows(
Exception.class,
() -> createThread(createThread),
"taskDetails can only be provided for threads of type Task");
}
@Test

Copilot uses AI. Check for mistakes.
void post_taskWithoutTaskDetails_400(TestNamespace ns) {
CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Task with no details")
.withAbout("<#E::table::some.table>")
.withType(ThreadType.Task);

assertThrows(
Exception.class,
() -> createThread(createThread),
"taskDetails is required for threads of type Task");
}

@Test
void post_descriptionTaskWithOldValueAndSuggestion_200(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s::description>", table.getFullyQualifiedName());
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

for (TaskType type : List.of(TaskType.RequestDescription, TaskType.UpdateDescription)) {
CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Desc task " + type)
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(type)
.withAssignees(List.of(assignee))
.withOldValue("old description")
.withSuggestion("new description"));

Thread thread = createThread(createThread);
assertNotNull(thread.getTask());
assertEquals(type, thread.getTask().getType());
assertEquals("old description", thread.getTask().getOldValue());
assertEquals("new description", thread.getTask().getSuggestion());
deleteThread(thread.getId());
}
}

@Test
void post_updateTagTaskWithValidJsonArrays_200(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about =
String.format("<#E::table::%s::columns::%s::tags>", table.getFullyQualifiedName(), "id");
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

String oldTags =
"[{\"tagFQN\":\"PersonalData.Personal\",\"source\":\"Classification\","
+ "\"labelType\":\"Manual\",\"state\":\"Confirmed\"}]";
String newTags =
"[{\"tagFQN\":\"PII.Sensitive\",\"source\":\"Classification\","
+ "\"labelType\":\"Manual\",\"state\":\"Confirmed\"}]";

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Update tags")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(TaskType.UpdateTag)
.withAssignees(List.of(assignee))
.withOldValue(oldTags)
.withSuggestion(newTags));

Thread thread = createThread(createThread);
assertNotNull(thread.getTask());
assertEquals(TaskType.UpdateTag, thread.getTask().getType());
deleteThread(thread.getId());
}

@Test
void post_approvalTaskWithOldValueAndSuggestion_200(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s>", table.getFullyQualifiedName());
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Please approve")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(TaskType.RequestApproval)
.withAssignees(List.of(assignee))
.withOldValue("Draft")
.withSuggestion("Approved"));

Thread thread = createThread(createThread);
assertNotNull(thread.getTask());
assertEquals(TaskType.RequestApproval, thread.getTask().getType());
assertEquals("Draft", thread.getTask().getOldValue());
assertEquals("Approved", thread.getTask().getSuggestion());
deleteThread(thread.getId());
}

@Test
void post_genericTaskWithOldValueAndSuggestion_200(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s>", table.getFullyQualifiedName());
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Generic task")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(TaskType.Generic)
.withAssignees(List.of(assignee))
.withOldValue("some value")
.withSuggestion("another value"));

Thread thread = createThread(createThread);
assertNotNull(thread.getTask());
assertEquals(TaskType.Generic, thread.getTask().getType());
deleteThread(thread.getId());
}

@Test
void post_testCaseFailureResolutionTaskWithOldValue_400(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s>", table.getFullyQualifiedName());
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Failure resolution")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(TaskType.RequestTestCaseFailureResolution)
.withAssignees(List.of(assignee))
.withOldValue("should be rejected"));

assertThrows(Exception.class, () -> createThread(createThread));
}

@Test
void post_recognizerFeedbackApprovalTaskWithSuggestion_400(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about = String.format("<#E::table::%s>", table.getFullyQualifiedName());
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Recognizer feedback")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(TaskType.RecognizerFeedbackApproval)
.withAssignees(List.of(assignee))
.withSuggestion("should be rejected"));

assertThrows(Exception.class, () -> createThread(createThread));
}

@Test
void post_tagTaskWithInvalidJson_400(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
String about =
String.format("<#E::table::%s::columns::%s::tags>", table.getFullyQualifiedName(), "id");
EntityReference assignee =
SdkClients.adminClient().users().getByName("admin").getEntityReference();

CreateThread createThread =
new CreateThread()
.withFrom(ADMIN_USER)
.withMessage("Bad tag json")
.withAbout(about)
.withType(ThreadType.Task)
.withTaskDetails(
new CreateTaskDetails()
.withType(TaskType.RequestTag)
.withAssignees(List.of(assignee))
.withSuggestion("not valid json"));

assertThrows(Exception.class, () -> createThread(createThread));
}

@Test
void list_threadsWithPagination(TestNamespace ns) throws Exception {
Table table = createTestTable(ns);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,15 @@ public Thread getTask(EntityLink about, TaskType taskType, TaskStatus taskStatus
private Thread createThread(ThreadContext threadContext) {
Thread thread = threadContext.getThread();
if (thread.getType() == ThreadType.Task) {
validateTaskDetails(thread);
validateAssignee(thread);
thread.getTask().withId(getNextTaskId());
} else if (thread.getType() == ThreadType.Announcement) {
// Validate start and end time for announcement
validateAnnouncement(thread);
} else if (thread.getTask() != null) {
throw new IllegalArgumentException(
"taskDetails can only be provided for threads of type Task");
}
Comment on lines 502 to 514
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

createThread currently rejects taskDetails for non-Task threads only in the final else if branch. If a client posts an Announcement thread with non-null taskDetails, it will hit the Announcement branch and bypass the rejection, so corrupted task payloads can still be persisted on announcements. Consider checking thread.getTask() != null for all non-ThreadType.Task threads (including Announcement) before the type-specific branches, or add the same rejection inside the Announcement branch.

Copilot uses AI. Check for mistakes.
store(threadContext);
storeRelationships(threadContext);
Expand Down Expand Up @@ -1189,6 +1193,45 @@ private static long convertSecondsToMilliseconds(long seconds) {
.toEpochMilli();
}

private void validateTaskDetails(Thread thread) {
TaskDetails task = thread.getTask();
if (task == null) {
throw new IllegalArgumentException("taskDetails is required for threads of type Task");
}
switch (task.getType()) {
case RequestTag, UpdateTag -> {
validateTagLabelArray(task.getOldValue(), "oldValue", task.getType());
validateTagLabelArray(task.getSuggestion(), "suggestion", task.getType());
}
case RequestTestCaseFailureResolution, RecognizerFeedbackApproval -> {
rejectField(task.getOldValue(), "oldValue", task.getType());
rejectField(task.getSuggestion(), "suggestion", task.getType());
}
case RequestDescription, UpdateDescription, RequestApproval, Generic -> {}
}
Comment on lines +1201 to +1211
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ’‘ Edge Case: Switch on TaskType lacks default branch for future enum values

The switch at line 1201 explicitly lists all 8 current TaskType values, but has no default branch. If a new TaskType is added to the enum in the future, it will silently pass through validateTaskDetails without any validation β€” the same class of bug this PR is fixing. Adding a default branch that either throws or logs a warning would make this future-proof.

Suggested fix:

switch (task.getType()) {
  case RequestTag, UpdateTag -> {
    validateTagLabelArray(task.getOldValue(), "oldValue", task.getType());
    validateTagLabelArray(task.getSuggestion(), "suggestion", task.getType());
  }
  case RequestTestCaseFailureResolution, RecognizerFeedbackApproval -> {
    rejectField(task.getOldValue(), "oldValue", task.getType());
    rejectField(task.getSuggestion(), "suggestion", task.getType());
  }
  case RequestDescription, UpdateDescription, RequestApproval, Generic -> {}
  default -> LOG.warn("No field validation defined for task type: {}", task.getType());
}

Was this helpful? React with πŸ‘ / πŸ‘Ž | Reply gitar fix to apply this suggestion

}
Comment thread
gitar-bot[bot] marked this conversation as resolved.

private void rejectField(Object value, String fieldName, TaskType taskType) {
if (value != null) {
throw new IllegalArgumentException(
String.format("taskDetails.%s is not applicable for task type %s", fieldName, taskType));
}
}

private void validateTagLabelArray(String value, String fieldName, TaskType taskType) {
if (value == null) {
return;
}
try {
JsonUtils.readObjects(value, TagLabel.class);
} catch (Exception e) {
throw new IllegalArgumentException(
String.format(
"taskDetails.%s must be a valid JSON array of tags for task type %s",
fieldName, taskType));
}
}

private void validateAssignee(Thread thread) {
if (thread != null && ThreadType.Task.equals(thread.getType())) {
String createdByUserName = thread.getCreatedBy();
Expand Down
Loading
Loading