Skip to content

Commit b105d65

Browse files
committed
Fix server-side role search behavior across role selectors (#27737)
* Fix server-side role search behavior across role selectors * address gitar * fix failing spec (cherry picked from commit c5e360a)
1 parent 521ea63 commit b105d65

20 files changed

Lines changed: 430 additions & 184 deletions

File tree

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SSOConfiguration.spec.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,15 +779,20 @@ test.describe('SSO Configuration Tests', () => {
779779
// Typing filters the visible options
780780
await field.click();
781781
await field.locator('input').fill('Data');
782+
await page.waitForResponse('/api/v1/roles/search?*');
782783
await expect(
783784
dropdown.locator(
784785
'.ant-select-item-option:not(.ant-select-item-option-disabled)'
785786
)
786-
).not.toHaveCount(0);
787+
).not.toHaveCount(0, { timeout: 15000 });
787788

788789
// Pressing Enter on a non-existent value does not create an arbitrary tag
789790
await field.locator('input').clear();
791+
const missingRoleSearchResponse = page.waitForResponse(
792+
'/api/v1/roles/search?*'
793+
);
790794
await field.locator('input').fill('NonExistentRoleXYZ123');
795+
await missingRoleSearchResponse;
791796
await field.locator('input').press('Enter');
792797
await expect(field.locator('.ant-select-selection-item')).toHaveCount(0);
793798
});

openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/AddRoleAndAssignToUser.spec.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
descriptionBox,
1919
generateRandomUsername,
2020
redirectToHomePage,
21-
uuid
21+
uuid,
2222
} from '../../utils/common';
2323
import { settingClick } from '../../utils/sidebar';
2424
import { visitUserProfilePage } from '../../utils/user';
@@ -80,9 +80,9 @@ test.describe.serial('Add role and assign it to the user', () => {
8080
test('Create new user and assign new role to him', async ({ page }) => {
8181
await settingClick(page, GlobalSettingOptions.USERS);
8282

83-
await page.waitForLoadState('networkidle');
84-
83+
const initialRolesResponse = page.waitForResponse('/api/v1/roles/search?*');
8584
await page.click('[data-testid="add-user"]');
85+
await initialRolesResponse;
8686

8787
await page.fill('[data-testid="email"]', user.email);
8888
await page.fill('[data-testid="displayName"]', userDisplayName);
@@ -99,7 +99,9 @@ test.describe.serial('Add role and assign it to the user', () => {
9999
await page.waitForSelector('.ant-select-dropdown', {
100100
state: 'visible',
101101
});
102+
const rolesSearchResponse = page.waitForResponse('/api/v1/roles/search?*');
102103
await page.fill('#roles', roleName);
104+
await rolesSearchResponse;
103105
await page.click(`[title="${roleName}"]`);
104106

105107
await page.keyboard.press('Escape');

openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Domains.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2687,7 +2687,9 @@ test.describe('Domains Rbac', () => {
26872687

26882688
// Add domain role to the user
26892689
await visitUserProfilePage(page, user1.responseData.name);
2690+
const initialRolesResponse = page.waitForResponse('/api/v1/roles/search?*');
26902691
await page.getByTestId('edit-roles-button').click();
2692+
await initialRolesResponse;
26912693

26922694
await page.locator('[data-testid="user-profile-edit-popover"]').isVisible();
26932695
const rolesCombobox = page.locator('input[role="combobox"]').nth(1);

openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/UserDetails.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,11 @@ test.describe('User with different Roles', () => {
503503

504504
await expect(adminPage.getByTestId('user-profile-roles')).toBeVisible();
505505

506+
const initialRolesResponse = adminPage.waitForResponse(
507+
'/api/v1/roles/search?*'
508+
);
506509
await adminPage.getByTestId('edit-roles-button').click();
510+
await initialRolesResponse;
507511

508512
await expect(
509513
adminPage.getByTestId('profile-edit-roles-select')
@@ -513,6 +517,15 @@ test.describe('User with different Roles', () => {
513517
state: 'visible',
514518
});
515519

520+
await adminPage
521+
.getByTestId('profile-edit-roles-select')
522+
.locator('input')
523+
.fill('Application');
524+
await adminPage.waitForResponse('/api/v1/roles/search?*');
525+
await adminPage
526+
.locator('.ant-select-item-option-content')
527+
.getByText('Application bot role', { exact: true })
528+
.waitFor({ state: 'visible' });
516529
await adminPage
517530
.locator('.ant-select-item-option-content')
518531
.getByText('Application bot role', { exact: true })

openmetadata-ui/src/main/resources/ui/playwright/utils/user.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ export const addUser = async (
690690
await waitForAllLoadersToDisappear(page);
691691
await page.click('[data-testid="add-user"]');
692692

693-
await page.waitForResponse('/api/v1/roles?default=false&limit=100&fields=');
693+
await page.waitForResponse('/api/v1/roles/search?*');
694694
await page.fill('[data-testid="email"]', email);
695695

696696
await page.fill('[data-testid="displayName"]', name);
@@ -701,11 +701,14 @@ export const addUser = async (
701701
await page.fill('#password', password);
702702
await page.fill('#confirmPassword', password);
703703

704-
await page.click('[data-testid="roles-dropdown"] > .ant-select-selector');
705-
await page.getByTestId('roles-dropdown').getByRole('combobox').fill(role);
706-
await page.waitForSelector('.ant-select-dropdown:visible', {
707-
state: 'visible',
708-
});
704+
const rolesCombobox = page
705+
.getByTestId('roles-dropdown')
706+
.getByRole('combobox');
707+
await expect(rolesCombobox).toBeVisible({ timeout: 120000 });
708+
await rolesCombobox.click();
709+
const rolesSearchResponse = page.waitForResponse('/api/v1/roles/search?*');
710+
await rolesCombobox.fill(role);
711+
await rolesSearchResponse;
709712
const roleOption = page
710713
.locator('.ant-select-dropdown:visible')
711714
.locator('.ant-select-item-option')

openmetadata-ui/src/main/resources/ui/src/components/Settings/Bot/BotDetails/BotDetails.component.tsx

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@
1414
import { CheckOutlined, CloseOutlined } from '@ant-design/icons';
1515
import { Button, Card, Col, Input, Row, Typography } from 'antd';
1616
import { AxiosError } from 'axios';
17-
import { toLower } from 'lodash';
18-
import { FC, useEffect, useMemo, useState } from 'react';
17+
import { debounce, toLower, uniqBy } from 'lodash';
18+
import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react';
1919
import { useTranslation } from 'react-i18next';
2020
import { ReactComponent as IconBotProfile } from '../../../../assets/svg/bot-profile.svg';
21-
import { PAGE_SIZE_LARGE, TERM_ADMIN } from '../../../../constants/constants';
21+
import { TERM_ADMIN } from '../../../../constants/constants';
2222
import { GlobalSettingOptions } from '../../../../constants/GlobalSettings.constants';
2323
import { useLimitStore } from '../../../../context/LimitsProvider/useLimitsStore';
2424
import { EntityType } from '../../../../enums/entity.enum';
2525
import { Role } from '../../../../generated/entity/teams/role';
26-
import { getRoles } from '../../../../rest/rolesAPIV1';
26+
import { searchRoles } from '../../../../rest/rolesAPIV1';
2727
import { getEntityName } from '../../../../utils/EntityUtils';
2828
import { getSettingPath } from '../../../../utils/RouterUtils';
2929
import { showErrorToast } from '../../../../utils/ToastUtils';
@@ -48,6 +48,8 @@ const BotDetails: FC<BotsDetailProps> = ({
4848
const [isDisplayNameEdit, setIsDisplayNameEdit] = useState(false);
4949
const [selectedRoles, setSelectedRoles] = useState<Array<string>>([]);
5050
const [roles, setRoles] = useState<Array<Role>>([]);
51+
const [isRolesLoading, setIsRolesLoading] = useState(false);
52+
const selectedRolesRef = useRef<string[]>([]);
5153
const { getResourceLimit, config } = useLimitStore();
5254

5355
const [disableFields, setDisableFields] = useState<string[]>(['token']);
@@ -74,21 +76,29 @@ const BotDetails: FC<BotsDetailProps> = ({
7476
}
7577
};
7678

77-
const fetchRoles = async () => {
79+
const fetchRoles = useCallback(async (query = '') => {
80+
setIsRolesLoading(true);
81+
7882
try {
79-
const { data } = await getRoles(
80-
'',
81-
undefined,
82-
undefined,
83-
false,
84-
PAGE_SIZE_LARGE
85-
);
86-
setRoles(data);
83+
const data = await searchRoles(query);
84+
setRoles((prevRoles) => {
85+
const selectedRoleOptions = prevRoles.filter((role) =>
86+
selectedRolesRef.current.includes(role.id)
87+
);
88+
89+
return uniqBy([...selectedRoleOptions, ...data], 'id');
90+
});
8791
} catch (err) {
88-
setRoles([]);
8992
showErrorToast(err as AxiosError);
93+
} finally {
94+
setIsRolesLoading(false);
9095
}
91-
};
96+
}, []);
97+
98+
const debouncedFetchRoles = useMemo(
99+
() => debounce(fetchRoles, 300),
100+
[fetchRoles]
101+
);
92102

93103
const onDisplayNameChange = (e: React.ChangeEvent<HTMLInputElement>) => {
94104
setDisplayName(e.target.value);
@@ -196,7 +206,9 @@ const BotDetails: FC<BotsDetailProps> = ({
196206
</Col>
197207
<Col span={24}>
198208
<RolesCard
209+
isRolesLoading={isRolesLoading}
199210
roles={roles}
211+
searchRolesOptions={debouncedFetchRoles}
200212
selectedRoles={selectedRoles}
201213
setSelectedRoles={(selectedRoles) =>
202214
setSelectedRoles(selectedRoles)
@@ -212,13 +224,36 @@ const BotDetails: FC<BotsDetailProps> = ({
212224
);
213225
};
214226

227+
useEffect(() => {
228+
selectedRolesRef.current = selectedRoles;
229+
}, [selectedRoles]);
230+
215231
useEffect(() => {
216232
fetchRoles();
217233
initLimits();
218234
}, []);
219235

236+
useEffect(() => {
237+
return () => {
238+
debouncedFetchRoles.cancel();
239+
};
240+
}, [debouncedFetchRoles]);
241+
220242
useEffect(() => {
221243
prepareSelectedRoles();
244+
setRoles((prevRoles) =>
245+
uniqBy(
246+
[
247+
...prevRoles,
248+
...((botUserData.roles ?? []).map((role) => ({
249+
id: role.id,
250+
name: role.name ?? '',
251+
displayName: role.displayName,
252+
})) as Role[]),
253+
],
254+
'id'
255+
)
256+
);
222257
}, [botUserData]);
223258

224259
return (

openmetadata-ui/src/main/resources/ui/src/components/Settings/Bot/BotDetails/BotDetails.test.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import { act, render, screen } from '@testing-library/react';
1515
import { MemoryRouter } from 'react-router-dom';
1616
import { OperationPermission } from '../../../../context/PermissionProvider/PermissionProvider.interface';
17+
import { searchRoles } from '../../../../rest/rolesAPIV1';
1718
import { getAuthMechanismForBotUser } from '../../../../rest/userAPI';
1819
import AccessTokenCard from '../../Users/AccessTokenCard/AccessTokenCard.component';
1920
import BotDetails from './BotDetails.component';
@@ -93,6 +94,10 @@ jest.mock('../../../../utils/PermissionsUtils', () => ({
9394
checkPermission: jest.fn().mockReturnValue(true),
9495
}));
9596

97+
jest.mock('../../../../rest/rolesAPIV1', () => ({
98+
searchRoles: jest.fn().mockResolvedValue([]),
99+
}));
100+
96101
const mockGetResourceLimit = jest.fn().mockResolvedValue({
97102
configuredLimit: { disabledFields: [] },
98103
});
@@ -146,6 +151,10 @@ jest.mock('../../../../context/LimitsProvider/useLimitsStore', () => ({
146151
}));
147152

148153
describe('Test BotsDetail Component', () => {
154+
beforeEach(() => {
155+
(searchRoles as jest.Mock).mockResolvedValue([]);
156+
});
157+
149158
it('Should render all child elements', async () => {
150159
await act(async () => {
151160
render(<BotDetails {...mockProp} />, {

0 commit comments

Comments
 (0)