Skip to content

Commit 6f68ccd

Browse files
sadpandajoeclaude
andcommitted
fix(list-view): read SelectOption.title from full option arg
Follow-up to sc-104554. The chart-list Owner filter regression added two unit tests in `Select.test.tsx`, both of which failed in CI: 1. `select filter with ReactNode label uses option title when serializing selection` revealed that `Select.tsx` was reading `selected.title` from the first argument of antd's `onChange`. With `labelInValue`, that argument is the `{label, value}` labeled-value only; the option's `title` lives on the second argument (the full option). So the title-preferring branch never ran and the label collapsed to `String(selected.value)` — the exact bug the fix was supposed to prevent. Read `option?.title` from the second arg instead. 2. Both new tests asserted `toHaveBeenCalledWith({label, value})` but `Filters/index.tsx` invokes the prop as `updateFilterValue(index, option)`. Updated assertions to match the real signature so the test enforces the intent against reality, not against a phantom 1-arg shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 36cf933 commit 6f68ccd

2 files changed

Lines changed: 12 additions & 9 deletions

File tree

superset-frontend/src/components/ListView/Filters/Select.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ test('select filter with ReactNode label uses option title when serializing sele
7474
await selectOption('John Doe', 'Owner');
7575

7676
await waitFor(() => {
77-
expect(mockUpdateFilterValue).toHaveBeenCalledWith({
77+
expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, {
7878
label: 'John Doe',
7979
value: 42,
8080
});
@@ -115,7 +115,7 @@ test('select filter falls back to stringified value when no string label or titl
115115
await selectOption('123', 'Something');
116116

117117
await waitFor(() => {
118-
expect(mockUpdateFilterValue).toHaveBeenCalledWith({
118+
expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, {
119119
label: '123',
120120
value: 123,
121121
});

superset-frontend/src/components/ListView/Filters/Select.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,22 @@ function SelectFilter(
5858
) {
5959
const [selectedOption, setSelectedOption] = useState(initialValue);
6060

61-
const onChange = (selected: SelectOption) => {
61+
const onChange = (selected: SelectOption, option?: SelectOption) => {
62+
// antd's `onChange` (with `labelInValue`) passes the `{label, value}`
63+
// labeled-value as the first arg and the full option (which carries
64+
// `title` and any other fields) as the second. Options may supply a
65+
// ReactNode label (e.g. OwnerSelectLabel for the chart list Owner
66+
// filter). Since this object is serialized into the URL and rehydrated
67+
// as the filter pill on return, we need a plain string. Prefer `title`
68+
// (set by callers to the human-readable name) before falling back to
69+
// the value.
6270
onSelect(
6371
selected
6472
? {
65-
// Options may supply a ReactNode label (e.g. OwnerSelectLabel for
66-
// the chart list Owner filter). Since this object is serialized
67-
// into the URL and rehydrated as the filter pill on return, we
68-
// need a plain string. Prefer `title` (set by callers to the
69-
// human-readable name) before falling back to the value.
7073
label:
7174
typeof selected.label === 'string'
7275
? selected.label
73-
: (selected.title ?? String(selected.value)),
76+
: (option?.title ?? String(selected.value)),
7477
value: selected.value,
7578
}
7679
: undefined,

0 commit comments

Comments
 (0)