Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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<Task> mentioned = adminClient.tasks().list(params).getData();
List<UUID> 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"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -493,28 +493,29 @@
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);
});
Expand Down Expand Up @@ -646,7 +647,7 @@

expect(patchResponse.ok()).toBe(true);

const page = await browser.newPage();

Check warning on line 650 in openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Tasks/ActivityFeed.spec.ts

View workflow job for this annotation

GitHub Actions / checkstyle

Prefer the `page` fixture (test.use({ storageState })) over browser.newPage() + manual login for single-user admin tests. For multi-user tests that need a second non-admin page, this warning is expected — no action needed
await adminUser.login(page);
await table.visitEntityPage(page);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,17 @@
}, [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]);

Check warning on line 67 in openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/TaskListV1.component.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useEffect has a missing dependency: 'handlePanelResize'. Either include it or remove the dependency array. If 'handlePanelResize' changes too often, find the parent component that defines it and wrap that definition in useCallback

const tasks = useMemo(
() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
DummyEntityActivityFeedComponent,
DummyFollowingActivityComponent,
DummySetActiveActivityComponent,
DummyTaskListStateComponent,
} from '../../../mocks/ActivityFeedProvider.mock';
import { mockUserData } from '../../../mocks/MyDataPage.mock';
import {
Expand Down Expand Up @@ -331,6 +332,103 @@
);
});

describe('a first-page task fetch replaces the previous result set', () => {
const renderTaskListState = () =>
render(
<ActivityFeedProvider>
<DummyTaskListStateComponent />
</ActivityFeedProvider>
);

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({

Check warning on line 376 in openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

This function expects no arguments, but 1 was provided
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({

Check warning on line 412 in openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

This function expects no arguments, but 1 was provided
data: [{ id: 'task-closed', createdAt: 2 }],
paging: { after: 'cursor-2' },
});
});
await act(async () => {
resolveFirst({

Check warning on line 418 in openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

This function expects no arguments, but 1 was provided
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(
<ActivityFeedProvider>
Expand Down Expand Up @@ -547,7 +645,7 @@
});

await act(async () => {
resolveSlowRequest({

Check warning on line 648 in openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

This function expects no arguments, but 1 was provided
data: [{ ...mockActivityEvents[0], summary: 'Stale result' }],
paging: {},
});
Expand Down
Loading
Loading