diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java index 5316cb7e1c5..6b975f7f1cd 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java @@ -22,6 +22,7 @@ import static org.openmetadata.catalog.type.Relationship.ADDRESSED_TO; import static org.openmetadata.catalog.type.Relationship.CREATED; import static org.openmetadata.catalog.type.Relationship.IS_ABOUT; import static org.openmetadata.catalog.type.Relationship.REPLIED_TO; +import static org.openmetadata.catalog.util.ChangeEventParser.getPlaintextDiff; import static org.openmetadata.catalog.util.EntityUtil.populateEntityReferences; import com.fasterxml.jackson.core.JsonProcessingException; @@ -44,6 +45,7 @@ import org.apache.commons.lang3.StringUtils; import org.jdbi.v3.sqlobject.transaction.Transaction; import org.json.JSONObject; import org.openmetadata.catalog.Entity; +import org.openmetadata.catalog.api.feed.CloseTask; import org.openmetadata.catalog.api.feed.EntityLinkThreadCount; import org.openmetadata.catalog.api.feed.ResolveTask; import org.openmetadata.catalog.api.feed.ThreadCount; @@ -165,17 +167,21 @@ public class FeedRepository { } public Thread get(String id) throws IOException { - return EntityUtil.validate(id, dao.feedDAO().findById(id), Thread.class); + Thread thread = EntityUtil.validate(id, dao.feedDAO().findById(id), Thread.class); + sortPosts(thread); + return thread; } public Thread getTask(Integer id) throws IOException { Thread task = EntityUtil.validate(id.toString(), dao.feedDAO().findByTaskId(id), Thread.class); + sortPosts(task); return populateAssignees(task); } - public PatchResponse closeTask(UriInfo uriInfo, Thread thread, String user) throws IOException { + public PatchResponse closeTask(UriInfo uriInfo, Thread thread, String user, CloseTask closeTask) + throws IOException { // Update the attributes - closeTask(thread, user); + closeTask(thread, user, closeTask.getComment()); Thread updatedHref = FeedResource.addHref(uriInfo, thread); return new PatchResponse<>(Status.OK, updatedHref, RestUtil.ENTITY_UPDATED); } @@ -324,17 +330,49 @@ public class FeedRepository { // Update the attributes task.withNewValue(resolveTask.getNewValue()); - closeTask(thread, user); + closeTask(thread, user, null); Thread updatedHref = FeedResource.addHref(uriInfo, thread); return new PatchResponse<>(Status.OK, updatedHref, RestUtil.ENTITY_UPDATED); } - private void addClosingPost(Thread thread, String user) { + private String getTagFQNs(List tags) { + return tags.stream().map(TagLabel::getTagFQN).collect(Collectors.joining(", ")); + } + + private void addClosingPost(Thread thread, String user, String closingComment) throws IOException { // Add a post to the task + String message; + if (closingComment != null) { + message = String.format("Closed the Task with comment - %s", closingComment); + } else { + // The task was resolved with an update. + // Add a default message to the Task thread with updated description/tag + TaskDetails task = thread.getTask(); + TaskType type = task.getType(); + String oldValue = StringUtils.EMPTY; + if (List.of(TaskType.RequestDescription, TaskType.UpdateDescription).contains(type)) { + if (task.getOldValue() != null) { + oldValue = task.getOldValue(); + } + message = + String.format("Resolved the Task with Description - %s", getPlaintextDiff(oldValue, task.getNewValue())); + } else if (List.of(TaskType.RequestTag, TaskType.UpdateTag).contains(type)) { + List tags; + if (task.getOldValue() != null) { + tags = JsonUtils.readObjects(task.getOldValue(), TagLabel.class); + oldValue = getTagFQNs(tags); + } + tags = JsonUtils.readObjects(task.getNewValue(), TagLabel.class); + String newValue = getTagFQNs(tags); + message = String.format("Resolved the Task with Tag(s) - %s", getPlaintextDiff(oldValue, newValue)); + } else { + message = "Resolved the Task."; + } + } Post post = new Post() .withId(UUID.randomUUID()) - .withMessage("Closed the Task.") + .withMessage(message) .withFrom(user) .withReactions(java.util.Collections.emptyList()) .withPostTs(System.currentTimeMillis()); @@ -345,13 +383,14 @@ public class FeedRepository { } } - private void closeTask(Thread thread, String user) throws JsonProcessingException { + private void closeTask(Thread thread, String user, String closingComment) throws IOException { TaskDetails task = thread.getTask(); task.withStatus(TaskStatus.Closed).withClosedBy(user).withClosedAt(System.currentTimeMillis()); thread.withTask(task).withUpdatedBy(user).withUpdatedAt(System.currentTimeMillis()); dao.feedDAO().update(thread.getId().toString(), JsonUtils.pojoToJson(thread)); - addClosingPost(thread, user); + addClosingPost(thread, user, closingComment); + sortPosts(thread); } private void storeMentions(Thread thread, String message) { @@ -403,6 +442,8 @@ public class FeedRepository { // Add mentions into field relationship table storeMentions(thread, post.getMessage()); + sortPostsInThreads(List.of(thread)); + return thread; } @@ -654,6 +695,7 @@ public class FeedRepository { }); } + sortPosts(thread); String change = patchUpdate(thread, post, updated) ? RestUtil.ENTITY_UPDATED : RestUtil.ENTITY_NO_CHANGE; return new PatchResponse<>(Status.OK, updated, change); } @@ -682,6 +724,7 @@ public class FeedRepository { // Update the attributes String change = patchUpdate(original, updated) ? RestUtil.ENTITY_UPDATED : RestUtil.ENTITY_NO_CHANGE; + sortPosts(updated); Thread updatedHref = FeedResource.addHref(uriInfo, updated); return new PatchResponse<>(Status.OK, updatedHref, change); } @@ -742,22 +785,35 @@ public class FeedRepository { } private boolean fieldsChanged(Thread original, Thread updated) { - // Patch supports isResolved, message, and reactions for now + // Patch supports isResolved, message, task assignees, and reactions for now return !original.getResolved().equals(updated.getResolved()) || !original.getMessage().equals(updated.getMessage()) || (Collections.isEmpty(original.getReactions()) && !Collections.isEmpty(updated.getReactions())) || (!Collections.isEmpty(original.getReactions()) && Collections.isEmpty(updated.getReactions())) || original.getReactions().size() != updated.getReactions().size() - || !original.getReactions().containsAll(updated.getReactions()); + || !original.getReactions().containsAll(updated.getReactions()) + || (original.getTask() != null + && (original.getTask().getAssignees().size() != updated.getTask().getAssignees().size() + || !original.getTask().getAssignees().containsAll(updated.getTask().getAssignees()))); + } + + private void sortPosts(Thread thread) { + thread.getPosts().sort(Comparator.comparing(Post::getPostTs)); + } + + private void sortPostsInThreads(List threads) { + for (Thread t : threads) { + sortPosts(t); + } } /** Limit the number of posts within each thread. */ private void limitPostsInThreads(List threads, int limitPosts) { for (Thread t : threads) { List posts = t.getPosts(); + sortPosts(t); if (posts.size() > limitPosts) { // Only keep the last "n" number of posts - posts.sort(Comparator.comparing(Post::getPostTs)); posts = posts.subList(posts.size() - limitPosts, posts.size()); t.withPosts(posts); } @@ -812,6 +868,7 @@ public class FeedRepository { } List threads = JsonUtils.readObjects(jsons, Thread.class); int totalCount = dao.feedDAO().listCountTasksAssignedTo(userTeamJsonPostgres, userTeamJsonMysql, status.toString()); + sortPostsInThreads(threads); return new FilteredThreads(threads, totalCount); } @@ -845,6 +902,7 @@ public class FeedRepository { } List threads = JsonUtils.readObjects(jsons, Thread.class); int totalCount = dao.feedDAO().listCountTasksAssignedBy(username, status.toString()); + sortPostsInThreads(threads); return new FilteredThreads(threads, totalCount); } @@ -866,6 +924,7 @@ public class FeedRepository { } List threads = JsonUtils.readObjects(jsons, Thread.class); int totalCount = dao.feedDAO().listCountThreadsByOwner(userId, teamIds, type, isResolved); + sortPostsInThreads(threads); return new FilteredThreads(threads, totalCount); } @@ -908,6 +967,7 @@ public class FeedRepository { dao.feedDAO() .listCountThreadsByMentions( user.getName(), teamNames, type, isResolved, Relationship.MENTIONED_IN.ordinal()); + sortPostsInThreads(threads); return new FilteredThreads(threads, totalCount); } @@ -931,6 +991,7 @@ public class FeedRepository { List threads = JsonUtils.readObjects(jsons, Thread.class); int totalCount = dao.feedDAO().listCountThreadsByFollows(userId, teamIds, type, isResolved, Relationship.FOLLOWS.ordinal()); + sortPostsInThreads(threads); return new FilteredThreads(threads, totalCount); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java index 56471b11b29..93267638c4d 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/feeds/FeedResource.java @@ -52,6 +52,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.SecurityContext; import javax.ws.rs.core.UriInfo; +import org.openmetadata.catalog.api.feed.CloseTask; import org.openmetadata.catalog.api.feed.CreatePost; import org.openmetadata.catalog.api.feed.CreateThread; import org.openmetadata.catalog.api.feed.ResolveTask; @@ -303,10 +304,10 @@ public class FeedResource { @Context UriInfo uriInfo, @Context SecurityContext securityContext, @PathParam("id") String id, - @Valid ResolveTask resolveTask) + @Valid CloseTask closeTask) throws IOException { Thread task = dao.getTask(Integer.parseInt(id)); - return dao.closeTask(uriInfo, task, securityContext.getUserPrincipal().getName()).toResponse(); + return dao.closeTask(uriInfo, task, securityContext.getUserPrincipal().getName(), closeTask).toResponse(); } @PATCH diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java index 0bbdea12f8d..584359e256d 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java @@ -216,7 +216,7 @@ public final class ChangeEventParser { case ADD: String fieldValue = getFieldValue(newFieldValue); if (Entity.FIELD_FOLLOWERS.equals(updatedField)) { - message = String.format("Started to follow **%s** `%s`", link.getEntityType(), link.getEntityFQN()); + message = String.format("Followed **%s** `%s`", link.getEntityType(), link.getEntityFQN()); } else if (fieldValue != null && !fieldValue.isEmpty()) { message = String.format("Added **%s**: `%s`", updatedField, fieldValue); } @@ -226,7 +226,7 @@ public final class ChangeEventParser { break; case DELETE: if (Entity.FIELD_FOLLOWERS.equals(updatedField)) { - message = String.format("Stopped following %s `%s`", link.getEntityType(), link.getEntityFQN()); + message = String.format("Unfollowed %s `%s`", link.getEntityType(), link.getEntityFQN()); } else { message = String.format("Deleted **%s**", updatedField); } @@ -310,7 +310,7 @@ public final class ChangeEventParser { return getPlainTextUpdateMessage(updatedField, oldValue.toString(), newValue.toString()); } - private static String getPlaintextDiff(String oldValue, String newValue) { + public static String getPlaintextDiff(String oldValue, String newValue) { // create a configured DiffRowGenerator String addMarker = ""; String removeMarker = ""; diff --git a/catalog-rest-service/src/main/resources/json/schema/api/feed/closeTask.json b/catalog-rest-service/src/main/resources/json/schema/api/feed/closeTask.json new file mode 100644 index 00000000000..87cf0c58709 --- /dev/null +++ b/catalog-rest-service/src/main/resources/json/schema/api/feed/closeTask.json @@ -0,0 +1,15 @@ +{ + "$id": "https://open-metadata.org/schema/api/feed/closeTask.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "CloseTaskRequest", + "description": "Close Task request", + "type": "object", + "properties": { + "comment": { + "description": "The closing comment explaining why the task is being closed.", + "type": "string" + } + }, + "required": ["comment"], + "additionalProperties": false +} diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java index 3328509cb2e..719cc5b48b9 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java @@ -29,6 +29,7 @@ import static org.openmetadata.catalog.exception.CatalogExceptionMessage.noPermi import static org.openmetadata.catalog.resources.EntityResourceTest.USER_ADDRESS_TAG_LABEL; import static org.openmetadata.catalog.security.SecurityUtil.authHeaders; import static org.openmetadata.catalog.security.SecurityUtil.getPrincipalName; +import static org.openmetadata.catalog.util.ChangeEventParser.getPlaintextDiff; import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS; import static org.openmetadata.catalog.util.TestUtils.ADMIN_USER_NAME; import static org.openmetadata.catalog.util.TestUtils.NON_EXISTENT_ENTITY; @@ -70,6 +71,7 @@ import org.junit.jupiter.params.provider.NullSource; import org.openmetadata.catalog.CatalogApplicationTest; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.data.CreateTable; +import org.openmetadata.catalog.api.feed.CloseTask; import org.openmetadata.catalog.api.feed.CreatePost; import org.openmetadata.catalog.api.feed.CreateThread; import org.openmetadata.catalog.api.feed.EntityLinkThreadCount; @@ -435,7 +437,9 @@ public class FeedResourceTest extends CatalogApplicationTest { assertEquals(TaskStatus.Closed, task.getStatus()); assertEquals(1, taskThread.getPostsCount()); assertEquals(1, taskThread.getPosts().size()); - assertEquals("Closed the Task.", taskThread.getPosts().get(0).getMessage()); + String diff = getPlaintextDiff("old description", "accepted description"); + String expectedMessage = String.format("Resolved the Task with Description - %s", diff); + assertEquals(expectedMessage, taskThread.getPosts().get(0).getMessage()); } @Test @@ -471,7 +475,7 @@ public class FeedResourceTest extends CatalogApplicationTest { String oldDescription = table.get().getColumns().stream().filter(c -> c.getName().equals("c1")).findFirst().get().getDescription(); - closeTask(taskId, userAuthHeaders); + closeTask(taskId, "closing comment", userAuthHeaders); // closing the task should not affect description of the table tables = TABLE_RESOURCE_TEST.listEntities(null, userAuthHeaders); @@ -491,7 +495,7 @@ public class FeedResourceTest extends CatalogApplicationTest { assertEquals(TaskStatus.Closed, task.getStatus()); assertEquals(1, taskThread.getPostsCount()); assertEquals(1, taskThread.getPosts().size()); - assertEquals("Closed the Task.", taskThread.getPosts().get(0).getMessage()); + assertEquals("Closed the Task with comment - closing comment", taskThread.getPosts().get(0).getMessage()); } @Test @@ -542,7 +546,9 @@ public class FeedResourceTest extends CatalogApplicationTest { assertEquals(TaskStatus.Closed, task.getStatus()); assertEquals(1, taskThread.getPostsCount()); assertEquals(1, taskThread.getPosts().size()); - assertEquals("Closed the Task.", taskThread.getPosts().get(0).getMessage()); + String diff = getPlaintextDiff("", USER_ADDRESS_TAG_LABEL.getTagFQN()); + String expectedMessage = String.format("Resolved the Task with Tag(s) - %s", diff); + assertEquals(expectedMessage, taskThread.getPosts().get(0).getMessage()); } private static Stream provideStringsForListThreads() { @@ -866,7 +872,7 @@ public class FeedResourceTest extends CatalogApplicationTest { assertEquals(initialThreadCount + 3, threads.getPaging().getTotal()); assertEquals(initialThreadCount + 3, threads.getData().size()); assertEquals( - String.format("Started to follow **table** `%s`", TABLE2.getFullyQualifiedName()), + String.format("Followed **table** `%s`", TABLE2.getFullyQualifiedName()), threads.getData().get(0).getMessage()); assertEquals("Message 2", threads.getData().get(1).getMessage()); @@ -1062,9 +1068,9 @@ public class FeedResourceTest extends CatalogApplicationTest { TestUtils.put(target, resolveTask, Status.OK, authHeaders); } - public static void closeTask(int id, Map authHeaders) throws HttpResponseException { + public static void closeTask(int id, String comment, Map authHeaders) throws HttpResponseException { WebTarget target = getResource("feed/tasks/" + id + "/close"); - TestUtils.put(target, new ResolveTask().withNewValue(""), Status.OK, authHeaders); + TestUtils.put(target, new CloseTask().withComment(comment), Status.OK, authHeaders); } public static ThreadList listTasks( diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index 79de1ab042d..d9a2ef9db27 100644 --- a/ingestion-core/src/metadata/_version.py +++ b/ingestion-core/src/metadata/_version.py @@ -7,5 +7,5 @@ Provides metadata version information. from incremental import Version -__version__ = Version("metadata", 0, 11, 0, dev=10) +__version__ = Version("metadata", 0, 11, 0, dev=11) __all__ = ["__version__"] diff --git a/openmetadata-ui/src/main/resources/ui/cypress/integration/Pages/myData.spec.js b/openmetadata-ui/src/main/resources/ui/cypress/integration/Pages/myData.spec.js index 124f609c46d..8330e8b9726 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/integration/Pages/myData.spec.js +++ b/openmetadata-ui/src/main/resources/ui/cypress/integration/Pages/myData.spec.js @@ -118,7 +118,7 @@ describe('MyData page should work', () => { cy.get('[data-testid="message-container"]') .eq(1) .scrollIntoView() - .contains(`Started to follow ${termObj.entity.slice(0, -1)}`) + .contains(`Followed ${termObj.entity.slice(0, -1)}`) .should('be.visible'); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/TaskDetailPage/TaskDetailPage.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/TaskDetailPage/TaskDetailPage.tsx index 6cd60a4fc47..a21edb22bda 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/TaskDetailPage/TaskDetailPage.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/TaskDetailPage/TaskDetailPage.tsx @@ -169,7 +169,7 @@ const TaskDetailPage = () => { taskDetail.task?.status, ThreadTaskStatus.Closed ); - // const isTaskOpen = isEqual(taskDetail.task?.status, ThreadTaskStatus.Open); + const isCreator = isEqual(taskDetail.createdBy, currentUser?.name); const isTaskActionEdit = isEqual(taskAction.key, TaskActionMode.EDIT); @@ -272,8 +272,8 @@ const TaskDetailPage = () => { const patch = compare(taskDetail, updatedTask); updateThread(taskDetail.id, patch) - .then((res: AxiosResponse) => { - setTaskDetail(res.data); + .then(() => { + fetchTaskDetail(); }) .catch((err: AxiosError) => showErrorToast(err)); setEditAssignee(false); @@ -294,14 +294,16 @@ const TaskDetailPage = () => { }) .catch((err: AxiosError) => showErrorToast(err)); } else { - showErrorToast('Cannot accept empty description'); + showErrorToast( + 'Cannot accept an empty description. Please add a description.' + ); } }; const onTaskReject = () => { if (comment) { updateTask(TaskOperation.REJECT, taskDetail.task?.id, { - newValue: comment, + comment, }) .then(() => { showSuccessToast('Task Closed Successfully'); @@ -314,7 +316,7 @@ const TaskDetailPage = () => { }) .catch((err: AxiosError) => showErrorToast(err)); } else { - showErrorToast('Cannot close task without comment'); + showErrorToast('Cannot close task without a comment'); } setModalVisible(false); }; @@ -805,7 +807,7 @@ const TaskDetailPage = () => { disabled: !comment, className: 'ant-btn-primary-custom', }} - okText="Close" + okText="Close with comment" title={`Close Task #${taskDetail.task?.id} ${taskDetail.message}`} visible={modalVisible} width={700}