Merge pull request #2831 from marmelab/fix-default-value-filter

[RFR] Fix default filter values cannot be removed by user
marmelab · Jan 30, 2019 · a466471 · a466471
2 parents 5a18708 + 3ada067
commit a466471
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 29 deletions.
diff --git a/cypress/integration/list.js b/cypress/integration/list.js
@@ -24,7 +24,9 @@ describe('List Page', () => {
             cy.viewport(1280, 500);
 
             cy.scrollTo(0, 200);
-            cy.get(ListPagePosts.elements.headroomUnpinned).should('not.be.visible');
+            cy.get(ListPagePosts.elements.headroomUnpinned).should(
+                'not.be.visible'
+            );
 
             cy.scrollTo(0, -100);
             cy.get(ListPagePosts.elements.headroomUnfixed).should('be.visible');
@@ -109,6 +111,15 @@ describe('List Page', () => {
             cy.contains('1-1 of 1');
             ListPagePosts.setFilterValue('q', '');
         });
+
+        it('should allow to disable alwaysOn filters with default value', () => {
+            LoginPage.navigate();
+            LoginPage.login('admin', 'password');
+            ListPageUsers.navigate();
+            cy.contains('1-2 of 2');
+            cy.get('button[tooltip="Remove this filter"]').click();
+            cy.contains('1-3 of 3');
+        });
     });
 
     describe('Bulk Actions', () => {
@@ -223,13 +234,15 @@ describe('List Page', () => {
         });
     });
 
-    describe("Sorting", () => {
+    describe('Sorting', () => {
         it('should display a sort arrow when clicking on a sortable column header', () => {
             ListPagePosts.toggleColumnSort('id');
             cy.get(ListPagePosts.elements.svg('id')).should('be.visible');
 
             ListPagePosts.toggleColumnSort('tags.name');
-            cy.get(ListPagePosts.elements.svg('tags.name')).should('be.visible');
+            cy.get(ListPagePosts.elements.svg('tags.name')).should(
+                'be.visible'
+            );
         });
 
         it('should hide the sort arrow when clicking on another sortable column header', () => {
@@ -241,10 +254,20 @@ describe('List Page', () => {
         it('should reverse the sort arrow when clicking on an already sorted column header', () => {
             ListPagePosts.toggleColumnSort('published_at');
             ListPagePosts.toggleColumnSort('tags.name');
-            cy.get(ListPagePosts.elements.svg('tags.name', '[class*=iconDirectionAsc]')).should('exist');
+            cy.get(
+                ListPagePosts.elements.svg(
+                    'tags.name',
+                    '[class*=iconDirectionAsc]'
+                )
+            ).should('exist');
 
             ListPagePosts.toggleColumnSort('tags.name');
-            cy.get(ListPagePosts.elements.svg('tags.name', '[class*=iconDirectionDesc]')).should('exist');
+            cy.get(
+                ListPagePosts.elements.svg(
+                    'tags.name',
+                    '[class*=iconDirectionDesc]'
+                )
+            ).should('exist');
         });
-    })
+    });
 });
diff --git a/packages/ra-core/src/controller/ListController.js b/packages/ra-core/src/controller/ListController.js
@@ -137,6 +137,31 @@ export class ListController extends Component {
         return true;
     }
 
+    /**
+     * Check if user has already set custom sort, page, or filters for this list
+     *
+     * User params come from the Redux store as the params props. By default,
+     * this object is:
+     *
+     * { filter: {}, order: null, page: 1, perPage: null, sort: null }
+     *
+     * To check if the user has custom params, we must compare the params
+     * to these initial values.
+     *
+     * @param {object} params
+     */
+    hasCustomParams(params) {
+        return (
+            params &&
+            params.filter &&
+            (Object.keys(params.filter).length > 0 ||
+                params.order != null ||
+                params.page !== 1 ||
+                params.perPage != null ||
+                params.sort != null)
+        );
+    }
+
     /**
      * Merge list params from 4 different sources:
      *   - the query string
@@ -148,10 +173,9 @@ export class ListController extends Component {
         const query =
             Object.keys(this.props.query).length > 0
                 ? this.props.query
-                : { ...this.props.params };
-        const filterDefaultValues = this.props.filterDefaultValues || {};
-
-        query.filter = { ...filterDefaultValues, ...query.filter };
+                : this.hasCustomParams(this.props.params)
+                    ? { ...this.props.params }
+                    : { filter: this.props.filterDefaultValues || {} };
 
         if (!query.sort) {
             query.sort = this.props.sort.field;
@@ -166,6 +190,11 @@ export class ListController extends Component {
         return query;
     }
 
+    getFilterValues() {
+        const query = this.getQuery();
+        return query.filter || {};
+    }
+
     updateData(query) {
         const params = query || this.getQuery();
         const { sort, order, page = 1, perPage, filter } = params;
@@ -190,7 +219,7 @@ export class ListController extends Component {
         this.changeParams({ type: SET_PER_PAGE, payload: perPage });
 
     setFilters = debounce(filters => {
-        if (isEqual(filters, this.props.filterValues)) {
+        if (isEqual(filters, this.getFilterValues())) {
             return;
         }
 
@@ -203,15 +232,15 @@ export class ListController extends Component {
         this.setState({ [filterName]: true });
         if (typeof defaultValue !== 'undefined') {
             this.setFilters({
-                ...this.props.filterValues,
+                ...this.getFilterValues(),
                 [filterName]: defaultValue,
             });
         }
     };
 
     hideFilter = filterName => {
         this.setState({ [filterName]: false });
-        const newFilters = removeKey(this.props.filterValues, filterName);
+        const newFilters = removeKey(this.getFilterValues(), filterName);
         this.setFilters(newFilters);
     };
 
@@ -255,9 +284,6 @@ export class ListController extends Component {
             selectedIds,
         } = this.props;
         const query = this.getQuery();
-
-        const queryFilterValues = query.filter || {};
-
         const resourceName = translate(`resources.${resource}.name`, {
             smart_count: 2,
             _: inflection.humanize(inflection.pluralize(resource)),
@@ -275,7 +301,7 @@ export class ListController extends Component {
             data,
             defaultTitle,
             displayedFilters: this.state,
-            filterValues: queryFilterValues,
+            filterValues: this.getFilterValues(),
             hasCreate,
             hideFilter: this.hideFilter,
             ids,
@@ -320,7 +346,6 @@ ListController.propTypes = {
     crudGetList: PropTypes.func.isRequired,
     data: PropTypes.object, // eslint-disable-line react/forbid-prop-types
     debounce: PropTypes.number,
-    filterValues: PropTypes.object, // eslint-disable-line react/forbid-prop-types
     hasCreate: PropTypes.bool,
     hasEdit: PropTypes.bool,
     hasList: PropTypes.bool,
@@ -345,7 +370,6 @@ ListController.propTypes = {
 ListController.defaultProps = {
     debounce: 500,
     filter: {},
-    filterValues: {},
     perPage: 10,
     sort: {
         field: 'id',
@@ -435,19 +459,12 @@ function mapStateToProps(state, props) {
         total: resourceState.list.total,
         data: resourceState.data,
         isLoading: state.admin.loading > 0,
-        filterValues: resourceState.list.params.filter,
         version: state.admin.ui.viewVersion,
     };
 }
 
-
-
 export default compose(
-    checkMinimumRequiredProps('List', [
-        'basePath',
-        'location',
-        'resource',
-    ]),
+    checkMinimumRequiredProps('List', ['basePath', 'location', 'resource']),
     connect(
         mapStateToProps,
         {

diff --git a/packages/ra-core/src/controller/ListController.spec.js b/packages/ra-core/src/controller/ListController.spec.js
@@ -25,7 +25,7 @@ describe('ListController', () => {
         location: {
             pathname: '',
         },
-        params: {},
+        params: { filter: {} },
         push: () => {},
         query: {},
         resource: '',
@@ -74,7 +74,7 @@ describe('ListController', () => {
                 ...defaultProps,
                 debounce: 200,
                 changeListParams: jest.fn(),
-                filterValues: { q: 'hello' },
+                params: { filter: { q: 'hello' } },
                 children: fakeComponent,
             };