Skip to content

Commit

Permalink
Fix #3080: Activity Feed: Blank feed on my-data page for updating use…
Browse files Browse the repository at this point in the history 
…r's role or team (#3131)
  • Loading branch information
@vivekratnavel
vivekratnavel committed Mar 4, 2022
1 parent 79589e3 commit bddb16e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 35 deletions.
15 changes: 15 additions & 0 deletions catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ public final class Entity {
public static final String AIRFLOW_PIPELINE = "airflowPipeline";
public static final String WEBHOOK = "webhook";

//
// List of entities whose changes should not be published to the Activity Feed
//
public static final List<String> ACTIVITY_FEED_EXCLUDED_ENTITIES = List.of(USER, TEAM, ROLE, POLICY, BOTS);

private Entity() {}

public static <T> void registerEntity(
Expand Down Expand Up @@ -185,6 +190,16 @@ public static boolean shouldHaveOwner(@NonNull String entityType) {
return !entityType.equals(TEAM);
}

/**
* Returns true if the change events of the given entity type should be published to the activity feed.
*
* @param entityType Type of the entity.
* @return true if change events of the entity should be published to activity feed, false otherwise
*/
public static boolean shouldDisplayEntityChangeOnFeed(@NonNull String entityType) {
return !ACTIVITY_FEED_EXCLUDED_ENTITIES.contains(entityType);
}

public static <T> EntityInterface<T> getEntityInterface(T entity) {
if (entity == null) {
return null;
Expand Down
21 changes: 13 additions & 8 deletions catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,15 @@ public Void process(ContainerRequestContext requestContext, ContainerResponseCon

// Add a new thread to the entity for every change event
// for the event to appear in activity feeds
List<Thread> threads = getThreads(responseContext, changeEvent);
if (threads != null) {
for (var thread : threads) {
feedDao.create(thread);
if (Entity.shouldDisplayEntityChangeOnFeed(changeEvent.getEntityType())) {
List<Thread> threads = getThreads(responseContext);
if (threads != null) {
for (var thread : threads) {
// Don't create a thread if there is no message
if (!thread.getMessage().isEmpty()) {
feedDao.create(thread);
}
}
}
}
}
Expand Down Expand Up @@ -171,7 +176,7 @@ private static ChangeEvent copyChangeEvent(ChangeEvent changeEvent) {
.withCurrentVersion(changeEvent.getCurrentVersion());
}

private List<Thread> getThreads(ContainerResponseContext responseContext, ChangeEvent changeEvent) {
private List<Thread> getThreads(ContainerResponseContext responseContext) {
Object entity = responseContext.getEntity();
if (entity == null) {
return null; // Response has no entity to produce change event from
Expand All @@ -184,14 +189,14 @@ private List<Thread> getThreads(ContainerResponseContext responseContext, Change
return null;
}

return getThreads(entity, entityInterface.getChangeDescription(), changeEvent);
return getThreads(entity, entityInterface.getChangeDescription());
}

private List<Thread> getThreads(Object entity, ChangeDescription changeDescription, ChangeEvent changeEvent) {
private List<Thread> getThreads(Object entity, ChangeDescription changeDescription) {
List<Thread> threads = new ArrayList<>();
var entityInterface = Entity.getEntityInterface(entity);

Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, entity, changeEvent);
Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, entity);

// Create an automated thread
for (var link : messages.keySet()) {
Expand Down
6 changes: 2 additions & 4 deletions catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.resources.feeds.MessageParser.EntityLink;
import org.openmetadata.catalog.type.ChangeDescription;
import org.openmetadata.catalog.type.ChangeEvent;
import org.openmetadata.catalog.type.EntityReference;
import org.openmetadata.catalog.type.FieldChange;

Expand All @@ -46,8 +45,7 @@ private enum CHANGE_TYPE {
DELETE
}

public static Map<EntityLink, String> getFormattedMessages(
ChangeDescription changeDescription, Object entity, ChangeEvent changeEvent) {
public static Map<EntityLink, String> getFormattedMessages(ChangeDescription changeDescription, Object entity) {
// Store a map of entityLink -> message
Map<EntityLink, String> messages;

Expand Down Expand Up @@ -82,7 +80,7 @@ private static Map<EntityLink, String> getFormattedMessages(
}

private static String getFieldValue(Object fieldValue) {
if (fieldValue != null) {
if (fieldValue != null && !fieldValue.toString().isEmpty()) {
try {
// Check if field value is a json string
JsonValue json = JsonUtils.readJson(fieldValue.toString());
Expand Down
31 changes: 8 additions & 23 deletions catalog-rest-service/src/test/java/org/openmetadata/catalog/util/ChangeEventParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.openmetadata.catalog.resources.databases.TableResourceTest;
import org.openmetadata.catalog.resources.feeds.MessageParser.EntityLink;
import org.openmetadata.catalog.type.ChangeDescription;
import org.openmetadata.catalog.type.ChangeEvent;
import org.openmetadata.catalog.type.EntityReference;
import org.openmetadata.catalog.type.FieldChange;
import org.openmetadata.catalog.type.TagLabel;
Expand All @@ -57,16 +56,14 @@ public void setup(TestInfo test) throws IOException, URISyntaxException {
@Test
void testFormattedMessages() throws JsonProcessingException {
ChangeDescription changeDescription = new ChangeDescription();
ChangeEvent changeEvent = new ChangeEvent();
// Simulate updating tags of an entity from tag1 -> tag2
FieldChange addTag = new FieldChange();
addTag.withName("tags").withNewValue("tag2");
FieldChange deleteTag = new FieldChange();
deleteTag.withName("tags").withOldValue("tag1");
changeDescription.withFieldsAdded(List.of(addTag)).withFieldsDeleted(List.of(deleteTag)).withPreviousVersion(1.0);
changeEvent.withChangeDescription(changeDescription).withPreviousVersion(1.0).withCurrentVersion(1.1);

Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, messages.size());

TagLabel tag1 = new TagLabel();
Expand All @@ -78,8 +75,7 @@ void testFormattedMessages() throws JsonProcessingException {
addTag.withNewValue(JsonUtils.pojoToJson(List.of(tag2)));
deleteTag.withOldValue(JsonUtils.pojoToJson(List.of(tag1)));

Map<EntityLink, String> jsonMessages =
ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
Map<EntityLink, String> jsonMessages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, jsonMessages.size());

// The entity links and values of both the messages should be the same
Expand All @@ -89,17 +85,15 @@ void testFormattedMessages() throws JsonProcessingException {
@Test
void testEntityReferenceFormat() throws JsonProcessingException {
ChangeDescription changeDescription = new ChangeDescription();
ChangeEvent changeEvent = new ChangeEvent();
// Simulate adding owner to a table
EntityReference entityReference = new EntityReference();
entityReference.withId(UUID.randomUUID()).withName("user1").withDisplayName("User One");
FieldChange addOwner = new FieldChange();
addOwner.withName("owner").withNewValue(JsonUtils.pojoToJson(entityReference));

changeDescription.withFieldsAdded(List.of(addOwner)).withPreviousVersion(1.0);
changeEvent.withChangeDescription(changeDescription).withPreviousVersion(1.0).withCurrentVersion(1.1);

Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, messages.size());

assertEquals("Added **owner**: `User One`", messages.values().iterator().next());
Expand All @@ -108,15 +102,13 @@ void testEntityReferenceFormat() throws JsonProcessingException {
@Test
void testUpdateOfString() {
ChangeDescription changeDescription = new ChangeDescription();
ChangeEvent changeEvent = new ChangeEvent();
// Simulate a change of description in table
FieldChange updateDescription = new FieldChange();
updateDescription.withName("description").withNewValue("new description").withOldValue("old description");

changeDescription.withFieldsUpdated(List.of(updateDescription)).withPreviousVersion(1.0);
changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(1.1);

Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, messages.size());

assertEquals(
Expand All @@ -135,11 +127,8 @@ void testUpdateOfString() {
.withFieldsDeleted(List.of(deleteDescription))
.withPreviousVersion(1.0);

changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(1.1);

// now test if both the type of updates give the same message
Map<EntityLink, String> updatedMessages =
ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
Map<EntityLink, String> updatedMessages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, updatedMessages.size());

assertEquals(messages.keySet().iterator().next(), updatedMessages.keySet().iterator().next());
Expand All @@ -149,7 +138,6 @@ void testUpdateOfString() {
@Test
void testMajorSchemaChange() {
ChangeDescription changeDescription = new ChangeDescription();
ChangeEvent changeEvent = new ChangeEvent();
// Simulate a change of column name in table
FieldChange addColumn = new FieldChange();
addColumn
Expand All @@ -167,9 +155,8 @@ void testMajorSchemaChange() {
.withFieldsAdded(List.of(addColumn))
.withFieldsDeleted(List.of(deleteColumn))
.withPreviousVersion(1.3);
changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(2.3);

Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
Map<EntityLink, String> messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, messages.size());

assertEquals(
Expand All @@ -186,9 +173,8 @@ void testMajorSchemaChange() {
.withFieldsAdded(List.of(addColumn))
.withFieldsDeleted(List.of(deleteColumn))
.withPreviousVersion(1.3);
changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(2.3);

messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, messages.size());

assertEquals(
Expand All @@ -205,9 +191,8 @@ void testMajorSchemaChange() {
.withFieldsAdded(List.of(addColumn))
.withFieldsDeleted(List.of(deleteColumn))
.withPreviousVersion(1.4);
changeEvent.withChangeDescription(changeDescription).withPreviousVersion(0.1).withCurrentVersion(2.4);

messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE, changeEvent);
messages = ChangeEventParser.getFormattedMessages(changeDescription, TABLE);
assertEquals(1, messages.size());

assertEquals(
Expand Down

0 comments on commit bddb16e

Please sign in to comment.