From e5aa71855961c31e346a15b57de611bfac4ae93a Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 6 Dec 2023 09:58:45 -0600 Subject: [PATCH] Fixes course run selection code and standardizes it (#2017) --- .../public/src/components/CourseInfoBox.js | 6 +- .../components/CourseProductDetailEnroll.js | 19 +-- .../CourseProductDetailEnroll_test.js | 12 +- .../src/containers/pages/CatalogPage.js | 4 +- frontend/public/src/lib/courseApi.js | 50 +++++++ frontend/public/src/lib/courseApi_test.js | 137 +++++++++++++++++- 6 files changed, 202 insertions(+), 26 deletions(-) diff --git a/frontend/public/src/components/CourseInfoBox.js b/frontend/public/src/components/CourseInfoBox.js index f24022b9f..d71ac2ec9 100644 --- a/frontend/public/src/components/CourseInfoBox.js +++ b/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 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 = diff --git a/frontend/public/src/components/CourseProductDetailEnroll.js b/frontend/public/src/components/CourseProductDetailEnroll.js index 3b063fa2b..af003b659 100644 --- a/frontend/public/src/components/CourseProductDetailEnroll.js +++ b/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,8 +626,8 @@ 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 @@ -642,7 +635,7 @@ export class CourseProductDetailEnroll extends React.Component< } } - return ( + return run ? ( <> { // $FlowFixMe: isLoading null or undefined @@ -680,7 +673,7 @@ export class CourseProductDetailEnroll extends React.Component< ) : null} - ) + ) : null } } diff --git a/frontend/public/src/components/CourseProductDetailEnroll_test.js b/frontend/public/src/components/CourseProductDetailEnroll_test.js index f5f9753e8..f7f5dcf53 100644 --- a/frontend/public/src/components/CourseProductDetailEnroll_test.js +++ b/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: [], diff --git a/frontend/public/src/containers/pages/CatalogPage.js b/frontend/public/src/containers/pages/CatalogPage.js index 66e144e73..f8149735a 100644 --- a/frontend/public/src/containers/pages/CatalogPage.js +++ b/frontend/public/src/containers/pages/CatalogPage.js @@ -401,7 +401,7 @@ export class CatalogPage extends React.Component { */ renderCourseCatalogCard(course: CourseDetailWithRuns) { return ( -
  • +
  • { */ renderProgramCatalogCard(program: Program) { return ( -
  • +
  • diff --git a/frontend/public/src/lib/courseApi.js b/frontend/public/src/lib/courseApi.js index 88e4370fc..c0de0a63a 100644 --- a/frontend/public/src/lib/courseApi.js +++ b/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 +) => { + /* + 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 - 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 + ) +} diff --git a/frontend/public/src/lib/courseApi_test.js b/frontend/public/src/lib/courseApi_test.js index 575700cdd..bba364461 100644 --- a/frontend/public/src/lib/courseApi_test.js +++ b/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]) + }) + }) + }) })