fix: Sorting charts/dashboards makes the applied filters ineffective (a…

…pache#27258)
brouberol · Feb 27, 2024 · 8b4dce7 · 8b4dce7
1 parent 4a1f66a
commit 8b4dce7
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 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
@@ -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
@@ -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
@@ -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<CardSortSelectOption>;
   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={<FormLabel>{t('Sort')}</FormLabel>}
         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
@@ -271,10 +271,11 @@ function ListView<T extends object = any>({
     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<T extends object = any>({
             )}
             {viewMode === 'card' && cardSortSelectOptions && (
               <CardSortSelect
-                initialSort={initialSort}
-                onChange={fetchData}
+                initialSort={sortBy}
+                onChange={(value: SortColumn[]) => setSortBy(value)}
                 options={cardSortSelectOptions}
-                pageIndex={pageIndex}
-                pageSize={pageSize}
               />
             )}
           </div>

diff --git 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
@@ -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,