diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/FeedResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/FeedResourceIT.java index 6cc6346f3194..84a60b373b5a 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/FeedResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/FeedResourceIT.java @@ -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 + 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); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java index 0364eb316305..b01e4a8130fa 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java @@ -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"); } store(threadContext); storeRelationships(threadContext); @@ -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 -> {} + } + } + + 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(); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/shared/TagsTask.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/shared/TagsTask.tsx index d2c4124b5091..19c40b1487a6 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/shared/TagsTask.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/shared/TagsTask.tsx @@ -50,6 +50,31 @@ const TagsTask: FC = ({ const isTaskClosed = task?.status === ThreadTaskStatus.Closed; + const parseTagLabels = (value?: string): TagLabel[] => { + try { + const parsed = JSON.parse(value ?? '[]'); + + return Array.isArray(parsed) ? parsed : []; + } catch { + return []; + } + }; + + const parsedOldValue = useMemo( + () => parseTagLabels(oldValue), + [oldValue] + ); + + const parsedNewValue = useMemo( + () => parseTagLabels(newValue), + [newValue] + ); + + const parsedSuggestion = useMemo( + () => parseTagLabels(suggestion), + [suggestion] + ); + const diffView = useMemo(() => { if (!oldValue && !newValue) { return ( @@ -61,15 +86,10 @@ const TagsTask: FC = ({ ); } else { return ( - + ); } - }, [oldValue, newValue]); + }, [oldValue, newValue, parsedOldValue, parsedNewValue]); /** * @@ -86,15 +106,10 @@ const TagsTask: FC = ({ ); } else { return ( - + ); } - }, [suggestion, oldValue]); + }, [suggestion, oldValue, parsedOldValue, parsedSuggestion]); return (
@@ -116,7 +131,7 @@ const TagsTask: FC = ({
{isTaskActionEdit && hasEditAccess ? (