Add support for closing task with comment (#5684)

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<Thread> closeTask(UriInfo uriInfo, Thread thread, String user) throws IOException {
+  public PatchResponse<Thread> 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<TagLabel> 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<TagLabel> 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<Thread> threads) {
+    for (Thread t : threads) {
+      sortPosts(t);
+    }
   }
 
   /** Limit the number of posts within each thread. */
   private void limitPostsInThreads(List<Thread> threads, int limitPosts) {
     for (Thread t : threads) {
       List<Post> 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<Thread> 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<Thread> 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<Thread> 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<Thread> 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);
   }
 

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

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 = "<!add>";
     String removeMarker = "<!remove>";

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
+}

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<Arguments> 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<String, String> authHeaders) throws HttpResponseException {
+  public static void closeTask(int id, String comment, Map<String, String> 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(

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__"]

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');
       });
 

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) => {
+      .then(() => {
-        setTaskDetail(res.data);
+        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}