diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TaskCommentsIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TaskCommentsIT.java index 610132cf292b..d996f0a7ae22 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TaskCommentsIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TaskCommentsIT.java @@ -14,6 +14,7 @@ package org.openmetadata.it.tests; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -37,6 +38,7 @@ import org.openmetadata.schema.type.TaskPriority; import org.openmetadata.sdk.client.OpenMetadataClient; import org.openmetadata.sdk.exceptions.ForbiddenException; +import org.openmetadata.sdk.models.ListParams; /** * Integration tests for Task Comments functionality. @@ -325,4 +327,51 @@ void test_commentHasTimestamp() { adminClient.tasks().delete(task.getId().toString(), java.util.Map.of("hardDelete", "true")); } } + + @Test + @Order(13) + void test_listByMentionedUser_returnsTaskFromCommentMention() { + Task mentioning = createTestTask(adminClient); + Task unrelated = createTestTask(adminClient); + + try { + adminClient + .tasks() + .addComment( + mentioning.getId().toString(), + String.format("Please review <#E::user::%s>", shared.USER2.getName())); + adminClient.tasks().addComment(unrelated.getId().toString(), "No mention here"); + + ListParams params = + new ListParams().addFilter("mentionedUser", shared.USER2.getName()).setLimit(1000); + List mentioned = adminClient.tasks().list(params).getData(); + List mentionedIds = mentioned.stream().map(Task::getId).toList(); + + assertTrue( + mentionedIds.contains(mentioning.getId()), + "mentionedUser filter must return the task whose comment mentions the user"); + assertFalse( + mentionedIds.contains(unrelated.getId()), + "mentionedUser filter must exclude tasks that do not mention the user"); + + // The UI sends the FQN, which is quoted for a dotted username, but a bare + // name has to resolve to the same mention rows or the filter silently + // returns nothing for those users. + ListParams byFqn = + new ListParams() + .addFilter("mentionedUser", shared.USER2.getFullyQualifiedName()) + .setLimit(1000); + assertTrue( + adminClient.tasks().list(byFqn).getData().stream() + .anyMatch(t -> t.getId().equals(mentioning.getId())), + "mentionedUser must match on the quoted FQN as well as the bare name"); + } finally { + adminClient + .tasks() + .delete(mentioning.getId().toString(), java.util.Map.of("hardDelete", "true")); + adminClient + .tasks() + .delete(unrelated.getId().toString(), java.util.Map.of("hardDelete", "true")); + } + } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java index 8bfd797e9302..77acbd80351b 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java @@ -283,13 +283,19 @@ private String getMentionedUserCondition() { if (mentionedUser == null) { return ""; } - queryParams.put("mentionedUserParam", mentionedUser); + // TaskRepository.storeMentions writes the task id into toFQN and the mentioned + // user into fromFQNHash (via @BindFQN). field_relationship has no toId column, so + // selecting one made every mentionedUser query fail with an SQLSyntaxErrorException. + // hashUserName quotes first, so a dotted name matches whether the caller sends the + // quoted FQN ("john.doe") or the bare name (john.doe) — bare would otherwise hash + // as three FQN segments and match nothing. + queryParams.put("mentionedUserHash", hashUserName(mentionedUser)); return String.format( - "(id IN (SELECT fr.toId FROM field_relationship fr " - + "WHERE fr.fromFQN = :mentionedUserParam " - + "AND fr.toType = 'task' " + "(id IN (SELECT fr.toFQN FROM field_relationship fr " + + "WHERE fr.fromFQNHash = :mentionedUserHash " + + "AND fr.toType = '%s' " + "AND fr.relation = %d))", - Relationship.MENTIONED_IN.ordinal()); + Entity.TASK, Relationship.MENTIONED_IN.ordinal()); } /** diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeedTabBadge.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeedTabBadge.spec.ts index 39285976bd42..c1988031d633 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeedTabBadge.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeedTabBadge.spec.ts @@ -75,7 +75,19 @@ async function switchToOpenFilter(page: import('@playwright/test').Page) { await page.getByTestId('open-tasks').click(); await tasksListResponse; } -test.describe('ActivityFeedTab — task filter badge and placeholder', () => { +const waitForMentionedTaskResponse = (page: import('@playwright/test').Page) => + page.waitForResponse((response) => { + if ( + response.request().method() !== 'GET' || + !response.url().includes('/api/v1/tasks') + ) { + return false; + } + + return Boolean(new URL(response.url()).searchParams.get('mentionedUser')); + }); + +test.describe('ActivityFeedTab — task filter badge, placeholder and mentions', () => { const table = new TableClass(); const assigneeUser = new UserClass(); @@ -229,4 +241,75 @@ test.describe('ActivityFeedTab — task filter badge and placeholder', () => { await afterAction(); } }); + + test('Mentions sub-tab lists only the tasks the user is mentioned in', async ({ + browser, + }) => { + const { page, apiContext, afterAction } = await performAdminLogin(browser, { + navigate: true, + }); + + // Own table: the assertions below are exact card counts and chromium runs + // fullyParallel, so sharing the describe-level table would make them depend + // on test order. + const mentionTable = new TableClass(); + + try { + await mentionTable.create(apiContext); + + const fqn = mentionTable.entityResponseData?.fullyQualifiedName as string; + const assignee = assigneeUser.responseData.name; + + await createOpenTask(apiContext, fqn, assignee); + const mentionedTask = await createOpenTask(apiContext, fqn, assignee); + + // A mention relationship is only written from the comment path, so the + // comment is what makes this task match ?mentionedUser=admin. + const commentResponse = await apiContext.post( + `/api/v1/tasks/${mentionedTask.id}/comments`, + { data: { message: 'Please take a look <#E::user::admin>' } } + ); + expect(commentResponse.ok()).toBe(true); + + await mentionTable.visitEntityPage(page); + await navigateToTasksPanel(page); + + // My Tasks lists every task about the entity. + await expect(page.getByTestId('task-feed-card')).toHaveCount(2); + + const mentionsResponse = waitForMentionedTaskResponse(page); + await page.getByTestId('mentions-toggle').click(); + await mentionsResponse; + + // The list has to actually switch — it used to keep rendering My Tasks + // because the mentions fetch wrote to the conversation feed state instead. + await expect(page.getByTestId('task-feed-card')).toHaveCount(1); + await expect( + page.getByTestId('task-feed-card').getByTestId('entity-link') + ).toBeVisible(); + await expect( + page.getByTestId('no-data-placeholder-container') + ).toHaveCount(0); + + // Switching back restores the full list — guards the paging-cursor reset. + const myTasksResponse = waitForTaskListResponse(page); + await page.getByTestId('my-tasks-toggle').click(); + await myTasksResponse; + + await expect(page.getByTestId('task-feed-card')).toHaveCount(2); + + // Landing on the mentions URL directly used to show the empty placeholder. + const mentionsAgain = waitForMentionedTaskResponse(page); + await page.getByTestId('mentions-toggle').click(); + await mentionsAgain; + + await page.reload(); + await waitForPageLoaded(page); + + await expect(page.getByTestId('task-feed-card')).toHaveCount(1); + } finally { + await mentionTable.delete(apiContext); + await afterAction(); + } + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Tasks/ActivityFeed.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Tasks/ActivityFeed.spec.ts index f54a27c66c9d..a66ba4022d14 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Tasks/ActivityFeed.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Tasks/ActivityFeed.spec.ts @@ -493,28 +493,29 @@ test.describe('Activity Feed - Entity Page', () => { await openResponse; await waitForPageLoaded(page); - await taskFilterButton.click(); - await expect( - page.locator('.task-filter-container').getByText(/mention/i) - ).toBeVisible(); + await expect(page.getByTestId('mentions-toggle')).toBeVisible(); + // Mentions renders the task list, so it has to query tasks-where-mentioned + // about this entity — not the conversation feed, whose results the mentions + // list never reads. const mentionsResponse = page.waitForResponse((response) => { if ( response.request().method() !== 'GET' || - !response.url().includes('/api/v1/feed') + !response.url().includes('/api/v1/tasks') ) { return false; } const requestUrl = new URL(response.url()); - return requestUrl.searchParams.get('filterType') === 'MENTIONS'; + return ( + Boolean(requestUrl.searchParams.get('mentionedUser')) && + requestUrl.searchParams.get('aboutEntity') === + table.entityResponseData?.fullyQualifiedName + ); }); - await page - .locator('.task-filter-container') - .getByText(/mention/i) - .click(); + await page.getByTestId('mentions-toggle').click(); await mentionsResponse; await waitForPageLoaded(page); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/TaskListV1.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/TaskListV1.component.tsx index 37614f1f12fc..290364c4a066 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/TaskListV1.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/TaskListV1.component.tsx @@ -54,12 +54,17 @@ const TaskListV1 = ({ }, [taskList, selectedTask, onTaskClick]); useEffect(() => { + // While a fetch is in flight the list is intentionally empty; collapsing the + // right panel here would flash the layout on every sub-tab/filter switch. + if (isLoading) { + return; + } if (isEmpty(taskList) && handlePanelResize) { handlePanelResize?.(true); } else { handlePanelResize?.(false); } - }, [taskList]); + }, [taskList, isLoading]); const tasks = useMemo( () => diff --git a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx index f8ed07026798..62315c0496c7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx @@ -32,6 +32,7 @@ import { DummyEntityActivityFeedComponent, DummyFollowingActivityComponent, DummySetActiveActivityComponent, + DummyTaskListStateComponent, } from '../../../mocks/ActivityFeedProvider.mock'; import { mockUserData } from '../../../mocks/MyDataPage.mock'; import { @@ -331,6 +332,103 @@ describe('ActivityFeedProvider', () => { ); }); + describe('a first-page task fetch replaces the previous result set', () => { + const renderTaskListState = () => + render( + + + + ); + + it('clears the rows and the paging cursor before the new response lands', async () => { + (listTasks as jest.Mock).mockResolvedValueOnce({ + data: [{ id: 'task-open', createdAt: 1 }], + paging: { after: 'cursor-1' }, + }); + + renderTaskListState(); + + await act(async () => { + fireEvent.click(screen.getByTestId('fetch-open')); + }); + + expect(screen.getByTestId('task-ids')).toHaveTextContent('task-open'); + expect(screen.getByTestId('paging-after')).toHaveTextContent('cursor-1'); + + let resolveClosed: (value: unknown) => void = () => undefined; + (listTasks as jest.Mock).mockReturnValueOnce( + new Promise((resolve) => { + resolveClosed = resolve; + }) + ); + + await act(async () => { + fireEvent.click(screen.getByTestId('fetch-closed')); + }); + + // Leaving the open rows and `cursor-1` in place is what kept the previous + // list on screen and let infinite scroll append the new query's next page + // onto it using the old cursor. + expect(screen.getByTestId('task-ids')).toBeEmptyDOMElement(); + expect(screen.getByTestId('paging-after')).toHaveTextContent('none'); + + await act(async () => { + resolveClosed({ + data: [{ id: 'task-closed', createdAt: 2 }], + paging: { after: 'cursor-2' }, + }); + }); + + expect(screen.getByTestId('task-ids')).toHaveTextContent('task-closed'); + expect(screen.getByTestId('paging-after')).toHaveTextContent('cursor-2'); + }); + + it('ignores a response that resolves after a newer request started', async () => { + let resolveFirst: (value: unknown) => void = () => undefined; + let resolveSecond: (value: unknown) => void = () => undefined; + + (listTasks as jest.Mock) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveFirst = resolve; + }) + ) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveSecond = resolve; + }) + ); + + renderTaskListState(); + + await act(async () => { + fireEvent.click(screen.getByTestId('fetch-open')); + }); + await act(async () => { + fireEvent.click(screen.getByTestId('fetch-closed')); + }); + + await act(async () => { + resolveSecond({ + data: [{ id: 'task-closed', createdAt: 2 }], + paging: { after: 'cursor-2' }, + }); + }); + await act(async () => { + resolveFirst({ + data: [{ id: 'task-open', createdAt: 1 }], + paging: { after: 'cursor-1' }, + }); + }); + + await waitFor(() => + expect(screen.getByTestId('task-ids')).toHaveTextContent('task-closed') + ); + + expect(screen.getByTestId('paging-after')).toHaveTextContent('cursor-2'); + }); + }); + it('should call postFeed with button click', async () => { render( diff --git a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.tsx index 0a8e10a3fd26..0c3613c2502c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.tsx @@ -105,6 +105,10 @@ const ActivityFeedProvider = ({ children, user }: Props) => { // overwrite newer results, so each request claims a sequence number and only the // latest one is allowed to commit. const activityRequestSeq = useRef(0); + // getTaskData and getFeedData write the same `loading` and `entityPaging`, so a + // single sequence guards both: an older request must not commit its rows, its + // paging cursor, or clear the loader after a newer one has started. + const listRequestSeq = useRef(0); // For regular feeds (conversations, announcements) const [entityThread, setEntityThread] = useState([]); const [selectedThread, setSelectedThread] = useState(); @@ -221,8 +225,17 @@ const ActivityFeedProvider = ({ children, user }: Props) => { taskStatusGroup?: TaskStatusGroup, limit?: number ) => { + const requestId = ++listRequestSeq.current; try { setLoading(true); + if (!after) { + // A first-page fetch replaces the result set. Dropping the rows and the + // paging cursor now is what stops the previous query's list staying on + // screen, and stops the infinite-scroll effect appending this query's + // next page onto it using the old cursor. + setTasks([]); + setEntityPaging({} as Paging); + } const feedFilterType = filterType ?? FeedFilter.ALL; const domain = activeDomain !== DEFAULT_DOMAIN_VALUE ? activeDomain : undefined; @@ -323,11 +336,19 @@ const ActivityFeedProvider = ({ children, user }: Props) => { }); } + if (listRequestSeq.current !== requestId) { + return; + } + const sortedTasks = orderBy(taskResponse.data, ['createdAt'], ['desc']); setTasks((prev) => (after ? [...prev, ...sortedTasks] : sortedTasks)); setEntityPaging(taskResponse.paging); } catch (err) { + if (listRequestSeq.current !== requestId) { + return; + } + showErrorToast( err as AxiosError, t('server.entity-fetch-error', { @@ -335,7 +356,9 @@ const ActivityFeedProvider = ({ children, user }: Props) => { }) ); } finally { - setLoading(false); + if (listRequestSeq.current === requestId) { + setLoading(false); + } } }, [currentUser, activeDomain] @@ -351,8 +374,13 @@ const ActivityFeedProvider = ({ children, user }: Props) => { taskStatusGroup?: TaskStatusGroup, limit?: number ) => { + const requestId = ++listRequestSeq.current; try { setLoading(true); + if (!after) { + setEntityThread([]); + setEntityPaging({} as Paging); + } const feedFilterType = filterType ?? FeedFilter.ALL; let userId = undefined; @@ -373,9 +401,17 @@ const ActivityFeedProvider = ({ children, user }: Props) => { userId, limit ); + if (listRequestSeq.current !== requestId) { + return; + } + setEntityThread((prev) => (after ? [...prev, ...data] : [...data])); setEntityPaging(paging); } catch (err) { + if (listRequestSeq.current !== requestId) { + return; + } + showErrorToast( err as AxiosError, t('server.entity-fetch-error', { @@ -383,10 +419,12 @@ const ActivityFeedProvider = ({ children, user }: Props) => { }) ); } finally { - setLoading(false); + if (listRequestSeq.current === requestId) { + setLoading(false); + } } }, - [currentUser, user, getTaskData] + [currentUser, user] ); // Here value is the post message and id can be thread id or post id. diff --git a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.test.tsx index 50c9bed220fd..aeeaba198005 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.test.tsx @@ -32,6 +32,10 @@ const mockFetchUserActivity = jest.fn(); let mockActivityEvents: { id: string; timestamp: number }[] = []; let mockConversationCount = 0; let mockActivityCount = 0; +let mockLoading = false; +let mockTasks: { id: string }[] = []; +let mockEntityPaging: { after?: string } = {}; +let mockIsInView = false; jest.mock('../../../hooks/useApplicationStore', () => ({ useApplicationStore: () => ({ @@ -57,7 +61,7 @@ jest.mock('../../../utils/useRequiredParams', () => ({ })); jest.mock('../../../hooks/useElementInView', () => ({ - useElementInView: () => [{ current: null }, false], + useElementInView: () => [{ current: null }, mockIsInView], })); jest.mock('../ActivityFeedProvider/ActivityFeedProvider', () => ({ @@ -67,9 +71,9 @@ jest.mock('../ActivityFeedProvider/ActivityFeedProvider', () => ({ entityThread: [], getFeedData: mockGetFeedData, getTaskData: mockGetTaskData, - loading: false, - entityPaging: {}, - tasks: [], + loading: mockLoading, + entityPaging: mockEntityPaging, + tasks: mockTasks, selectedTask: null, setActiveTask: jest.fn(), activityEvents: mockActivityEvents, @@ -134,8 +138,11 @@ jest.mock('../ActivityFeedList/ActivityFeedListV1New.component', () => jest.mock('../ActivityFeedList/TaskListV1.component', () => jest .fn() - .mockImplementation(({ emptyPlaceholderText }) => ( -
{emptyPlaceholderText}
+ .mockImplementation(({ emptyPlaceholderText, isLoading, onAfterClose }) => ( +
+
)) ); @@ -194,6 +201,10 @@ describe('ActivityFeedTab', () => { mockActivityEvents = []; mockConversationCount = 0; mockActivityCount = 0; + mockLoading = false; + mockTasks = []; + mockEntityPaging = {}; + mockIsInView = false; mockGetTaskCounts.mockResolvedValue({ open: 0, inProgress: 0, @@ -268,30 +279,137 @@ describe('ActivityFeedTab', () => { }); }); - describe('Bug 1 — feedFilter uses ActivityFeedTabs.MENTIONS enum', () => { - it('calls getFeedData with FeedFilter.MENTIONS when mentions tab is active', async () => { + describe('Mentions sub-tab fetches tasks the user is mentioned in', () => { + it('calls getTaskData with FeedFilter.MENTIONS and never getFeedData', async () => { renderComponent(ActivityFeedTabs.MENTIONS); - await waitFor(() => { - const calls = mockGetFeedData.mock.calls; - const mentionsCall = calls.find( - ([feedFilter]) => feedFilter === FeedFilter.MENTIONS - ); + await waitFor(() => + expect(mockGetTaskData).toHaveBeenCalledWith( + FeedFilter.MENTIONS, + undefined, + EntityType.TABLE, + 'test.db.table', + 'open' + ) + ); - expect(mentionsCall).toBeDefined(); - }); + // The mentions list renders off provider `tasks`, so routing it through + // getFeedData (which writes entityThread) left the previous My Tasks + // list on screen. + expect(mockGetFeedData).not.toHaveBeenCalled(); }); - it('does not call getFeedData with FeedFilter.MENTIONS when tasks tab is active', async () => { + it('renders the task list, not the feed list, on the mentions sub-tab', async () => { + renderComponent(ActivityFeedTabs.MENTIONS); + + await waitFor(() => + expect(screen.getByTestId('task-list')).toBeInTheDocument() + ); + + expect(screen.queryByTestId('feed-list')).not.toBeInTheDocument(); + expect(screen.getByText('message.no-mentions')).toBeInTheDocument(); + }); + + it('does not pass FeedFilter.MENTIONS on the my-tasks sub-tab', async () => { renderComponent(ActivityFeedTabs.TASKS); await waitFor(() => expect(mockGetTaskData).toHaveBeenCalled()); - const mentionsCall = mockGetFeedData.mock.calls.find( - ([feedFilter]) => feedFilter === FeedFilter.MENTIONS + expect( + mockGetTaskData.mock.calls.find( + ([feedFilter]) => feedFilter === FeedFilter.MENTIONS + ) + ).toBeUndefined(); + expect( + mockGetFeedData.mock.calls.find( + ([feedFilter]) => feedFilter === FeedFilter.MENTIONS + ) + ).toBeUndefined(); + }); + }); + + describe('Sub-tab changes show the loader, never a stale list', () => { + it('switches the in-list loader back on for a first-page refetch after paginating', async () => { + mockTasks = [{ id: 'stale-my-task' }]; + // Scrolled to the bottom with a cursor: the one path that legitimately + // turns the in-list loader off. + mockEntityPaging = { after: 'cursor-1' }; + mockIsInView = true; + + renderComponent(ActivityFeedTabs.TASKS); + + await waitFor(() => + expect(mockGetTaskData).toHaveBeenCalledWith( + undefined, + 'cursor-1', + EntityType.TABLE, + 'test.db.table', + 'open' + ) ); - expect(mentionsCall).toBeUndefined(); + await waitFor(() => + expect(screen.getByTestId('task-list')).toHaveAttribute( + 'data-loading', + 'false' + ) + ); + + // onAfterClose refetches the first page, and the provider clears `tasks` + // for it. Once pagination has cleared isFirstLoad, only switching it back + // on keeps the loader up — otherwise the emptied list renders the + // "no tasks" placeholder next to the pagination spinner. + mockLoading = true; + fireEvent.click(screen.getByTestId('task-after-close')); + + await waitFor(() => + expect(screen.getByTestId('task-list')).toHaveAttribute( + 'data-loading', + 'true' + ) + ); + }); + + it('brings the loader back when the sub-tab changes via the URL', async () => { + mockTasks = [{ id: 'stale-my-task' }]; + // A scrolled-to-bottom list with a cursor pages in, which is the one path + // that legitimately turns the in-list loader off. + mockEntityPaging = { after: 'cursor-1' }; + mockIsInView = true; + + const { rerender } = renderComponent(ActivityFeedTabs.TASKS); + + await waitFor(() => + expect(mockGetTaskData).toHaveBeenCalledWith( + undefined, + 'cursor-1', + EntityType.TABLE, + 'test.db.table', + 'open' + ) + ); + + mockLoading = true; + // Entity pages never pass the `subTab` prop — the switch arrives as a URL + // param, which is why keying the loader reset off `subTab` was a no-op and + // left the previous sub-tab's list on screen. + mockUseRequiredParams.mockReturnValue({ + tab: 'activity_feed', + subTab: ActivityFeedTabs.MENTIONS, + }); + + rerender( + + + + ); + + await waitFor(() => + expect(screen.getByTestId('task-list')).toHaveAttribute( + 'data-loading', + 'true' + ) + ); }); }); @@ -373,5 +491,24 @@ describe('ActivityFeedTab', () => { ).toBeInTheDocument(); }); }); + + it('fires exactly one fetch per task filter change', async () => { + renderComponent(ActivityFeedTabs.TASKS); + + await waitFor(() => expect(mockGetTaskData).toHaveBeenCalled()); + + fireEvent.click(screen.getByTestId('user-profile-page-task-filter-icon')); + fireEvent.click(await screen.findByTestId('closed-tasks')); + + // The fetch effect already refires on taskFilter, so the handler calling + // getTaskData itself as well fired two identical requests per click. + await waitFor(() => + expect( + mockGetTaskData.mock.calls.filter( + ([, , , , statusGroup]) => statusGroup === 'closed' + ) + ).toHaveLength(1) + ); + }); }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx index 0f802a50700c..4cbf5176057c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx @@ -181,25 +181,33 @@ export const ActivityFeedTab = ({ () => activeTab === ActivityFeedTabs.MENTIONS, [activeTab] ); - useEffect(() => { - setIsFirstLoad(true); - }, [subTab]); - - const handleTabChange = (subTab: string) => { - setIsFirstLoad(true); - navigate( - entityUtilClassBase.getEntityLink( - entityType, - fqn, - EntityTabs.ACTIVITY_FEED, - subTab - ), - { replace: true } - ); - setActiveThread(); - setActiveTask(); - setIsFullWidth(false); - }; + + // Both sub-tabs of the Tasks pane render TaskListV1 off `tasks`, so both have + // to fetch through getTaskData. Keeping the render branch and the fetch branch + // on a single predicate is what stops them drifting apart again. + const isTaskListTab = useMemo( + () => isTaskActiveTab || isMentionTabSelected, + [isTaskActiveTab, isMentionTabSelected] + ); + + const handleTabChange = useCallback( + (subTab: string) => { + setIsFirstLoad(true); + navigate( + entityUtilClassBase.getEntityLink( + entityType, + fqn, + EntityTabs.ACTIVITY_FEED, + subTab + ), + { replace: true } + ); + setActiveThread(); + setActiveTask(); + setIsFullWidth(false); + }, + [entityType, fqn, navigate, setActiveThread, setActiveTask] + ); const placeholderText = useMemo(() => { if (activeTab === ActivityFeedTabs.ALL) { @@ -352,8 +360,12 @@ export const ActivityFeedTab = ({ const handleFeedFetchFromFeedList = useCallback( (after?: string) => { - setIsFirstLoad(false); - if (isTaskActiveTab) { + // Only a "load more" page keeps the current list on screen. A first-page + // refetch replaces it, so the in-list loader has to be switched back ON — + // once pagination has cleared this flag, `isFirstLoad && loading` is false + // and the cleared list renders the empty placeholder next to the spinner. + setIsFirstLoad(!after); + if (isTaskListTab) { getTaskData(feedFilter, after, entityType, fqn, taskFilter); } else { getFeedData( @@ -367,7 +379,7 @@ export const ActivityFeedTab = ({ } }, [ - isTaskActiveTab, + isTaskListTab, feedFilter, entityType, fqn, @@ -380,7 +392,12 @@ export const ActivityFeedTab = ({ useEffect(() => { if (fqn) { - if (isTaskActiveTab) { + // Every dep here identifies a different query, so this is always a + // first-page fetch that replaces the list — sub-tab (via feedFilter), task + // filter, entity or domain. The loader has to be on for the window where + // the provider has cleared the rows but the response has not landed. + setIsFirstLoad(true); + if (isTaskListTab) { getTaskData(feedFilter, undefined, entityType, fqn, taskFilter); } else { getFeedData( @@ -402,12 +419,12 @@ export const ActivityFeedTab = ({ taskFilter, getFeedData, getTaskData, - isTaskActiveTab, + isTaskListTab, ]); useEffect(() => { // Activity events only render on the ALL tab; skip the fetch on Tasks/Mentions. - if (isTaskActiveTab || isMentionTabSelected) { + if (isTaskListTab) { return; } if (fqn && entityType && !isUserEntity) { @@ -420,8 +437,7 @@ export const ActivityFeedTab = ({ entityType, isUserEntity, userId, - isTaskActiveTab, - isMentionTabSelected, + isTaskListTab, fetchEntityActivity, fetchUserActivity, ]); @@ -461,7 +477,7 @@ export const ActivityFeedTab = ({ const handleFeedClick = useCallback( (feed: Thread) => { - if (!feed && (isTaskActiveTab || isMentionTabSelected)) { + if (!feed && isTaskListTab) { setIsFullWidth(false); } if (selectedThread?.id !== feed?.id) { @@ -471,25 +487,19 @@ export const ActivityFeedTab = ({ setActiveActivity(undefined); } }, - [ - setActiveThread, - setActiveActivity, - isTaskActiveTab, - isMentionTabSelected, - selectedThread, - ] + [setActiveThread, setActiveActivity, isTaskListTab, selectedThread] ); const handleTaskClick = useCallback( (task: Task) => { - if (!task && isTaskActiveTab) { + if (!task && isTaskListTab) { setIsFullWidth(false); } if (selectedTask?.id !== task?.id) { setActiveTask(task); } }, - [setActiveTask, isTaskActiveTab, selectedTask] + [setActiveTask, isTaskListTab, selectedTask] ); const handleActivityClick = useCallback( @@ -513,15 +523,16 @@ export const ActivityFeedTab = ({ [loading] ); - const handleUpdateTaskFilter = (filter: TaskStatusGroup) => { + // The fetch effect above already refires on `taskFilter`; calling getTaskData + // here as well fired two identical requests per filter click. + const handleUpdateTaskFilter = useCallback((filter: TaskStatusGroup) => { setTaskFilter(filter); - getTaskData(feedFilter, undefined, entityType, fqn, filter); - }; + }, []); - const handleAfterTaskClose = () => { + const handleAfterTaskClose = useCallback(() => { handleFeedFetchFromFeedList(); fetchFeedsCount(); - }; + }, [handleFeedFetchFromFeedList, fetchFeedsCount]); const taskFilterOptions = useMemo( () => [ { @@ -618,7 +629,7 @@ export const ActivityFeedTab = ({ options={[ { label: ( - + {t('label.my-task-plural')} @@ -627,7 +638,7 @@ export const ActivityFeedTab = ({ }, { label: ( - + {t('label.mention-plural')} @@ -641,12 +652,12 @@ export const ActivityFeedTab = ({ ); }, [t, handleTabChange]); - const handlePanelResize = (isFullWidth: boolean) => { + const handlePanelResize = useCallback((isFullWidth: boolean) => { setIsFullWidth(isFullWidth); - }; + }, []); const getRightPanelContent = () => { - if ((isTaskActiveTab || isMentionTabSelected) && selectedTask) { + if (isTaskListTab && selectedTask) { return (
{entityType === EntityType.TABLE ? ( @@ -811,7 +822,7 @@ export const ActivityFeedTab = ({ layoutType === ActivityFeedLayoutType.THREE_PANEL, })} id="center-container"> - {(isTaskActiveTab || isMentionTabSelected) && ( + {isTaskListTab && (
)} - {isTaskActiveTab || isMentionTabSelected ? ( + {isTaskListTab ? ( )} {!isFirstLoad && loader} - {!isEmpty( - isTaskActiveTab || isMentionTabSelected ? tasks : entityThread - ) && - !loading && ( -
} - style={{ height: '2px' }} - /> - )} + {!isEmpty(isTaskListTab ? tasks : entityThread) && !loading && ( +
} + style={{ height: '2px' }} + /> + )}
{layoutType === ActivityFeedLayoutType.THREE_PANEL && ( diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/TestCaseIncidentTab/TestCaseIncidentTab.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/TestCaseIncidentTab/TestCaseIncidentTab.component.tsx index d060473a90e2..e8fbb329cb59 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/TestCaseIncidentTab/TestCaseIncidentTab.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/IncidentManager/TestCaseIncidentTab/TestCaseIncidentTab.component.tsx @@ -13,6 +13,7 @@ import { Typography } from 'antd'; import classNames from 'classnames'; +import { isEmpty } from 'lodash'; import { lazy, RefObject, @@ -175,12 +176,12 @@ const TestCaseIncidentTab = () => { - {loader} + {!isEmpty(tasks) && loader}
{ return

{t(CHILDREN_LABEL)}

; }; +/** + * Exposes the task list and the paging cursor so a test can observe what the + * provider holds *between* a filter switch and the response landing. + */ +export const DummyTaskListStateComponent = () => { + const { getTaskData, tasks, entityPaging } = useActivityFeedProvider(); + + const fetchTasks = (statusGroup: TaskStatusGroup) => { + getTaskData( + FeedFilter.OWNER_OR_FOLLOWS, + undefined, + EntityType.TABLE, + 'db.schema.tbl', + statusGroup + ); + }; + + return ( +
+
+ ); +}; + export const DummyChildrenDeletePostComponent = () => { const { t } = useTranslation(); const { deleteFeed } = useActivityFeedProvider();