Skip to content

Commit

Permalink
Updating glossary reviewers should propagate reviewers in glossary te…
Browse files Browse the repository at this point in the history 
…rm (#16580)

* highlight inherited reviewer in glossary

* locales

* use glossary name for search query

* fix glossary version cypress

* add union datatype for subfields

* Adding reviewer to glossary also adds them as an assignee to the task

* add glossary approval cypress

---------

Co-authored-by: sonikashah <[email protected]>
  • Loading branch information
@karanh37 @sonika-shah
karanh37 and sonika-shah authored Jun 10, 2024
1 parent 945ec7c commit 4c8bf1c
Show file tree
Hide file tree
Showing 27 changed files with 315 additions and 52 deletions.
7 changes: 7 additions & 0 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,13 @@ default List<String> listAfter(ListFilter filter, int limit, String after) {

@SqlQuery("select fqnhash FROM glossary_term_entity where fqnhash LIKE CONCAT(:fqnhash, '.%')")
List<String> getNestedChildrenByFQN(@BindFQN("fqnhash") String fqnhash);

default List<String> getAllTerms(String fqnPrefix) {
return getAllTermsInternal((FullyQualifiedName.quoteName(fqnPrefix)));
}

@SqlQuery("select json FROM glossary_term_entity where fqnhash LIKE CONCAT(:fqnhash, '.%')")
List<String> getAllTermsInternal(@BindFQN("fqnhash") String fqnhash);
}

interface IngestionPipelineDAO extends EntityDAO<IngestionPipeline> {
Expand Down
28 changes: 28 additions & 0 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,15 @@
import org.openmetadata.service.resources.glossary.GlossaryResource;
import org.openmetadata.service.util.EntityUtil.Fields;
import org.openmetadata.service.util.FullyQualifiedName;
import org.openmetadata.service.util.JsonUtils;

@Slf4j
public class GlossaryRepository extends EntityRepository<Glossary> {
private static final String UPDATE_FIELDS = "";
private static final String PATCH_FIELDS = "";

FeedRepository feedRepository = Entity.getFeedRepository();

public GlossaryRepository() {
super(
GlossaryResource.COLLECTION_PATH,
Expand Down Expand Up @@ -321,6 +324,12 @@ private void updateAssetIndexesOnGlossaryUpdate(Glossary original, Glossary upda
}
}

private List<GlossaryTerm> getAllTerms(Glossary glossary) {
// Get all the hierarchically nested terms of the glossary
List<String> jsons = daoCollection.glossaryTermDAO().getAllTerms(glossary.getName());
return JsonUtils.readObjects(jsons, GlossaryTerm.class);
}

/** Handles entity updated from PUT and POST operation. */
public class GlossaryUpdater extends EntityUpdater {
public GlossaryUpdater(Glossary original, Glossary updated, Operation operation) {
Expand Down Expand Up @@ -364,6 +373,25 @@ public void updateName(Glossary original, Glossary updated) {
}
}

@Override
public void updateReviewers() {
super.updateReviewers();
GlossaryTermRepository repository =
(GlossaryTermRepository) Entity.getEntityRepository(GLOSSARY_TERM);

// adding the reviewer in glossary should add the person as assignee to the task - for all
// draft terms present in glossary
if (!original.getReviewers().equals(updated.getReviewers())) {

List<GlossaryTerm> childTerms = getAllTerms(updated);
for (GlossaryTerm term : childTerms) {
if (term.getStatus().equals(Status.DRAFT)) {
repository.updateTaskWithNewReviewers(term);
}
}
}
}

public void invalidateGlossary(UUID classificationId) {
// Glossary name changed. Invalidate the glossary and its children terms
CACHE_WITH_ID.invalidate(new ImmutablePair<>(GLOSSARY, classificationId));
Expand Down
71 changes: 53 additions & 18 deletions ...metadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@
import org.openmetadata.schema.type.api.BulkResponse;
import org.openmetadata.service.Entity;
import org.openmetadata.service.exception.CatalogExceptionMessage;
import org.openmetadata.service.exception.EntityNotFoundException;
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;
import org.openmetadata.service.resources.feeds.MessageParser.EntityLink;
import org.openmetadata.service.resources.glossary.GlossaryTermResource;
import org.openmetadata.service.search.SearchRequest;
Expand All @@ -104,6 +106,8 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {

private static GlossaryTerm valueBeforeUpdate = new GlossaryTerm();

FeedRepository feedRepository = Entity.getFeedRepository();

public GlossaryTermRepository() {
super(
GlossaryTermResource.COLLECTION_PATH,
Expand Down Expand Up @@ -786,6 +790,48 @@ private void updateAssetIndexesOnGlossaryTermUpdate(GlossaryTerm original, Gloss
}
}

private List<GlossaryTerm> getNestedTerms(GlossaryTerm glossaryTerm) {
// Get all the hierarchically nested child terms of the glossary term
List<String> jsons =
daoCollection.glossaryTermDAO().getAllTermsInternal(glossaryTerm.getFullyQualifiedName());
return JsonUtils.readObjects(jsons, GlossaryTerm.class);
}

protected void updateTaskWithNewReviewers(GlossaryTerm term) {
try {

MessageParser.EntityLink about =
new MessageParser.EntityLink(GLOSSARY_TERM, term.getFullyQualifiedName());
Thread originalTask = feedRepository.getTask(about, TaskType.RequestApproval);

// Update assignees only for open approval tasks
if (TaskStatus.Open.equals(originalTask.getTask().getStatus())) {

term =
Entity.getEntityByName(
Entity.GLOSSARY_TERM,
term.getFullyQualifiedName(),
"id,fullyQualifiedName,reviewers",
Include.ALL);

Thread updatedTask = JsonUtils.deepCopy(originalTask, Thread.class);
updatedTask.getTask().withAssignees(new ArrayList<>(term.getReviewers()));
JsonPatch patch = JsonUtils.getJsonPatch(originalTask, updatedTask);
RestUtil.PatchResponse<Thread> thread =
feedRepository.patchThread(
null, originalTask.getId(), updatedTask.getUpdatedBy(), patch);

// Send WebSocket Notification
WebsocketNotificationHandler.handleTaskNotification(thread.entity());
}
} catch (EntityNotFoundException e) {
LOG.info(
"{} Task not found for glossary term {}",
TaskType.RequestApproval,
term.getFullyQualifiedName());
}
}

/** Handles entity updated from PUT and POST operation. */
public class GlossaryTermUpdater extends EntityUpdater {
public GlossaryTermUpdater(GlossaryTerm original, GlossaryTerm updated, Operation operation) {
Expand All @@ -795,27 +841,16 @@ public GlossaryTermUpdater(GlossaryTerm original, GlossaryTerm updated, Operatio
@Override
public void updateReviewers() {
super.updateReviewers();

// adding the reviewer should add the person as assignee to the task
if ((!nullOrEmpty(updated.getReviewers())
&& !original.getReviewers().equals(updated.getReviewers()))
&& updated.getStatus() == Status.DRAFT) {
EntityLink about = new EntityLink(GLOSSARY_TERM, updated.getFullyQualifiedName());
FeedRepository feedRepository = Entity.getFeedRepository();
Thread originalTask = feedRepository.getTask(about, TaskType.RequestApproval);

if (TaskStatus.Open.equals(originalTask.getTask().getStatus())) {

Thread updatedTask = JsonUtils.deepCopy(originalTask, Thread.class);
updatedTask.getTask().withAssignees(updated.getReviewers());
JsonPatch patch = JsonUtils.getJsonPatch(originalTask, updatedTask);

RestUtil.PatchResponse<Thread> thread =
feedRepository.patchThread(
null, originalTask.getId(), updatedTask.getUpdatedBy(), patch);
if (!original.getReviewers().equals(updated.getReviewers())) {

// Send WebSocket Notification
WebsocketNotificationHandler.handleTaskNotification(thread.entity());
List<GlossaryTerm> childTerms = getNestedTerms(updated);
childTerms.add(updated);
for (GlossaryTerm term : childTerms) {
if (term.getStatus().equals(Status.DRAFT)) {
updateTaskWithNewReviewers(term);
}
}
}
}
Expand Down
82 changes: 82 additions & 0 deletions ...rvice/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@
import static org.openmetadata.service.util.TestUtils.validateTagLabel;

import java.io.IOException;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.ws.rs.core.Response.Status;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -62,6 +65,7 @@
import org.openmetadata.schema.entity.data.Glossary;
import org.openmetadata.schema.entity.data.GlossaryTerm;
import org.openmetadata.schema.entity.data.Table;
import org.openmetadata.schema.entity.feed.Thread;
import org.openmetadata.schema.type.ApiStatus;
import org.openmetadata.schema.type.ChangeDescription;
import org.openmetadata.schema.type.Column;
Expand All @@ -70,6 +74,7 @@
import org.openmetadata.schema.type.ProviderType;
import org.openmetadata.schema.type.TagLabel;
import org.openmetadata.schema.type.TagLabel.TagSource;
import org.openmetadata.schema.type.TaskStatus;
import org.openmetadata.schema.type.csv.CsvImportResult;
import org.openmetadata.service.Entity;
import org.openmetadata.service.exception.CatalogExceptionMessage;
Expand Down Expand Up @@ -150,6 +155,42 @@ void patch_addDeleteReviewers(TestInfo test) throws IOException {
glossary =
patchEntityAndCheck(glossary, origJson, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change);

// Create a glossary term and assign USER2 as a reviewer
GlossaryTermResourceTest glossaryTermResourceTest = new GlossaryTermResourceTest();
CreateGlossaryTerm createGlossaryTerm =
glossaryTermResourceTest
.createRequest("GLOSSARY_TERM1", "", "", null)
.withRelatedTerms(List.of(GLOSSARY1_TERM1.getFullyQualifiedName()))
.withGlossary(glossary.getName())
.withReviewers(listOf(USER2_REF));
GlossaryTerm GLOSSARY_TERM1 =
glossaryTermResourceTest.createEntity(createGlossaryTerm, ADMIN_AUTH_HEADERS);

// Verify that the term has both the glossary's reviewer and its own reviewer
List<EntityReference> reviewers = listOf(USER1_REF, USER2_REF);
reviewers.sort(Comparator.comparing(EntityReference::getName));
assertEquals(GLOSSARY_TERM1.getReviewers().size(), reviewers.size());

// Compare the reviewer IDs of both lists to ensure they match
List<UUID> glossaryTermReviewerIds =
GLOSSARY_TERM1.getReviewers().stream()
.map(EntityReference::getId)
.collect(Collectors.toList());
assertEquals(glossaryTermReviewerIds, listOf(USER1_REF.getId(), USER2_REF.getId()));

// Verify that the task assignees are the same as the term reviewers
Thread approvalTask =
glossaryTermResourceTest.assertApprovalTask(GLOSSARY_TERM1, TaskStatus.Open);
assertEquals(
GLOSSARY_TERM1.getReviewers().size(), approvalTask.getTask().getAssignees().size());

// Compare the reviewer IDs of both lists to ensure they match
List<UUID> taskAssigneeIds =
approvalTask.getTask().getAssignees().stream()
.map(EntityReference::getId)
.collect(Collectors.toList());
assertEquals(glossaryTermReviewerIds, taskAssigneeIds);

// Remove a reviewer USER1 in PATCH request
// Changes from this PATCH is consolidated with the previous changes
origJson = JsonUtils.pojoToJson(glossary);
Expand All @@ -160,6 +201,47 @@ void patch_addDeleteReviewers(TestInfo test) throws IOException {
CHANGE_CONSOLIDATED); // PATCH operation update is consolidated in a user session
fieldAdded(change, "reviewers", List.of(USER2_REF));
patchEntityAndCheck(glossary, origJson, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change);

// Verify that USER1_REF is removed from the reviewers for the terms inside the glossary
GLOSSARY_TERM1 =
glossaryTermResourceTest.getEntity(GLOSSARY_TERM1.getId(), "reviewers", ADMIN_AUTH_HEADERS);
reviewers = listOf(USER2_REF);
reviewers.sort(Comparator.comparing(EntityReference::getName));
assertEquals(GLOSSARY_TERM1.getReviewers(), reviewers);

// Create a child term under GLOSSARY_TERM1 and ensure the reviewers are inherited from parent
// term
createGlossaryTerm =
glossaryTermResourceTest
.createRequest("CHILD_TERM1", "", "", null)
.withRelatedTerms(List.of(GLOSSARY1_TERM1.getFullyQualifiedName()))
.withGlossary(glossary.getName())
.withParent(GLOSSARY_TERM1.getFullyQualifiedName())
.withReviewers(listOf(DATA_CONSUMER_REF));
GlossaryTerm CHILD_TERM1 =
glossaryTermResourceTest.createEntity(createGlossaryTerm, ADMIN_AUTH_HEADERS);

reviewers = listOf(USER2_REF, DATA_CONSUMER_REF);
reviewers.sort(Comparator.comparing(EntityReference::getName));
assertEquals(CHILD_TERM1.getReviewers().size(), reviewers.size());

// Compare the reviewer IDs of both lists to ensure they match
List<UUID> childTermReviewerIds =
CHILD_TERM1.getReviewers().stream()
.map(EntityReference::getId)
.collect(Collectors.toList());
assertEquals(childTermReviewerIds, listOf(DATA_CONSUMER_REF.getId(), USER2_REF.getId()));

// Verify that the task assignees are the same as the child term reviewers
approvalTask = glossaryTermResourceTest.assertApprovalTask(CHILD_TERM1, TaskStatus.Open);
assertEquals(CHILD_TERM1.getReviewers().size(), approvalTask.getTask().getAssignees().size());

// Compare the reviewer IDs of both lists to ensure they match
taskAssigneeIds =
approvalTask.getTask().getAssignees().stream()
.map(EntityReference::getId)
.collect(Collectors.toList());
assertEquals(childTermReviewerIds, taskAssigneeIds);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...e/src/test/java/org/openmetadata/service/resources/glossary/GlossaryTermResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ void testInheritedPermissionFromParent(TestInfo test) throws IOException {
g, t1, "t11", null, DATA_STEWARD.getEntityReference(), authHeaders(DATA_STEWARD.getName()));
}

private Thread assertApprovalTask(GlossaryTerm term, TaskStatus expectedTaskStatus)
protected Thread assertApprovalTask(GlossaryTerm term, TaskStatus expectedTaskStatus)
throws HttpResponseException {
String entityLink =
new EntityLink(Entity.GLOSSARY_TERM, term.getFullyQualifiedName()).getLinkString();
Expand Down
5 changes: 5 additions & 0 deletions openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Glossary.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,11 @@ describe('Glossary page should work properly', { tags: 'Governance' }, () => {
glossary: GLOSSARY_1,
glossaryTerm: GLOSSARY_1.terms[1],
});

approveGlossaryTermWorkflow({
glossary: GLOSSARY_1,
glossaryTerm: GLOSSARY_1.terms[2],
});
cy.logout();
Cypress.session.clearAllSavedSessions();
cy.login();
Expand Down
10 changes: 10 additions & 0 deletions openmetadata-ui/src/main/resources/ui/src/assets/svg/ic-inherit.svg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 6 additions & 3 deletions .../resources/ui/src/components/Entity/EntityLineage/NodeChildren/NodeChildren.component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import { isEmpty } from 'lodash';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { BORDER_COLOR } from '../../../../constants/constants';
import { LINEAGE_COLUMN_NODE_SUPPORTED } from '../../../../constants/Lineage.constants';
import {
DATATYPES_HAVING_SUBFIELDS,
LINEAGE_COLUMN_NODE_SUPPORTED,
} from '../../../../constants/Lineage.constants';
import { useLineageProvider } from '../../../../context/LineageProvider/LineageProvider';
import { LineageLayerView } from '../../../../context/LineageProvider/LineageProvider.interface';
import { EntityType } from '../../../../enums/entity.enum';
Expand Down Expand Up @@ -111,7 +114,7 @@ const NodeChildren = ({ node, isConnectable }: NodeChildrenProps) => {
<Panel header={headerContent} key={record.fullyQualifiedName ?? ''}>
{record?.children?.map((child) => {
const { fullyQualifiedName, dataType } = child;
if (['RECORD', 'STRUCT', 'ARRAY'].includes(dataType)) {
if (DATATYPES_HAVING_SUBFIELDS.includes(dataType)) {
return renderRecord(child);
} else {
const isColumnTraced = tracedColumns.includes(
Expand All @@ -136,7 +139,7 @@ const NodeChildren = ({ node, isConnectable }: NodeChildrenProps) => {
const renderColumnsData = useCallback(
(column: Column) => {
const { fullyQualifiedName, dataType } = column;
if (['RECORD', 'STRUCT', 'ARRAY'].includes(dataType)) {
if (DATATYPES_HAVING_SUBFIELDS.includes(dataType)) {
return renderRecord(column);
} else {
const isColumnTraced = tracedColumns.includes(fullyQualifiedName ?? '');
Expand Down
Loading

0 comments on commit 4c8bf1c

Please sign in to comment.