qf-jonathan opened a new pull request, #37235: URL: https://github.com/apache/superset/pull/37235
### SUMMARY Fixes a critical bug where editing roles with many users fails due to URL length limits, plus a secondary N+1 query optimization. #### 1. Fix "Request Line is too large" error when editing roles with many users (Primary) **Problem:** When editing a role with hundreds/thousands of users, the request to `/api/v1/security/users/?q=(filters:!((col:id,opr:in,value:!($USER_IDs)))...)` would fail because the URL became too long. This caused the user list to fail loading, and saving changes would inadvertently remove all role memberships. **Solution:** Changed from filtering by user IDs (`col: 'id', opr: 'in', value: user_ids`) to filtering by role relationship (`col: 'roles', opr: 'rel_m_m', value: role_id`). The URL length now remains constant regardless of the number of users. #### 2. Fix N+1 query issue in `/api/v1/security/roles/` endpoint (Secondary) Also I noticed that the Changed SQLAlchemy loading strategy from `joinedload` to `selectinload` for `Role.permissions`, `Role.user`, and `Role.groups`. This eliminates N+1 queries when accessing `[user.id for user in role.user]`, `[perm.id for perm in role.permissions]`, and `[group.id for group in role.groups]` for each role. Added a test to verify the correct filter parameters are sent to the API. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF https://github.com/user-attachments/assets/b598fc0b-4ef7-4829-ae43-319c16f7a29f ### TESTING INSTRUCTIONS 1. Create a role with 2000+ users assigned 2. Navigate to Settings > Security > List Roles and click edit on that role 3. Verify the modal opens and displays users (previously would fail with "Request Line is too large") 4. Make a change and save 5. Verify all users remain assigned (previously would remove all memberships) 6. Check network tab - `/api/v1/security/users/` should use `rel_m_m` filter with short URL ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] 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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
