Fix just the counts from Catalog (#2008)

* holding changes * simplified just counts * prettier and remove unnecessary package * update type info * update tests to include count in payloads for new selectors & to respect that the count is no longer from state * fix fmt
mitodl · Nov 29, 2023 · d1c4560 · d1c4560
1 parent 3dd62ee
commit d1c4560
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 15 deletions.
diff --git a/frontend/public/src/containers/pages/CatalogPage.js b/frontend/public/src/containers/pages/CatalogPage.js
@@ -4,13 +4,15 @@ import moment from "moment"
 import { getStartDateText } from "../../lib/util"
 
 import {
+  coursesCountSelector,
   coursesSelector,
   coursesNextPageSelector,
   coursesQuery,
   coursesQueryKey
 } from "../../lib/queries/catalogCourses"
 
 import {
+  programsCountSelector,
   programsSelector,
   programsNextPageSelector,
   programsQuery,
@@ -37,7 +39,10 @@ type Props = {
   programs: ?Array<Program>,
   forceRequest: () => Promise<*>,
   coursesNextPage: ?string,
-  programsNextPage: ?string
+  programsNextPage: ?string,
+  programsCount: number,
+  coursesCount: number,
+  departments: ?Array<Department>
 }
 
 // Department filter name for all items.
@@ -361,11 +366,32 @@ export class CatalogPage extends React.Component<Props> {
    * Returns the number of courseRuns or programs based on the selected catalog tab.
    */
   renderNumberOfCatalogItems() {
-    const { coursesIsLoading, programsIsLoading } = this.props
-    if (this.state.tabSelected === COURSES_TAB && !coursesIsLoading) {
-      return this.state.filteredCourses.length
-    } else if (this.state.tabSelected === PROGRAMS_TAB && !programsIsLoading) {
-      return this.state.filteredPrograms.length
+    const { coursesCount, programsCount, departments } = this.props
+    if (
+      this.state.tabSelected === PROGRAMS_TAB &&
+      this.state.selectedDepartment === ALL_DEPARTMENTS
+    ) {
+      return programsCount
+    } else if (
+      this.state.tabSelected === PROGRAMS_TAB &&
+      this.state.selectedDepartment !== ALL_DEPARTMENTS
+    ) {
+      return departments.find(
+        department => department.name === this.state.selectedDepartment
+      ).programs
+    }
+    if (
+      this.state.tabSelected === COURSES_TAB &&
+      this.state.selectedDepartment === ALL_DEPARTMENTS
+    ) {
+      return coursesCount
+    } else if (
+      this.state.tabSelected === COURSES_TAB &&
+      this.state.selectedDepartment !== ALL_DEPARTMENTS
+    ) {
+      return departments.find(
+        department => department.name === this.state.selectedDepartment
+      ).courses
     }
   }
 
@@ -645,7 +671,7 @@ const getNextProgramPage = page =>
 const mapPropsToConfig = () => [
   coursesQuery(1),
   programsQuery(1),
-  departmentsQuery()
+  departmentsQuery(1)
 ]
 
 const mapDispatchToProps = {
@@ -655,8 +681,10 @@ const mapDispatchToProps = {
 
 const mapStateToProps = createStructuredSelector({
   courses:              coursesSelector,
+  coursesCount:         coursesCountSelector,
   coursesNextPage:      coursesNextPageSelector,
   programs:             programsSelector,
+  programsCount:        programsCountSelector,
   programsNextPage:     programsNextPageSelector,
   departments:          departmentsSelector,
   coursesIsLoading:     pathOr(true, ["queries", coursesQueryKey, "isPending"]),

diff --git a/frontend/public/src/containers/pages/CatalogPage_test.js b/frontend/public/src/containers/pages/CatalogPage_test.js
@@ -174,6 +174,7 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   1,
             results: courses
           }
         }
@@ -213,9 +214,11 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   1,
             results: courses
           },
           programs: {
+            count:   5,
             results: programs
           },
           departments: [
@@ -511,6 +514,7 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   3,
             results: courses
           },
           programs: {
@@ -596,10 +600,12 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   2,
             results: courses,
             next:    "http://fake.com/api/courses/?page=2"
           },
           programs: {
+            count:   2,
             results: programs
           }
         }
@@ -615,7 +621,8 @@ describe("CatalogPage", function() {
     expect(JSON.stringify(inner.state().allCoursesRetrieved)).equals(
       JSON.stringify(courses)
     )
-    expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
+    // one shows visually, but the total is 2
+    expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
     expect(inner.state().courseQueryPage).equals(1)
 
     // Mock the second page of course API results.
@@ -636,7 +643,7 @@ describe("CatalogPage", function() {
       "GET"
     )
 
-    // Should expect 2 courses to be displayed in the catalog now.
+    // Should expect 2 courses to be visually displayed in the catalog now. Total count should stay 2.
     expect(inner.state().courseQueryPage).equals(2)
     expect(JSON.stringify(inner.state().allCoursesRetrieved)).equals(
       JSON.stringify([displayedCourse, displayedCourse])
@@ -664,10 +671,12 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   1,
             results: courses,
             next:    null
           },
           programs: {
+            count:   1,
             results: programs
           }
         }
@@ -718,10 +727,12 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   1,
             results: courses,
             next:    "http://fake.com/api/courses/?page=2"
           },
           programs: {
+            count:   1,
             results: programs
           }
         }
@@ -777,10 +788,12 @@ describe("CatalogPage", function() {
         },
         entities: {
           courses: {
+            count:   1,
             results: courses,
             next:    "http://fake.com/api/courses/?page=2"
           },
           programs: {
+            count:   2,
             results: programs,
             next:    "http://fake.com/api/courses/?page=2"
           },
@@ -807,14 +820,16 @@ describe("CatalogPage", function() {
     expect(JSON.stringify(inner.state().allProgramsRetrieved)).equals(
       JSON.stringify(programs)
     )
-    expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
+    // While there is only one showing, there are still 2 total. The total should be shown.
+    expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
     expect(inner.state().programQueryPage).equals(1)
 
     // Mock the second page of program API results.
     helper.handleRequestStub.returns({
       body: {
         next:    null,
-        results: programs
+        results: programs,
+        count:   2
       }
     })
 
@@ -836,6 +851,7 @@ describe("CatalogPage", function() {
     expect(JSON.stringify(inner.state().filteredPrograms)).equals(
       JSON.stringify([displayedProgram, displayedProgram])
     )
+    // This should still be 2 because we haven't changed the filter - no matter if one or two have loaded, there are 2
     expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
   })
 })
diff --git a/frontend/public/src/flow/courseTypes.js b/frontend/public/src/flow/courseTypes.js
@@ -147,5 +147,7 @@ export type LearnerRecord = {
 
 export type Department = {
   name: string,
-  id: number
+  id: number,
+  courses: number,
+  programs: number,
 }
diff --git a/frontend/public/src/lib/queries/catalogCourses.js b/frontend/public/src/lib/queries/catalogCourses.js
@@ -4,6 +4,12 @@ import { nextState } from "./util"
 
 export const coursesSelector = pathOr(null, ["entities", "courses", "results"])
 
+export const coursesCountSelector = pathOr(null, [
+  "entities",
+  "courses",
+  "count"
+])
+
 export const coursesNextPageSelector = pathOr(null, [
   "entities",
   "courses",

diff --git a/frontend/public/src/lib/queries/departments.js b/frontend/public/src/lib/queries/departments.js
@@ -6,11 +6,11 @@ export const departmentsSelector = pathOr(null, ["entities", "departments"])
 
 export const departmentsQueryKey = "departments"
 
-export const departmentsQuery = () => ({
+export const departmentsQuery = page => ({
   queryKey:  departmentsQueryKey,
-  url:       `/api/departments`,
+  url:       `/api/v2/departments/?page=${page}`,
   transform: json => ({
-    departments: json
+    departments: json.results
   }),
   update: {
     departments: nextState

diff --git a/frontend/public/src/lib/queries/programs.js b/frontend/public/src/lib/queries/programs.js
@@ -14,6 +14,12 @@ export const programsNextPageSelector = pathOr(null, [
   "next"
 ])
 
+export const programsCountSelector = pathOr(null, [
+  "entities",
+  "programs",
+  "count"
+])
+
 export const programsQueryKey = "programs"
 
 export const programsQuery = page => ({