sfirke commented on code in PR #45591:
URL: https://github.com/apache/superset/pull/45591#discussion_r4368237859
##########
superset-frontend/src/features/groups/utils.ts:
##########
@@ -18,10 +18,75 @@
*/
import { t } from '@apache-superset/core/translation';
import { SupersetClient } from '@superset-ui/core';
+import {
+ DEFAULT_SORT_COMPARATOR,
+ type LabeledValue,
+} from '@superset-ui/core/components';
import rison from 'rison';
+import {
+ SUBJECT_DETAIL_PROP,
+ SUBJECT_OPTION_FILTER_PROPS,
+ SUBJECT_TEXT_LABEL_PROP,
+ SubjectSelectLabel,
+} from 'src/features/subjects/SubjectSelectLabel';
import { getUserDisplayLabel } from 'src/features/users/utils';
import { FormValues } from './types';
+type UserRecord = { id: number; email?: string } & Parameters<
+ typeof getUserDisplayLabel
+>[0];
+
+/**
+ * Option props AsyncSelect's client-side re-filter should match against for
+ * user options: the display name, the email and the username. These mirror
+ * the columns the server-side `user_all_text` filter searches, so a user the
+ * server matched is never hidden by the picker.
+ */
+export const USER_OPTION_FILTER_PROPS = [
+ ...SUBJECT_OPTION_FILTER_PROPS,
+ 'username',
+];
+
+/**
+ * Build a select option for a user picker.
+ *
+ * The option shows the email under the display name, like the subject-based
+ * owner pickers do, so that a server-side match on email is visible to the
+ * person searching. Plain-text copies of both lines, plus the username, are
+ * attached under the `USER_OPTION_FILTER_PROPS` keys so AsyncSelect's
+ * client-side re-filter can match them (a React label is otherwise filtered
+ * as an empty string), and the display name under `title` so ListView can
+ * serialize the chosen option's label into the URL. The username is not
+ * rendered, so a username match surfaces the user without explaining why.
+ */
+export const toUserSelectOption = (user: UserRecord) => {
+ const textLabel = getUserDisplayLabel(user);
+ return {
+ value: user.id,
+ label: SubjectSelectLabel({ label: textLabel, secondaryLabel: user.email
}),
Review Comment:
Good catches, both confirmed. For the typed token, antd's token lookup is
keyed by the label plus any `optionFilterProp`, so AsyncSelect now passes
`optionFilterProp="title"`. Because filtering always goes through our
`filterOption`, that prop only affects the token lookup, and it uses the same
`title` convention the paste path already relies on. (The user pickers are the
only AsyncSelect options that carry a `title`.) For the announcement, rc-select
spreads the option's own `aria-*` attributes over the label-derived default, so
`toUserSelectOption` now sets `aria-label` to the display name. I added an
AsyncSelect test that types `Jane Smith,` against React-label options, plus
Role and Group picker tests that options are announced by name rather than id.
Each test fails with its fix reverted. Fixed in ee3e79bf92.
---
🤖 _Drafted by Claude Code, reviewed and approved by @sfirke._
##########
superset-frontend/src/features/groups/utils.ts:
##########
@@ -47,8 +112,13 @@ export const fetchUserOptions = async (
pageSize: number,
addDangerToast: (msg: string) => void,
) => {
- const query = rison.encode({
- filter: filterValue,
+ // The ListView filter path passes the raw input, so trim here to keep both
+ // callers from sending a whitespace-only search.
+ const searchTerm = filterValue.trim();
Review Comment:
Agreed. `fetchUserOptions` now normalizes the term with the same
`stripSurroundingQuotes` helper the client re-filter uses (now exported from
the Select index), so `"Jane Sm"` goes to the server as `Jane Sm`. That helper
also trims, so the whitespace-only guard still holds. Covered by a new
`fetchUserOptions` test. Fixed in ee3e79bf92.
---
🤖 _Drafted by Claude Code, reviewed and approved by @sfirke._
--
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]
