You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2026/04/17 22:35:40 UTC
(superset) branch fix-role-mgmt-429-414 created (now aaf6c884925)
This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a change to branch fix-role-mgmt-429-414
in repository https://gitbox.apache.org/repos/asf/superset.git
at aaf6c884925 fix(roles): resolve HTTP 429 and 414 errors on role management page
This branch includes the following new commits:
new aaf6c884925 fix(roles): resolve HTTP 429 and 414 errors on role management page
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(roles): resolve HTTP 429 and 414 errors on role management page
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a commit to branch fix-role-mgmt-429-414
in repository https://gitbox.apache.org/repos/asf/superset.git
commit aaf6c884925691610bc31be906eb9187a6772be7
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri Apr 17 22:35:27 2026 +0000
fix(roles): resolve HTTP 429 and 414 errors on role management page
- Replace permissions hydration in edit modal with GET
/api/v1/security/roles/{id}/permissions/ — one request per role,
role ID in path so URL length is constant regardless of permission count
(fixes 414 URI Too Long for roles with many permissions)
- Batch fetchPaginatedData page requests in groups of 5 instead of
firing all pages simultaneously via Promise.all (fixes 429 Too Many
Requests caused by 225 concurrent requests for a 226-page result set)
- Update tests to reflect new permissions endpoint and response shape
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
.../src/features/roles/RoleListEditModal.test.tsx | 99 ++++++++++------------
.../src/features/roles/RoleListEditModal.tsx | 56 ++++++------
superset-frontend/src/utils/fetchOptions.ts | 21 +++--
3 files changed, 84 insertions(+), 92 deletions(-)
diff --git a/superset-frontend/src/features/roles/RoleListEditModal.test.tsx b/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
index 5129b6318ef..31542863260 100644
--- a/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
+++ b/superset-frontend/src/features/roles/RoleListEditModal.test.tsx
@@ -233,16 +233,19 @@ 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/')) {
+ if (
+ endpoint?.includes(
+ `/api/v1/security/roles/${mockRole.id}/permissions/`,
+ )
+ ) {
// 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' },
+ permission_name: 'can_read',
+ view_menu_name: 'Dashboard',
},
],
},
@@ -276,7 +279,11 @@ describe('RoleListEditModal', () => {
mockToasts.addDangerToast.mockClear();
const mockGet = SupersetClient.get as jest.Mock;
mockGet.mockImplementation(({ endpoint }) => {
- if (endpoint?.includes('/api/v1/security/permissions-resources/')) {
+ if (
+ endpoint?.includes(
+ `/api/v1/security/roles/${mockRole.id}/permissions/`,
+ )
+ ) {
return Promise.reject(new Error('network error'));
}
if (endpoint?.includes('/api/v1/security/groups/')) {
@@ -346,24 +353,26 @@ describe('RoleListEditModal', () => {
};
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}` },
- }));
+ if (endpoint?.includes(`/api/v1/security/roles/${roleA.id}/permissions/`)) {
return Promise.resolve({
- json: { count: result.length, result },
+ json: {
+ result: roleA.permission_ids.map(pid => ({
+ id: pid,
+ permission_name: `perm_${pid}`,
+ view_menu_name: `view_${pid}`,
+ })),
+ },
+ });
+ }
+ if (endpoint?.includes(`/api/v1/security/roles/${roleB.id}/permissions/`)) {
+ return Promise.resolve({
+ json: {
+ result: roleB.permission_ids.map(pid => ({
+ id: pid,
+ permission_name: `perm_${pid}`,
+ view_menu_name: `view_${pid}`,
+ })),
+ },
});
}
return Promise.resolve({ json: { count: 0, result: [] } });
@@ -380,7 +389,7 @@ describe('RoleListEditModal', () => {
await waitFor(() => {
const permCall = mockGet.mock.calls.find(([c]) =>
- c.endpoint.includes('/api/v1/security/permissions-resources/'),
+ c.endpoint.includes(`/api/v1/security/roles/${roleA.id}/permissions/`),
);
expect(permCall).toBeTruthy();
});
@@ -400,26 +409,16 @@ describe('RoleListEditModal', () => {
await waitFor(() => {
const permCalls = mockGet.mock.calls.filter(([c]) =>
- c.endpoint.includes('/api/v1/security/permissions-resources/'),
+ c.endpoint.includes(`/api/v1/security/roles/${roleB.id}/permissions/`),
);
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 () => {
+ test('fetches permissions via role endpoint and groups by id for hydration', async () => {
const mockGet = SupersetClient.get as jest.Mock;
mockGet.mockResolvedValue({
json: {
@@ -434,8 +433,11 @@ describe('RoleListEditModal', () => {
expect(mockGet).toHaveBeenCalled();
});
+ // Permissions should be fetched via the role's permissions endpoint (no ID list in URL)
const permissionCall = mockGet.mock.calls.find(([call]) =>
- call.endpoint.includes('/api/v1/security/permissions-resources/'),
+ call.endpoint.includes(
+ `/api/v1/security/roles/${mockRole.id}/permissions/`,
+ ),
)?.[0];
const groupsCall = mockGet.mock.calls.find(([call]) =>
call.endpoint.includes('/api/v1/security/groups/'),
@@ -447,26 +449,17 @@ describe('RoleListEditModal', () => {
throw new Error('Expected hydration calls to be defined');
}
- const permissionQuery = permissionCall.endpoint.match(/\?q=(.+)/);
+ // Permission endpoint has no query params (role ID is in the path)
+ expect(permissionCall.endpoint).toBe(
+ `/api/v1/security/roles/${mockRole.id}/permissions/`,
+ );
+
+ // Groups still use the id-in filter
const groupsQuery = groupsCall.endpoint.match(/\?q=(.+)/);
- expect(permissionQuery).toBeTruthy();
expect(groupsQuery).toBeTruthy();
- if (!permissionQuery || !groupsQuery) {
- throw new Error('Expected query params to be present');
+ if (!groupsQuery) {
+ throw new Error('Expected groups 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,
diff --git a/superset-frontend/src/features/roles/RoleListEditModal.tsx b/superset-frontend/src/features/roles/RoleListEditModal.tsx
index 42581ecfe2b..2dfb4bcdcb2 100644
--- a/superset-frontend/src/features/roles/RoleListEditModal.tsx
+++ b/superset-frontend/src/features/roles/RoleListEditModal.tsx
@@ -33,6 +33,7 @@ import {
SelectOption,
} from 'src/features/roles/types';
import { useToasts } from 'src/components/MessageToasts/withToasts';
+import { SupersetClient } from '@superset-ui/core';
import { fetchPaginatedData } from 'src/utils/fetchOptions';
import { type UserObject } from 'src/pages/UsersList/types';
import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon';
@@ -146,41 +147,34 @@ function RoleListEditModal({
}, [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[]) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/security/roles/${id}/permissions/`,
+ })
+ .then(response => {
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]);
+ setRolePermissions(
+ (
+ response.json?.result as Array<{
+ id: number;
+ permission_name: string;
+ view_menu_name: string;
+ }>
+ ).map(p => ({
+ value: p.id,
+ label: formatPermissionLabel(p.permission_name, p.view_menu_name),
+ })),
+ );
+ })
+ .catch(() => {
+ addDangerToast(t('There was an error loading permissions.'));
+ })
+ .finally(() => {
+ setLoadingRolePermissions(false);
+ });
+ }, [addDangerToast, id]);
useEffect(() => {
if (!stableGroupIds.length) {
diff --git a/superset-frontend/src/utils/fetchOptions.ts b/superset-frontend/src/utils/fetchOptions.ts
index d9ac7b067ed..05faa0b718c 100644
--- a/superset-frontend/src/utils/fetchOptions.ts
+++ b/superset-frontend/src/utils/fetchOptions.ts
@@ -88,16 +88,21 @@ export const fetchPaginatedData = async ({
}
const totalPages = Math.ceil(totalItems / pageSize);
+ const concurrencyLimit = 5;
+ const allResults = [...firstPageResults];
- const requests = Array.from({ length: totalPages - 1 }, (_, i) =>
- fetchPage(i + 1),
- );
- const remainingResults = await Promise.all(requests);
+ for (let batch = 1; batch < totalPages; batch += concurrencyLimit) {
+ const batchEnd = Math.min(batch + concurrencyLimit, totalPages);
+ // eslint-disable-next-line no-await-in-loop
+ const batchResults = await Promise.all(
+ Array.from({ length: batchEnd - batch }, (_, i) =>
+ fetchPage(batch + i),
+ ),
+ );
+ allResults.push(...batchResults.flatMap(res => res.results));
+ }
- setData([
- ...firstPageResults,
- ...remainingResults.flatMap(res => res.results),
- ]);
+ setData(allResults);
} catch (err) {
addDangerToast(t(errorMessage));
} finally {