Skip to content

Commit

Permalink
clean up PR to clarify intent for ENT-9410
Browse files Browse the repository at this point in the history
  • Loading branch information
pwnage101 committed Oct 21, 2024
1 parent 4c6f3fc commit 623a0af
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 145 deletions.
30 changes: 8 additions & 22 deletions src/components/app/data/hooks/useCourseMetadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@ import {
determineAllocatedAssignmentsForCourse,
getAvailableCourseRuns,
transformCourseMetadataByAllocatedCourseRunAssignments,
isRunUnrestricted,
} from '../utils';
import useLateEnrollmentBufferDays from './useLateEnrollmentBufferDays';
import useRedeemablePolicies from './useRedeemablePolicies';
import useEnterpriseCustomerContainsContent from './useEnterpriseCustomerContainsContent';

/**
* Retrieves the course metadata for the given enterprise customer and course key.
* @returns {Types.UseQueryResult}} The query results for the course metadata.
*/
export default function useCourseMetadata(queryOptions = {}, catalogUuid = undefined) {
export default function useCourseMetadata(queryOptions = {}) {
const { select, ...queryOptionsRest } = queryOptions;
const { courseKey } = useParams();
const [searchParams] = useSearchParams();
Expand All @@ -26,11 +24,6 @@ export default function useCourseMetadata(queryOptions = {}, catalogUuid = undef
hasAssignedCourseRuns,
hasMultipleAssignedCourseRuns,
} = determineAllocatedAssignmentsForCourse({ courseKey, redeemableLearnerCreditPolicies });
const {
data: {
restrictedRunsAllowed,
},
} = useEnterpriseCustomerContainsContent([courseKey]);
// `requestUrl.searchParams` uses `URLSearchParams`, which decodes `+` as a space, so we
// need to replace it with `+` again to be a valid course run key.
let courseRunKey = searchParams.get('course_run_key')?.replaceAll(' ', '+');
Expand All @@ -49,22 +42,15 @@ export default function useCourseMetadata(queryOptions = {}, catalogUuid = undef
if (!data) {
return data;
}
// First stage filters out any runs that are unavailable for universal reasons, such as enrollment windows and
// published states.
const basicAvailableCourseRuns = getAvailableCourseRuns({ course: data, lateEnrollmentBufferDays });
// Second stage filters out any *restricted* runs that are not redeemable via a specific catalog (if
// catalogUuid is defined), or via any of the customer's catalogs (if catalogUuid is undefined).
const availableAndUnrestrictedCourseRuns = basicAvailableCourseRuns.filter(
courseRunMetadata => isRunUnrestricted({
restrictedRunsAllowed,
courseKey,
courseRunMetadata,
catalogUuid,
}),
);
// NOTE: The results from this call includes restricted runs, some of
// which might not be ACTUALLY available depending on the subsidy being
// applied. However, we don't know the subsidy being applied at this
// point of the code, so just return all of the basically available
// restricted runs regardless of catalog inclusion.
const availableCourseRuns = getAvailableCourseRuns({ course: data, lateEnrollmentBufferDays });
let transformedData = {
...data,
availableCourseRuns: availableAndUnrestrictedCourseRuns,
availableCourseRuns,
};
// This logic should appropriately handle multiple course runs being assigned, and return the appropriate metadata
transformedData = transformCourseMetadataByAllocatedCourseRunAssignments({
Expand Down
93 changes: 12 additions & 81 deletions src/components/app/data/hooks/useCourseMetadata.test.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { v4 as uuidv4 } from 'uuid';
import { renderHook } from '@testing-library/react-hooks';
import { QueryClientProvider } from '@tanstack/react-query';
import dayjs from 'dayjs';
Expand Down Expand Up @@ -116,7 +115,11 @@ describe('useCourseMetadata', () => {
expect.objectContaining({
data: {
...mockCourseMetadata,
availableCourseRuns: [mockCourseMetadata.courseRuns[0]],
availableCourseRuns: [
mockCourseMetadata.courseRuns[0],
mockCourseMetadata.courseRuns[2],
mockCourseMetadata.courseRuns[3],
],
},
isLoading: false,
isFetching: false,
Expand Down Expand Up @@ -148,7 +151,11 @@ describe('useCourseMetadata', () => {
original: mockCourseMetadata,
transformed: {
...mockCourseMetadata,
availableCourseRuns: [mockCourseMetadata.courseRuns[0]],
availableCourseRuns: [
mockCourseMetadata.courseRuns[0],
mockCourseMetadata.courseRuns[2],
mockCourseMetadata.courseRuns[3],
],
},
},
isLoading: false,
Expand Down Expand Up @@ -178,10 +185,10 @@ describe('useCourseMetadata', () => {

const availableCourseRuns = [
mockCourseMetadata.courseRuns[0], // This is marketable, enrollable, unrestricted, etc.
mockCourseMetadata.courseRuns[2], // This is restricted, but that doesn't disqualify it.
];
const unavailableCourseRuns = [
mockCourseMetadata.courseRuns[1], // This one is unavailable due to being unmarketable.
mockCourseMetadata.courseRuns[2], // This one is unavailable due to being restricted.
];
// Copy all the above runs to make both assigned and unassigned versions.
const courseRunsMatrix = {
Expand Down Expand Up @@ -301,7 +308,7 @@ describe('useCourseMetadata', () => {
expect.objectContaining({
data: {
...mockCourseMetadata,
// The requested run is available, unrestricted, and assigned, so should appear in both lists below:
// The requested run is available and assigned, so should appear in both lists below:
courseRuns: mockCourseRun,
availableCourseRuns: mockCourseRun,
},
Expand All @@ -310,80 +317,4 @@ describe('useCourseMetadata', () => {
}),
);
});
it('should return restricted course run if allowed for customer', async () => {
useParams.mockReturnValue({ courseKey: 'edX+DemoX' });
useLateEnrollmentBufferDays.mockReturnValue(undefined);
useSearchParams.mockReturnValue([new URLSearchParams({})]);
useEnterpriseCustomerContainsContent.mockReturnValue({
data: {
restrictedRunsAllowed: {
'edX+DemoX': {
// The restricted run (index 2) is simulated to be "allowed" for some of the customer's catalogs.
// This should be enough to get it to be included in the output.
[mockCourseMetadata.courseRuns[2].key]: { catalogUuids: [uuidv4(), uuidv4()] },
},
},
},
});

// This test is all about if the restricted run is allowed for ANY catalog
// for the customer, so do not pass a specific catalog UUID.
const { result, waitForNextUpdate } = renderHook(() => useCourseMetadata(), { wrapper: Wrapper });
await waitForNextUpdate();

expect(result.current).toEqual(
expect.objectContaining({
data: {
...mockCourseMetadata,
availableCourseRuns: [
mockCourseMetadata.courseRuns[0], // Include the completely unrestricted run.
// skip unmarketable run `mockCourseMetadata.courseRuns[1]`.
mockCourseMetadata.courseRuns[2], // Include restricted, but allowed runs.
// skip restricted run `mockCourseMetadata.courseRuns[3]` as it isn't part of any catalog for the customer.
],
},
isLoading: false,
isFetching: false,
}),
);
});
it('should return restricted course run if allowed for specific catalog', async () => {
useParams.mockReturnValue({ courseKey: 'edX+DemoX' });
useLateEnrollmentBufferDays.mockReturnValue(undefined);
useSearchParams.mockReturnValue([new URLSearchParams({})]);
const catalogA = uuidv4();
const catalogB = uuidv4();
useEnterpriseCustomerContainsContent.mockReturnValue({
data: {
restrictedRunsAllowed: {
'edX+DemoX': {
// The restricted runs (index 2 & 3) are simulated to be "allowed" for different catalogs.
// Only one will get to be included in the output.
[mockCourseMetadata.courseRuns[2].key]: { catalogUuids: [catalogA] },
[mockCourseMetadata.courseRuns[3].key]: { catalogUuids: [catalogB] },
},
},
},
});

// This test is all about if the restricted run is allowed for a SPECIFIC catalog (catalogA).
const { result, waitForNextUpdate } = renderHook(() => useCourseMetadata({}, catalogA), { wrapper: Wrapper });
await waitForNextUpdate();

expect(result.current).toEqual(
expect.objectContaining({
data: {
...mockCourseMetadata,
availableCourseRuns: [
mockCourseMetadata.courseRuns[0], // Include the completely unrestricted run.
// skip unmarketable run `mockCourseMetadata.courseRuns[1]`.
mockCourseMetadata.courseRuns[2], // Include the restricted, but allowed run.
// skip restricted run `mockCourseMetadata.courseRuns[3]` DESPITE its inclusion in one catalog.
],
},
isLoading: false,
isFetching: false,
}),
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ export default function useCourseRedemptionEligibility(queryOptions = {}) {
...queryCanRedeem(enterpriseCustomer.uuid, courseMetadata, lateEnrollmentBufferDays),
enabled: !!courseMetadata,
select: (data) => {
// Among other things, transformCourseRedemptionEligibility() removes
// restricted runs that fail the policy's can-redeem check.
const transformedData = transformCourseRedemptionEligibility({
courseMetadata,
canRedeemData: data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ const mockExpectedUseCouseRedemptionEligibilityReturn = transformCourseRedemptio
courseMetadata: mockCourseMetadata,
courseRunKey: mockCourseRunKey,
canRedeemData: mockCanRedeemData,
restrictedRunsAllowed: undefined,
});

describe('useCourseRedemptionEligibility', () => {
Expand Down
42 changes: 1 addition & 41 deletions src/components/app/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,18 +539,11 @@ export function getAvailableCourseRuns({ course, lateEnrollmentBufferDays }) {
return today.isBefore(bufferedEnrollDeadline);
};

const availableCourseRuns = course.courseRuns.filter(
return course.courseRuns.filter(
isDefinedAndNotNull(lateEnrollmentBufferDays)
? lateEnrollmentAvailableCourseRunsFilter
: standardAvailableCourseRunsFilter,
);

// ENT-9359 (epic for Custom Presentations/Restricted Runs):
// Temporarily hide all restricted runs unconditionally on the course about
// page during implementation of the overall feature. ENT-9410 is most likely
// the ticket to replace this code with something to actually show restricted
// runs conditionally.
return availableCourseRuns.filter((courseRun) => !courseRun.restrictionType);
}

export function getCatalogsForSubsidyRequests({
Expand Down Expand Up @@ -894,36 +887,3 @@ export function transformCourseMetadataByAllocatedCourseRunAssignments({
}
return courseMetadata;
}

/*
* Determine if a given run is unrestricted for either a SPECIFIC CATALOG or ANY CATALOG for the current customer.
*
* By centralizing the restricted run checking logic (i.e. limit any additional code that accesses
* restrictedRunsAllowed) there will be only one place to edit/fix it.
*
* DEPRECATED
*/
export function isRunUnrestricted({
restrictedRunsAllowed,
courseKey,
courseRunMetadata,
catalogUuid,
}) {
if (!courseRunMetadata?.restrictionType) {
return true;
}
if (courseRunMetadata?.restrictionType !== ENTERPRISE_RESTRICTION_TYPE) {
return false;
}
// Get all the catalogs (for one customer) that have access to a specific restricted run.
const allowedCatalogs = restrictedRunsAllowed?.[courseKey]?.[courseRunMetadata.key]?.catalogUuids;
if (!(allowedCatalogs instanceof Array)) {
return false;
}
if (catalogUuid) {
// If a catalogUuid is supplied, determine if the given run is unrestricted for a SPECIFIC CATALOG.
return allowedCatalogs.includes(catalogUuid);
}
// If a catalogUuid is not supplied, determine if the given run is unrestricted for ANY CATALOG for the customer.
return allowedCatalogs.length > 0;
}

0 comments on commit 623a0af

Please sign in to comment.