You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/02/27 20:44:23 UTC

(superset) 06/08: fix: Sorting charts/dashboards makes the applied filters ineffective (#27258)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 0d2cf7c93807950d8c705fc6120714de5337fddc
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Tue Feb 27 08:20:42 2024 -0500

    fix: Sorting charts/dashboards makes the applied filters ineffective (#27258)
    
    (cherry picked from commit 8b4dce71d6cbe3b48c8847c2f641bd7dd5de3e3c)
---
 .../cypress-base/cypress/e2e/chart_list/list.test.ts   |  7 +++++++
 .../cypress/e2e/dashboard_list/list.test.ts            |  7 +++++++
 .../src/components/ListView/CardSortSelect.tsx         | 18 +++++++++---------
 superset-frontend/src/components/ListView/ListView.tsx |  9 ++++-----
 superset-frontend/src/components/ListView/types.ts     |  4 +---
 superset-frontend/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 44f348edc5..3cd1f91b49 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 7dfb7cd673..917ca10455 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 479355881f..2a77d51cfb 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<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
index 82cf6b1878..93847ca5d8 100644
--- 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
index 0c1314f263..a526d6870d 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 8a8c57cb62..ec80e4cff5 100644
--- a/superset-frontend/src/components/ListView/utils.ts
+++ b/superset-frontend/src/components/ListView/utils.ts
@@ -221,7 +221,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 = {
@@ -257,6 +257,7 @@ export function useListViewState({
     pageCount,
     gotoPage,
     setAllFilters,
+    setSortBy,
     selectedFlatRows,
     toggleAllRowsSelected,
     state: { pageIndex, pageSize, sortBy, filters },
@@ -374,6 +375,7 @@ export function useListViewState({
     rows,
     selectedFlatRows,
     setAllFilters,
+    setSortBy,
     state: { pageIndex, pageSize, sortBy, filters, internalFilters, viewMode },
     toggleAllRowsSelected,
     applyFilterValue,