chore: PR feedback

openedx · Aug 12, 2024 · 2313a27 · 2313a27
1 parent 6c4c544
commit 2313a27
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 39 deletions.
diff --git a/src/components/app/data/hooks/useCourseMetadata.js b/src/components/app/data/hooks/useCourseMetadata.js
@@ -3,9 +3,9 @@ import { useParams, useSearchParams } from 'react-router-dom';
 
 import { queryCourseMetadata } from '../queries';
 import {
-  determineAllocatedCourseRuns,
+  determineAllocatedCourseRunAssignmentsForCourse,
   getAvailableCourseRuns,
-  transformCourseMetadataByAllocationCourseRun,
+  transformCourseMetadataByAllocatedCourseRunAssignments,
 } from '../utils';
 import useLateEnrollmentBufferDays from './useLateEnrollmentBufferDays';
 import useRedeemablePolicies from './useRedeemablePolicies';
@@ -23,13 +23,14 @@ export default function useCourseMetadata(queryOptions = {}) {
     allocatedCourseRunAssignmentKeys,
     hasAssignedCourseRuns,
     hasMultipleAssignedCourseRuns,
-  } = determineAllocatedCourseRuns({
+  } = determineAllocatedCourseRunAssignmentsForCourse({
     courseKey,
     redeemableLearnerCreditPolicies,
   });
   // `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(' ', '+');
+  // only override `courseRunKey` when learner has a single allocated assignment
   if (!courseRunKey && hasAssignedCourseRuns) {
     courseRunKey = hasMultipleAssignedCourseRuns ? null : allocatedCourseRunAssignmentKeys[0];
   }
@@ -50,7 +51,7 @@ export default function useCourseMetadata(queryOptions = {}) {
         availableCourseRuns,
       };
       // This logic should appropriately handle multiple course runs being assigned, and return the appropriate metadata
-      transformedData = transformCourseMetadataByAllocationCourseRun({
+      transformedData = transformCourseMetadataByAllocatedCourseRunAssignments({
         hasMultipleAssignedCourseRuns,
         courseMetadata: transformedData,
         allocatedCourseRunAssignmentKeys,

diff --git a/src/components/app/data/utils.js b/src/components/app/data/utils.js
@@ -748,39 +748,35 @@ export function isEnrollmentUpgradeable(enrollment) {
   return canUpgradeToVerifiedEnrollment;
 }
 
-export function determineAllocatedCourseRuns({
+export function determineAllocatedCourseRunAssignmentsForCourse({
   redeemableLearnerCreditPolicies,
   courseKey,
 }) {
   const { learnerContentAssignments } = redeemableLearnerCreditPolicies;
-  if (learnerContentAssignments.hasAllocatedAssignments) {
-    let allocatedCourseRunAssignments = learnerContentAssignments.allocatedAssignments.filter(
-      (assignment) => assignment?.isAssignedCourseRun,
-    );
-    if (courseKey) {
-      allocatedCourseRunAssignments = allocatedCourseRunAssignments.filter(
-        (assignment) => assignment?.parentContentKey === courseKey,
-      );
-    }
-    const allocatedCourseRunAssignmentKeys = allocatedCourseRunAssignments.map(assignment => assignment.contentKey);
-    const hasAssignedCourseRuns = allocatedCourseRunAssignmentKeys.length > 0;
-    const hasMultipleAssignedCourseRuns = allocatedCourseRunAssignmentKeys.length > 1;
+  // note: checking the non-happy path first, with early return so happy path code isn't nested in conditional.
+  if (!learnerContentAssignments.hasAllocatedAssignments) {
     return {
-      allocatedCourseRunAssignmentKeys,
-      allocatedCourseRunAssignments,
-      hasAssignedCourseRuns,
-      hasMultipleAssignedCourseRuns,
+      allocatedCourseRunAssignmentKeys: [],
+      allocatedCourseRunAssignments: [],
+      hasAssignedCourseRuns: false,
+      hasMultipleAssignedCourseRuns: false,
     };
   }
+  const allocatedCourseRunAssignments = learnerContentAssignments.allocatedAssignments.filter((assignment) => (
+    assignment.isAssignedCourseRun && assignment.parentContentKey === courseKey
+  ));
+  const allocatedCourseRunAssignmentKeys = allocatedCourseRunAssignments.map(assignment => assignment.contentKey);
+  const hasAssignedCourseRuns = allocatedCourseRunAssignmentKeys.length > 0;
+  const hasMultipleAssignedCourseRuns = allocatedCourseRunAssignmentKeys.length > 1;
   return {
-    allocatedCourseRunAssignmentKeys: [],
-    allocatedCourseRunAssignments: [],
-    hasAssignedCourseRuns: false,
-    hasMultipleAssignedCourseRuns: false,
+    allocatedCourseRunAssignmentKeys,
+    allocatedCourseRunAssignments,
+    hasAssignedCourseRuns,
+    hasMultipleAssignedCourseRuns,
   };
 }
 
-export function transformCourseMetadataByAllocationCourseRun({
+export function transformCourseMetadataByAllocatedCourseRunAssignments({
   hasMultipleAssignedCourseRuns,
   courseMetadata,
   allocatedCourseRunAssignmentKeys,

diff --git a/src/components/course/data/courseLoader.js b/src/components/course/data/courseLoader.js
@@ -1,7 +1,7 @@
 import { generatePath, redirect } from 'react-router-dom';
 
 import {
-  determineAllocatedCourseRuns,
+  determineAllocatedCourseRunAssignmentsForCourse,
   determineLearnerHasContentAssignmentsOnly,
   extractEnterpriseCustomer,
   getCatalogsForSubsidyRequests,
@@ -21,7 +21,7 @@ import {
   queryRedeemablePolicies,
   querySubscriptions,
   queryUserEntitlements,
-  transformCourseMetadataByAllocationCourseRun,
+  transformCourseMetadataByAllocatedCourseRunAssignments,
 } from '../../app/data';
 import { ensureAuthenticatedUser } from '../../app/routes/data';
 import { getCourseTypeConfig, getLinkToCourse, pathContainsCourseTypeSlug } from './utils';
@@ -50,7 +50,11 @@ export default function makeCourseLoader(queryClient) {
       authenticatedUser,
       enterpriseSlug,
     });
-    const subsidyQueries = Promise.all([
+    const redeemableLearnerCreditPolicies = await queryClient.ensureQueryData(queryRedeemablePolicies({
+      enterpriseUuid: enterpriseCustomer.uuid,
+      lmsUserId: authenticatedUser.userId,
+    }));
+    const otherSubsidyQueries = Promise.all([
       queryClient.ensureQueryData(queryRedeemablePolicies({
         enterpriseUuid: enterpriseCustomer.uuid,
         lmsUserId: authenticatedUser.userId,
@@ -62,18 +66,16 @@ export default function makeCourseLoader(queryClient) {
       queryClient.ensureQueryData(queryCouponCodeRequests(enterpriseCustomer.uuid, authenticatedUser.email)),
       queryClient.ensureQueryData(queryBrowseAndRequestConfiguration(enterpriseCustomer.uuid)),
     ]);
-    const redeemableLearnerCreditPoliciesLoader = await queryClient.ensureQueryData(queryRedeemablePolicies({
-      enterpriseUuid: enterpriseCustomer.uuid,
-      lmsUserId: authenticatedUser.userId,
-    }));
+
     const {
       allocatedCourseRunAssignmentKeys,
       hasAssignedCourseRuns,
       hasMultipleAssignedCourseRuns,
-    } = determineAllocatedCourseRuns({
+    } = determineAllocatedCourseRunAssignmentsForCourse({
       courseKey,
-      redeemableLearnerCreditPolicies: redeemableLearnerCreditPoliciesLoader,
+      redeemableLearnerCreditPolicies,
     });
+    // only override `courseRunKey` when learner has a single allocated assignment
     if (!courseRunKey && hasAssignedCourseRuns) {
       courseRunKey = hasMultipleAssignedCourseRuns ? null : allocatedCourseRunAssignmentKeys[0];
     }
@@ -86,9 +88,9 @@ export default function makeCourseLoader(queryClient) {
           return null;
         }
         const lateEnrollmentBufferDays = getLateEnrollmentBufferDays(
-          redeemableLearnerCreditPoliciesLoader.redeemablePolicies,
+          redeemableLearnerCreditPolicies.redeemablePolicies,
         );
-        const transformedCourseMetadata = transformCourseMetadataByAllocationCourseRun({
+        const transformedCourseMetadata = transformCourseMetadataByAllocatedCourseRunAssignments({
           hasMultipleAssignedCourseRuns,
           courseMetadata,
           allocatedCourseRunAssignmentKeys,
@@ -101,8 +103,7 @@ export default function makeCourseLoader(queryClient) {
       queryClient.ensureQueryData(queryUserEntitlements()),
       queryClient.ensureQueryData(queryEnterpriseCustomerContainsContent(enterpriseCustomer.uuid, [courseKey])),
       queryClient.ensureQueryData(queryCourseReviews(courseKey)),
-      subsidyQueries.then(async (subsidyResponses) => {
-        const redeemableLearnerCreditPolicies = subsidyResponses[0];
+      otherSubsidyQueries.then(async (subsidyResponses) => {
         const { customerAgreement, subscriptionPlan, subscriptionLicense } = subsidyResponses[1];
         const { hasCurrentEnterpriseOffers, currentEnterpriseOffers } = subsidyResponses[2];
         const {