Fixes #3090 Glossary Term approval workflow (#13269)

common/src/main/java/org/openmetadata/common/utils/CommonUtil.java

@@ -72,7 +72,7 @@ public final class CommonUtil {
         String fileName = e.nextElement().getName();
         if (pattern.matcher(fileName).matches()) {
           retval.add(fileName);
-          LOG.info("Adding file from jar {}", fileName);
+          LOG.debug("Adding file from jar {}", fileName);
         }
       }
     } catch (Exception ignored) {
@@ -90,7 +90,7 @@ public final class CommonUtil {
           .map(
               path -> {
                 String relativePath = root.relativize(path).toString();
-                LOG.info("Adding directory file {}", relativePath);
+                LOG.debug("Adding directory file {}", relativePath);
                 return relativePath;
               })
           .collect(Collectors.toSet());

openmetadata-service/src/main/java/org/openmetadata/service/exception/CatalogExceptionMessage.java

@@ -136,6 +136,10 @@ public final class CatalogExceptionMessage {
     return String.format("Principal: CatalogPrincipal{name='%s'} is not admin", name);
   }
 
+  public static String notReviewer(String name) {
+    return String.format("User '%s' is not a reviewer", name);
+  }
+
   public static String permissionDenied(
       String user, MetadataOperation operation, String roleName, String policyName, String ruleName) {
     if (roleName != null) {

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ClassificationRepository.java

@@ -107,13 +107,13 @@ public class ClassificationRepository extends EntityRepository<Classification> {
 
   @Override
   @SuppressWarnings("unused")
-  public void postUpdate(Classification entity) {
+  public void postUpdate(Classification original, Classification updated) {
     String scriptTxt = "for (k in params.keySet()) { ctx._source.put(k, params.get(k)) }";
-    if (entity.getDisabled() != null) {
+    if (updated.getDisabled() != null) {
-      scriptTxt = "ctx._source.disabled=" + entity.getDisabled();
+      scriptTxt = "ctx._source.disabled=" + updated.getDisabled();
     }
     searchClient.updateSearchEntityUpdated(
-        JsonUtils.deepCopy(entity, Classification.class), scriptTxt, "classification.fullyQualifiedName");
+        JsonUtils.deepCopy(updated, Classification.class), scriptTxt, "classification.fullyQualifiedName");
   }
 
   @Override

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java

@@ -1276,6 +1276,13 @@ public interface CollectionDAO {
         @Bind("toType") String toType,
         @Bind("relation") int relation);
 
+    @SqlQuery(
+        "SELECT fromFQN, fromType, json FROM field_relationship WHERE "
+            + "toFQNHash = :toFQNHash AND toType = :toType AND relation = :relation")
+    @RegisterRowMapper(FromFieldMapper.class)
+    List<Triple<String, String, String>> findFrom(
+        @BindFQN("toFQNHash") String toFQNHash, @Bind("toType") String toType, @Bind("relation") int relation);
+
     @SqlQuery(
         "SELECT fromFQN, toFQN, json FROM field_relationship WHERE "
             + "fromFQNHash LIKE CONCAT(:fqnPrefixHash, '%') AND fromType = :fromType AND toType = :toType "
@@ -1349,6 +1356,13 @@ public interface CollectionDAO {
         @Bind("toType") String toType,
         @Bind("relation") int relation);
 
+    class FromFieldMapper implements RowMapper<Triple<String, String, String>> {
+      @Override
+      public Triple<String, String, String> map(ResultSet rs, StatementContext ctx) throws SQLException {
+        return Triple.of(rs.getString("fromFQN"), rs.getString("fromType"), rs.getString("json"));
+      }
+    }
+
     class ToFieldMapper implements RowMapper<Triple<String, String, String>> {
       @Override
       public Triple<String, String, String> map(ResultSet rs, StatementContext ctx) throws SQLException {

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java

@@ -701,10 +701,10 @@ public abstract class EntityRepository<T extends EntityInterface> {
   }
 
   @SuppressWarnings("unused")
-  public void postUpdate(T entity) {
+  protected void postUpdate(T original, T updated) {
     if (supportsSearchIndex) {
       String scriptTxt = "for (k in params.keySet()) { ctx._source.put(k, params.get(k)) }";
-      searchClient.updateSearchEntityUpdated(JsonUtils.deepCopy(entity, entityClass), scriptTxt, "");
+      searchClient.updateSearchEntityUpdated(JsonUtils.deepCopy(updated, entityClass), scriptTxt, "");
     }
   }
 
@@ -775,7 +775,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
             .withCurrentVersion(entity.getVersion())
             .withPreviousVersion(change.getPreviousVersion());
     entity.setChangeDescription(change);
-    postUpdate(entity);
+    postUpdate(entity, entity);
     return new PutResponse<>(Status.OK, changeEvent, RestUtil.ENTITY_FIELDS_CHANGED);
   }
 
@@ -1754,6 +1754,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
 
       // Store the updated entity
       storeUpdate();
+      postUpdate(original, updated);
     }
 
     public void entitySpecificUpdate() {

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java

@@ -49,6 +49,7 @@ import lombok.Getter;
 import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.Triple;
 import org.json.JSONObject;
 import org.openmetadata.schema.EntityInterface;
 import org.openmetadata.schema.api.feed.CloseTask;
@@ -127,11 +128,11 @@ public class FeedRepository {
     return dao.feedDAO().getTaskId();
   }
 
-  public class ThreadContext {
+  public static class ThreadContext {
     @Getter protected final Thread thread;
-    @Getter @Setter protected final EntityLink about;
+    @Getter protected final EntityLink about;
     @Getter @Setter protected EntityInterface aboutEntity;
-    @Getter private EntityReference createdBy;
+    @Getter private final EntityReference createdBy;
 
     ThreadContext(Thread thread) {
       this.thread = thread;
@@ -251,6 +252,23 @@ public class FeedRepository {
     storeMentions(thread, thread.getMessage());
   }
 
+  public Thread getTask(EntityLink about, TaskType taskType) {
+    List<Triple<String, String, String>> tasks =
+        dao.fieldRelationshipDAO()
+            .findFrom(about.getFullyQualifiedFieldValue(), about.getFullyQualifiedFieldType(), IS_ABOUT.ordinal());
+    for (Triple<String, String, String> task : tasks) {
+      if (task.getMiddle().equals(Entity.THREAD)) {
+        String threadId = task.getLeft();
+        Thread thread = EntityUtil.validate(threadId, dao.feedDAO().findById(threadId), Thread.class);
+        if (thread.getTask() != null && thread.getTask().getType() == taskType) {
+          return thread;
+        }
+      }
+    }
+    throw new EntityNotFoundException(
+        String.format("Task for entity %s of type %s was not found", about.getEntityType(), taskType));
+  }
+
   private Thread createThread(ThreadContext threadContext) {
     Thread thread = threadContext.getThread();
     if (thread.getType() == ThreadType.Task) {
@@ -279,8 +297,7 @@ public class FeedRepository {
 
   public PatchResponse<Thread> closeTask(UriInfo uriInfo, Thread thread, String user, CloseTask closeTask) {
     // Update the attributes
-    ThreadContext threadContext = getThreadContext(thread);
+    closeTask(thread, user, closeTask);
-    closeTask(threadContext, user, closeTask);
     Thread updatedHref = FeedResource.addHref(uriInfo, thread);
     return new PatchResponse<>(Status.OK, updatedHref, RestUtil.ENTITY_UPDATED);
   }
@@ -305,7 +322,7 @@ public class FeedRepository {
 
     // Update the attributes
     threadContext.getThread().getTask().withNewValue(resolveTask.getNewValue());
-    closeTask(threadContext, user, new CloseTask());
+    closeTask(threadContext.getThread(), user, new CloseTask());
   }
 
   private static String getTagFQNs(List<TagLabel> tags) {
@@ -340,8 +357,8 @@ public class FeedRepository {
     addPostToThread(thread.getId().toString(), post, user);
   }
 
-  private void closeTask(ThreadContext threadContext, String user, CloseTask closeTask) {
+  public void closeTask(Thread thread, String user, CloseTask closeTask) {
-    Thread thread = threadContext.getThread();
+    ThreadContext threadContext = getThreadContext(thread);
     TaskDetails task = thread.getTask();
     if (task.getStatus() != Open) {
       return;

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java

@@ -17,11 +17,13 @@
 package org.openmetadata.service.jdbi3;
 
 import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
+import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
 import static org.openmetadata.schema.type.Include.ALL;
 import static org.openmetadata.service.Entity.FIELD_REVIEWERS;
 import static org.openmetadata.service.Entity.GLOSSARY;
 import static org.openmetadata.service.Entity.GLOSSARY_TERM;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.invalidGlossaryTermMove;
+import static org.openmetadata.service.exception.CatalogExceptionMessage.notReviewer;
 import static org.openmetadata.service.resources.EntityResource.searchClient;
 import static org.openmetadata.service.util.EntityUtil.entityReferenceMatch;
 import static org.openmetadata.service.util.EntityUtil.getId;
@@ -32,22 +34,36 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
+import javax.json.JsonPatch;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.openmetadata.schema.EntityInterface;
 import org.openmetadata.schema.api.data.TermReference;
+import org.openmetadata.schema.api.feed.CloseTask;
+import org.openmetadata.schema.api.feed.ResolveTask;
 import org.openmetadata.schema.entity.data.Glossary;
 import org.openmetadata.schema.entity.data.GlossaryTerm;
+import org.openmetadata.schema.entity.data.GlossaryTerm.Status;
+import org.openmetadata.schema.entity.feed.Thread;
 import org.openmetadata.schema.type.EntityReference;
 import org.openmetadata.schema.type.Include;
 import org.openmetadata.schema.type.ProviderType;
 import org.openmetadata.schema.type.Relationship;
 import org.openmetadata.schema.type.TagLabel;
 import org.openmetadata.schema.type.TagLabel.TagSource;
+import org.openmetadata.schema.type.TaskDetails;
+import org.openmetadata.schema.type.TaskStatus;
+import org.openmetadata.schema.type.TaskType;
+import org.openmetadata.schema.type.ThreadType;
 import org.openmetadata.service.Entity;
 import org.openmetadata.service.exception.CatalogExceptionMessage;
 import org.openmetadata.service.jdbi3.CollectionDAO.EntityRelationshipRecord;
+import org.openmetadata.service.jdbi3.FeedRepository.TaskWorkflow;
+import org.openmetadata.service.jdbi3.FeedRepository.ThreadContext;
+import org.openmetadata.service.resources.feeds.FeedResource;
+import org.openmetadata.service.resources.feeds.MessageParser.EntityLink;
 import org.openmetadata.service.resources.glossary.GlossaryTermResource;
+import org.openmetadata.service.security.AuthorizationException;
 import org.openmetadata.service.util.EntityUtil;
 import org.openmetadata.service.util.EntityUtil.Fields;
 import org.openmetadata.service.util.FullyQualifiedName;
@@ -108,18 +124,21 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
 
   @Override
   public void prepare(GlossaryTerm entity, boolean update) {
+    List<EntityReference> parentReviewers = null;
     // Validate parent term
     GlossaryTerm parentTerm =
         entity.getParent() != null
-            ? getByName(null, entity.getParent().getFullyQualifiedName(), getFields("owner"))
+            ? Entity.getEntity(entity.getParent(), "owner,reviewers", Include.NON_DELETED)
             : null;
     if (parentTerm != null) {
+      parentReviewers = parentTerm.getReviewers();
       entity.setParent(parentTerm.getEntityReference());
     }
 
     // Validate glossary
-    Glossary glossary = Entity.getEntity(entity.getGlossary(), "", Include.NON_DELETED);
+    Glossary glossary = Entity.getEntity(entity.getGlossary(), "reviewers", Include.NON_DELETED);
     entity.setGlossary(glossary.getEntityReference());
+    parentReviewers = parentReviewers != null ? parentReviewers : glossary.getReviewers();
 
     validateHierarchy(entity);
 
@@ -128,6 +147,11 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
 
     // Validate reviewers
     EntityUtil.populateEntityReferences(entity.getReviewers());
+
+    if (!update || entity.getStatus() == null) {
+      // If parentTerm or glossary has reviewers set, the glossary term can only be created in `Draft` mode
+      entity.setStatus(!nullOrEmpty(parentReviewers) ? Status.DRAFT : Status.APPROVED);
+    }
   }
 
   @Override
@@ -191,6 +215,32 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
     return new GlossaryTermUpdater(original, updated, operation);
   }
 
+  protected void postCreate(GlossaryTerm entity) {
+    if (entity.getStatus() == Status.DRAFT) {
+      // Create an approval task for glossary term in draft mode
+      createApprovalTask(entity, entity.getReviewers());
+    }
+  }
+
+  @Override
+  public void postUpdate(GlossaryTerm original, GlossaryTerm updated) {
+    if (original.getStatus() == Status.DRAFT) {
+      if (updated.getStatus() == Status.APPROVED) {
+        closeApprovalTask(updated, "Approved the glossary term");
+      } else if (updated.getStatus() == Status.REJECTED) {
+        closeApprovalTask(updated, "Rejected the glossary term");
+      }
+    }
+  }
+
+  @Override
+  protected void preDelete(GlossaryTerm entity, String deletedBy) {
+    // A glossary term in `Draft` state can only be deleted by the reviewers
+    if (Status.DRAFT.equals(entity.getStatus())) {
+      checkUpdatedByReviewer(entity, deletedBy);
+    }
+  }
+
   @Override
   protected void postDelete(GlossaryTerm entity) {
     // Cleanup all the tag labels using this glossary term
@@ -212,6 +262,42 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
     }
   }
 
+  public TaskWorkflow getTaskWorkflow(ThreadContext threadContext) {
+    validateTaskThread(threadContext);
+    TaskType taskType = threadContext.getThread().getTask().getType();
+    if (EntityUtil.isApprovalTask(taskType)) {
+      return new ApprovalTaskWorkflow(threadContext);
+    }
+    return super.getTaskWorkflow(threadContext);
+  }
+
+  public static class ApprovalTaskWorkflow extends TaskWorkflow {
+    ApprovalTaskWorkflow(ThreadContext threadContext) {
+      super(threadContext);
+    }
+
+    @Override
+    public EntityInterface performTask(String user, ResolveTask resolveTask) {
+      GlossaryTerm glossaryTerm = (GlossaryTerm) threadContext.getAboutEntity();
+      glossaryTerm.setStatus(Status.APPROVED);
+      return glossaryTerm;
+    }
+
+    @Override
+    protected void closeTask(String user, CloseTask closeTask) {
+      // Closing task results in glossary term going from `Draft` to `Rejected`
+      GlossaryTerm term = (GlossaryTerm) threadContext.getAboutEntity();
+      if (term.getStatus() == Status.DRAFT) {
+        String origJson = JsonUtils.pojoToJson(term);
+        term.setStatus(Status.REJECTED);
+        String updatedJson = JsonUtils.pojoToJson(term);
+        JsonPatch patch = JsonUtils.getJsonPatch(origJson, updatedJson);
+        EntityRepository<?> repository = threadContext.getEntityRepository();
+        repository.patch(null, term.getId(), user, patch);
+      }
+    }
+  }
+
   @Override
   public void restoreFromSearch(GlossaryTerm entity) {
     if (supportsSearchIndex) {
@@ -245,6 +331,52 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
     }
   }
 
+  private void checkUpdatedByReviewer(GlossaryTerm term, String updatedBy) {
+    // Only list of allowed reviewers can change the status from DRAFT to APPROVED
+    List<EntityReference> reviewers = term.getReviewers();
+    if (!nullOrEmpty(reviewers)) {
+      // Updating user must be one of the reviewers
+      boolean isReviewer =
+          reviewers.stream()
+              .anyMatch(e -> e.getName().equals(updatedBy) || e.getFullyQualifiedName().equals(updatedBy));
+      if (!isReviewer) {
+        throw new AuthorizationException(notReviewer(updatedBy));
+      }
+    }
+  }
+
+  private void createApprovalTask(GlossaryTerm entity, List<EntityReference> parentReviewers) {
+    TaskDetails taskDetails =
+        new TaskDetails()
+            .withAssignees(FeedResource.formatAssignees(parentReviewers))
+            .withType(TaskType.RequestApproval)
+            .withStatus(TaskStatus.Open);
+
+    EntityLink about = new EntityLink(entityType, entity.getFullyQualifiedName());
+    Thread thread =
+        new Thread()
+            .withId(UUID.randomUUID())
+            .withThreadTs(System.currentTimeMillis())
+            .withMessage("Approval required for ") // TODO fix this
+            .withCreatedBy(entity.getUpdatedBy())
+            .withAbout(about.getLinkString())
+            .withType(ThreadType.Task)
+            .withTask(taskDetails)
+            .withUpdatedBy(entity.getUpdatedBy())
+            .withUpdatedAt(System.currentTimeMillis());
+    FeedRepository feedRepository = Entity.getFeedRepository();
+    feedRepository.create(thread);
+  }
+
+  private void closeApprovalTask(GlossaryTerm entity, String comment) {
+    EntityLink about = new EntityLink(GLOSSARY_TERM, entity.getFullyQualifiedName());
+    FeedRepository feedRepository = Entity.getFeedRepository();
+    Thread taskThread = feedRepository.getTask(about, TaskType.RequestApproval);
+    if (TaskStatus.Open.equals(taskThread.getTask().getStatus())) {
+      feedRepository.closeTask(taskThread, entity.getUpdatedBy(), new CloseTask().withComment(comment));
+    }
+  }
+
   /** Handles entity updated from PUT and POST operation. */
   public class GlossaryTermUpdater extends EntityUpdater {
     public GlossaryTermUpdater(GlossaryTerm original, GlossaryTerm updated, Operation operation) {
@@ -273,7 +405,14 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
     }
 
     private void updateStatus(GlossaryTerm origTerm, GlossaryTerm updatedTerm) {
-      // TODO Only list of allowed reviewers can change the status from DRAFT to APPROVED
+      if (origTerm.getStatus() == updatedTerm.getStatus()) {
+        return;
+      }
+      // Only reviewers can change from DRAFT status to APPROVED/REJECTED status
+      if (origTerm.getStatus() == Status.DRAFT
+          && (updatedTerm.getStatus() == Status.APPROVED || updatedTerm.getStatus() == Status.REJECTED)) {
+        checkUpdatedByReviewer(origTerm, updatedTerm.getUpdatedBy());
+      }
       recordChange("status", origTerm.getStatus(), updatedTerm.getStatus());
     }
 

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java

@@ -418,7 +418,7 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
     List<ResultSummary> resultSummaries = listOrEmpty(testSuite.getTestCaseResultSummary());
     for (UUID testCaseId : testCaseIds) {
       TestCase testCase = Entity.getEntity(Entity.TEST_CASE, testCaseId, "*", Include.ALL);
-      postUpdate(testCase);
+      postUpdate(testCase, testCase);
       // Get the latest result to set the testSuite summary field
       String result =
           daoCollection

openmetadata-service/src/main/java/org/openmetadata/service/security/DefaultAuthorizer.java

@@ -13,6 +13,7 @@
 
 package org.openmetadata.service.security;
 
+import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
 import static org.openmetadata.schema.type.Permission.Access.ALLOW;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.notAdmin;
 
@@ -71,6 +72,9 @@ public class DefaultAuthorizer implements Authorizer {
     if (subjectContext.isAdmin()) {
       return;
     }
+    if (isReviewer(resourceContext, subjectContext)) {
+      return; // Reviewer of a resource gets admin level privilege on the resource
+    }
     PolicyEvaluator.hasPermission(subjectContext, resourceContext, operationContext);
   }
 
@@ -123,4 +127,15 @@ public class DefaultAuthorizer implements Authorizer {
     }
     return loggedInUser;
   }
+
+  private boolean isReviewer(ResourceContextInterface resourceContext, SubjectContext subjectContext) {
+    if (resourceContext.getEntity() == null) {
+      return false;
+    }
+    String updatedBy = subjectContext.getUser().getName();
+    List<EntityReference> reviewers = resourceContext.getEntity().getReviewers();
+    return !nullOrEmpty(reviewers)
+        ? reviewers.stream().anyMatch(e -> e.getName().equals(updatedBy) || e.getFullyQualifiedName().equals(updatedBy))
+        : false;
+  }
 }

openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java

@@ -86,6 +86,9 @@ public class ResourceContext<T extends EntityInterface> implements ResourceConte
       if (entityRepository.isSupportsDomain()) {
         fields = EntityUtil.addField(fields, Entity.FIELD_DOMAIN);
       }
+      if (entityRepository.isSupportsReviewers()) {
+        fields = EntityUtil.addField(fields, Entity.FIELD_REVIEWERS);
+      }
       Fields fieldList = entityRepository.getFields(fields);
       try {
         if (id != null) {

openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java

@@ -545,6 +545,10 @@ public final class EntityUtil {
     return taskType == TaskType.RequestTag || taskType == TaskType.UpdateTag;
   }
 
+  public static boolean isApprovalTask(TaskType taskType) {
+    return taskType == TaskType.RequestApproval;
+  }
+
   public static Column findColumn(List<Column> columns, String columnName) {
     return columns.stream()
         .filter(c -> c.getName().equals(columnName))

openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java

@@ -371,7 +371,7 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
     Glossary glossary = createEntity(createRequest("importExportTest"), ADMIN_AUTH_HEADERS);
     String user1 = USER1.getName();
     String user2 = USER2.getName();
-    String team1 = TEAM1.getName();
+    String team11 = TEAM11.getName();
 
     // CSV Header "parent" "name" "displayName" "description" "synonyms" "relatedTerms" "references" "tags",
     // "reviewers", "owner", "status"
@@ -382,8 +382,8 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
                 ",g1,dsp1,\"dsc1,1\",h1;h2;h3,,term1;http://term1,Tier.Tier1,%s;%s,user;%s,%s",
                 user1, user2, user1, "Approved"),
             String.format(
-                ",g2,dsp2,dsc3,h1;h3;h3,,term2;https://term2,Tier.Tier2,%s,user;%s,%s", user1, user2, "Draft"),
+                ",g2,dsp2,dsc3,h1;h3;h3,,term2;https://term2,Tier.Tier2,%s,user;%s,%s", user1, user2, "Approved"),
-            String.format("importExportTest.g1,g11,dsp2,dsc11,h1;h3;h3,,,,%s,team;%s,%s", user1, team1, "Deprecated"));
+            String.format("importExportTest.g1,g11,dsp2,dsc11,h1;h3;h3,,,,%s,team;%s,%s", user1, team11, "Draft"));
 
     // Update terms with change in description
     List<String> updateRecords =
@@ -392,12 +392,11 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
                 ",g1,dsp1,new-dsc1,h1;h2;h3,,term1;http://term1,Tier.Tier1,%s;%s,user;%s,%s",
                 user1, user2, user1, "Approved"),
             String.format(
-                ",g2,dsp2,new-dsc3,h1;h3;h3,,term2;https://term2,Tier.Tier2,%s,user;%s,%s", user1, user2, "Draft"),
+                ",g2,dsp2,new-dsc3,h1;h3;h3,,term2;https://term2,Tier.Tier2,%s,user;%s,%s", user1, user2, "Approved"),
-            String.format(
+            String.format("importExportTest.g1,g11,dsp2,new-dsc11,h1;h3;h3,,,,%s,team;%s,%s", user1, team11, "Draft"));
-                "importExportTest.g1,g11,dsp2,new-dsc11,h1;h3;h3,,,,%s,team;%s,%s", user1, team1, "Deprecated"));
 
     // Add new row to existing rows
-    List<String> newRecords = listOf(",g3,dsp0,dsc0,h1;h2;h3,,term0;http://term0,Tier.Tier3,,,Draft");
+    List<String> newRecords = listOf(",g3,dsp0,dsc0,h1;h2;h3,,term0;http://term0,Tier.Tier3,,,Approved");
     testImportExport(glossary.getName(), GlossaryCsv.HEADERS, createRecords, updateRecords, newRecords);
   }
 

openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java

@@ -17,6 +17,7 @@
 package org.openmetadata.service.resources.glossary;
 
 import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static javax.ws.rs.core.Response.Status.FORBIDDEN;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.openmetadata.common.utils.CommonUtil.listOf;
 import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
@@ -26,7 +27,9 @@ import static org.openmetadata.service.Entity.GLOSSARY;
 import static org.openmetadata.service.Entity.GLOSSARY_TERM;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.entityIsNotEmpty;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.glossaryTermMismatch;
+import static org.openmetadata.service.exception.CatalogExceptionMessage.notReviewer;
 import static org.openmetadata.service.resources.databases.TableResourceTest.getColumn;
+import static org.openmetadata.service.security.SecurityUtil.authHeaders;
 import static org.openmetadata.service.util.EntityUtil.fieldAdded;
 import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
 import static org.openmetadata.service.util.EntityUtil.fieldUpdated;
@@ -37,6 +40,11 @@ import static org.openmetadata.service.util.EntityUtil.toTagLabels;
 import static org.openmetadata.service.util.TestUtils.*;
 import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE;
 import static org.openmetadata.service.util.TestUtils.UpdateType.NO_CHANGE;
+import static org.openmetadata.service.util.TestUtils.assertEntityReferenceNames;
+import static org.openmetadata.service.util.TestUtils.assertListNotEmpty;
+import static org.openmetadata.service.util.TestUtils.assertListNotNull;
+import static org.openmetadata.service.util.TestUtils.assertListNull;
+import static org.openmetadata.service.util.TestUtils.assertResponse;
 
 import java.io.IOException;
 import java.net.URI;
@@ -57,18 +65,25 @@ import org.openmetadata.schema.api.data.CreateGlossary;
 import org.openmetadata.schema.api.data.CreateGlossaryTerm;
 import org.openmetadata.schema.api.data.CreateTable;
 import org.openmetadata.schema.api.data.TermReference;
+import org.openmetadata.schema.api.feed.ResolveTask;
 import org.openmetadata.schema.entity.data.Glossary;
 import org.openmetadata.schema.entity.data.GlossaryTerm;
 import org.openmetadata.schema.entity.data.GlossaryTerm.Status;
 import org.openmetadata.schema.entity.data.Table;
+import org.openmetadata.schema.entity.feed.Thread;
 import org.openmetadata.schema.entity.type.Style;
 import org.openmetadata.schema.type.ChangeDescription;
 import org.openmetadata.schema.type.Column;
 import org.openmetadata.schema.type.EntityReference;
 import org.openmetadata.schema.type.TagLabel;
+import org.openmetadata.schema.type.TaskDetails;
+import org.openmetadata.schema.type.TaskStatus;
 import org.openmetadata.service.Entity;
 import org.openmetadata.service.resources.EntityResourceTest;
 import org.openmetadata.service.resources.databases.TableResourceTest;
+import org.openmetadata.service.resources.feeds.FeedResource.ThreadList;
+import org.openmetadata.service.resources.feeds.FeedResourceTest;
+import org.openmetadata.service.resources.feeds.MessageParser.EntityLink;
 import org.openmetadata.service.util.EntityUtil;
 import org.openmetadata.service.util.FullyQualifiedName;
 import org.openmetadata.service.util.JsonUtils;
@@ -79,6 +94,7 @@ import org.openmetadata.service.util.TestUtils.UpdateType;
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, CreateGlossaryTerm> {
   private final GlossaryResourceTest glossaryTest = new GlossaryResourceTest();
+  private final FeedResourceTest taskTest = new FeedResourceTest();
 
   public GlossaryTermResourceTest() {
     super(
@@ -254,16 +270,116 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     fieldDeleted(change, "reviewers", List.of(USER1_REF));
     fieldDeleted(change, "synonyms", List.of("synonym1"));
     fieldDeleted(change, "references", List.of(reference1));
-    term = patchEntityAndCheck(term, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
-
-    // Change GlossaryTerm status from DRAFT to Approved
-    origJson = JsonUtils.pojoToJson(term);
-    term.withStatus(Status.APPROVED);
-    change = getChangeDescription(term.getVersion());
-    fieldUpdated(change, "status", Status.DRAFT, Status.APPROVED);
     patchEntityAndCheck(term, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
   }
 
+  @Test
+  void test_GlossaryTermApprovalWorkflow(TestInfo test) throws IOException {
+    //
+    // glossary1 create without reviewers is created with Approved status
+    //
+    CreateGlossary createGlossary = glossaryTest.createRequest(getEntityName(test, 1)).withReviewers(null);
+    Glossary glossary1 = glossaryTest.createEntity(createGlossary, ADMIN_AUTH_HEADERS);
+
+    // term g1t1 under glossary1 is created in Approved mode without reviewers
+    GlossaryTerm g1t1 = createTerm(glossary1, null, "g1t1");
+    assertEquals(Status.APPROVED, g1t1.getStatus());
+
+    //
+    // glossary2 created with reviewers user1, user2
+    // Glossary term g2t1 created under it are in `Draft` status. Automatically a Request Approval task is created.
+    // Only a reviewer can change the status to `Approved`. When the status changes to `Approved`, the Request Approval
+    // task is automatically resolved.
+    //
+    createGlossary =
+        glossaryTest
+            .createRequest(getEntityName(test, 2))
+            .withReviewers(listOf(USER1.getFullyQualifiedName(), USER2.getFullyQualifiedName()));
+    Glossary glossary2 = glossaryTest.createEntity(createGlossary, ADMIN_AUTH_HEADERS);
+
+    // Creating a glossary term g2t1 should be in `Draft` mode (because glossary has reviewers)
+    GlossaryTerm g2t1 = createTerm(glossary2, null, "g2t1");
+    assertEquals(Status.DRAFT, g2t1.getStatus());
+    assertApprovalTask(g2t1, TaskStatus.Open); // A Request Approval task is opened
+
+    // Non reviewer - even Admin - can't change the `Draft` to `Approved` status using PATCH
+    String json = JsonUtils.pojoToJson(g2t1);
+    g2t1.setStatus(Status.APPROVED);
+    assertResponse(() -> patchEntity(g2t1.getId(), json, g2t1, ADMIN_AUTH_HEADERS), FORBIDDEN, notReviewer("admin"));
+
+    // A reviewer can change the `Draft` to `Approved` status using PATCH or PUT
+    GlossaryTerm g2t1Updated = patchEntity(g2t1.getId(), json, g2t1, authHeaders(USER1.getName()));
+    assertEquals(Status.APPROVED, g2t1Updated.getStatus());
+    assertApprovalTask(g2t1, TaskStatus.Closed); // The Request Approval task is closed
+
+    //
+    // Glossary terms g2t2 created is in `Draft` status. Automatically a Request Approval task is created.
+    // Only a reviewer can resolve the task. Resolving the task changes g2t1 status from `Draft` to `Approved`.
+    //
+    GlossaryTerm g2t2 = createTerm(glossary2, null, "g2t2");
+    assertEquals(Status.DRAFT, g2t2.getStatus());
+    Thread approvalTask = assertApprovalTask(g2t2, TaskStatus.Open); // A Request Approval task is opened
+    int taskId = approvalTask.getTask().getId();
+
+    // Even admin can't resolve the task
+    ResolveTask resolveTask = new ResolveTask().withNewValue(Status.APPROVED.value());
+    assertResponse(
+        () -> taskTest.resolveTask(taskId, resolveTask, ADMIN_AUTH_HEADERS), FORBIDDEN, notReviewer("admin"));
+
+    // Reviewer resolves the task. Glossary is approved. And task is resolved.
+    taskTest.resolveTask(taskId, resolveTask, authHeaders(USER1.getName()));
+    assertApprovalTask(g2t2, TaskStatus.Closed); // A Request Approval task is opened
+    g2t2 = getEntity(g2t2.getId(), authHeaders(USER1.getName()));
+    assertEquals(Status.APPROVED, g2t2.getStatus());
+
+    //
+    // Glossary terms g2t3 created is in `Draft` status. Automatically a Request Approval task is created.
+    // Only a reviewer can close the task. Closing the task moves g2t1 from `Draft` to `Rejected` state.
+    //
+    GlossaryTerm g2t3 = createTerm(glossary2, null, "g2t3");
+    assertEquals(Status.DRAFT, g2t3.getStatus());
+    approvalTask = assertApprovalTask(g2t3, TaskStatus.Open); // A Request Approval task is opened
+    int taskId2 = approvalTask.getTask().getId();
+
+    // Even admin can't close the task
+    assertResponse(() -> taskTest.closeTask(taskId2, "comment", ADMIN_AUTH_HEADERS), FORBIDDEN, notReviewer("admin"));
+
+    // Reviewer closes the task. Glossary term is rejected. And task is resolved.
+    taskTest.closeTask(taskId2, "Rejected", authHeaders(USER1.getName()));
+    assertApprovalTask(g2t3, TaskStatus.Closed); // A Request Approval task is opened
+    g2t3 = getEntity(g2t3.getId(), authHeaders(USER1.getName()));
+    assertEquals(Status.REJECTED, g2t3.getStatus());
+
+    //
+    // Glossary terms g2t4 created is in `Draft` status. Automatically a Request Approval task is created.
+    // Only a reviewer changes the status to `Rejected`. This automatically closes Request Approval task.
+    //
+    final GlossaryTerm g2t4 = createTerm(glossary2, null, "g2t4");
+    assertEquals(Status.DRAFT, g2t4.getStatus());
+    assertApprovalTask(g2t4, TaskStatus.Open); // A Request Approval task is opened
+
+    // Non reviewer - even Admin - can't change the `Draft` to `Approved` status using PATCH
+    String json2 = JsonUtils.pojoToJson(g2t4);
+    g2t4.setStatus(Status.REJECTED);
+    assertResponse(() -> patchEntity(g2t4.getId(), json2, g2t4, ADMIN_AUTH_HEADERS), FORBIDDEN, notReviewer("admin"));
+
+    // A reviewer can change the `Draft` to `Rejected` status using PATCH
+    GlossaryTerm g2t4Updated = patchEntity(g2t4.getId(), json2, g2t4, authHeaders(USER1.getName()));
+    assertEquals(Status.REJECTED, g2t4Updated.getStatus());
+    assertApprovalTask(g2t4, TaskStatus.Closed); // The Request Approval task is closed
+  }
+
+  private Thread assertApprovalTask(GlossaryTerm term, TaskStatus expectedTaskStatus) throws HttpResponseException {
+    String entityLink = new EntityLink(Entity.GLOSSARY_TERM, term.getFullyQualifiedName()).getLinkString();
+    ThreadList threads = taskTest.listTasks(entityLink, null, null, expectedTaskStatus, 100, ADMIN_AUTH_HEADERS);
+    assertEquals(threads.getData().size(), 1);
+    Thread taskThread = threads.getData().get(0);
+    TaskDetails taskDetails = taskThread.getTask();
+    assertNotNull(taskDetails);
+    assertEquals(expectedTaskStatus, taskDetails.getStatus());
+    return taskThread;
+  }
+
   @Test
   void patch_addDeleteTags(TestInfo test) throws IOException {
     // Create glossary term1 in glossary g1
@@ -363,7 +479,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     // Create glossary term t12, t121, t1211 under t1
     GlossaryTerm t12 = createTerm(g1, t1, "t12");
     GlossaryTerm t121 = createTerm(g1, t12, "t121");
-    GlossaryTerm t1211 = createTerm(g1, t121, "t121");
+    createTerm(g1, t121, "t121");
 
     // Assign glossary terms to a table
     // t1 assigned to table. t11 assigned column1 and t111 assigned to column2
@@ -472,9 +588,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
     assertEquals(fqn, entity.getFullyQualifiedName());
     assertEquals(entity.getStyle(), request.getStyle());
     // Validate glossary that holds this term is present
-    validateEntityReference(entity.getGlossary());
+    assertReference(request.getGlossary(), entity.getGlossary());
-    // TODO fix this
-    //    assertTrue(EntityUtil.entityReferenceMatch.test(request.getGlossary(), entity.getGlossary()));
 
     if (request.getParent() != null) {
       assertReference(request.getParent(), entity.getParent());

openmetadata-spec/src/main/resources/json/schema/entity/data/glossaryTerm.json

@@ -25,8 +25,7 @@
     },
     "status": {
       "type": "string",
-      "enum": ["Draft", "Approved", "Deprecated"],
+      "enum": ["Draft", "Approved", "Deprecated", "Rejected"]
-      "default": "Draft"
     }
   },
   "properties": {

openmetadata-spec/src/main/resources/json/schema/entity/feed/thread.json

@@ -15,6 +15,7 @@
         "UpdateDescription",
         "RequestTag",
         "UpdateTag",
+        "RequestApproval",
         "Generic"
       ],
       "javaEnums": [
@@ -30,6 +31,9 @@
         {
           "name": "UpdateTag"
         },
+        {
+          "name": "RequestApproval"
+        },
         {
           "name": "Generic"
         }