From b45ed2c8de8be7668b337a0203463679e897067c Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Thu, 7 Apr 2022 23:14:10 -0700 Subject: [PATCH] Fix #3878: Backend : Getting 500 internal server error for follows activity feed filter (#3927) --- .../catalog/jdbi3/CollectionDAO.java | 7 +++- .../catalog/jdbi3/FeedRepository.java | 12 +++++-- .../resources/feeds/FeedResourceTest.java | 32 +++++++++++++++++++ .../src/main/resources/ui/src/jsons/en.ts | 1 + .../pages/MyDataPage/MyDataPage.component.tsx | 12 ++++--- .../resources/ui/src/utils/StringsUtils.ts | 16 ++++++++-- 6 files changed, 68 insertions(+), 12 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java index 0e934a0c1ec..b4ac8f77a9d 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java @@ -654,6 +654,7 @@ public interface CollectionDAO { + "LIMIT :limit") List listThreadsByFollowsBefore( @Bind("userId") String userId, + @BindList("teamIds") List teamIds, @Bind("limit") int limit, @Bind("before") long before, @Bind("resolved") boolean resolved, @@ -668,6 +669,7 @@ public interface CollectionDAO { + "LIMIT :limit") List listThreadsByFollowsAfter( @Bind("userId") String userId, + @BindList("teamIds") List teamIds, @Bind("limit") int limit, @Bind("after") long after, @Bind("resolved") boolean resolved, @@ -679,7 +681,10 @@ public interface CollectionDAO { + "((fromEntity='user' AND fromId= :userId) OR " + "(fromEntity='team' AND fromId IN ())) AND relation= :relation)") int listCountThreadsByFollows( - @Bind("userId") String userId, @Bind("resolved") boolean resolved, @Bind("relation") int relation); + @Bind("userId") String userId, + @BindList("teamIds") List teamIds, + @Bind("resolved") boolean resolved, + @Bind("relation") int relation); @SqlQuery( "SELECT json FROM thread_entity WHERE updatedAt > :before AND resolved = :resolved AND id in (" 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 a1c2ec72f2f..e38fed67d35 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 @@ -550,13 +550,19 @@ public class FeedRepository { private FilteredThreads getThreadsByFollows( String userId, int limit, long time, boolean isResolved, PaginationType paginationType) throws IOException { List jsons; + List teamIds = getTeamIds(userId); if (paginationType == PaginationType.BEFORE) { - jsons = dao.feedDAO().listThreadsByFollowsBefore(userId, limit, time, isResolved, Relationship.FOLLOWS.ordinal()); + jsons = + dao.feedDAO() + .listThreadsByFollowsBefore(userId, teamIds, limit, time, isResolved, Relationship.FOLLOWS.ordinal()); } else { - jsons = dao.feedDAO().listThreadsByFollowsAfter(userId, limit, time, isResolved, Relationship.FOLLOWS.ordinal()); + jsons = + dao.feedDAO() + .listThreadsByFollowsAfter(userId, teamIds, limit, time, isResolved, Relationship.FOLLOWS.ordinal()); } List threads = JsonUtils.readObjects(jsons, Thread.class); - int totalCount = dao.feedDAO().listCountThreadsByFollows(userId, isResolved, Relationship.FOLLOWS.ordinal()); + int totalCount = + dao.feedDAO().listCountThreadsByFollows(userId, teamIds, isResolved, Relationship.FOLLOWS.ordinal()); return new FilteredThreads(threads, totalCount); } 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 a42f2540ca5..49073901809 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 @@ -14,6 +14,7 @@ package org.openmetadata.catalog.resources.feeds; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static javax.ws.rs.core.Response.Status.NOT_FOUND; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -518,6 +519,31 @@ public class FeedResourceTest extends CatalogApplicationTest { assertEquals(2, threads.getPaging().getTotal()); } + @Test + void list_threadsWithFollowsFilter() throws HttpResponseException { + // Get the initial thread count of TABLE2 + String entityLink = String.format("<#E/table/%s>", TABLE2.getFullyQualifiedName()); + int initialThreadCount = listThreads(entityLink, null, AUTH_HEADERS).getPaging().getTotal(); + + // Create threads + createAndCheck(create().withMessage("Message 1").withAbout(entityLink), ADMIN_AUTH_HEADERS); + + createAndCheck(create().withMessage("Message 2").withAbout(entityLink), ADMIN_AUTH_HEADERS); + + // Make the USER follow TABLE2 + followTable(TABLE2.getId(), USER.getId(), AUTH_HEADERS); + + ThreadList threads = listThreadsWithFilter(USER.getId().toString(), FilterType.FOLLOWS.toString(), AUTH_HEADERS); + assertEquals(initialThreadCount + 2, threads.getPaging().getTotal()); + assertEquals(initialThreadCount + 2, threads.getData().size()); + assertEquals("Message 2", threads.getData().get(0).getMessage()); + + // Filter by follows for another user should return 0 threads + threads = listThreadsWithFilter(USER2.getId().toString(), FilterType.FOLLOWS.toString(), AUTH_HEADERS); + assertEquals(0, threads.getPaging().getTotal()); + assertEquals(0, threads.getData().size()); + } + @Test void list_threadsWithInvalidFilter() { assertResponse( @@ -679,6 +705,12 @@ public class FeedResourceTest extends CatalogApplicationTest { return TestUtils.get(target, ThreadList.class, authHeaders); } + public static void followTable(UUID tableId, UUID userId, Map authHeaders) + throws HttpResponseException { + WebTarget target = getResource("tables/" + tableId + "/followers"); + TestUtils.put(target, userId.toString(), CREATED, authHeaders); + } + public static ThreadList listThreadsWithFilter(String userId, String filterType, Map authHeaders) throws HttpResponseException { WebTarget target = getResource("feed"); diff --git a/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts b/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts index 390cb4f6faa..ddbf0c92759 100644 --- a/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts +++ b/openmetadata-ui/src/main/resources/ui/src/jsons/en.ts @@ -54,6 +54,7 @@ const jsonData = { 'fetch-data-error': 'Error while fetching data!', 'fetch-database-details-error': 'Error while fetching database details!', 'fetch-database-tables-error': 'Error while fetching database tables!', + 'fetch-activity-feed-error': 'Error while fetching activity feeds!', 'fetch-entity-feed-error': 'Error while fetching entity feeds!', 'fetch-entity-feed-count-error': 'Error while fetching entity feed count!', 'fetch-entity-count-error': 'Error while fetching entity count!', diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.component.tsx index 92ec29dc487..334c2dc5dc8 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.component.tsx @@ -238,11 +238,13 @@ const MyDataPage = () => { setEntityThread((prevData) => [...prevData, ...data]); }) - .catch(() => { - showToast({ - variant: 'error', - body: 'Error while fetching the Activity Feed', - }); + .catch((err: AxiosError) => { + handleShowErrorToast( + getErrorText( + err, + jsonData['api-error-messages']['fetch-activity-feed-error'] + ) + ); }) .finally(() => { setIsFeedLoading(false); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/StringsUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/StringsUtils.ts index 93b3bce38e5..b3ae971f8c1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/StringsUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/StringsUtils.ts @@ -106,7 +106,17 @@ export const getErrorText = ( value: AxiosError | string, fallbackText: string ): string => { - return ( - (isString(value) ? value : value.response?.data?.message) || fallbackText - ); + let errorText; + if (isString(value)) { + return value; + } else { + errorText = value.response?.data?.message; + if (!errorText) { + // if error text is undefined or null or empty, try responseMessage in data + errorText = value.response?.data?.responseMessage; + } + } + + // if error text is still empty, return the fallback text + return errorText || fallbackText; };