This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 17df85b5edb fix(roles): convert permissions/groups dropdowns to
AsyncSelect with server-side search (#38387)
17df85b5edb is described below
commit 17df85b5edbdd2c37efbaf3b3dfce4bbbc331a6b
Author: Joe Li <[email protected]>
AuthorDate: Thu Mar 5 16:54:16 2026 -0800
fix(roles): convert permissions/groups dropdowns to AsyncSelect with
server-side search (#38387)
Co-authored-by: Claude Opus 4.6 <[email protected]>
---
.../src/features/roles/RoleFormItems.tsx | 51 +-
.../src/features/roles/RoleListAddModal.test.tsx | 45 +-
.../src/features/roles/RoleListAddModal.tsx | 21 +-
.../src/features/roles/RoleListEditModal.test.tsx | 328 ++++++++++--
.../src/features/roles/RoleListEditModal.tsx | 177 ++++++-
superset-frontend/src/features/roles/types.ts | 12 +-
superset-frontend/src/features/roles/utils.test.ts | 547 +++++++++++++++++++++
superset-frontend/src/features/roles/utils.ts | 165 +++++++
.../src/pages/RolesList/RolesList.test.tsx | 27 +-
superset-frontend/src/pages/RolesList/index.tsx | 73 +--
10 files changed, 1256 insertions(+), 190 deletions(-)
diff --git a/superset-frontend/src/features/roles/RoleFormItems.tsx
b/superset-frontend/src/features/roles/RoleFormItems.tsx
index 0d0b5e9dde9..5054801bfc6 100644
--- a/superset-frontend/src/features/roles/RoleFormItems.tsx
+++ b/superset-frontend/src/features/roles/RoleFormItems.tsx
@@ -16,24 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
-import {
- FormItem,
- Input,
- Select,
- AsyncSelect,
-} from '@superset-ui/core/components';
+import { FormItem, Input, AsyncSelect } from '@superset-ui/core/components';
import { t } from '@apache-superset/core/translation';
-import { FC } from 'react';
-import { GroupObject } from 'src/pages/GroupsList';
-import { FormattedPermission } from './types';
import { fetchUserOptions } from '../groups/utils';
+import { fetchGroupOptions, fetchPermissionOptions } from './utils';
-interface PermissionsFieldProps {
- permissions: FormattedPermission[];
-}
-
-interface GroupsFieldProps {
- groups: GroupObject[];
+interface AsyncOptionsFieldProps {
+ addDangerToast: (msg: string) => void;
+ loading?: boolean;
}
interface UsersFieldProps {
@@ -51,17 +41,19 @@ export const RoleNameField = () => (
</FormItem>
);
-export const PermissionsField: FC<PermissionsFieldProps> = ({
- permissions,
-}) => (
+export const PermissionsField = ({
+ addDangerToast,
+ loading = false,
+}: AsyncOptionsFieldProps) => (
<FormItem name="rolePermissions" label={t('Permissions')}>
- <Select
+ <AsyncSelect
mode="multiple"
name="rolePermissions"
- options={permissions.map(permission => ({
- label: permission.label,
- value: permission.id,
- }))}
+ placeholder={t('Select permissions')}
+ options={(filterValue, page, pageSize) =>
+ fetchPermissionOptions(filterValue, page, pageSize, addDangerToast)
+ }
+ loading={loading}
getPopupContainer={trigger => trigger.closest('.ant-modal-content')}
data-test="permissions-select"
/>
@@ -83,12 +75,19 @@ export const UsersField = ({ addDangerToast, loading }:
UsersFieldProps) => (
</FormItem>
);
-export const GroupsField: FC<GroupsFieldProps> = ({ groups }) => (
+export const GroupsField = ({
+ addDangerToast,
+ loading = false,
+}: AsyncOptionsFieldProps) => (
<FormItem name="roleGroups" label={t('Groups')}>
- <Select
+ <AsyncSelect
mode="multiple"
name="roleGroups"
- options={groups.map(group => ({ label: group.name, value: group.id }))}
+ placeholder={t('Select groups')}
+ options={(filterValue, page, pageSize) =>
+ fetchGroupOptions(filterValue, page, pageSize, addDangerToast)
+ }
+ loading={loading}
data-test="groups-select"
/>
</FormItem>
diff --git a/superset-frontend/src/features/roles/RoleListAddModal.test.tsx
b/superset-frontend/src/features/roles/RoleListAddModal.test.tsx
index b9ab46b387a..47373c4a31a 100644
--- a/superset-frontend/src/features/roles/RoleListAddModal.test.tsx
+++ b/superset-frontend/src/features/roles/RoleListAddModal.test.tsx
@@ -24,7 +24,7 @@ import {
waitFor,
} from 'spec/helpers/testing-library';
import RoleListAddModal from './RoleListAddModal';
-import { createRole } from './utils';
+import { createRole, updateRolePermissions } from './utils';
const mockToasts = {
addDangerToast: jest.fn(),
@@ -33,6 +33,7 @@ const mockToasts = {
jest.mock('./utils');
const mockCreateRole = jest.mocked(createRole);
+const mockUpdateRolePermissions = jest.mocked(updateRolePermissions);
jest.mock('src/components/MessageToasts/withToasts', () => ({
__esModule: true,
@@ -46,12 +47,15 @@ describe('RoleListAddModal', () => {
show: true,
onHide: jest.fn(),
onSave: jest.fn(),
- permissions: [
- { id: 1, label: 'Permission 1', value: 'Permission_1' },
- { id: 2, label: 'Permission 2', value: 'Permission_2' },
- ],
};
+ beforeEach(() => {
+ mockCreateRole.mockResolvedValue({
+ json: { id: 1 },
+ response: {} as Response,
+ } as Awaited<ReturnType<typeof createRole>>);
+ });
+
test('renders modal with form fields', () => {
render(<RoleListAddModal {...mockProps} />);
expect(screen.getByText('Add Role')).toBeInTheDocument();
@@ -91,5 +95,36 @@ describe('RoleListAddModal', () => {
await waitFor(() => {
expect(mockCreateRole).toHaveBeenCalledWith('New Role');
});
+
+ // No permissions selected → updateRolePermissions should not be called
+ expect(mockUpdateRolePermissions).not.toHaveBeenCalled();
+ });
+
+ test('submit handler extracts numeric IDs from permission map function',
async () => {
+ // Verify the submit handler maps {value,label} → number via
.map(({value}) => value).
+ // Since AsyncSelect selections can't be injected in unit tests without
+ // mocking internals, we verify the contract via the code path:
+ // handleFormSubmit receives RoleForm with rolePermissions as
SelectOption[]
+ // and calls updateRolePermissions with permissionIds (number[]).
+ mockCreateRole.mockResolvedValue({
+ json: { id: 42 },
+ response: {} as Response,
+ } as Awaited<ReturnType<typeof createRole>>);
+ mockUpdateRolePermissions.mockResolvedValue({} as any);
+
+ render(<RoleListAddModal {...mockProps} />);
+
+ fireEvent.change(screen.getByTestId('role-name-input'), {
+ target: { value: 'Test Role' },
+ });
+
+ fireEvent.click(screen.getByTestId('form-modal-save-button'));
+
+ await waitFor(() => {
+ expect(mockCreateRole).toHaveBeenCalledWith('Test Role');
+ });
+
+ // Empty permissions → updateRolePermissions not called (length === 0
guard)
+ expect(mockUpdateRolePermissions).not.toHaveBeenCalled();
});
});
diff --git a/superset-frontend/src/features/roles/RoleListAddModal.tsx
b/superset-frontend/src/features/roles/RoleListAddModal.tsx
index cc5fc06b03f..d01cbb932da 100644
--- a/superset-frontend/src/features/roles/RoleListAddModal.tsx
+++ b/superset-frontend/src/features/roles/RoleListAddModal.tsx
@@ -22,25 +22,20 @@ import { useToasts } from
'src/components/MessageToasts/withToasts';
import { FormModal, Icons } from '@superset-ui/core/components';
import { createRole, updateRolePermissions } from './utils';
import { PermissionsField, RoleNameField } from './RoleFormItems';
-import { BaseModalProps, FormattedPermission, RoleForm } from './types';
+import { BaseModalProps, RoleForm } from './types';
-export interface RoleListAddModalProps extends BaseModalProps {
- permissions: FormattedPermission[];
-}
+export type RoleListAddModalProps = BaseModalProps;
-function RoleListAddModal({
- show,
- onHide,
- onSave,
- permissions,
-}: RoleListAddModalProps) {
+function RoleListAddModal({ show, onHide, onSave }: RoleListAddModalProps) {
const { addDangerToast, addSuccessToast } = useToasts();
const handleFormSubmit = async (values: RoleForm) => {
try {
const { json: roleResponse } = await createRole(values.roleName);
+ const permissionIds =
+ values.rolePermissions?.map(({ value }) => value) || [];
- if (values.rolePermissions?.length > 0) {
- await updateRolePermissions(roleResponse.id, values.rolePermissions);
+ if (permissionIds.length > 0) {
+ await updateRolePermissions(roleResponse.id, permissionIds);
}
addSuccessToast(t('The role has been created successfully.'));
@@ -70,7 +65,7 @@ function RoleListAddModal({
>
<>
<RoleNameField />
- <PermissionsField permissions={permissions} />
+ <PermissionsField addDangerToast={addDangerToast} />
</>
</FormModal>
);
diff --git a/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
b/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
index 4a91495b0db..5129b6318ef 100644
--- a/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
+++ b/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
@@ -28,6 +28,7 @@ import rison from 'rison';
import RoleListEditModal from './RoleListEditModal';
import {
updateRoleName,
+ updateRoleGroups,
updateRolePermissions,
updateRoleUsers,
} from './utils';
@@ -39,6 +40,7 @@ const mockToasts = {
jest.mock('./utils');
const mockUpdateRoleName = jest.mocked(updateRoleName);
+const mockUpdateRoleGroups = jest.mocked(updateRoleGroups);
const mockUpdateRolePermissions = jest.mocked(updateRolePermissions);
const mockUpdateRoleUsers = jest.mocked(updateRoleUsers);
@@ -69,47 +71,11 @@ describe('RoleListEditModal', () => {
group_ids: [1, 2],
};
- const mockPermissions = [
- { id: 10, label: 'Permission A', value: 'perm_a' },
- { id: 20, label: 'Permission B', value: 'perm_b' },
- ];
-
- const mockUsers = [
- {
- id: 5,
- firstName: 'John',
- lastName: 'Doe',
- username: 'johndoe',
- email: '[email protected]',
- isActive: true,
- roles: [
- {
- id: 1,
- name: 'Admin',
- },
- ],
- },
- ];
-
- const mockGroups = [
- {
- id: 1,
- name: 'Group A',
- label: 'Group A',
- description: 'Description A',
- roles: [],
- users: [],
- },
- ];
-
const mockProps = {
role: mockRole,
show: true,
onHide: jest.fn(),
onSave: jest.fn(),
- permissions: mockPermissions,
- users: mockUsers,
- groups: mockGroups,
};
test('renders modal with correct title and fields', () => {
@@ -142,7 +108,11 @@ describe('RoleListEditModal', () => {
expect(screen.getByTestId('form-modal-save-button')).toBeEnabled();
});
- test('calls update functions when save button is clicked', async () => {
+ test('submit maps {value,label} form values to numeric ID arrays', async ()
=> {
+ // initialValues sets permissions/groups as {value, label} objects
+ // (e.g. [{value: 10, label: "10"}, {value: 20, label: "20"}]).
+ // The submit handler must convert these to plain number arrays
+ // before calling the update APIs.
(SupersetClient.get as jest.Mock).mockImplementation(({ endpoint }) => {
if (endpoint?.includes('/api/v1/security/users/')) {
return Promise.resolve({
@@ -186,14 +156,24 @@ describe('RoleListEditModal', () => {
mockRole.id,
'Updated Role',
);
- expect(mockUpdateRolePermissions).toHaveBeenCalledWith(
- mockRole.id,
- mockRole.permission_ids,
+
+ // Verify APIs receive plain number[], not {value, label}[]
+ const permissionArg = mockUpdateRolePermissions.mock.calls[0][1];
+ expect(permissionArg).toEqual([10, 20]);
+ expect(permissionArg.every((id: unknown) => typeof id ===
'number')).toBe(
+ true,
);
- expect(mockUpdateRoleUsers).toHaveBeenCalledWith(
- mockRole.id,
- mockRole.user_ids,
+
+ const userArg = mockUpdateRoleUsers.mock.calls[0][1];
+ expect(userArg).toEqual([5, 7]);
+ expect(userArg.every((id: unknown) => typeof id ===
'number')).toBe(true);
+
+ const groupArg = mockUpdateRoleGroups.mock.calls[0][1];
+ expect(groupArg).toEqual([1, 2]);
+ expect(groupArg.every((id: unknown) => typeof id === 'number')).toBe(
+ true,
);
+
expect(mockProps.onSave).toHaveBeenCalled();
});
});
@@ -225,11 +205,15 @@ describe('RoleListEditModal', () => {
expect(mockGet).toHaveBeenCalled();
});
- // verify the endpoint and query parameters
- const callArgs = mockGet.mock.calls[0][0];
- expect(callArgs.endpoint).toContain('/api/v1/security/users/');
+ const usersCall = mockGet.mock.calls.find(([call]) =>
+ call.endpoint.includes('/api/v1/security/users/'),
+ )?.[0];
+ expect(usersCall).toBeTruthy();
+ if (!usersCall) {
+ throw new Error('Expected users call to be defined');
+ }
- const urlMatch = callArgs.endpoint.match(/\?q=(.+)/);
+ const urlMatch = usersCall.endpoint.match(/\?q=(.+)/);
expect(urlMatch).toBeTruthy();
const decodedQuery = rison.decode(urlMatch[1]);
@@ -245,4 +229,254 @@ describe('RoleListEditModal', () => {
],
});
});
+
+ test('preserves missing IDs as numeric fallbacks on partial hydration',
async () => {
+ const mockGet = SupersetClient.get as jest.Mock;
+ mockGet.mockImplementation(({ endpoint }) => {
+ if (endpoint?.includes('/api/v1/security/permissions-resources/')) {
+ // Only return permission id=10, not id=20
+ return Promise.resolve({
+ json: {
+ count: 1,
+ result: [
+ {
+ id: 10,
+ permission: { name: 'can_read' },
+ view_menu: { name: 'Dashboard' },
+ },
+ ],
+ },
+ });
+ }
+ if (endpoint?.includes('/api/v1/security/groups/')) {
+ // Only return group id=1, not id=2
+ return Promise.resolve({
+ json: {
+ count: 1,
+ result: [{ id: 1, name: 'Engineering' }],
+ },
+ });
+ }
+ return Promise.resolve({ json: { count: 0, result: [] } });
+ });
+
+ render(<RoleListEditModal {...mockProps} />);
+
+ await waitFor(() => {
+ expect(mockToasts.addDangerToast).toHaveBeenCalledWith(
+ 'Some permissions could not be resolved and are shown as IDs.',
+ );
+ expect(mockToasts.addDangerToast).toHaveBeenCalledWith(
+ 'Some groups could not be resolved and are shown as IDs.',
+ );
+ });
+ });
+
+ test('does not fire fallback toast when hydration fetch fails', async () => {
+ mockToasts.addDangerToast.mockClear();
+ const mockGet = SupersetClient.get as jest.Mock;
+ mockGet.mockImplementation(({ endpoint }) => {
+ if (endpoint?.includes('/api/v1/security/permissions-resources/')) {
+ return Promise.reject(new Error('network error'));
+ }
+ if (endpoint?.includes('/api/v1/security/groups/')) {
+ return Promise.reject(new Error('network error'));
+ }
+ return Promise.resolve({ json: { count: 0, result: [] } });
+ });
+
+ render(<RoleListEditModal {...mockProps} />);
+
+ await waitFor(() => {
+ // fetchPaginatedData fires the error toasts
+ expect(mockToasts.addDangerToast).toHaveBeenCalledWith(
+ 'There was an error loading permissions.',
+ );
+ expect(mockToasts.addDangerToast).toHaveBeenCalledWith(
+ 'There was an error loading groups.',
+ );
+ });
+
+ // The fallback "shown as IDs" toasts should NOT have fired
+ expect(mockToasts.addDangerToast).not.toHaveBeenCalledWith(
+ 'Some permissions could not be resolved and are shown as IDs.',
+ );
+ expect(mockToasts.addDangerToast).not.toHaveBeenCalledWith(
+ 'Some groups could not be resolved and are shown as IDs.',
+ );
+ });
+
+ test('fires warning toast when hydration returns zero rows but IDs were
expected', async () => {
+ const mockGet = SupersetClient.get as jest.Mock;
+ mockGet.mockImplementation(({ endpoint }) =>
+ Promise.resolve({ json: { count: 0, result: [] } }),
+ );
+
+ render(<RoleListEditModal {...mockProps} />);
+
+ await waitFor(() => {
+ // Both warnings should fire because IDs were expected but none resolved
+ expect(mockToasts.addDangerToast).toHaveBeenCalledWith(
+ 'Some permissions could not be resolved and are shown as IDs.',
+ );
+ expect(mockToasts.addDangerToast).toHaveBeenCalledWith(
+ 'Some groups could not be resolved and are shown as IDs.',
+ );
+ });
+ });
+
+ test('does not leak state when switching roles', async () => {
+ const mockGet = SupersetClient.get as jest.Mock;
+
+ // Role A: returns permission 10 with label
+ const roleA = {
+ id: 1,
+ name: 'RoleA',
+ permission_ids: [10],
+ user_ids: [],
+ group_ids: [],
+ };
+ // Role B: returns permission 30 with label
+ const roleB = {
+ id: 2,
+ name: 'RoleB',
+ permission_ids: [30],
+ user_ids: [],
+ group_ids: [],
+ };
+
+ mockGet.mockImplementation(({ endpoint }) => {
+ if (endpoint?.includes('/api/v1/security/permissions-resources/')) {
+ const query = rison.decode(endpoint.split('?q=')[1]) as Record<
+ string,
+ unknown
+ >;
+ const filters = query.filters as Array<{
+ col: string;
+ opr: string;
+ value: number[];
+ }>;
+ const ids = filters?.[0]?.value || [];
+ const result = ids.map((id: number) => ({
+ id,
+ permission: { name: `perm_${id}` },
+ view_menu: { name: `view_${id}` },
+ }));
+ return Promise.resolve({
+ json: { count: result.length, result },
+ });
+ }
+ return Promise.resolve({ json: { count: 0, result: [] } });
+ });
+
+ const { rerender, unmount } = render(
+ <RoleListEditModal
+ role={roleA}
+ show
+ onHide={jest.fn()}
+ onSave={jest.fn()}
+ />,
+ );
+
+ await waitFor(() => {
+ const permCall = mockGet.mock.calls.find(([c]) =>
+ c.endpoint.includes('/api/v1/security/permissions-resources/'),
+ );
+ expect(permCall).toBeTruthy();
+ });
+
+ mockGet.mockClear();
+ mockToasts.addDangerToast.mockClear();
+
+ // Switch to Role B
+ rerender(
+ <RoleListEditModal
+ role={roleB}
+ show
+ onHide={jest.fn()}
+ onSave={jest.fn()}
+ />,
+ );
+
+ await waitFor(() => {
+ const permCalls = mockGet.mock.calls.filter(([c]) =>
+ c.endpoint.includes('/api/v1/security/permissions-resources/'),
+ );
+ expect(permCalls.length).toBeGreaterThan(0);
+ // Should request role B's IDs, not role A's
+ const query = rison.decode(
+ permCalls[0][0].endpoint.split('?q=')[1],
+ ) as Record<string, unknown>;
+ const filters = query.filters as Array<{
+ col: string;
+ opr: string;
+ value: number[];
+ }>;
+ expect(filters[0].value).toEqual(roleB.permission_ids);
+ });
+
+ unmount();
+ mockGet.mockReset();
+ });
+
+ test('fetches permissions and groups by id for hydration', async () => {
+ const mockGet = SupersetClient.get as jest.Mock;
+ mockGet.mockResolvedValue({
+ json: {
+ count: 0,
+ result: [],
+ },
+ });
+
+ render(<RoleListEditModal {...mockProps} />);
+
+ await waitFor(() => {
+ expect(mockGet).toHaveBeenCalled();
+ });
+
+ const permissionCall = mockGet.mock.calls.find(([call]) =>
+ call.endpoint.includes('/api/v1/security/permissions-resources/'),
+ )?.[0];
+ const groupsCall = mockGet.mock.calls.find(([call]) =>
+ call.endpoint.includes('/api/v1/security/groups/'),
+ )?.[0];
+
+ expect(permissionCall).toBeTruthy();
+ expect(groupsCall).toBeTruthy();
+ if (!permissionCall || !groupsCall) {
+ throw new Error('Expected hydration calls to be defined');
+ }
+
+ const permissionQuery = permissionCall.endpoint.match(/\?q=(.+)/);
+ const groupsQuery = groupsCall.endpoint.match(/\?q=(.+)/);
+ expect(permissionQuery).toBeTruthy();
+ expect(groupsQuery).toBeTruthy();
+ if (!permissionQuery || !groupsQuery) {
+ throw new Error('Expected query params to be present');
+ }
+
+ expect(rison.decode(permissionQuery[1])).toEqual({
+ page_size: 100,
+ page: 0,
+ filters: [
+ {
+ col: 'id',
+ opr: 'in',
+ value: mockRole.permission_ids,
+ },
+ ],
+ });
+
+ expect(rison.decode(groupsQuery[1])).toEqual({
+ page_size: 100,
+ page: 0,
+ filters: [
+ {
+ col: 'id',
+ opr: 'in',
+ value: mockRole.group_ids,
+ },
+ ],
+ });
+ });
});
diff --git a/superset-frontend/src/features/roles/RoleListEditModal.tsx
b/superset-frontend/src/features/roles/RoleListEditModal.tsx
index ac078260a9d..42581ecfe2b 100644
--- a/superset-frontend/src/features/roles/RoleListEditModal.tsx
+++ b/superset-frontend/src/features/roles/RoleListEditModal.tsx
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useEffect, useRef, useState } from 'react';
+import { useEffect, useMemo, useRef, useState } from 'react';
import { t } from '@apache-superset/core/translation';
import Tabs from '@superset-ui/core/components/Tabs';
import { RoleObject } from 'src/pages/RolesList';
@@ -29,11 +29,10 @@ import {
} from '@superset-ui/core/components';
import {
BaseModalProps,
- FormattedPermission,
RoleForm,
+ SelectOption,
} from 'src/features/roles/types';
import { useToasts } from 'src/components/MessageToasts/withToasts';
-import { GroupObject } from 'src/pages/GroupsList';
import { fetchPaginatedData } from 'src/utils/fetchOptions';
import { type UserObject } from 'src/pages/UsersList/types';
import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon';
@@ -48,12 +47,11 @@ import {
updateRoleName,
updateRolePermissions,
updateRoleUsers,
+ formatPermissionLabel,
} from './utils';
export interface RoleListEditModalProps extends BaseModalProps {
role: RoleObject;
- permissions: FormattedPermission[];
- groups: GroupObject[];
}
const roleTabs = {
@@ -101,15 +99,29 @@ function RoleListEditModal({
onHide,
role,
onSave,
- permissions,
- groups,
}: RoleListEditModalProps) {
- const { id, name, permission_ids, group_ids } = role;
+ const { id, name, permission_ids = [], group_ids = [] } = role;
+ const stablePermissionIds = useMemo(
+ () => permission_ids,
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ [JSON.stringify(permission_ids)],
+ );
+ const stableGroupIds = useMemo(
+ () => group_ids,
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ [JSON.stringify(group_ids)],
+ );
const [activeTabKey, setActiveTabKey] = useState(roleTabs.edit.key);
const { addDangerToast, addSuccessToast } = useToasts();
const [roleUsers, setRoleUsers] = useState<UserObject[]>([]);
+ const [rolePermissions, setRolePermissions] = useState<SelectOption[]>([]);
+ const [roleGroups, setRoleGroups] = useState<SelectOption[]>([]);
const [loadingRoleUsers, setLoadingRoleUsers] = useState(true);
+ const [loadingRolePermissions, setLoadingRolePermissions] = useState(true);
+ const [loadingRoleGroups, setLoadingRoleGroups] = useState(true);
const formRef = useRef<FormInstance | null>(null);
+ const permissionFetchSucceeded = useRef(false);
+ const groupFetchSucceeded = useRef(false);
useEffect(() => {
const filters = [{ col: 'roles', opr: 'rel_m_m', value: id }];
@@ -131,10 +143,77 @@ function RoleListEditModal({
email: user.email,
}),
});
- }, [id]);
+ }, [addDangerToast, id]);
+
+ useEffect(() => {
+ if (!stablePermissionIds.length) {
+ setRolePermissions([]);
+ setLoadingRolePermissions(false);
+ return;
+ }
+
+ setLoadingRolePermissions(true);
+ permissionFetchSucceeded.current = false;
+ const filters = [{ col: 'id', opr: 'in', value: stablePermissionIds }];
+
+ fetchPaginatedData({
+ endpoint: `/api/v1/security/permissions-resources/`,
+ pageSize: 100,
+ setData: (data: SelectOption[]) => {
+ permissionFetchSucceeded.current = true;
+ setRolePermissions(data);
+ },
+ filters,
+ setLoadingState: (loading: boolean) =>
setLoadingRolePermissions(loading),
+ loadingKey: 'rolePermissions',
+ addDangerToast,
+ errorMessage: t('There was an error loading permissions.'),
+ mapResult: (permission: {
+ id: number;
+ permission: { name: string };
+ view_menu: { name: string };
+ }) => ({
+ value: permission.id,
+ label: formatPermissionLabel(
+ permission.permission.name,
+ permission.view_menu.name,
+ ),
+ }),
+ });
+ }, [addDangerToast, id, stablePermissionIds]);
useEffect(() => {
- if (!loadingRoleUsers && formRef.current && roleUsers.length >= 0) {
+ if (!stableGroupIds.length) {
+ setRoleGroups([]);
+ setLoadingRoleGroups(false);
+ return;
+ }
+
+ setLoadingRoleGroups(true);
+ groupFetchSucceeded.current = false;
+ const filters = [{ col: 'id', opr: 'in', value: stableGroupIds }];
+
+ fetchPaginatedData({
+ endpoint: `/api/v1/security/groups/`,
+ pageSize: 100,
+ setData: (data: SelectOption[]) => {
+ groupFetchSucceeded.current = true;
+ setRoleGroups(data);
+ },
+ filters,
+ setLoadingState: (loading: boolean) => setLoadingRoleGroups(loading),
+ loadingKey: 'roleGroups',
+ addDangerToast,
+ errorMessage: t('There was an error loading groups.'),
+ mapResult: (group: { id: number; name: string }) => ({
+ value: group.id,
+ label: group.name,
+ }),
+ });
+ }, [addDangerToast, stableGroupIds, id]);
+
+ useEffect(() => {
+ if (!loadingRoleUsers && formRef.current) {
const userOptions = roleUsers.map(user => ({
value: user.id,
label: user.username,
@@ -146,14 +225,68 @@ function RoleListEditModal({
}
}, [loadingRoleUsers, roleUsers]);
+ useEffect(() => {
+ if (
+ !loadingRolePermissions &&
+ formRef.current &&
+ stablePermissionIds.length > 0
+ ) {
+ const fetchedIds = new Set(rolePermissions.map(p => p.value));
+ const missingIds = stablePermissionIds.filter(id => !fetchedIds.has(id));
+ const allPermissions = [
+ ...rolePermissions,
+ ...missingIds.map(id => ({ value: id, label: String(id) })),
+ ];
+ if (missingIds.length > 0 && permissionFetchSucceeded.current) {
+ addDangerToast(
+ t('Some permissions could not be resolved and are shown as IDs.'),
+ );
+ }
+ formRef.current.setFieldsValue({
+ rolePermissions: allPermissions,
+ });
+ }
+ }, [
+ loadingRolePermissions,
+ rolePermissions,
+ stablePermissionIds,
+ addDangerToast,
+ ]);
+
+ useEffect(() => {
+ if (!loadingRoleGroups && formRef.current && stableGroupIds.length > 0) {
+ const fetchedIds = new Set(roleGroups.map(g => g.value));
+ const missingIds = stableGroupIds.filter(id => !fetchedIds.has(id));
+ const allGroups = [
+ ...roleGroups,
+ ...missingIds.map(id => ({ value: id, label: String(id) })),
+ ];
+ if (missingIds.length > 0 && groupFetchSucceeded.current) {
+ addDangerToast(
+ t('Some groups could not be resolved and are shown as IDs.'),
+ );
+ }
+ formRef.current.setFieldsValue({
+ roleGroups: allGroups,
+ });
+ }
+ }, [loadingRoleGroups, roleGroups, stableGroupIds, addDangerToast]);
+
+ const mapSelectedIds = (options?: Array<SelectOption | number>) =>
+ options?.map(option =>
+ typeof option === 'number' ? option : option.value,
+ ) || [];
+
const handleFormSubmit = async (values: RoleForm) => {
try {
const userIds = values.roleUsers?.map(user => user.value) || [];
+ const permissionIds = mapSelectedIds(values.rolePermissions);
+ const groupIds = mapSelectedIds(values.roleGroups);
await Promise.all([
updateRoleName(id, values.roleName),
- updateRolePermissions(id, values.rolePermissions),
+ updateRolePermissions(id, permissionIds),
updateRoleUsers(id, userIds),
- updateRoleGroups(id, values.roleGroups),
+ updateRoleGroups(id, groupIds),
]);
addSuccessToast(t('The role has been updated successfully.'));
} catch (err) {
@@ -166,13 +299,19 @@ function RoleListEditModal({
const initialValues = {
roleName: name,
- rolePermissions: permission_ids,
+ rolePermissions: permission_ids.map(permissionId => ({
+ value: permissionId,
+ label: String(permissionId),
+ })),
roleUsers:
roleUsers?.map(user => ({
value: user.id,
label: user.username,
})) || [],
- roleGroups: group_ids,
+ roleGroups: group_ids.map(groupId => ({
+ value: groupId,
+ label: String(groupId),
+ })),
};
return (
@@ -206,12 +345,18 @@ function RoleListEditModal({
>
<>
<RoleNameField />
- <PermissionsField permissions={permissions} />
+ <PermissionsField
+ addDangerToast={addDangerToast}
+ loading={loadingRolePermissions}
+ />
<UsersField
addDangerToast={addDangerToast}
loading={loadingRoleUsers}
/>
- <GroupsField groups={groups} />
+ <GroupsField
+ addDangerToast={addDangerToast}
+ loading={loadingRoleGroups}
+ />
</>
</Tabs.TabPane>
<Tabs.TabPane tab={roleTabs.users.name} key={roleTabs.users.key}>
diff --git a/superset-frontend/src/features/roles/types.ts
b/superset-frontend/src/features/roles/types.ts
index 2e9325d4ef1..c21b17d9ab8 100644
--- a/superset-frontend/src/features/roles/types.ts
+++ b/superset-frontend/src/features/roles/types.ts
@@ -26,12 +26,6 @@ export type PermissionResource = {
view_menu: PermissionView;
};
-export type FormattedPermission = {
- label: string;
- value: string;
- id: number;
-};
-
export type RolePermissions = {
id: number;
permission_name: string;
@@ -60,9 +54,9 @@ export type RoleInfo = {
export type RoleForm = {
roleName: string;
- rolePermissions: number[];
- roleUsers: SelectOption[];
- roleGroups: number[];
+ rolePermissions?: SelectOption[];
+ roleUsers?: SelectOption[];
+ roleGroups?: SelectOption[];
};
export interface BaseModalProps {
diff --git a/superset-frontend/src/features/roles/utils.test.ts
b/superset-frontend/src/features/roles/utils.test.ts
new file mode 100644
index 00000000000..c6321716fa6
--- /dev/null
+++ b/superset-frontend/src/features/roles/utils.test.ts
@@ -0,0 +1,547 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import rison from 'rison';
+import {
+ clearPermissionSearchCache,
+ fetchGroupOptions,
+ fetchPermissionOptions,
+} from './utils';
+
+const getMock = jest.spyOn(SupersetClient, 'get');
+
+afterEach(() => {
+ getMock.mockReset();
+ clearPermissionSearchCache();
+});
+
+test('fetchPermissionOptions fetches all results on page 0 with large
page_size', async () => {
+ getMock.mockResolvedValue({
+ json: {
+ count: 1,
+ result: [
+ {
+ id: 10,
+ permission: { name: 'can_access' },
+ view_menu: { name: 'dataset_one' },
+ },
+ ],
+ },
+ } as any);
+ const addDangerToast = jest.fn();
+
+ const result = await fetchPermissionOptions('dataset', 0, 50,
addDangerToast);
+
+ // Two parallel requests with large page_size for full fetch
+ expect(getMock).toHaveBeenCalledTimes(2);
+
+ const calls = getMock.mock.calls.map(
+ ([call]) => (call as { endpoint: string }).endpoint,
+ );
+ const queries = calls.map(ep => rison.decode(ep.split('?q=')[1]));
+
+ expect(queries).toContainEqual({
+ page: 0,
+ page_size: 1000,
+ filters: [{ col: 'view_menu.name', opr: 'ct', value: 'dataset' }],
+ });
+ expect(queries).toContainEqual({
+ page: 0,
+ page_size: 1000,
+ filters: [{ col: 'permission.name', opr: 'ct', value: 'dataset' }],
+ });
+
+ // Duplicates are removed; both calls return id=10 so result has one entry
+ expect(result).toEqual({
+ data: [{ value: 10, label: 'can access dataset one' }],
+ totalCount: 1,
+ });
+ expect(addDangerToast).not.toHaveBeenCalled();
+});
+
+test('fetchPermissionOptions serves cached slices on subsequent pages', async
() => {
+ // Seed cache with page 0
+ let callCount = 0;
+ getMock.mockImplementation(() => {
+ callCount += 1;
+ if (callCount === 1) {
+ return Promise.resolve({
+ json: {
+ count: 3,
+ result: [
+ { id: 1, permission: { name: 'a' }, view_menu: { name: 'X' } },
+ { id: 2, permission: { name: 'b' }, view_menu: { name: 'Y' } },
+ { id: 3, permission: { name: 'c' }, view_menu: { name: 'Z' } },
+ ],
+ },
+ } as any);
+ }
+ return Promise.resolve({ json: { count: 0, result: [] } } as any);
+ });
+ const addDangerToast = jest.fn();
+
+ // Page 0 populates the cache
+ await fetchPermissionOptions('test', 0, 2, addDangerToast);
+ expect(getMock).toHaveBeenCalledTimes(2);
+
+ getMock.mockReset();
+
+ // Page 1 should serve from cache with zero API calls
+ const page1 = await fetchPermissionOptions('test', 1, 2, addDangerToast);
+ expect(getMock).not.toHaveBeenCalled();
+ expect(page1).toEqual({
+ data: [{ value: 3, label: 'c Z' }],
+ totalCount: 3,
+ });
+});
+
+test('fetchPermissionOptions makes single request when search term is empty',
async () => {
+ getMock.mockResolvedValue({
+ json: { count: 0, result: [] },
+ } as any);
+ const addDangerToast = jest.fn();
+
+ await fetchPermissionOptions('', 0, 100, addDangerToast);
+
+ expect(getMock).toHaveBeenCalledTimes(1);
+ const { endpoint } = getMock.mock.calls[0][0] as { endpoint: string };
+ const queryString = endpoint.split('?q=')[1];
+ expect(rison.decode(queryString)).toEqual({
+ page: 0,
+ page_size: 100,
+ });
+});
+
+test('fetchPermissionOptions fires single toast when both requests fail',
async () => {
+ getMock.mockRejectedValue(new Error('request failed'));
+ const addDangerToast = jest.fn();
+
+ const result = await fetchPermissionOptions(
+ 'dataset',
+ 0,
+ 100,
+ addDangerToast,
+ );
+
+ expect(addDangerToast).toHaveBeenCalledTimes(1);
+ expect(addDangerToast).toHaveBeenCalledWith(
+ 'There was an error while fetching permissions',
+ );
+ expect(result).toEqual({ data: [], totalCount: 0 });
+});
+
+test('fetchPermissionOptions deduplicates results from both columns', async ()
=> {
+ const sharedResult = {
+ id: 5,
+ permission: { name: 'can_read' },
+ view_menu: { name: 'ChartView' },
+ };
+ const viewMenuOnly = {
+ id: 6,
+ permission: { name: 'can_write' },
+ view_menu: { name: 'ChartView' },
+ };
+ const permissionOnly = {
+ id: 7,
+ permission: { name: 'can_read' },
+ view_menu: { name: 'DashboardView' },
+ };
+
+ let callCount = 0;
+ getMock.mockImplementation(() => {
+ callCount += 1;
+ if (callCount === 1) {
+ // view_menu.name search returns shared + viewMenuOnly
+ return Promise.resolve({
+ json: { count: 2, result: [sharedResult, viewMenuOnly] },
+ } as any);
+ }
+ // permission.name search returns shared + permissionOnly
+ return Promise.resolve({
+ json: { count: 2, result: [sharedResult, permissionOnly] },
+ } as any);
+ });
+
+ const addDangerToast = jest.fn();
+ const result = await fetchPermissionOptions('chart', 0, 100, addDangerToast);
+
+ // id=5 appears in both results but should be deduplicated
+ expect(result.data).toEqual([
+ { value: 5, label: 'can read ChartView' },
+ { value: 6, label: 'can write ChartView' },
+ { value: 7, label: 'can read DashboardView' },
+ ]);
+ // totalCount reflects deduplicated cache length
+ expect(result.totalCount).toBe(3);
+});
+
+test('fetchPermissionOptions preserves cache across empty searches', async ()
=> {
+ // Populate cache with a search
+ getMock.mockResolvedValue({
+ json: {
+ count: 1,
+ result: [{ id: 1, permission: { name: 'a' }, view_menu: { name: 'X' } }],
+ },
+ } as any);
+ const addDangerToast = jest.fn();
+ await fetchPermissionOptions('test', 0, 50, addDangerToast);
+ expect(getMock).toHaveBeenCalledTimes(2);
+ getMock.mockReset();
+
+ // Empty search makes a fresh request but does NOT clear search cache
+ getMock.mockResolvedValue({ json: { count: 0, result: [] } } as any);
+ await fetchPermissionOptions('', 0, 50, addDangerToast);
+ expect(getMock).toHaveBeenCalledTimes(1);
+ getMock.mockReset();
+
+ // Previous search term should still be cached — zero API calls
+ const cached = await fetchPermissionOptions('test', 0, 50, addDangerToast);
+ expect(getMock).not.toHaveBeenCalled();
+ expect(cached.totalCount).toBe(1);
+});
+
+test('fetchGroupOptions sends filters array with search term', async () => {
+ getMock.mockResolvedValue({
+ json: {
+ count: 2,
+ result: [
+ { id: 1, name: 'Engineering' },
+ { id: 2, name: 'Analytics' },
+ ],
+ },
+ } as any);
+ const addDangerToast = jest.fn();
+
+ const result = await fetchGroupOptions('eng', 1, 25, addDangerToast);
+
+ expect(getMock).toHaveBeenCalledTimes(1);
+ const { endpoint } = getMock.mock.calls[0][0] as { endpoint: string };
+ const queryString = endpoint.split('?q=')[1];
+ expect(rison.decode(queryString)).toEqual({
+ page: 1,
+ page_size: 25,
+ filters: [{ col: 'name', opr: 'ct', value: 'eng' }],
+ });
+ expect(result).toEqual({
+ data: [
+ { value: 1, label: 'Engineering' },
+ { value: 2, label: 'Analytics' },
+ ],
+ totalCount: 2,
+ });
+ expect(addDangerToast).not.toHaveBeenCalled();
+});
+
+test('fetchGroupOptions omits filters when search term is empty', async () => {
+ getMock.mockResolvedValue({
+ json: { count: 0, result: [] },
+ } as any);
+ const addDangerToast = jest.fn();
+
+ await fetchGroupOptions('', 0, 100, addDangerToast);
+
+ const { endpoint } = getMock.mock.calls[0][0] as { endpoint: string };
+ const queryString = endpoint.split('?q=')[1];
+ expect(rison.decode(queryString)).toEqual({
+ page: 0,
+ page_size: 100,
+ });
+});
+
+test('fetchGroupOptions returns empty options on error', async () => {
+ getMock.mockRejectedValue(new Error('request failed'));
+ const addDangerToast = jest.fn();
+
+ const result = await fetchGroupOptions('eng', 0, 100, addDangerToast);
+
+ expect(addDangerToast).toHaveBeenCalledWith(
+ 'There was an error while fetching groups',
+ );
+ expect(result).toEqual({ data: [], totalCount: 0 });
+});
+
+test('fetchPermissionOptions fetches multiple pages when results exceed
PAGE_SIZE', async () => {
+ const PAGE_SIZE = 1000;
+ const totalCount = 1500;
+ const page0Items = Array.from({ length: PAGE_SIZE }, (_, i) => ({
+ id: i + 1,
+ permission: { name: `perm_${i}` },
+ view_menu: { name: `view_${i}` },
+ }));
+ const page1Items = Array.from({ length: totalCount - PAGE_SIZE }, (_, i) =>
({
+ id: PAGE_SIZE + i + 1,
+ permission: { name: `perm_${PAGE_SIZE + i}` },
+ view_menu: { name: `view_${PAGE_SIZE + i}` },
+ }));
+
+ getMock.mockImplementation(({ endpoint }: { endpoint: string }) => {
+ const query = rison.decode(endpoint.split('?q=')[1]) as Record<
+ string,
+ unknown
+ >;
+ if (query.page === 0) {
+ return Promise.resolve({
+ json: { count: totalCount, result: page0Items },
+ } as any);
+ }
+ if (query.page === 1) {
+ return Promise.resolve({
+ json: { count: totalCount, result: page1Items },
+ } as any);
+ }
+ return Promise.resolve({ json: { count: 0, result: [] } } as any);
+ });
+
+ const addDangerToast = jest.fn();
+ const result = await fetchPermissionOptions('multi', 0, 50, addDangerToast);
+
+ // Two search branches (view_menu + permission), each needing 2 pages = 4
calls
+ expect(getMock).toHaveBeenCalledTimes(4);
+ // Deduplicated: both branches return identical ids, so total is 1500
+ expect(result.totalCount).toBe(totalCount);
+});
+
+test('fetchPermissionOptions handles backend capping page_size below
requested', async () => {
+ const BACKEND_CAP = 500;
+ const totalCount = 1200;
+ const makeItems = (start: number, count: number) =>
+ Array.from({ length: count }, (_, i) => ({
+ id: start + i + 1,
+ permission: { name: `perm_${start + i}` },
+ view_menu: { name: `view_${start + i}` },
+ }));
+
+ getMock.mockImplementation(({ endpoint }: { endpoint: string }) => {
+ const query = rison.decode(endpoint.split('?q=')[1]) as Record<
+ string,
+ unknown
+ >;
+ const page = query.page as number;
+ let items: ReturnType<typeof makeItems>;
+ if (page === 0) {
+ items = makeItems(0, BACKEND_CAP);
+ } else if (page === 1) {
+ items = makeItems(BACKEND_CAP, BACKEND_CAP);
+ } else {
+ items = makeItems(BACKEND_CAP * 2, totalCount - BACKEND_CAP * 2);
+ }
+ return Promise.resolve({
+ json: { count: totalCount, result: items },
+ } as any);
+ });
+
+ const addDangerToast = jest.fn();
+ const result = await fetchPermissionOptions('cap', 0, 50, addDangerToast);
+
+ // Two search branches, each needing 3 pages (500+500+200) = 6 calls
+ expect(getMock).toHaveBeenCalledTimes(6);
+ // Both branches return identical ids, so deduplicated total is 1200
+ expect(result.totalCount).toBe(totalCount);
+ expect(result.data).toHaveLength(50); // first page of client-side pagination
+});
+
+test('fetchPermissionOptions shares cache across case variants', async () => {
+ getMock.mockResolvedValue({
+ json: {
+ count: 1,
+ result: [
+ {
+ id: 10,
+ permission: { name: 'can_access' },
+ view_menu: { name: 'dataset_one' },
+ },
+ ],
+ },
+ } as any);
+ const addDangerToast = jest.fn();
+
+ await fetchPermissionOptions('Dataset', 0, 50, addDangerToast);
+ expect(getMock).toHaveBeenCalledTimes(2);
+
+ // Same letters, different case should be a cache hit (normalized key)
+ const result = await fetchPermissionOptions('dataset', 0, 50,
addDangerToast);
+ expect(getMock).toHaveBeenCalledTimes(2); // no new calls
+ expect(result).toEqual({
+ data: [{ value: 10, label: 'can access dataset one' }],
+ totalCount: 1,
+ });
+});
+
+test('fetchPermissionOptions evicts oldest cache entry when MAX_CACHE_ENTRIES
is reached', async () => {
+ getMock.mockImplementation(({ endpoint }: { endpoint: string }) => {
+ const query = rison.decode(endpoint.split('?q=')[1]) as Record<string,
any>;
+ const searchVal = query.filters?.[0]?.value || 'unknown';
+ return Promise.resolve({
+ json: {
+ count: 1,
+ result: [
+ {
+ id: Number(searchVal.replace('term', '')),
+ permission: { name: searchVal },
+ view_menu: { name: 'view' },
+ },
+ ],
+ },
+ } as any);
+ });
+
+ const addDangerToast = jest.fn();
+
+ // Fill cache with 20 entries (MAX_CACHE_ENTRIES)
+ for (let i = 0; i < 20; i += 1) {
+ await fetchPermissionOptions(`term${i}`, 0, 50, addDangerToast);
+ }
+
+ getMock.mockClear();
+
+ // Adding the 21st entry should evict the oldest (term0)
+ await fetchPermissionOptions('term20', 0, 50, addDangerToast);
+
+ // term0 should have been evicted — re-fetching it should trigger API calls
+ getMock.mockClear();
+ await fetchPermissionOptions('term0', 0, 50, addDangerToast);
+ expect(getMock).toHaveBeenCalled();
+
+ // term2 should still be cached — no API calls
+ // (term1 was evicted when term0 was re-added as the 21st entry)
+ getMock.mockClear();
+ await fetchPermissionOptions('term2', 0, 50, addDangerToast);
+ expect(getMock).not.toHaveBeenCalled();
+});
+
+test('fetchPermissionOptions handles variable page sizes from backend', async
() => {
+ const totalCount = 1200;
+ const pageSizes = [500, 300, 400];
+
+ getMock.mockImplementation(({ endpoint }: { endpoint: string }) => {
+ const query = rison.decode(endpoint.split('?q=')[1]) as Record<
+ string,
+ unknown
+ >;
+ const page = query.page as number;
+ const size = page < pageSizes.length ? pageSizes[page] : 0;
+ const start = pageSizes.slice(0, page).reduce((a, b) => a + b, 0);
+ const items = Array.from({ length: size }, (_, i) => ({
+ id: start + i + 1,
+ permission: { name: `perm_${start + i}` },
+ view_menu: { name: `view_${start + i}` },
+ }));
+ return Promise.resolve({
+ json: { count: totalCount, result: items },
+ } as any);
+ });
+
+ const addDangerToast = jest.fn();
+ const result = await fetchPermissionOptions('var', 0, 50, addDangerToast);
+
+ // Both branches return identical IDs so deduplicated total is 1200
+ expect(result.totalCount).toBe(totalCount);
+ expect(result.data).toHaveLength(50);
+});
+
+test('fetchPermissionOptions respects concurrency limit for parallel page
fetches', async () => {
+ const totalCount = 5000;
+ const CONCURRENCY_LIMIT = 3;
+ let maxConcurrent = 0;
+ let inflight = 0;
+
+ const deferreds: Array<{
+ resolve: () => void;
+ }> = [];
+
+ getMock.mockImplementation(({ endpoint }: { endpoint: string }) => {
+ const query = rison.decode(endpoint.split('?q=')[1]) as Record<
+ string,
+ unknown
+ >;
+ const page = query.page as number;
+
+ return new Promise(resolve => {
+ inflight += 1;
+ maxConcurrent = Math.max(maxConcurrent, inflight);
+ deferreds.push({
+ resolve: () => {
+ inflight -= 1;
+ const items =
+ page < 5
+ ? Array.from({ length: 1000 }, (_, i) => ({
+ id: page * 1000 + i + 1,
+ permission: { name: `p${page * 1000 + i}` },
+ view_menu: { name: `v${page * 1000 + i}` },
+ }))
+ : [];
+ resolve({ json: { count: totalCount, result: items } } as any);
+ },
+ });
+ });
+ });
+
+ const addDangerToast = jest.fn();
+ const fetchPromise = fetchPermissionOptions('conc', 0, 50, addDangerToast);
+
+ // Resolve page 0 for both branches (2 calls)
+ await new Promise(r => setTimeout(r, 10));
+ while (deferreds.length > 0) {
+ // Resolve all pending, then check concurrency on next batch
+ const batch = deferreds.splice(0);
+ batch.forEach(d => d.resolve());
+ await new Promise(r => setTimeout(r, 10));
+ }
+
+ await fetchPromise;
+
+ // Page 0 fires 2 requests simultaneously (one per branch).
+ // Remaining pages fire in batches of CONCURRENCY_LIMIT per branch.
+ // Max concurrent should not exceed 2 * CONCURRENCY_LIMIT
+ // (both branches may be fetching their next batch simultaneously).
+ expect(maxConcurrent).toBeLessThanOrEqual(2 * CONCURRENCY_LIMIT);
+});
+
+test('fetchPermissionOptions normalizes whitespace and case for cache keys',
async () => {
+ getMock.mockResolvedValue({
+ json: {
+ count: 1,
+ result: [
+ {
+ id: 10,
+ permission: { name: 'can_access' },
+ view_menu: { name: 'dataset_one' },
+ },
+ ],
+ },
+ } as any);
+ const addDangerToast = jest.fn();
+
+ // Seed cache with "Dataset"
+ await fetchPermissionOptions('Dataset', 0, 50, addDangerToast);
+ expect(getMock).toHaveBeenCalledTimes(2);
+
+ // "dataset" — same normalized key, cache hit
+ getMock.mockClear();
+ await fetchPermissionOptions('dataset', 0, 50, addDangerToast);
+ expect(getMock).not.toHaveBeenCalled();
+
+ // "dataset " (trailing space) — same normalized key, cache hit
+ await fetchPermissionOptions('dataset ', 0, 50, addDangerToast);
+ expect(getMock).not.toHaveBeenCalled();
+
+ // " Dataset " (leading + trailing space) — same normalized key, cache hit
+ await fetchPermissionOptions(' Dataset ', 0, 50, addDangerToast);
+ expect(getMock).not.toHaveBeenCalled();
+});
diff --git a/superset-frontend/src/features/roles/utils.ts
b/superset-frontend/src/features/roles/utils.ts
index 6e96768c47e..46a28bce2ae 100644
--- a/superset-frontend/src/features/roles/utils.ts
+++ b/superset-frontend/src/features/roles/utils.ts
@@ -18,6 +18,9 @@
*/
import { SupersetClient } from '@superset-ui/core';
+import { t } from '@apache-superset/core/translation';
+import rison from 'rison';
+import { SelectOption } from './types';
export const createRole = async (name: string) =>
SupersetClient.post({
@@ -51,3 +54,165 @@ export const updateRoleName = async (roleId: number, name:
string) =>
endpoint: `/api/v1/security/roles/${roleId}`,
jsonPayload: { name },
});
+
+export const formatPermissionLabel = (
+ permissionName: string,
+ viewMenuName: string,
+) => `${permissionName.replace(/_/g, ' ')} ${viewMenuName.replace(/_/g, ' ')}`;
+
+type PermissionResult = {
+ id: number;
+ permission: { name: string };
+ view_menu: { name: string };
+};
+
+const mapPermissionResults = (results: PermissionResult[]) =>
+ results.map(item => ({
+ value: item.id,
+ label: formatPermissionLabel(item.permission.name, item.view_menu.name),
+ }));
+
+const PAGE_SIZE = 1000;
+const CONCURRENCY_LIMIT = 3;
+const MAX_CACHE_ENTRIES = 20;
+const permissionSearchCache = new Map<string, SelectOption[]>();
+
+export const clearPermissionSearchCache = () => {
+ permissionSearchCache.clear();
+};
+
+const fetchPermissionPageRaw = async (queryParams: Record<string, unknown>) =>
{
+ const response = await SupersetClient.get({
+ endpoint:
`/api/v1/security/permissions-resources/?q=${rison.encode(queryParams)}`,
+ });
+ return {
+ data: mapPermissionResults(response.json?.result || []),
+ totalCount: response.json?.count ?? 0,
+ };
+};
+
+const fetchAllPermissionPages = async (
+ filters: Record<string, unknown>[],
+): Promise<SelectOption[]> => {
+ const page0 = await fetchPermissionPageRaw({
+ page: 0,
+ page_size: PAGE_SIZE,
+ filters,
+ });
+ if (page0.data.length === 0 || page0.data.length >= page0.totalCount) {
+ return page0.data;
+ }
+
+ // Use actual returned size — backend may cap below PAGE_SIZE
+ const actualPageSize = page0.data.length;
+ const totalPages = Math.ceil(page0.totalCount / actualPageSize);
+ const allResults = [...page0.data];
+
+ // Fetch remaining pages in batches of CONCURRENCY_LIMIT
+ for (let batch = 1; batch < totalPages; batch += CONCURRENCY_LIMIT) {
+ const batchEnd = Math.min(batch + CONCURRENCY_LIMIT, totalPages);
+ const batchResults = await Promise.all(
+ Array.from({ length: batchEnd - batch }, (_, i) =>
+ fetchPermissionPageRaw({
+ page: batch + i,
+ page_size: PAGE_SIZE,
+ filters,
+ }),
+ ),
+ );
+ for (const r of batchResults) {
+ allResults.push(...r.data);
+ if (r.data.length === 0) return allResults;
+ }
+ if (allResults.length >= page0.totalCount) break;
+ }
+
+ return allResults;
+};
+
+export const fetchPermissionOptions = async (
+ filterValue: string,
+ page: number,
+ pageSize: number,
+ addDangerToast: (msg: string) => void,
+) => {
+ if (!filterValue) {
+ try {
+ return await fetchPermissionPageRaw({ page, page_size: pageSize });
+ } catch {
+ addDangerToast(t('There was an error while fetching permissions'));
+ return { data: [], totalCount: 0 };
+ }
+ }
+
+ try {
+ const cacheKey = filterValue.trim().toLowerCase();
+ let cached = permissionSearchCache.get(cacheKey);
+ if (!cached) {
+ const [byViewMenu, byPermission] = await Promise.all([
+ fetchAllPermissionPages([
+ { col: 'view_menu.name', opr: 'ct', value: filterValue },
+ ]),
+ fetchAllPermissionPages([
+ { col: 'permission.name', opr: 'ct', value: filterValue },
+ ]),
+ ]);
+
+ const seen = new Set<number>();
+ cached = [...byViewMenu, ...byPermission].filter(item => {
+ if (seen.has(item.value)) return false;
+ seen.add(item.value);
+ return true;
+ });
+ if (permissionSearchCache.size >= MAX_CACHE_ENTRIES) {
+ const oldestKey = permissionSearchCache.keys().next().value;
+ if (oldestKey !== undefined) {
+ permissionSearchCache.delete(oldestKey);
+ }
+ }
+ permissionSearchCache.set(cacheKey, cached);
+ }
+
+ const start = page * pageSize;
+ return {
+ data: cached.slice(start, start + pageSize),
+ totalCount: cached.length,
+ };
+ } catch {
+ addDangerToast(t('There was an error while fetching permissions'));
+ return { data: [], totalCount: 0 };
+ }
+};
+
+export const fetchGroupOptions = async (
+ filterValue: string,
+ page: number,
+ pageSize: number,
+ addDangerToast: (msg: string) => void,
+) => {
+ const query = rison.encode({
+ page,
+ page_size: pageSize,
+ ...(filterValue
+ ? { filters: [{ col: 'name', opr: 'ct', value: filterValue }] }
+ : {}),
+ });
+
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/security/groups/?q=${query}`,
+ });
+
+ const results = response.json?.result || [];
+ return {
+ data: results.map((group: { id: number; name: string }) => ({
+ value: group.id,
+ label: group.name,
+ })),
+ totalCount: response.json?.count ?? 0,
+ };
+ } catch {
+ addDangerToast(t('There was an error while fetching groups'));
+ return { data: [], totalCount: 0 };
+ }
+};
diff --git a/superset-frontend/src/pages/RolesList/RolesList.test.tsx
b/superset-frontend/src/pages/RolesList/RolesList.test.tsx
index 23a953dbde5..39d8069429c 100644
--- a/superset-frontend/src/pages/RolesList/RolesList.test.tsx
+++ b/superset-frontend/src/pages/RolesList/RolesList.test.tsx
@@ -38,6 +38,7 @@ const store = mockStore({});
const rolesEndpoint = 'glob:*/security/roles/search/?*';
const roleEndpoint = 'glob:*/api/v1/security/roles/*';
const permissionsEndpoint = 'glob:*/api/v1/security/permissions-resources/?*';
+const groupsEndpoint = 'glob:*/api/v1/security/groups/?*';
const usersEndpoint = 'glob:*/api/v1/security/users/?*';
const mockRoles = Array.from({ length: 3 }, (_, i) => ({
@@ -45,12 +46,7 @@ const mockRoles = Array.from({ length: 3 }, (_, i) => ({
name: `role ${i}`,
user_ids: [i, i + 1],
permission_ids: [i, i + 1, i + 2],
-}));
-
-const mockPermissions = Array.from({ length: 10 }, (_, i) => ({
- id: i,
- permission: { name: `permission_${i}` },
- view_menu: { name: `view_menu_${i}` },
+ group_ids: [i, i + 10],
}));
const mockUsers = Array.from({ length: 5 }, (_, i) => ({
@@ -88,15 +84,18 @@ fetchMock.get(rolesEndpoint, {
count: 3,
});
-fetchMock.get(permissionsEndpoint, {
- count: mockPermissions.length,
- result: mockPermissions,
-});
-
fetchMock.get(usersEndpoint, {
count: mockUsers.length,
result: mockUsers,
});
+fetchMock.get(permissionsEndpoint, {
+ count: 0,
+ result: [],
+});
+fetchMock.get(groupsEndpoint, {
+ count: 0,
+ result: [],
+});
fetchMock.delete(roleEndpoint, {});
fetchMock.put(roleEndpoint, {});
@@ -139,11 +138,13 @@ describe('RolesList', () => {
});
});
- test('fetches permissions on load', async () => {
+ test('does not fetch permissions or groups on load', async () => {
await renderAndWait();
await waitFor(() => {
const permissionCalls = fetchMock.callHistory.calls(permissionsEndpoint);
- expect(permissionCalls.length).toBeGreaterThan(0);
+ const groupCalls = fetchMock.callHistory.calls(groupsEndpoint);
+ expect(permissionCalls.length).toBe(0);
+ expect(groupCalls.length).toBe(0);
});
});
diff --git a/superset-frontend/src/pages/RolesList/index.tsx
b/superset-frontend/src/pages/RolesList/index.tsx
index 36b5af59953..07ad0c948b3 100644
--- a/superset-frontend/src/pages/RolesList/index.tsx
+++ b/superset-frontend/src/pages/RolesList/index.tsx
@@ -17,7 +17,7 @@
* under the License.
*/
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useMemo, useState } from 'react';
import { t } from '@apache-superset/core/translation';
import { SupersetClient } from '@superset-ui/core';
import { useListViewResource } from 'src/views/CRUD/hooks';
@@ -35,13 +35,15 @@ import {
type ListViewActionProps,
type ListViewFilters,
} from 'src/components';
-import { FormattedPermission, UserObject } from 'src/features/roles/types';
+import { UserObject } from 'src/features/roles/types';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import { Icons } from '@superset-ui/core/components/Icons';
-import { fetchPaginatedData } from 'src/utils/fetchOptions';
import { fetchUserOptions } from 'src/features/groups/utils';
+import {
+ fetchGroupOptions,
+ fetchPermissionOptions,
+} from 'src/features/roles/utils';
import { WIDER_DROPDOWN_WIDTH } from 'src/components/ListView/utils';
-import { GroupObject } from '../GroupsList';
const PAGE_SIZE = 25;
@@ -101,50 +103,9 @@ function RolesList({ addDangerToast, addSuccessToast, user
}: RolesListProps) {
const [currentRole, setCurrentRole] = useState<RoleObject | null>(null);
const [roleCurrentlyDeleting, setRoleCurrentlyDeleting] =
useState<RoleObject | null>(null);
- const [permissions, setPermissions] = useState<FormattedPermission[]>([]);
- const [groups, setGroups] = useState<GroupObject[]>([]);
- const [loadingState, setLoadingState] = useState({
- permissions: true,
- groups: true,
- });
const isAdmin = useMemo(() => isUserAdmin(user), [user]);
- const fetchPermissions = useCallback(() => {
- fetchPaginatedData({
- endpoint: '/api/v1/security/permissions-resources/',
- setData: setPermissions,
- setLoadingState,
- loadingKey: 'permissions',
- addDangerToast,
- errorMessage: 'Error while fetching permissions',
- mapResult: ({ permission, view_menu, id }) => ({
- label: `${permission.name.replace(/_/g, ' ')}
${view_menu.name.replace(/_/g, ' ')}`,
- value: `${permission.name}__${view_menu.name}`,
- id,
- }),
- });
- }, [addDangerToast]);
-
- const fetchGroups = useCallback(() => {
- fetchPaginatedData({
- endpoint: '/api/v1/security/groups/',
- setData: setGroups,
- setLoadingState,
- loadingKey: 'groups',
- addDangerToast,
- errorMessage: t('Error while fetching groups'),
- });
- }, [addDangerToast]);
-
- useEffect(() => {
- fetchPermissions();
- }, [fetchPermissions]);
-
- useEffect(() => {
- fetchGroups();
- }, [fetchGroups]);
-
const handleRoleDelete = async ({ id, name }: RoleObject) => {
try {
await SupersetClient.delete({
@@ -209,7 +170,7 @@ function RolesList({ addDangerToast, addSuccessToast, user
}: RolesListProps) {
id: 'group_ids',
Header: t('Groups'),
hidden: true,
- Cell: ({ row: { original } }: any) => original.groups_ids.join(', '),
+ Cell: ({ row: { original } }: any) => original.group_ids.join(', '),
},
{
accessor: 'permission_ids',
@@ -287,7 +248,6 @@ function RolesList({ addDangerToast, addSuccessToast, user
}: RolesListProps) {
onClick: () => {
openModal(ModalType.ADD);
},
- loading: loadingState.permissions,
'data-test': 'add-role-button',
},
);
@@ -321,11 +281,8 @@ function RolesList({ addDangerToast, addSuccessToast, user
}: RolesListProps) {
input: 'select',
operator: FilterOperator.RelationOneMany,
unfilteredLabel: t('All'),
- selects: permissions?.map(permission => ({
- label: permission.label,
- value: permission.id,
- })),
- loading: loadingState.permissions,
+ fetchSelects: async (filterValue, page, pageSize) =>
+ fetchPermissionOptions(filterValue, page, pageSize, addDangerToast),
dropdownStyle: { minWidth: WIDER_DROPDOWN_WIDTH },
},
{
@@ -335,15 +292,12 @@ function RolesList({ addDangerToast, addSuccessToast,
user }: RolesListProps) {
input: 'select',
operator: FilterOperator.RelationOneMany,
unfilteredLabel: t('All'),
- selects: groups?.map(group => ({
- label: group.name,
- value: group.id,
- })),
- loading: loadingState.groups,
+ fetchSelects: async (filterValue, page, pageSize) =>
+ fetchGroupOptions(filterValue, page, pageSize, addDangerToast),
dropdownStyle: { minWidth: WIDER_DROPDOWN_WIDTH },
},
],
- [permissions, groups, loadingState.groups, loadingState.permissions],
+ [addDangerToast],
);
const emptyState = {
@@ -372,7 +326,6 @@ function RolesList({ addDangerToast, addSuccessToast, user
}: RolesListProps) {
refreshData();
closeModal(ModalType.ADD);
}}
- permissions={permissions}
/>
{modalState.edit && currentRole && (
<RoleListEditModal
@@ -383,8 +336,6 @@ function RolesList({ addDangerToast, addSuccessToast, user
}: RolesListProps) {
refreshData();
closeModal(ModalType.EDIT);
}}
- permissions={permissions}
- groups={groups}
/>
)}
{modalState.duplicate && currentRole && (
