rebenitez1802 commented on code in PR #42592:
URL: https://github.com/apache/superset/pull/42592#discussion_r3764425266
##########
superset-frontend/src/features/roles/RoleFormItems.tsx:
##########
@@ -60,6 +60,13 @@ export const PermissionsField = ({
placeholder={t('Select permissions')}
options={options}
loading={loading}
+ // fetchPermissionOptions already filters server-side against the raw
+ // permission/view_menu names. AsyncSelect's default client-side
+ // re-filter checks the search term against the rendered label, but
+ // that label has had underscores replaced with spaces
+ // (formatPermissionLabel), so a raw-name search term never matches
+ // it and the correctly-fetched option gets hidden. See #42041.
+ filterOption={false}
Review Comment:
Per @sadpandajoe's Aug-8 review, `filterOption={false}` here has two
edge-case regressions that an underscore-normalizing predicate fixes cleanly
(both verified against the `AsyncSelect` source):
1. **Small installs** (total permission/view-menu rows ≤ the default page
size of 100): once `AsyncSelect` has loaded every option it sets
`allValuesLoaded` and short-circuits the server loader on later searches
(`fetchPage` returns early), so with the client filter disabled a search shows
the *entire* list unnarrowed instead of filtering it.
2. **Edit flow:** the role's existing permissions are injected at the front
of the options and sorted selected-first, and `filterSort` still runs when
`filterOption={false}`, so the first search leaves that whole block pinned
above the actual server matches.
A normalizing predicate restores client-side narrowing while still matching
a raw `stg_silver` search against the displayed `stg silver` label
(`formatPermissionLabel` returns a plain string, so no ReactNode hazard). It
doesn't reintroduce a hidden-match gap: the loader is only skipped when the
full set is already cached, so the predicate then filters a complete list.
```suggestion
// formatPermissionLabel renders the raw permission/view_menu name
with
// underscores replaced by spaces, so AsyncSelect's default
client-side
// re-filter never matches a raw-name search term (e.g. "stg_silver")
// against the displayed label ("stg silver") and hides the
// server-matched option. Normalize both sides so client-side
narrowing
// still works without hiding valid matches. See #42041.
filterOption={(input, option) =>
String(option?.label ?? '')
.toLowerCase()
.replace(/_/g, ' ')
.includes(input.toLowerCase().replace(/_/g, ' '))
}
```
--
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]
