diff --git a/superset-frontend/src/components/ListView/Filters/Select.test.tsx b/superset-frontend/src/components/ListView/Filters/Select.test.tsx new file mode 100644 index 00000000000..a0bedb32f20 --- /dev/null +++ b/superset-frontend/src/components/ListView/Filters/Select.test.tsx @@ -0,0 +1,267 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { createRef } from 'react'; +import { + render, + screen, + selectOption, + waitFor, +} from 'spec/helpers/testing-library'; +import { ListViewFilterOperator } from '../types'; +import UIFilters from './index'; +import SelectFilter from './Select'; +import type { FilterHandler } from './types'; + +const mockUpdateFilterValue = jest.fn(); + +beforeEach(() => { + mockUpdateFilterValue.mockClear(); +}); + +test('select filter with ReactNode label uses option title when serializing selection', async () => { + // Regression for sc-104554: the chart-list Owner filter renders options + // with ReactNode labels (name + email). The value passed to + // updateFilterValue is serialized into URL / filter state and re-used to + // render the filter pill on return. It must carry the plain-text name + // (from `title`) and not fall back to the numeric user id. + const ReactNodeLabel = ( +
+ John Doe + john@example.com +
+ ); + + const fetchSelects = jest.fn().mockResolvedValue({ + data: [ + { + label: ReactNodeLabel, + value: 42, + title: 'John Doe', + }, + ], + totalCount: 1, + }); + + const filters = [ + { + Header: 'Owner', + key: 'owner', + id: 'owners', + input: 'select' as const, + operator: ListViewFilterOperator.RelationManyMany, + unfilteredLabel: 'All', + fetchSelects, + paginate: true, + }, + ]; + + render( + , + ); + + await selectOption('John Doe', 'Owner'); + + await waitFor(() => { + expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { + label: 'John Doe', + value: 42, + }); + }); +}); + +test('select filter falls back to stringified value when no string label or title is available', async () => { + const fetchSelects = jest.fn().mockResolvedValue({ + data: [ + { + label: 123, + value: 123, + }, + ], + totalCount: 1, + }); + + const filters = [ + { + Header: 'Something', + key: 'something', + id: 'something', + input: 'select' as const, + operator: ListViewFilterOperator.RelationOneMany, + unfilteredLabel: 'All', + fetchSelects, + }, + ]; + + render( + , + ); + + await selectOption('123', 'Something'); + + await waitFor(() => { + expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { + label: '123', + value: 123, + }); + }); +}); + +test('plain select with string label passes label through unchanged', async () => { + // Happy-path coverage for the typeof-string branch in onChange, exercised + // through the non-async Select wrapper (selects array, no fetchSelects). + const filters = [ + { + Header: 'Status', + key: 'status', + id: 'status', + input: 'select' as const, + operator: ListViewFilterOperator.Equals, + unfilteredLabel: 'All', + selects: [ + { label: 'Published', value: 7 }, + { label: 'Draft', value: 8 }, + ], + }, + ]; + + render( + , + ); + + await selectOption('Published', 'Status'); + + await waitFor(() => { + expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { + label: 'Published', + value: 7, + }); + }); +}); + +test('plain select with ReactNode label uses option title when serializing selection', async () => { + // Parallel coverage to the AsyncSelect ReactNode-with-title test, against + // the non-async Select wrapper. Guards against the two wrappers ever + // diverging on antd's two-arg onChange shape. + const ReactNodeLabel = ( +
+ Jane Roe + jane@example.com +
+ ); + + const filters = [ + { + Header: 'Owner', + key: 'owner', + id: 'owners', + input: 'select' as const, + operator: ListViewFilterOperator.RelationManyMany, + unfilteredLabel: 'All', + selects: [{ label: ReactNodeLabel, value: 99, title: 'Jane Roe' }], + }, + ]; + + render( + , + ); + + await selectOption('Jane Roe', 'Owner'); + + await waitFor(() => { + expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { + label: 'Jane Roe', + value: 99, + }); + }); +}); + +test('clearFilter notifies onSelect with undefined and isClear=true', () => { + // The isClear flag is what allows the parent (Filters/index) to suppress + // onFilterUpdate side-effects when the user clears the filter rather than + // picking a new value. Lock that contract in. + const mockOnSelect = jest.fn(); + const ref = createRef(); + + render( + , + ); + + ref.current?.clearFilter(); + + expect(mockOnSelect).toHaveBeenCalledWith(undefined, true); +}); + +test('rehydrates filter pill from initialValue with plain-string label', async () => { + // The user-visible regression: after URL/state rehydration the filter pill + // must render the human-readable name, not the numeric user id. The fix + // ensures the persisted label is a string; this test asserts that string + // is what surfaces in the rendered combobox selection. + const filters = [ + { + Header: 'Owner', + key: 'owner', + id: 'owners', + input: 'select' as const, + operator: ListViewFilterOperator.RelationManyMany, + unfilteredLabel: 'All', + fetchSelects: jest.fn().mockResolvedValue({ data: [], totalCount: 0 }), + paginate: true, + }, + ]; + + render( + , + ); + + await waitFor(() => { + expect(screen.getByText('John Doe')).toBeInTheDocument(); + }); +}); diff --git a/superset-frontend/src/components/ListView/Filters/Select.tsx b/superset-frontend/src/components/ListView/Filters/Select.tsx index 5e38d99bd84..cecee38a274 100644 --- a/superset-frontend/src/components/ListView/Filters/Select.tsx +++ b/superset-frontend/src/components/ListView/Filters/Select.tsx @@ -58,14 +58,22 @@ function SelectFilter( ) { const [selectedOption, setSelectedOption] = useState(initialValue); - const onChange = (selected: SelectOption) => { + const onChange = (selected: SelectOption, option?: SelectOption) => { + // antd's `onChange` (with `labelInValue`) passes the `{label, value}` + // labeled-value as the first arg and the full option (which carries + // `title` and any other fields) as the second. Options may supply a + // ReactNode label (e.g. OwnerSelectLabel for the chart list Owner + // filter). Since this object is serialized into the URL and rehydrated + // as the filter pill on return, we need a plain string. Prefer `title` + // (set by callers to the human-readable name) before falling back to + // the value. onSelect( selected ? { label: typeof selected.label === 'string' ? selected.label - : String(selected.value), + : (option?.title ?? String(selected.value)), value: selected.value, } : undefined, diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index 16dcf339dd9..f1f2ad28ced 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -26,6 +26,10 @@ export interface SortColumn { export interface SelectOption { label: ReactNode; value: any; + // Plain-text representation of the option. Callers should set this when + // `label` is a ReactNode so that the option can be serialized (e.g. into + // URL filter state) without losing the human-readable name. + title?: string; [key: string]: unknown; }