This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a273fe4d621 fix(list-view): preserve user name in filter pill after
navigation (#39505)
a273fe4d621 is described below
commit a273fe4d621b91da608bc7be24fd0c3a778cd3da
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Wed May 20 16:54:49 2026 -0700
fix(list-view): preserve user name in filter pill after navigation (#39505)
Co-authored-by: Joe Li <[email protected]>
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
---
.../components/ListView/Filters/Select.test.tsx | 267 +++++++++++++++++++++
.../src/components/ListView/Filters/Select.tsx | 12 +-
superset-frontend/src/components/ListView/types.ts | 4 +
3 files changed, 281 insertions(+), 2 deletions(-)
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 = (
+ <div>
+ <span>John Doe</span>
+ <span>[email protected]</span>
+ </div>
+ );
+
+ 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(
+ <UIFilters
+ filters={filters}
+ internalFilters={[]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ 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: <span>123</span>,
+ value: 123,
+ },
+ ],
+ totalCount: 1,
+ });
+
+ const filters = [
+ {
+ Header: 'Something',
+ key: 'something',
+ id: 'something',
+ input: 'select' as const,
+ operator: ListViewFilterOperator.RelationOneMany,
+ unfilteredLabel: 'All',
+ fetchSelects,
+ },
+ ];
+
+ render(
+ <UIFilters
+ filters={filters}
+ internalFilters={[]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ 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(
+ <UIFilters
+ filters={filters}
+ internalFilters={[]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ 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 = (
+ <div>
+ <span>Jane Roe</span>
+ <span>[email protected]</span>
+ </div>
+ );
+
+ 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(
+ <UIFilters
+ filters={filters}
+ internalFilters={[]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ 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<FilterHandler>();
+
+ render(
+ <SelectFilter
+ Header="Owner"
+ initialValue={{ label: 'John Doe', value: 42 }}
+ onSelect={mockOnSelect}
+ selects={[{ label: 'John Doe', value: 42, title: 'John Doe' }]}
+ ref={ref}
+ />,
+ );
+
+ 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(
+ <UIFilters
+ filters={filters}
+ internalFilters={[
+ {
+ id: 'owners',
+ operator: ListViewFilterOperator.RelationManyMany,
+ value: { label: 'John Doe', value: 42 },
+ },
+ ]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ 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;
}
