Fix and speedup filters

marmelab · Oct 1, 2016 · 00be458 · 00be458
1 parent ff374f1
commit 00be458
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 82 deletions.
diff --git a/src/mui/list/FilterForm.js b/src/mui/list/FilterForm.js
@@ -6,12 +6,13 @@ import ActionHide from 'material-ui/svg-icons/action/highlight-off';
 
 export class FilterForm extends Component {
     getShownFilters() {
-        const { filters, displayedFilters, currentFilters } = this.props;
+        const { filters, displayedFilters, currentFilters, initialValues } = this.props;
         return filters
             .filter(filterElement =>
                 filterElement.props.alwaysOn ||
                 displayedFilters[filterElement.props.source] ||
-                currentFilters[filterElement.props.source]
+                currentFilters[filterElement.props.source] ||
+                initialValues[filterElement.props.source]
             );
     }
 
@@ -50,6 +51,7 @@ FilterForm.propTypes = {
     filters: PropTypes.arrayOf(PropTypes.node).isRequired,
     displayedFilters: PropTypes.object.isRequired,
     hideFilter: PropTypes.func.isRequired,
+    initialValues: PropTypes.object,
 };
 
 FilterForm.defaultProps = {

diff --git a/src/mui/list/List.js b/src/mui/list/List.js
@@ -17,6 +17,7 @@ import { changeListParams as changeListParamsAction } from '../../actions/listAc
 export class List extends Component {
     constructor(props) {
         super(props);
+        this.debouncedSetFilters = debounce(this.setFilters.bind(this), 500);
         this.state = {};
     }
 
@@ -32,21 +33,28 @@ export class List extends Component {
          || nextProps.query.filter !== this.props.query.filter) {
             this.updateData(Object.keys(nextProps.query).length > 0 ? nextProps.query : nextProps.params);
         }
+        if (nextProps.filters !== this.props.filters) {
+            const nextFilters = nextProps.filters;
+            Object.keys(nextFilters).forEach(filterName => {
+                if (nextFilters[filterName] === '') {
+                    delete nextFilters[filterName];
+                }
+            });
+            this.debouncedSetFilters(nextFilters);
+        }
     }
 
-    componentWillUpdate(nextProps) {
-        this.debouncedUpdateFilter(nextProps.filters, this.props.filters);
+    shouldComponentUpdate(nextProps, nextState) {
+        if (nextProps.isLoading === this.props.isLoading && nextState === this.state) {
+            return false;
+        }
+        return true;
     }
 
     getBasePath() {
         return this.props.location.pathname;
     }
 
-    refresh(event) {
-        event.stopPropagation();
-        this.updateData();
-    }
-
     getQuery() {
         return (Object.keys(this.props.query).length > 0) ? this.props.query : { ...this.props.params };
     }
@@ -61,30 +69,22 @@ export class List extends Component {
         this.changeParams({ type: SET_SORT, payload: event.currentTarget.dataset.sort });
     }
 
-    setPage = (page) => this.changeParams({ type: SET_PAGE, payload: page });
-
-    setFilter = (field, value) => {
-        this.changeParams({ type: SET_FILTER, payload: { field, value } });
+    refresh = (event) => {
+        event.stopPropagation();
+        this.updateData();
     }
 
-    debouncedUpdateFilter = debounce((nextFilters, currentFilters) => {
-        if (nextFilters !== currentFilters) {
-            const allFilters = new Set([
-                ...Object.keys(currentFilters),
-                ...Object.keys(nextFilters),
-            ]);
+    setPage = (page) => this.changeParams({ type: SET_PAGE, payload: page });
 
-            allFilters.forEach(filter => {
-                this.setFilter(filter, nextFilters[filter]);
-            });
-        }
-    }, 1000);
+    setFilters = (filters) => {
+        this.changeParams({ type: SET_FILTER, payload: filters });
+    }
 
     showFilter = (filterName) => this.setState({ [filterName]: true });
 
     hideFilter = (filterName) => {
         this.setState({ [filterName]: false });
-        this.setFilter(filterName);
+        this.setFilters({ ...this.props.filters, [filterName]: undefined });
     }
 
     changeParams(action) {
@@ -109,7 +109,7 @@ export class List extends Component {
                         context: 'button',
                     })}
                     {hasCreate && <CreateButton basePath={basePath} />}
-                    <FlatButton primary label="Refresh" onClick={::this.refresh} icon={<NavigationRefresh />} />
+                    <FlatButton primary label="Refresh" onClick={this.refresh} icon={<NavigationRefresh />} />
                 </CardActions>
                 <CardTitle title={<Title title={title} defaultTitle={`${inflection.humanize(inflection.pluralize(resource))} List`} />} />
                 {filter && React.createElement(filter, {
@@ -175,7 +175,7 @@ function mapStateToProps(state, props) {
         total: resourceState.list.total,
         data: resourceState.data,
         isLoading: state.admin.loading > 0,
-        filters: getFormValues('filterForm')(state),
+        filters: getFormValues('filterForm')(state) || resourceState.list.params.filter,
     };
 }
 

diff --git a/src/reducer/resource/list/queryReducer.js b/src/reducer/resource/list/queryReducer.js
@@ -33,19 +33,7 @@ export default (previousState, { type, payload }) => {
         return { ...previousState, page: payload };
 
     case SET_FILTER: {
-        const filter = { ...previousState.filter };
-
-        if (typeof payload.value === 'undefined' || payload.value.length === 0) {
-            delete filter[payload.field];
-        } else {
-            filter[payload.field] = payload.value;
-        }
-
-        return {
-            ...previousState,
-            page: 1,
-            filter,
-        };
+        return { ...previousState, page: 1, filter: payload };
     }
 
     default:

diff --git a/src/reducer/resource/list/queryReducer.spec.js b/src/reducer/resource/list/queryReducer.spec.js
@@ -6,12 +6,8 @@ describe('Query Reducer', () => {
         it('should add new filter with given value when set', () => {
             const updatedState = queryReducer({}, {
                 type: 'SET_FILTER',
-                payload: {
-                    field: 'title',
-                    value: 'foo',
-                },
+                payload: { title: 'foo' },
             });
-
             assert.deepEqual(updatedState.filter, { title: 'foo' });
         });
 
@@ -22,47 +18,12 @@ describe('Query Reducer', () => {
                 },
             }, {
                 type: 'SET_FILTER',
-                payload: {
-                    field: 'title',
-                    value: 'bar',
-                },
+                payload: { title: 'bar' },
             });
 
             assert.deepEqual(updatedState.filter, { title: 'bar' });
         });
 
-        it('should remove existing filter if value is empty', () => {
-            const updatedState = queryReducer({
-                filter: {
-                    title: '',
-                },
-            }, {
-                type: 'SET_FILTER',
-                payload: {
-                    field: 'title',
-                    value: '',
-                },
-            });
-
-            assert.deepEqual(updatedState.filter, {});
-        });
-
-        it('should remove existing filter if value is undefined', () => {
-            const updatedState = queryReducer({
-                filter: {
-                    title: 'foo',
-                },
-            }, {
-                type: 'SET_FILTER',
-                payload: {
-                    field: 'title',
-                    value: undefined,
-                },
-            });
-
-            assert.deepEqual(updatedState.filter, {});
-        });
-
         it('should reset page to 1', () => {
             const updatedState = queryReducer({ page: 3 }, { type: 'SET_FILTER', payload: {} });
             assert.equal(updatedState.page, 1);

diff --git a/src/sideEffect/saga/crudFetch.js b/src/sideEffect/saga/crudFetch.js
@@ -34,11 +34,11 @@ const crudFetch = (restClient, successSideEffects = () => [], failureSideEffects
             }
         }
         const sideEffects = successSideEffects(type, meta.resource, payload, response);
-        yield put({ type: FETCH_END });
         yield [
             ...sideEffects.map(a => put(a)),
             put({ type: `${type}_SUCCESS`, payload: response, meta }),
         ];
+        yield put({ type: FETCH_END });
     }
 
     return function *watchCrudFetch() {