From 8b4dce71d6cbe3b48c8847c2f641bd7dd5de3e3c Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Tue, 27 Feb 2024 08:20:42 -0500 Subject: [PATCH] fix: Sorting charts/dashboards makes the applied filters ineffective (#27258) --- .../cypress/e2e/chart_list/list.test.ts | 7 +++++++ .../cypress/e2e/dashboard_list/list.test.ts | 7 +++++++ .../src/components/ListView/CardSortSelect.tsx | 18 +++++++++--------- .../src/components/ListView/ListView.tsx | 9 ++++----- .../src/components/ListView/types.ts | 4 +--- .../src/components/ListView/utils.ts | 4 +++- 6 files changed, 31 insertions(+), 18 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts b/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts index 44f348edc50f5..3cd1f91b49003 100644 --- a/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts @@ -173,6 +173,13 @@ describe('Charts list', () => { orderAlphabetical(); cy.getBySel('styled-card').first().contains('% Rural'); }); + + it('should preserve other filters when sorting', () => { + cy.getBySel('styled-card').should('have.length', 25); + setFilter('Type', 'Big Number'); + setFilter('Sort', 'Least recently modified'); + cy.getBySel('styled-card').should('have.length', 3); + }); }); describe('common actions', () => { diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts index 7dfb7cd673d7f..917ca104550d2 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts @@ -117,6 +117,13 @@ describe('Dashboards list', () => { orderAlphabetical(); cy.getBySel('styled-card').first().contains('Supported Charts Dashboard'); }); + + it('should preserve other filters when sorting', () => { + cy.getBySel('styled-card').should('have.length', 5); + setFilter('Status', 'Published'); + setFilter('Sort', 'Least recently modified'); + cy.getBySel('styled-card').should('have.length', 3); + }); }); describe('common actions', () => { diff --git a/superset-frontend/src/components/ListView/CardSortSelect.tsx b/superset-frontend/src/components/ListView/CardSortSelect.tsx index 479355881f900..2a77d51cfb94e 100644 --- a/superset-frontend/src/components/ListView/CardSortSelect.tsx +++ b/superset-frontend/src/components/ListView/CardSortSelect.tsx @@ -21,7 +21,7 @@ import { styled, t } from '@superset-ui/core'; import { Select } from 'src/components'; import { FormLabel } from 'src/components/Form'; import { SELECT_WIDTH } from './utils'; -import { CardSortSelectOption, FetchDataConfig, SortColumn } from './types'; +import { CardSortSelectOption, SortColumn } from './types'; const SortContainer = styled.div` display: inline-flex; @@ -32,22 +32,22 @@ const SortContainer = styled.div` `; interface CardViewSelectSortProps { - onChange: (conf: FetchDataConfig) => any; + onChange: (value: SortColumn[]) => void; options: Array; initialSort?: SortColumn[]; - pageIndex: number; - pageSize: number; } export const CardSortSelect = ({ initialSort, onChange, options, - pageIndex, - pageSize, }: CardViewSelectSortProps) => { const defaultSort = - (initialSort && options.find(({ id }) => id === initialSort[0].id)) || + (initialSort && + options.find( + ({ id, desc }) => + id === initialSort[0].id && desc === initialSort[0].desc, + )) || options[0]; const [value, setValue] = useState({ @@ -72,7 +72,7 @@ export const CardSortSelect = ({ desc: originalOption.desc, }, ]; - onChange({ pageIndex, pageSize, sortBy, filters: [] }); + onChange(sortBy); } }; @@ -82,7 +82,7 @@ export const CardSortSelect = ({ ariaLabel={t('Sort')} header={{t('Sort')}} labelInValue - onChange={(value: CardSortSelectOption) => handleOnChange(value)} + onChange={handleOnChange} options={formattedOptions} showSearch value={value} diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index 82cf6b187846a..93847ca5d879d 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -271,10 +271,11 @@ function ListView({ pageCount = 1, gotoPage, applyFilterValue, + setSortBy, selectedFlatRows, toggleAllRowsSelected, setViewMode, - state: { pageIndex, pageSize, internalFilters, viewMode }, + state: { pageIndex, pageSize, internalFilters, sortBy, viewMode }, query, } = useListViewState({ bulkSelectColumnConfig, @@ -350,11 +351,9 @@ function ListView({ )} {viewMode === 'card' && cardSortSelectOptions && ( setSortBy(value)} options={cardSortSelectOptions} - pageIndex={pageIndex} - pageSize={pageSize} /> )} diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index 005d8fde7cc02..1082c764f58ee 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -23,8 +23,6 @@ export interface SortColumn { desc?: boolean; } -export type SortColumns = SortColumn[]; - export interface SelectOption { label: string; value: any; @@ -84,7 +82,7 @@ export interface FilterValue { export interface FetchDataConfig { pageIndex: number; pageSize: number; - sortBy: SortColumns; + sortBy: SortColumn[]; filters: FilterValue[]; } diff --git a/superset-frontend/src/components/ListView/utils.ts b/superset-frontend/src/components/ListView/utils.ts index 31a9368a1ab41..03540125df267 100644 --- a/superset-frontend/src/components/ListView/utils.ts +++ b/superset-frontend/src/components/ListView/utils.ts @@ -220,7 +220,7 @@ export function useListViewState({ query.sortColumn && query.sortOrder ? [{ id: query.sortColumn, desc: query.sortOrder === 'desc' }] : initialSort, - [query.sortColumn, query.sortOrder], + [initialSort, query.sortColumn, query.sortOrder], ); const initialState = { @@ -256,6 +256,7 @@ export function useListViewState({ pageCount, gotoPage, setAllFilters, + setSortBy, selectedFlatRows, toggleAllRowsSelected, state: { pageIndex, pageSize, sortBy, filters }, @@ -373,6 +374,7 @@ export function useListViewState({ rows, selectedFlatRows, setAllFilters, + setSortBy, state: { pageIndex, pageSize, sortBy, filters, internalFilters, viewMode }, toggleAllRowsSelected, applyFilterValue,