Fixes course run selection code and standardizes it (#2017)

frontend/public/src/components/CourseInfoBox.js

@@ -6,6 +6,7 @@ import {
   formatLocalePrice,
   getStartDateText
 } from "../lib/util"
+import { getFirstRelevantRun } from "../lib/courseApi"
 import moment from "moment-timezone"
 
 import type { BaseCourseRun } from "../flow/courseTypes"
@@ -51,10 +52,7 @@ export default class CourseInfoBox extends React.PureComponent<CourseInfoBoxProp
     }
 
     const course = courses[0]
-
-    const run = course.next_run_id
-      ? course.courseruns.find(elem => elem.id === course.next_run_id)
-      : course.courseruns[0]
+    const run = getFirstRelevantRun(course, courseRuns)
     const product = run && run.products.length > 0 && run.products[0]
 
     const isArchived =

frontend/public/src/components/CourseProductDetailEnroll.js

@@ -29,6 +29,7 @@ import {
 import { formatPrettyDate, emptyOrNil } from "../lib/util"
 import moment from "moment-timezone"
 import {
+  getFirstRelevantRun,
   isFinancialAssistanceAvailable,
   isWithinEnrollmentPeriod
 } from "../lib/courseApi"
@@ -198,15 +199,6 @@ export class CourseProductDetailEnroll extends React.Component<
     }
   }
 
-  getFirstUnexpiredRun = () => {
-    const { courses, courseRuns } = this.props
-    return courseRuns
-      ? courses && courses[0].next_run_id
-        ? courseRuns.find(elem => elem.id === courses[0].next_run_id)
-        : courseRuns[0]
-      : null
-  }
-
   getCurrentCourseRun = (): EnrollmentFlaggedCourseRun => {
     return this.state.currentCourseRun
   }
@@ -298,6 +290,7 @@ export class CourseProductDetailEnroll extends React.Component<
   }
 
   updateDate(run: EnrollmentFlaggedCourseRun) {
+    // for original design - not used in course infobox design
     let date = emptyOrNil(run.start_date)
       ? undefined
       : moment(new Date(run.start_date))
@@ -633,16 +626,16 @@ export class CourseProductDetailEnroll extends React.Component<
     let run,
       product = null
 
-    if (courseRuns) {
-      run = this.getFirstUnexpiredRun()
+    if (courses && courseRuns) {
+      run = getFirstRelevantRun(courses[0], courseRuns)
 
       if (run) {
         product = run && run.products ? run.products[0] : null
         this.updateDate(run)
       }
     }
 
-    return (
+    return run ? (
       <>
         {
           // $FlowFixMe: isLoading null or undefined
@@ -680,7 +673,7 @@ export class CourseProductDetailEnroll extends React.Component<
           </>
         ) : null}
       </>
-    )
+    ) : null
   }
 }
 

frontend/public/src/components/CourseProductDetailEnroll_test.js

@@ -592,14 +592,17 @@ describe("CourseProductDetailEnroll", () => {
 
   it("CourseInfoBox renders a date selector with Enrolled text if the user is enrolled in one", async () => {
     const secondCourseRun = makeCourseRunDetail()
-    const enrollment = {
+    const enrollmentOne = {
       ...makeCourseRunEnrollment(),
       run: secondCourseRun
     }
-
+    const enrollmentTwo = {
+      ...makeCourseRunEnrollment(),
+      run: courseRun
+    }
     const entities = {
       currentUser: currentUser,
-      enrollments: [enrollment],
+      enrollments: [enrollmentOne, enrollmentTwo],
       courseRuns:  [courseRun, secondCourseRun]
     }
 
@@ -618,6 +621,7 @@ describe("CourseProductDetailEnroll", () => {
     assert.isTrue(selectorBar.exists())
 
     const enrolledItem = infobox.find(".more-dates-link.enrolled")
+
     assert.isTrue(enrolledItem.exists())
   })
 
@@ -642,8 +646,6 @@ describe("CourseProductDetailEnroll", () => {
       courseruns: [courseRun]
     }
 
-    console.log(course)
-
     const entities = {
       currentUser: currentUser,
       enrollments: [],

frontend/public/src/containers/pages/CatalogPage.js

@@ -401,7 +401,7 @@ export class CatalogPage extends React.Component<Props> {
    */
   renderCourseCatalogCard(course: CourseDetailWithRuns) {
     return (
-      <li>
+      <li key={`course-card-${course.id}`}>
         <a href={course.page.page_url} key={course.id}>
           <div className="col catalog-item">
             <img
@@ -427,7 +427,7 @@ export class CatalogPage extends React.Component<Props> {
    */
   renderProgramCatalogCard(program: Program) {
     return (
-      <li>
+      <li key={`program-card-${program.id}`}>
         <a href={program.page.page_url} key={program.id}>
           <div className="col catalog-item">
             <div className="program-image-and-badge">

frontend/public/src/lib/courseApi.js

@@ -12,6 +12,7 @@ import type Moment from "moment"
 import type {
   CourseRunDetail,
   CourseRun,
+  CourseDetail,
   RequirementNode,
   LearnerRecord,
   ProgramRequirement,
@@ -216,3 +217,52 @@ export const learnerProgramIsCompleted = (learnerRecord: LearnerRecord) => {
 
   return requirementsDone && electivesDone
 }
+
+export const getFirstRelevantRun = (
+  course: CourseDetail,
+  courseRuns: Array<CourseRunDetail>
+) => {
+  /*
+   Finds the next most relevant course run:
+   - If the course has a next_run_id, return that run
+   - If there are future runs, return the run closer to now
+   - If all runs are in the past, return the most recently ended run
+   - If there are no runs, return null
+
+   Args:
+   course: CourseDetail - the course itself
+   courseRuns: Array<CourseRunDetail> - the course runs to search
+
+   Returns: CourseRunDetail
+  */
+
+  if (course.courseruns.length === 0 || courseRuns.length === 0) {
+    return null
+  }
+
+  if (course.next_run_id) {
+    return courseRuns.find(run => run.id === course.next_run_id)
+  }
+
+  const now = moment()
+
+  if (
+    courseRuns.some(
+      run => run.start_date && moment(run.start_date).isSameOrAfter(now)
+    )
+  ) {
+    return courseRuns
+      .filter(
+        run => run.start_date && moment(run.start_date).isSameOrAfter(now)
+      )
+      .reduce((prev, curr) =>
+        moment(curr.start_date).isBefore(moment(prev.start_date)) ? curr : prev
+      )
+  }
+
+  return courseRuns
+    .filter(run => run.start_date && moment(run.start_date).isBefore(now))
+    .reduce((prev, curr) =>
+      moment(curr.start_date).isBefore(moment(prev.start_date)) ? prev : curr
+    )
+}

frontend/public/src/lib/courseApi_test.js

@@ -6,7 +6,8 @@ import {
   isFinancialAssistanceAvailable,
   learnerProgramIsCompleted,
   extractCoursesFromNode,
-  walkNodes
+  walkNodes,
+  getFirstRelevantRun
 } from "./courseApi"
 import { assert } from "chai"
 import moment from "moment"
@@ -18,7 +19,8 @@ import {
   makeProgramWithOnlyRequirements,
   makeProgramWithOnlyElectives,
   makeProgramWithNoElectivesOrRequirements,
-  makeProgram
+  makeProgram,
+  makeCourseDetailWithRuns
 } from "../factories/course"
 import { makeUser } from "../factories/user"
 
@@ -32,12 +34,18 @@ describe("Course API", () => {
     farPast = moment()
       .add(-50, "days")
       .toISOString(),
+    farFarPast = moment()
+      .add(-100, "days")
+      .toISOString(),
     future = moment()
       .add(10, "days")
       .toISOString(),
     farFuture = moment()
       .add(50, "days")
       .toISOString(),
+    farFarFuture = moment()
+      .add(100, "days")
+      .toISOString(),
     exampleUrl = "http://example.com"
   let courseRun: CourseRunDetail, user: LoggedInUser
 
@@ -315,4 +323,129 @@ describe("Course API", () => {
       })
     })
   })
+
+  describe("getFirstRelevantRun", () => {
+    it("returns null if there aren't any supplied course runs", () => {
+      const course = makeCourseDetailWithRuns()
+      const result = getFirstRelevantRun(course, [])
+      assert.equal(result, null)
+    })
+
+    it("returns null if there aren't course runs in the course", () => {
+      const testCourse = {
+        ...makeCourseDetailWithRuns(),
+        courseruns: []
+      }
+      const result = getFirstRelevantRun(testCourse, [makeCourseRunDetail()])
+      assert.equal(result, null)
+    })
+    ;[
+      ["in order", true],
+      ["out of order", false]
+    ].forEach(([isInOrderDesc, isInOrder]) => {
+      it(`returns the first run that is in the future and runs are ${isInOrderDesc}`, () => {
+        const runs = [
+          {
+            ...makeCourseRunDetail(),
+            start_date:       future,
+            enrollment_start: future,
+            end_date:         farFuture,
+            enrollment_end:   farFuture
+          },
+          {
+            ...makeCourseRunDetail(),
+            start_date:       farFuture,
+            enrollment_start: farFuture,
+            end_date:         farFarFuture,
+            enrollment_end:   farFarFuture
+          }
+        ]
+
+        if (!isInOrder) {
+          runs.reverse()
+        }
+
+        const testCourse = {
+          ...makeCourseDetailWithRuns(),
+          next_run_id: null,
+          courseruns:  runs
+        }
+
+        const result = getFirstRelevantRun(testCourse, runs)
+        assert.equal(result, isInOrder ? runs[0] : runs[1])
+      })
+    })
+    ;[
+      ["in order", true],
+      ["out of order", false]
+    ].forEach(([isInOrderDesc, isInOrder]) => {
+      it(`returns the first run that is in the future and runs are ${isInOrderDesc}`, () => {
+        const runs = [
+          {
+            ...makeCourseRunDetail(),
+            start_date:       future,
+            enrollment_start: future,
+            end_date:         farFuture,
+            enrollment_end:   farFuture
+          },
+          {
+            ...makeCourseRunDetail(),
+            start_date:       farPast,
+            enrollment_start: farPast,
+            end_date:         past,
+            enrollment_end:   past
+          }
+        ]
+
+        if (!isInOrder) {
+          runs.reverse()
+        }
+
+        const testCourse = {
+          ...makeCourseDetailWithRuns(),
+          next_run_id: null,
+          courseruns:  runs
+        }
+
+        const result = getFirstRelevantRun(testCourse, runs)
+        assert.equal(result, isInOrder ? runs[0] : runs[1])
+      })
+    })
+    ;[
+      ["in order", true],
+      ["out of order", false]
+    ].forEach(([isInOrderDesc, isInOrder]) => {
+      it(`returns the most recent run that is in the past if all runs are done and runs are ${isInOrderDesc}`, () => {
+        const runs = [
+          {
+            ...makeCourseRunDetail(),
+            start_date:       farPast,
+            enrollment_start: farPast,
+            end_date:         past,
+            enrollment_end:   past
+          },
+          {
+            ...makeCourseRunDetail(),
+            start_date:       farFarPast,
+            enrollment_start: farFarPast,
+            end_date:         farPast,
+            enrollment_end:   farPast
+          }
+        ]
+
+        if (!isInOrder) {
+          runs.reverse()
+        }
+
+        const testCourse = {
+          ...makeCourseDetailWithRuns(),
+          next_run_id: null,
+          courseruns:  runs
+        }
+
+        const result = getFirstRelevantRun(testCourse, runs)
+        assert.equal(result, isInOrder ? runs[0] : runs[1])
+      })
+    })
+  })
 })