Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: more consistency and clarity around late enrollment variables #1131

Merged
merged 1 commit into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/app/data/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export { default as useIsAssignmentsOnlyLearner } from './useIsAssignmentsOnlyLe
export { default as useNProgressLoader } from './useNProgressLoader';
export { default as useNotices } from './useNotices';
export { default as useLearnerSkillLevels } from './useLearnerSkillLevels';
export { default as useLateRedemptionBufferDays } from './useLateRedemptionBufferDays';
export { default as useLateEnrollmentBufferDays } from './useLateEnrollmentBufferDays';
export { default as useProgramDetails } from './useProgramDetails';
export { default as useLearnerProgramProgressData } from './useLearnerProgramProgressData';
export { default as useLearnerPathwayProgressData } from './useLearnerPathwayProgressData';
Expand Down
6 changes: 3 additions & 3 deletions src/components/app/data/hooks/useCourseMetadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useParams, useSearchParams } from 'react-router-dom';

import { queryCourseMetadata } from '../queries';
import { getAvailableCourseRuns } from '../utils';
import useLateRedemptionBufferDays from './useLateRedemptionBufferDays';
import useLateEnrollmentBufferDays from './useLateEnrollmentBufferDays';

/**
* Retrieves the course metadata for the given enterprise customer and course key.
Expand All @@ -16,7 +16,7 @@ export default function useCourseMetadata(queryOptions = {}) {
// `requestUrl.searchParams` uses `URLSearchParams`, which decodes `+` as a space, so we
// need to replace it with `+` again to be a valid course run key.
const courseRunKey = searchParams.get('course_run_key')?.replaceAll(' ', '+');
const isEnrollableBufferDays = useLateRedemptionBufferDays({
const lateEnrollmentBufferDays = useLateEnrollmentBufferDays({
enabled: !!courseKey,
});
return useQuery({
Expand All @@ -27,7 +27,7 @@ export default function useCourseMetadata(queryOptions = {}) {
if (!data) {
return data;
}
const availableCourseRuns = getAvailableCourseRuns({ course: data, isEnrollableBufferDays });
const availableCourseRuns = getAvailableCourseRuns({ course: data, lateEnrollmentBufferDays });
const transformedData = {
...data,
availableCourseRuns,
Expand Down
6 changes: 3 additions & 3 deletions src/components/app/data/hooks/useCourseMetadata.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import dayjs from 'dayjs';
import { useParams, useSearchParams } from 'react-router-dom';
import { queryClient } from '../../../../utils/tests';
import { fetchCourseMetadata } from '../services';
import useLateRedemptionBufferDays from './useLateRedemptionBufferDays';
import useLateEnrollmentBufferDays from './useLateEnrollmentBufferDays';
import useCourseMetadata from './useCourseMetadata';

jest.mock('./useEnterpriseCustomer');
jest.mock('./useLateRedemptionBufferDays');
jest.mock('./useLateEnrollmentBufferDays');

jest.mock('../services', () => ({
...jest.requireActual('../services'),
Expand Down Expand Up @@ -41,7 +41,7 @@ describe('useCourseMetadata', () => {
jest.clearAllMocks();
fetchCourseMetadata.mockResolvedValue(mockCourseMetadata);
useParams.mockReturnValue({ courseKey: 'edX+DemoX' });
useLateRedemptionBufferDays.mockReturnValue(undefined);
useLateEnrollmentBufferDays.mockReturnValue(undefined);
useSearchParams.mockReturnValue([new URLSearchParams({ course_run_key: 'course-v1:edX+DemoX+T2024' })]);
});
it('should handle resolved value correctly with no select function passed', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useQuery } from '@tanstack/react-query';
import useCourseMetadata from './useCourseMetadata';
import { queryCanRedeem } from '../queries';
import useEnterpriseCustomer from './useEnterpriseCustomer';
import useLateRedemptionBufferDays from './useLateRedemptionBufferDays';
import useLateEnrollmentBufferDays from './useLateEnrollmentBufferDays';

export function transformCourseRedemptionEligibility({
courseMetadata,
Expand Down Expand Up @@ -45,10 +45,10 @@ export default function useCourseRedemptionEligibility(queryOptions = {}) {
const { select, ...queryOptionsRest } = queryOptions;
const { data: enterpriseCustomer } = useEnterpriseCustomer();
const { data: courseMetadata } = useCourseMetadata();
const isEnrollableBufferDays = useLateRedemptionBufferDays();
const lateEnrollmentBufferDays = useLateEnrollmentBufferDays();

return useQuery({
...queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, isEnrollableBufferDays),
...queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, lateEnrollmentBufferDays),
enabled: !!courseMetadata,
select: (data) => {
const transformedData = transformCourseRedemptionEligibility({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import useEnterpriseCustomer from './useEnterpriseCustomer';
import { queryClient } from '../../../../utils/tests';
import { fetchCanRedeem } from '../services';
import useCourseMetadata from './useCourseMetadata';
import { useCourseRedemptionEligibility, useLateRedemptionBufferDays } from './index';
import { useCourseRedemptionEligibility, useLateEnrollmentBufferDays } from './index';
import { transformCourseRedemptionEligibility } from './useCourseRedemptionEligibility';

jest.mock('./useEnterpriseCustomer');
jest.mock('./useCourseMetadata');
jest.mock('./useLateRedemptionBufferDays');
jest.mock('./useLateEnrollmentBufferDays');
jest.mock('../services', () => ({
...jest.requireActual('../services'),
fetchCanRedeem: jest.fn().mockResolvedValue(null),
Expand Down Expand Up @@ -86,7 +86,7 @@ describe('useCourseRedemptionEligibility', () => {
fetchCanRedeem.mockResolvedValue(mockCanRedeemData);
useParams.mockReturnValue({ courseRunKey: mockCourseRunKey });
useCourseMetadata.mockReturnValue({ data: mockCourseMetadata });
useLateRedemptionBufferDays.mockReturnValue(undefined);
useLateEnrollmentBufferDays.mockReturnValue(undefined);
});
it('should handle resolved value correctly', async () => {
const { result, waitForNextUpdate } = renderHook(() => useCourseRedemptionEligibility(), { wrapper: Wrapper });
Expand Down
8 changes: 8 additions & 0 deletions src/components/app/data/hooks/useLateEnrollmentBufferDays.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { getLateEnrollmentBufferDays } from '../utils';
import useRedeemablePolicies from './useRedeemablePolicies';

export default function useLateEnrollmentBufferDays(queryOptions = {}) {
const { data } = useRedeemablePolicies(queryOptions);
const { redeemablePolicies } = data || {};
return getLateEnrollmentBufferDays(redeemablePolicies);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import dayjs from 'dayjs';
import { AppContext } from '@edx/frontend-platform/react';
import { queryClient } from '../../../../utils/tests';
import { fetchRedeemablePolicies } from '../services';
import useLateRedemptionBufferDays from './useLateRedemptionBufferDays';
import useLateEnrollmentBufferDays from './useLateEnrollmentBufferDays';
import { authenticatedUserFactory, enterpriseCustomerFactory } from '../services/data/__factories__';
import useEnterpriseCustomer from './useEnterpriseCustomer';
import { LATE_ENROLLMENTS_BUFFER_DAYS } from '../../../../config/constants';
Expand Down Expand Up @@ -63,7 +63,7 @@ const mockRedeemablePolicies = {
};
const mockEnterpriseCustomer = enterpriseCustomerFactory();
const mockAuthenticatedUser = authenticatedUserFactory();
describe('useLateRedemptionBufferDays', () => {
describe('useLateEnrollmentBufferDays', () => {
const Wrapper = ({ children }) => (
<QueryClientProvider client={queryClient()}>
<AppContext.Provider value={{ authenticatedUser: mockAuthenticatedUser }}>
Expand All @@ -77,7 +77,7 @@ describe('useLateRedemptionBufferDays', () => {
fetchRedeemablePolicies.mockResolvedValue(mockRedeemablePolicies);
});
it('should handle resolved value correctly', async () => {
const { result, waitForNextUpdate } = renderHook(() => useLateRedemptionBufferDays(), { wrapper: Wrapper });
const { result, waitForNextUpdate } = renderHook(() => useLateEnrollmentBufferDays(), { wrapper: Wrapper });
await waitForNextUpdate();
expect(result.current).toEqual(LATE_ENROLLMENTS_BUFFER_DAYS);
});
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('useLateRedemptionBufferDays', () => {
};
fetchRedeemablePolicies.mockResolvedValue(updatedMockRedeemablePolicies);

const { result, waitForNextUpdate } = renderHook(() => useLateRedemptionBufferDays(), { wrapper: Wrapper });
const { result, waitForNextUpdate } = renderHook(() => useLateEnrollmentBufferDays(), { wrapper: Wrapper });
await waitForNextUpdate();
expect(result.current).toEqual(undefined);
});
Expand Down
8 changes: 0 additions & 8 deletions src/components/app/data/hooks/useLateRedemptionBufferDays.js

This file was deleted.

4 changes: 2 additions & 2 deletions src/components/app/data/queries/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,10 @@ export function queryCanRedeemContextQueryKey(enterpriseUuid, courseKey) {
* ._ctx.canRedeem(availableCourseRunKeys)
* @returns {Types.QueryOptions}
*/
export function queryCanRedeem(enterpriseUuid, courseMetadata, isEnrollableBufferDays) {
export function queryCanRedeem(enterpriseUuid, courseMetadata, lateEnrollmentBufferDays) {
const availableCourseRuns = getAvailableCourseRuns({
course: courseMetadata,
isEnrollableBufferDays,
lateEnrollmentBufferDays,
});
const availableCourseRunKeys = availableCourseRuns.map(({ key }) => key);
return queries
Expand Down
17 changes: 8 additions & 9 deletions src/components/app/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ export function retrieveErrorMessage(error) {
* @returns {number|undefined} - Returns the number of late redemption buffer days
* if any policy has late redemption enabled.
*/
export function getLateRedemptionBufferDays(redeemablePolicies) {
export function getLateEnrollmentBufferDays(redeemablePolicies) {
if (!redeemablePolicies) {
return undefined;
}
Expand All @@ -443,8 +443,7 @@ export function getLateRedemptionBufferDays(redeemablePolicies) {
// itself back to False after a finite period of time.
policy.isLateRedemptionEnabled
));
const isEnrollableBufferDays = anyPolicyHasLateRedemptionEnabled ? LATE_ENROLLMENTS_BUFFER_DAYS : undefined;
return isEnrollableBufferDays;
return anyPolicyHasLateRedemptionEnabled ? LATE_ENROLLMENTS_BUFFER_DAYS : undefined;
}

// See https://2u-internal.atlassian.net/wiki/spaces/WS/pages/8749811/Enroll+button+and+Course+Run+Selector+Logic
Expand All @@ -466,10 +465,10 @@ export function isArchived(courseRun) {
* This function is used by logic that determines which runs should be visible on the course about page.
*
* @param {object} course - The course containing runs which will be a superset of the returned runs.
* @param {number} isEnrollableBufferDays - number of days to buffer the enrollment end date, or undefined.
* @param {number} lateEnrollmentBufferDays - number of days to buffer the enrollment end date, or undefined.
* @returns List of course runs.
*/
export function getAvailableCourseRuns({ course, isEnrollableBufferDays }) {
export function getAvailableCourseRuns({ course, lateEnrollmentBufferDays }) {
if (!course?.courseRuns) {
return [];
}
Expand All @@ -483,7 +482,7 @@ export function getAvailableCourseRuns({ course, isEnrollableBufferDays }) {
// but the rules around the following fields are relaxed:
//
// * courseRun.isEnrollable: This field represents the enrollment window actually stored in the database. However,
// during late enrollment we expand the end date of the enrollment window by isEnrollableBufferDays.
// during late enrollment we expand the end date of the enrollment window by lateEnrollmentBufferDays.
// * courseRun.isMarketable: This field is True when the run is published, has seats, and has a marketing URL. Since
// late enrollment potentially means enrolling into an unpublished run, we must ignore the run state.
const lateEnrollmentAvailableCourseRunsFilter = (courseRun) => {
Expand All @@ -501,13 +500,13 @@ export function getAvailableCourseRuns({ course, isEnrollableBufferDays }) {
// In cases where we don't expect the buffer to change behavior, fallback to the backend-provided value.
return standardAvailableCourseRunsFilter(courseRun);
}
const bufferedEnrollDeadline = dayjs(courseRun.enrollmentEnd).add(isEnrollableBufferDays, 'day');
const bufferedEnrollDeadline = dayjs(courseRun.enrollmentEnd).add(lateEnrollmentBufferDays, 'day');
return today.isBefore(bufferedEnrollDeadline);
};

// isEnrollableBufferDays is used as a heuristic to determine if the late enrollment feature is enabled.
// lateEnrollmentBufferDays being set is used as a heuristic to determine if the late enrollment feature is enabled.
return course.courseRuns.filter(
isDefinedAndNotNull(isEnrollableBufferDays)
isDefinedAndNotNull(lateEnrollmentBufferDays)
? lateEnrollmentAvailableCourseRunsFilter
: standardAvailableCourseRunsFilter,
);
Expand Down
5 changes: 3 additions & 2 deletions src/components/app/data/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,8 +619,9 @@ describe('getAvailableCourseRuns', () => {
MockDate.set('2023-07-05T00:00:00Z');
expect(getAvailableCourseRuns({ course: sampleCourseRunDataWithRecentRuns.courseData }))
.toEqual(sampleCourseRunDataWithRecentRuns.courseData.courseRuns.slice(0, 1));
expect(getAvailableCourseRuns({ course: sampleCourseRunDataWithRecentRuns.courseData, isEnrollableBufferDays: 60 }))
.toEqual(sampleCourseRunDataWithRecentRuns.courseData.courseRuns.slice(0, 3));
expect(getAvailableCourseRuns(
{ course: sampleCourseRunDataWithRecentRuns.courseData, lateEnrollmentBufferDays: 60 },
)).toEqual(sampleCourseRunDataWithRecentRuns.courseData.courseRuns.slice(0, 3));
});
it('returns empty array if course runs are not available', () => {
sampleCourseRunData.courseData.courseRuns = [];
Expand Down
8 changes: 5 additions & 3 deletions src/components/course/data/courseLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
queryEnterpriseCourseEnrollments,
extractEnterpriseCustomer,
queryRedeemablePolicies,
getLateRedemptionBufferDays,
getLateEnrollmentBufferDays,
querySubscriptions,
queryLicenseRequests,
queryCouponCodeRequests,
Expand Down Expand Up @@ -77,9 +77,11 @@ export default function makeCourseLoader(queryClient) {
enterpriseUuid: enterpriseCustomer.uuid,
lmsUserId: authenticatedUser.userId,
}));
const isEnrollableBufferDays = getLateRedemptionBufferDays(redeemableLearnerCreditPolicies.redeemablePolicies);
const lateEnrollmentBufferDays = getLateEnrollmentBufferDays(
redeemableLearnerCreditPolicies.redeemablePolicies,
);
return queryClient.ensureQueryData(
queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, isEnrollableBufferDays),
queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, lateEnrollmentBufferDays),
);
}),
queryClient.ensureQueryData(queryEnterpriseCourseEnrollments(enterpriseCustomer.uuid)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import {
extractEnterpriseCustomer,
getLateRedemptionBufferDays,
getLateEnrollmentBufferDays,
queryCanRedeem,
queryCourseMetadata,
queryRedeemablePolicies,
Expand Down Expand Up @@ -41,9 +41,9 @@
enterpriseUuid: enterpriseCustomer.uuid,
lmsUserId: authenticatedUser.userId,
}));
const isEnrollableBufferDays = getLateRedemptionBufferDays(redeemableLearnerCreditPolicies.redeemablePolicies);
const lateEnrollmentBufferDays = getLateEnrollmentBufferDays(redeemableLearnerCreditPolicies.redeemablePolicies);

Check warning on line 44 in src/components/course/routes/externalCourseEnrollmentLoader.js

View check run for this annotation

Codecov / codecov/patch

src/components/course/routes/externalCourseEnrollmentLoader.js#L44

Added line #L44 was not covered by tests
const canRedeem = await queryClient.ensureQueryData(
queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, isEnrollableBufferDays),
queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, lateEnrollmentBufferDays),
);
const hasSuccessfulRedemption = !!canRedeem.find(r => r.contentKey === courseRunKey)?.hasSuccessfulRedemption;
if (hasSuccessfulRedemption) {
Expand Down
Loading