feat(List Roles): Migrate FAB view to React #32432

EnxDev · 2025-02-28T18:57:43Z

SUMMARY

Migrate List Roles FAB view to React

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

TESTING INSTRUCTIONS

Add in your superset_config.py the following lines:

FAB_ADD_SECURITY_API = True

Run superset init
Go to settings on the frontend and click on List Roles

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai · 2025-02-28T18:57:47Z

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

rusackas · 2025-03-03T18:21:19Z

Thank you! I can't wait to see the last of these disappear :D

michael-s-molina · 2025-03-05T11:25:14Z

Hi @EnxDev. Thanks for working on this! Let me know when it's ready for review as it's still in draft. Regarding this:

FAB_ADD_SECURITY_API = True
FAB_API_MAX_PAGE_SIZE = -1
Run superset init

It would probably require instructions on UPDATING.md.

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Unclear Empty Object Initialization ▹ view	🧠 Not in standard

Files scanned

File Path	Reviewed
superset/views/roles.py	✅
superset-frontend/src/features/roles/types.ts	✅
superset-frontend/src/features/roles/utils.ts	✅
superset-frontend/src/components/ListView/types.ts	✅
superset-frontend/src/dashboard/components/menu/BackgroundStyleDropdown.tsx	✅
superset-frontend/src/components/ListView/Filters/Select.tsx	✅
superset-frontend/src/features/roles/RoleFormItems.tsx	✅
superset-frontend/src/features/roles/RoleListDuplicateModal.tsx	✅
superset-frontend/src/features/roles/RoleListAddModal.tsx	✅
superset-frontend/src/components/ListView/Filters/index.tsx	✅
superset-frontend/src/views/routes.tsx	✅
superset-frontend/src/features/roles/RoleListEditModal.tsx	✅
superset-frontend/src/features/home/SubMenu.tsx	✅
superset/security/api.py	✅
superset/initialization/init.py	✅
superset-frontend/src/pages/RolesList/index.tsx	✅
superset/config.py	✅
superset/security/manager.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

Check out our docs on how you can make Korbit work best for you and your team.

Customize Korbit for your organization through the Korbit Console.

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ [email protected]

superset-frontend/src/features/roles/RoleListDuplicateModal.tsx

+      onSave={onSave}
+      formSubmitHandler={handleFormSubmit}
+      requiredFields={['roleName']}
+      initialValues={{}}


mistercrunch · 2025-03-26T23:55:58Z

Highly recommend setting up pre-commit -> https://superset.apache.org/docs/contributing/development/#git-hooks

….com/apache/superset into enxdev/feat/fab-list-roles-migration

landryb · 2025-04-09T13:08:10Z

fwiw, i've found a small regression from this PR, some of the requests done are invalid URLs according to RFCs, and when superset is behind a java proxy, those trigger 400 codes.

i've locally fixed them this way:

--- i/superset-frontend/src/pages/RolesList/index.tsx
+++ w/superset-frontend/src/pages/RolesList/index.tsx
@@ -114,7 +114,7 @@ function RolesList({ addDangerToast, addSuccessToast, user }: RolesListProps) {
 
       const fetchPage = async (pageIndex: number) => {
         const response = await SupersetClient.get({
-          endpoint: `api/v1/security/permissions-resources/?q={"page_size":${pageSize}, "page":${pageIndex}}`,
+          endpoint: `api/v1/security/permissions-resources/?q=(page_size:${pageSize},page:${pageIndex})`,
         });
 
         return {
@@ -163,7 +163,7 @@ function RolesList({ addDangerToast, addSuccessToast, user }: RolesListProps) {
 
       const fetchPage = async (pageIndex: number) => {
         const response = await SupersetClient.get({
-          endpoint: `api/v1/security/users/?q={"page_size":${pageSize},"page":${pageIndex}}`,
+          endpoint: `api/v1/security/users/?q=(page_size:${pageSize},page:${pageIndex})`,
         });
         return response.json;
       };

can do a PR with that if that is deemed reasonable/acceptable.

with this fixed, the new roles page works fine.

geido · 2025-04-09T14:17:42Z

fwiw, i've found a small regression from this PR, some of the requests done are invalid URLs according to RFCs, and when superset is behind a java proxy, those trigger 400 codes.

i've locally fixed them this way:

--- i/superset-frontend/src/pages/RolesList/index.tsx
+++ w/superset-frontend/src/pages/RolesList/index.tsx
@@ -114,7 +114,7 @@ function RolesList({ addDangerToast, addSuccessToast, user }: RolesListProps) {
 
       const fetchPage = async (pageIndex: number) => {
         const response = await SupersetClient.get({
-          endpoint: `api/v1/security/permissions-resources/?q={"page_size":${pageSize}, "page":${pageIndex}}`,
+          endpoint: `api/v1/security/permissions-resources/?q=(page_size:${pageSize},page:${pageIndex})`,
         });
 
         return {
@@ -163,7 +163,7 @@ function RolesList({ addDangerToast, addSuccessToast, user }: RolesListProps) {
 
       const fetchPage = async (pageIndex: number) => {
         const response = await SupersetClient.get({
-          endpoint: `api/v1/security/users/?q={"page_size":${pageSize},"page":${pageIndex}}`,
+          endpoint: `api/v1/security/users/?q=(page_size:${pageSize},page:${pageIndex})`,
         });
         return response.json;
       };

can do a PR with that if that is deemed reasonable/acceptable.

with this fixed, the new roles page works fine.

@landryb a PR would be awesome! Please, feel free to ping me when you have it.

landryb · 2025-04-09T14:31:33Z

can do a PR with that if that is deemed reasonable/acceptable.
with this fixed, the new roles page works fine.

@landryb a PR would be awesome! Please, feel free to ping me when you have it.

that's #33060

feat(List Roles): Migrate FAB view

cdf0345

EnxDev requested review from geido and michael-s-molina February 28, 2025 18:57

pull-request-size bot added the size/XXL label Feb 28, 2025

EnxDev added the frontend:refactor Related to refactoring the frontend label Feb 28, 2025

EnxDev added 2 commits March 6, 2025 14:46

Merge branch 'master' into enxdev/feat/fab-list-roles-migration

779b45c

clean up

7ad74f1

github-actions bot added the api Related to the REST API label Mar 6, 2025

sadpandajoe added the review:draft label Mar 6, 2025

EnxDev and others added 5 commits March 21, 2025 23:49

Merge branch master into enxdev/feat/fab-list-roles-migration

c96601c

feat(unit tests): add unit tests for RoleList page and modals

e8a0ccb

chore(license): add missing license to utils.ts

2b1ee2e

chore(FAB): Bump FAB to 4.6.1

81ed1a5

chore(ListRoles): Clean up

3c1b0b5

geido marked this pull request as ready for review March 26, 2025 17:52

geido requested review from betodealmeida, eschutho, mistercrunch, rusackas and sadpandajoe as code owners March 26, 2025 17:52

dosubot bot added the change:frontend Requires changing the frontend label Mar 26, 2025

korbit-ai bot reviewed Mar 26, 2025

View reviewed changes

superset-frontend/src/features/roles/RoleListDuplicateModal.tsx

onSave={onSave}

formSubmitHandler={handleFormSubmit}

requiredFields={['roleName']}

initialValues={{}}

This comment was marked as resolved.

Sign in to view

EnxDev added 3 commits March 28, 2025 18:05

fix tests

06169dd

fix tests

cd853a1

Merge branch 'master' into enxdev/feat/fab-list-roles-migration

5d255e7

michael-s-molina removed the review:draft label Mar 31, 2025

EnxDev and others added 3 commits April 1, 2025 00:24

fix py tests

a79066f

Merge branch 'enxdev/feat/fab-list-roles-migration' of https://github…

cdff6e7

….com/apache/superset into enxdev/feat/fab-list-roles-migration

Merge

c7ec2c7

geido force-pushed the enxdev/feat/fab-list-roles-migration branch from c8aa718 to 840562a Compare April 2, 2025 10:22

fix(ListRole): UPDATING.md and CI passes

07d5a77

geido force-pushed the enxdev/feat/fab-list-roles-migration branch from 840562a to 07d5a77 Compare April 2, 2025 10:36

geido approved these changes Apr 2, 2025

View reviewed changes

geido merged commit 4f0020d into master Apr 2, 2025
48 checks passed

geido deleted the enxdev/feat/fab-list-roles-migration branch April 2, 2025 11:06

landryb mentioned this pull request Apr 9, 2025

some superset requests are invalid urls for the proxy/gateway/tomcat georchestra/superset#13

Closed

landryb mentioned this pull request Apr 9, 2025

fix(list roles): dont send invalid querystrings #33060

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(List Roles): Migrate FAB view to React #32432

feat(List Roles): Migrate FAB view to React #32432

Uh oh!

EnxDev commented Feb 28, 2025 •

edited by geido

Loading

Uh oh!

korbit-ai bot commented Feb 28, 2025

Uh oh!

rusackas commented Mar 3, 2025

Uh oh!

michael-s-molina commented Mar 5, 2025

Uh oh!

korbit-ai bot left a comment •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

mistercrunch commented Mar 26, 2025

Uh oh!

Uh oh!

landryb commented Apr 9, 2025

Uh oh!

geido commented Apr 9, 2025

Uh oh!

landryb commented Apr 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

feat(List Roles): Migrate FAB view to React #32432

feat(List Roles): Migrate FAB view to React #32432

Uh oh!

Conversation

EnxDev commented Feb 28, 2025 • edited by geido Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Feb 28, 2025

Uh oh!

rusackas commented Mar 3, 2025

Uh oh!

michael-s-molina commented Mar 5, 2025

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Interacting with Korbit

Customizing Korbit

Feedback and Support

Uh oh!

This comment was marked as resolved.

Uh oh!

mistercrunch commented Mar 26, 2025

Uh oh!

Uh oh!

landryb commented Apr 9, 2025

Uh oh!

geido commented Apr 9, 2025

Uh oh!

landryb commented Apr 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

EnxDev commented Feb 28, 2025 •

edited by geido

Loading

korbit-ai bot left a comment •

edited

Loading