refactor: course filters (#303)

Co-authored-by: Maxwell Frank <[email protected]>

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,18 +26,14 @@ export const ActiveCourseFilters = ({
           {formatMessage(messages[filter])}
         </Chip>
       ))}
-      <Button variant="link" onClick={setFilters.clear}>
+      <Button variant="link" onClick={clearFilters}>
         {formatMessage(messages.clearAll)}
       </Button>
     </div>
   );
 };
 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,22 +3,33 @@ import { useToggle } from '@openedx/paragon';
 
 import { StrictDict } from 'utils';
 import track from 'tracking';
+import { reduxHooks } from 'hooks';
 
 import * as module from './hooks';
 
 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">