refactor: course filters (#303)

Co-authored-by: Maxwell Frank <mfrank@2u.com>

src/containers/CourseFilterControls/ActiveCourseFilters.jsx

@@ -4,16 +4,17 @@ import { useIntl } from '@edx/frontend-platform/i18n';
 
 import { Button, Chip } from '@openedx/paragon';
 import { CloseSmall } from '@openedx/paragon/icons';
+import { reduxHooks } from 'hooks';
 
 import messages from './messages';
 import './index.scss';
 
 export const ActiveCourseFilters = ({
   filters,
-  setFilters,
   handleRemoveFilter,
 }) => {
   const { formatMessage } = useIntl();
+  const clearFilters = reduxHooks.useClearFilters();
   return (
     <div id="course-list-active-filters">
       {filters.map(filter => (
@@ -25,7 +26,7 @@ export const ActiveCourseFilters = ({
           {formatMessage(messages[filter])}
         </Chip>
       ))}
-      <Button variant="link" onClick={setFilters.clear}>
+      <Button variant="link" onClick={clearFilters}>
         {formatMessage(messages.clearAll)}
       </Button>
     </div>
@@ -33,10 +34,6 @@ export const ActiveCourseFilters = ({
 };
 ActiveCourseFilters.propTypes = {
   filters: PropTypes.arrayOf(PropTypes.string).isRequired,
-  setFilters: PropTypes.shape({
-    remove: PropTypes.func,
-    clear: PropTypes.func,
-  }).isRequired,
   handleRemoveFilter: PropTypes.func.isRequired,
 };
 

src/containers/CourseFilterControls/ActiveCourseFilters.test.jsx

@@ -6,10 +6,6 @@ import ActiveCourseFilters from './ActiveCourseFilters';
 describe('ActiveCourseFilters', () => {
   const props = {
     filters: Object.values(FilterKeys),
-    setFilters: {
-      remove: jest.fn().mockName('setFilters.remove'),
-      clear: jest.fn().mockName('setFilters.clear'),
-    },
     handleRemoveFilter: jest.fn().mockName('handleRemoveFilter'),
   };
   describe('snapshot', () => {

src/containers/CourseFilterControls/CourseFilterControls.jsx

@@ -27,7 +27,6 @@ export const CourseFilterControls = ({
   sortBy,
   setSortBy,
   filters,
-  setFilters,
 }) => {
   const { formatMessage } = useIntl();
   const hasCourses = reduxHooks.useHasCourses();
@@ -41,7 +40,6 @@ export const CourseFilterControls = ({
     handleSortChange,
   } = useCourseFilterControlsData({
     filters,
-    setFilters,
     setSortBy,
   });
   const { width } = useWindowSize();
@@ -112,10 +110,6 @@ CourseFilterControls.propTypes = {
   sortBy: PropTypes.string.isRequired,
   setSortBy: PropTypes.func.isRequired,
   filters: PropTypes.arrayOf(PropTypes.string).isRequired,
-  setFilters: PropTypes.shape({
-    add: PropTypes.func.isRequired,
-    remove: PropTypes.func.isRequired,
-  }).isRequired,
 };
 
 export default CourseFilterControls;

src/containers/CourseFilterControls/CourseFilterControls.test.jsx

@@ -23,10 +23,6 @@ describe('CourseFilterControls', () => {
     sortBy: 'test-sort-by',
     setSortBy: jest.fn().mockName('setSortBy'),
     filters: ['test-filter'],
-    setFilters: {
-      add: jest.fn().mockName('setFilters.add'),
-      remove: jest.fn().mockName('setFilters.remove'),
-    },
   };
 
   useCourseFilterControlsData.mockReturnValue({

src/containers/CourseFilterControls/__snapshots__/ActiveCourseFilters.test.jsx.snap

@@ -30,7 +30,7 @@ exports[`ActiveCourseFilters snapshot renders 1`] = `
     Upgraded
   </Chip>
   <Button
-    onClick={[MockFunction setFilters.clear]}
+    onClick={[Function]}
     variant="link"
   >
     Clear all

src/containers/CourseFilterControls/hooks.js

@@ -3,6 +3,7 @@ import { useToggle } from '@openedx/paragon';
 
 import { StrictDict } from 'utils';
 import track from 'tracking';
+import { reduxHooks } from 'hooks';
 
 import * as module from './hooks';
 
@@ -10,15 +11,25 @@ export const state = StrictDict({
   target: (val) => React.useState(val), // eslint-disable-line
 });
 
+/**
+ * Sets up a toggle for the modal as well as helper functions for handling changes to the form controls.
+ *
+ * @param {array} filters Currently active course filters
+ * @param {function} setSortBy Set function for sorting the course list
+ * @returns {object} data and functions for managing the CourseFilterControls component
+ */
 export const useCourseFilterControlsData = ({
   filters,
-  setFilters,
   setSortBy,
 }) => {
   const [isOpen, toggleOpen, toggleClose] = useToggle(false);
   const [target, setTarget] = module.state.target(null);
+
+  const addFilter = reduxHooks.useAddFilter();
+  const removeFilter = reduxHooks.useRemoveFilter();
+
   const handleFilterChange = ({ target: { checked, value } }) => {
-    const update = checked ? setFilters.add : setFilters.remove;
+    const update = checked ? addFilter : removeFilter;
     update(value);
   };
   const handleSortChange = ({ target: { value } }) => {

src/containers/CourseFilterControls/hooks.test.js

@@ -1,6 +1,8 @@
 import { useToggle } from '@openedx/paragon';
 
 import { MockUseState } from 'testUtils';
+import { reduxHooks } from 'hooks';
+
 import track from 'tracking';
 
 import * as hooks from './hooks';
@@ -12,18 +14,28 @@ jest.mock('tracking', () => ({
   },
 }));
 
+jest.mock('hooks', () => ({
+  reduxHooks: {
+    useAddFilter: jest.fn(),
+    useRemoveFilter: jest.fn(),
+  },
+}));
+
 const state = new MockUseState(hooks);
 
 describe('CourseFilterControls hooks', () => {
   let out;
   const filters = ['a', 'b', 'c'];
   const setSortBy = jest.fn();
-  const setFilters = {
-    add: jest.fn(),
-    remove: jest.fn(),
-  };
+
+  const removeFilter = jest.fn();
+  reduxHooks.useRemoveFilter.mockReturnValue(removeFilter);
+  const addFilter = jest.fn();
+  reduxHooks.useAddFilter.mockReturnValue(addFilter);
+
   const toggleOpen = jest.fn();
   const toggleClose = jest.fn();
+
   describe('state values', () => {
     state.testGetter(state.keys.target);
   });
@@ -37,7 +49,6 @@ describe('CourseFilterControls hooks', () => {
       state.mock();
       out = hooks.useCourseFilterControlsData({
         filters,
-        setFilters,
         setSortBy,
       });
     });
@@ -66,7 +77,6 @@ describe('CourseFilterControls hooks', () => {
       state.mockVal(state.keys.target, 'foo');
       out = hooks.useCourseFilterControlsData({
         filters,
-        setFilters,
         setSortBy,
       });
       expect(out.isOpen).toEqual(true);
@@ -81,14 +91,14 @@ describe('CourseFilterControls hooks', () => {
           value,
         },
       });
-      expect(setFilters.add).toHaveBeenCalledWith(value);
+      expect(addFilter).toHaveBeenCalledWith(value);
       out.handleFilterChange({
         target: {
           checked: false,
           value,
         },
       });
-      expect(setFilters.remove).toHaveBeenCalledWith(value);
+      expect(removeFilter).toHaveBeenCalledWith(value);
     });
     test('handle sort change', () => {
       const value = 'a';

src/containers/CourseList/hooks.js

@@ -1,6 +1,6 @@
 import React from 'react';
 
-import { useCheckboxSetValues, useWindowSize, breakpoints } from '@openedx/paragon';
+import { useWindowSize, breakpoints } from '@openedx/paragon';
 import queryString from 'query-string';
 
 import { ListPageSize, SortKeys } from 'data/constants/app';
@@ -18,31 +18,40 @@ export const state = StrictDict({
   sortBy: (val) => React.useState(val), // eslint-disable-line
 });
 
+/**
+ * Filters are fetched from the store and used to generate a list of "visible" courses.
+ * Other values returned and used for the layout of the CourseList component are:
+ * the current page number, the sorting method, and whether or not to enable filters and pagination.
+ *
+ * @returns data for the CourseList component
+ */
 export const useCourseListData = () => {
-  const [filters, setFilters] = useCheckboxSetValues([]);
-  const [sortBy, setSortBy] = module.state.sortBy(SortKeys.enrolled);
+  const filters = reduxHooks.useFilters();
+  const removeFilter = reduxHooks.useRemoveFilter();
   const pageNumber = reduxHooks.usePageNumber();
+  const setPageNumber = reduxHooks.useSetPageNumber();
+
+  const [sortBy, setSortBy] = module.state.sortBy(SortKeys.enrolled);
+
   const querySearch = queryString.parse(window.location.search, { parseNumbers: true });
 
-  const { numPages, visible } = reduxHooks.useCurrentCourseList({
+  const { numPages, visibleList } = reduxHooks.useCurrentCourseList({
     sortBy,
     filters,
     pageSize: querySearch?.disable_pagination === 1 ? 0 : ListPageSize,
   });
 
-  const handleRemoveFilter = (filter) => () => setFilters.remove(filter);
-  const setPageNumber = reduxHooks.useSetPageNumber();
+  const handleRemoveFilter = (filter) => () => removeFilter(filter);
 
   return {
     pageNumber,
     numPages,
     setPageNumber,
-    visibleList: visible,
+    visibleList,
     filterOptions: {
       sortBy,
       setSortBy,
       filters,
-      setFilters,
       handleRemoveFilter,
     },
     showFilters: filters.length > 0,

src/containers/CourseList/hooks.test.js

@@ -1,5 +1,3 @@
-import * as paragon from '@openedx/paragon';
-
 import queryString from 'query-string';
 
 import { MockUseState } from 'testUtils';
@@ -12,6 +10,8 @@ jest.mock('hooks', () => ({
     useCurrentCourseList: jest.fn(),
     usePageNumber: jest.fn(() => 23),
     useSetPageNumber: jest.fn(),
+    useFilters: jest.fn(),
+    useRemoveFilter: jest.fn(),
   },
 }));
 
@@ -24,20 +24,22 @@ const state = new MockUseState(hooks);
 const testList = ['a', 'b'];
 const testListData = {
   numPages: 52,
-  visible: testList,
+  visibleList: testList,
 };
 const testSortBy = 'fake sort option';
 const testFilters = ['some', 'fake', 'filters'];
-const testSetFilters = { add: jest.fn(), remove: jest.fn() };
-const testCheckboxSetValues = [testFilters, testSetFilters];
+
 const setPageNumber = jest.fn(val => ({ setPageNumber: val }));
 reduxHooks.useSetPageNumber.mockReturnValue(setPageNumber);
 
+const removeFilter = jest.fn();
+reduxHooks.useRemoveFilter.mockReturnValue(removeFilter);
+reduxHooks.useFilters.mockReturnValue(['some', 'fake', 'filters']);
+
 describe('CourseList hooks', () => {
   let out;
 
   reduxHooks.useCurrentCourseList.mockReturnValue(testListData);
-  paragon.useCheckboxSetValues.mockImplementation(() => testCheckboxSetValues);
 
   describe('state values', () => {
     state.testGetter(state.keys.sortBy);
@@ -80,12 +82,12 @@ describe('CourseList hooks', () => {
       });
       test('numPages and visible list load from useCurrentCourseList hook', () => {
         expect(out.numPages).toEqual(testListData.numPages);
-        expect(out.visibleList).toEqual(testListData.visible);
+        expect(out.visibleList).toEqual(testListData.visibleList);
       });
       test('showFilters is true iff filters is not empty', () => {
         expect(out.showFilters).toEqual(true);
         state.mockVal(state.keys.sortBy, testSortBy);
-        paragon.useCheckboxSetValues.mockReturnValueOnce([[], testSetFilters]);
+        reduxHooks.useFilters.mockReturnValueOnce([]);
         out = hooks.useCourseListData();
         // don't show filter when list is empty.
         expect(out.showFilters).toEqual(false);
@@ -95,15 +97,14 @@ describe('CourseList hooks', () => {
           expect(out.filterOptions.sortBy).toEqual(testSortBy);
           expect(out.filterOptions.setSortBy).toEqual(state.setState.sortBy);
         });
-        test('filters and setFilters passed by useCheckboxSetValues', () => {
+        test('filters passed by useFilters hook', () => {
           expect(out.filterOptions.filters).toEqual(testFilters);
-          expect(out.filterOptions.setFilters).toEqual(testSetFilters);
         });
         test('handleRemoveFilter creates callback to call setFilter.remove', () => {
           const cb = out.filterOptions.handleRemoveFilter(testFilters[0]);
-          expect(testSetFilters.remove).not.toHaveBeenCalled();
+          expect(removeFilter).not.toHaveBeenCalled();
           cb();
-          expect(testSetFilters.remove).toHaveBeenCalledWith(testFilters[0]);
+          expect(removeFilter).toHaveBeenCalledWith(testFilters[0]);
         });
         test('setPageNumber dispatches setPageNumber action with passed value', () => {
           expect(out.setPageNumber(2)).toEqual(setPageNumber(2));

src/containers/CourseList/index.jsx

@@ -17,6 +17,11 @@ import messages from './messages';
 
 import './index.scss';
 
+/**
+ * Renders the list of CourseCards, as well as the controls (CourseFilterControls) for modifying the list.
+ * Also houses the NoCoursesView to display if the user hasn't enrolled in any courses.
+ * @returns List of courses as CourseCards
+ */
 export const CourseList = () => {
   const { formatMessage } = useIntl();
   const hasCourses = reduxHooks.useHasCourses();
@@ -28,6 +33,7 @@ export const CourseList = () => {
     visibleList,
   } = useCourseListData();
   const isCollapsed = useIsCollapsed();
+
   return (
     <div className="course-list-container">
       <div className="course-list-heading-container">

src/data/redux/app/reducer.js

@@ -10,15 +10,17 @@ const initialState = {
   enterpriseDashboard: {},
   platformSettings: {},
   suggestedCourses: [],
-  filterState: {},
   selectSessionModal: {},
+  filters: [],
 };
 
 export const cardId = (val) => `card-${val}`;
 
 export const today = Date.now();
 
-// eslint-disable-next-line no-unused-vars
+/**
+ * Creates a redux slice with actions to load dashboard data and manage visual layout
+ */
 const app = createSlice({
   name: 'app',
   initialState,
@@ -49,6 +51,22 @@ const app = createSlice({
       selectSessionModal: { cardId: payload },
     }),
     setPageNumber: (state, { payload }) => ({ ...state, pageNumber: payload }),
+    setFilters: (state, { payload }) => ({
+      ...state,
+      filters: payload,
+    }),
+    addFilter: (state, { payload }) => ({
+      ...state,
+      filters: [...state.filters, payload],
+    }),
+    removeFilter: (state, { payload }) => ({
+      ...state,
+      filters: state.filters.filter(item => item !== payload),
+    }),
+    clearFilters: (state) => ({
+      ...state,
+      filters: [],
+    }),
   },
 });
 

src/data/redux/app/reducer.test.js

@@ -14,12 +14,14 @@ describe('app reducer', () => {
     it('returns initial state', () => {
       expect(reducer(undefined, {})).toEqual(initialState);
     });
+    const initialFilter = 'initial filter';
     const testState = {
       ...initialState,
       enrollments: [],
       courseData: {
       },
       entitlement: [],
+      filters: [initialFilter],
     };
     describe('action handlers', () => {
       describe('loadCourses', () => {
@@ -93,6 +95,30 @@ describe('app reducer', () => {
           });
         });
       });
+      describe('filters', () => {
+        const newFilter = 'new filter';
+        let out;
+        beforeEach(() => {
+          out = reducer(testState, {});
+        });
+        it('overwrites the filters object when using setFilters', () => {
+          expect(out.filters).toEqual([initialFilter]);
+          out = reducer(testState, actions.setFilters([newFilter]));
+          expect(out.filters).toEqual([newFilter]);
+        });
+        it('adds a filter when using addFilter', () => {
+          out = reducer(testState, actions.addFilter(newFilter));
+          expect(out.filters).toEqual([initialFilter, newFilter]);
+        });
+        it('removes a filter when using removeFilter', () => {
+          out = reducer(testState, actions.removeFilter(initialFilter));
+          expect(out.filters).toEqual([]);
+        });
+        it('clears the filters when using clearFilters', () => {
+          out = reducer(testState, actions.clearFilters());
+          expect(out.filters).toEqual([]);
+        });
+      });
     });
   });
 });

src/data/redux/app/selectors/currentList.js

@@ -50,7 +50,7 @@ export const visibleList = (state, {
     };
   }
   return {
-    visible: list.slice((pageNumber - 1) * pageSize, pageNumber * pageSize),
+    visibleList: list.slice((pageNumber - 1) * pageSize, pageNumber * pageSize),
     numPages: Math.ceil(list.length / pageSize),
   };
 };

src/data/redux/app/selectors/currentList.test.js

@@ -171,7 +171,7 @@ describe('courseList selector module', () => {
     });
     it('returns visible page based on passed page size and stored pageNumber', () => {
       // page 3, 2 per page. [0 1] [2 3] [4 5] ...
-      expect(out.visible).toEqual([testList[4], testList[5]]);
+      expect(out.visibleList).toEqual([testList[4], testList[5]]);
     });
     it('returns number of pages based on page size and list length', () => {
       expect(out.numPages).toEqual(6);

src/data/redux/app/selectors/simpleSelectors.js

@@ -15,6 +15,7 @@ export const simpleSelectors = StrictDict({
   enterpriseDashboard: mkSimpleSelector(app => app.enterpriseDashboard || {}),
   selectSessionModal: mkSimpleSelector(app => app.selectSessionModal),
   pageNumber: mkSimpleSelector(app => app.pageNumber),
+  filters: mkSimpleSelector(app => app.filters),
   socialShareSettings: mkSimpleSelector(app => app.socialShareSettings),
 });
 

src/data/redux/hooks/app.js

@@ -9,6 +9,7 @@ const actions = redux.actions.app;
 
 /** Simple Selectors **/
 export const usePageNumber = () => useSelector(selectors.pageNumber);
+export const useFilters = () => useSelector(selectors.filters);
 export const useEmailConfirmationData = () => useSelector(selectors.emailConfirmation);
 export const useEnterpriseDashboardData = () => useSelector(selectors.enterpriseDashboard);
 export const usePlatformSettingsData = () => useSelector(selectors.platformSettings);
@@ -77,6 +78,26 @@ export const useSetPageNumber = () => {
   return (value) => dispatch(actions.setPageNumber(value));
 };
 
+export const useSetFilters = () => {
+  const dispatch = useDispatch();
+  return (value) => dispatch(actions.setFilters(value));
+};
+
+export const useAddFilter = () => {
+  const dispatch = useDispatch();
+  return (value) => dispatch(actions.addFilter(value));
+};
+
+export const useRemoveFilter = () => {
+  const dispatch = useDispatch();
+  return (value) => dispatch(actions.removeFilter(value));
+};
+
+export const useClearFilters = () => {
+  const dispatch = useDispatch();
+  return (value) => dispatch(actions.clearFilters(value));
+};
+
 export const useLoadData = () => {
   const dispatch = useDispatch();
   return ({ courses, ...globalData }) => {

src/data/redux/index.js

@@ -10,6 +10,14 @@ const modules = {
   requests,
 };
 
+/**
+ * Extracts keys from the modules object and the provided propName parameter to locate the
+ * corresponding object for that propName.
+ * Example: moduleProps('reducer') will return an aggregated object containing the reducer for each module
+ *
+ * @param {string} propName Used to locate the prop in each module
+ * @returns {object} Aggregated values for the provided propName
+ */
 const moduleProps = (propName) => Object.keys(modules).reduce(
   (obj, moduleKey) => {
     const value = modules[moduleKey][propName];