feat: assign by course run implementation #1292

brobro10000 · 2024-09-03T21:03:22Z

Ability to assign learner credit assignments by individual course runs. This is a hard cutover from assigning from the top level course key to the individual course run.
Displays an enroll-by date for each applicable course run that currently verifies that the enroll-by date of the course run is before the subsidyExpirationDate- MAX_ALLOWABLE_REFUND_THRESHOLD_DAYS to ensure course runs that are far in the future beyond the policies term are not displayed but respect the ability to provide refunds within the subsidy expiration date.

This PR also includes:

Updates display of prices to include a range if multiple unique prices are available for runs based on content_price
Updates course page url on the allocation table to the correct URL (courseRun key --> top level key)
Update heuristic that determines displaying start date for self paced courses based whether the user has time to complete the course or within 14 days of today. If not, default the start_date to today
Supports the late enrollment flow for displaying course runs by determining if the policy allows the user to display late enrollment runs.
Updates tests and sample algolia return to include course_runs to accurately test the changes.

Catalog search results card.

Allocation screen:

Zero state was also added for edge cases that the enroll-by date for all course runs is beyond the subsidyExpirationDate+STALE_ENROLLMENT_DROPOFF_DAYS threshold
Catalog search zero state:

For all changes

Ensure adequate tests are in place (or reviewed existing tests cover changes)

Only if submitting a visual change

Ensure to attach screenshots
Ensure to have UX team confirm screenshots

codecov · 2024-09-05T16:48:59Z

Codecov Report

Attention: Patch coverage is 91.01796% with 15 lines in your changes missing coverage. Please review.

Project coverage is 85.64%. Comparing base (d1a270f) to head (b5f6cd3).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...components/learner-credit-management/data/utils.js	78.00%	9 Missing and 2 partials ⚠️
...ent-modal/AssignmentAllocationHelpCollapsibles.jsx	91.66%	1 Missing ⚠️
.../assignment-modal/AssignmentModalmportantDates.jsx	94.73%	1 Missing ⚠️
...ment/assignment-modal/NewAssignmentModalButton.jsx	85.71%	1 Missing ⚠️
...it-management/cards/data/useCourseCardMetadata.jsx	96.87%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1292      +/-   ##
==========================================
+ Coverage   85.60%   85.64%   +0.04%     
==========================================
  Files         566      567       +1     
  Lines       12467    12590     +123     
  Branches     2602     2634      +32     
==========================================
+ Hits        10672    10783     +111     
- Misses       1737     1748      +11     
- Partials       58       59       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx

adamstankiewicz · 2024-09-11T12:29:23Z

src/components/learner-credit-management/assignment-modal/NewAssignmentModalDropdown.jsx

+  const [emptyState] = useState(courseRuns.length === 0);
+  return (
+    <Dropdown id={courseKey}>
+      <Dropdown.Toggle variant="primary" id="assign-by-course-runs-dropdown">


Given there are multiple "Assign" CTAs rendered in the search results, I believe this id should be unique per search result?

The NewAssisgnmentModalButton component passes a top level course courseKey value to each dropdown component (each card result). Logging the output of each id on the top level <Dropdown /> (iamge 1) and <Dropdown.Item /> (image 2) component show that they are unique 👍🏽

What about id="assign-by-course-runs-dropdown", though? Is the id on the top-level Dropdown necessary? On the Paragon docs site, it looks like it's only passed on the Dropdown.Toggle. Should the courseKey be included as part of this id to make it unique?

src/components/learner-credit-management/assignment-modal/NewAssignmentModalDropdown.jsx

adamstankiewicz · 2024-09-11T13:45:50Z

src/components/learner-credit-management/assignment-modal/AssignmentModalmportantDates.jsx

+  return (
+    <section className="assignments-important-dates small">
+      {enrollByDate && (
+        <AssignmentModalImportantDate label={intl.formatMessage(messages.enrollByDate)}>


Nice abstraction of AssignmentModalImportantDate :)

src/components/learner-credit-management/cards/CourseCardFooterActions.jsx

src/components/learner-credit-management/cards/data/useCourseCardMetadata.jsx

src/components/learner-credit-management/assignment-modal/AssignmentModalmportantDates.jsx

src/components/learner-credit-management/BudgetDetailPageOverviewAvailability.jsx

src/components/learner-credit-management/assignment-modal/AssignmentModalmportantDates.jsx

src/components/learner-credit-management/assignment-modal/NewAssignmentModalButton.jsx

src/components/learner-credit-management/cards/CourseCardFooterActions.jsx

src/components/learner-credit-management/cards/data/useCourseCardMetadata.jsx

adamstankiewicz · 2024-09-12T19:45:41Z

src/components/learner-credit-management/cards/data/useCourseCardMetadata.jsx

+  const footerText = intl.formatMessage(messages.courseFooterMessage, {
+    courseRuns: assignableCourseRuns.length,
+    pluralText: pluralText('date', assignableCourseRuns.length),
+  });


nice construction of the i18n message here!

adamstankiewicz · 2024-09-12T19:46:19Z

src/components/learner-credit-management/data/constants.js

@@ -22,6 +22,12 @@ export const API_FIELDS_BY_TABLE_COLUMN_ACCESSOR = {
  courseListPrice: 'course_list_price',
 };

+// Course pace text


Thank you for making sure we stick to a single constants file within this module 🙌

adamstankiewicz · 2024-09-12T19:47:14Z

src/components/learner-credit-management/data/utils.js

+  return weeksToComplete <= differenceInWeeks;
+};
+
+const isWithinMinimumStartDateThreshold = ({ start }) => dayjs(start).isBefore(dayjs().subtract(START_DATE_DEFAULT_TO_TODAY_THRESHOLD_DAYS, 'days'));


👨‍🍳 💋

src/index.scss

adamstankiewicz · 2024-09-12T19:55:10Z

src/utils.js

+ * @param punctuation
+ * @returns {string}
+ */
+const pluralText = (


[inform] I think what you have here is fine, but just as a heads up, it looks like there is some support for defining plural messages within the message format (ICU syntax, see https://formatjs.io/docs/core-concepts/icu-syntax/#plural-format) or perhaps FormattedPlural (docs).

Leaving comment open as reference 👍🏽

pwnage101 · 2024-09-16T19:46:53Z

src/components/learner-credit-management/data/constants.js

+// Maximum days allowed from enrollment for a refund on assignments related to policies
+export const MAX_ALLOWABLE_REFUND_THRESHOLD_DAYS = 14;
+
+// Start date threshold to default to today days, sets start date to today if course start date is beyond this value


Nit:

Suggested change

// Start date threshold to default to today days, sets start date to today if course start date is beyond this value

// When the start date is before this number of days before today, display the alternate start date (fixed to today).

src/components/learner-credit-management/assignment-modal/AssignmentModalContent.jsx

src/components/learner-credit-management/assignment-modal/NewAssignmentModalButton.jsx

src/components/learner-credit-management/cards/data/useCourseCardMetadata.jsx

src/components/learner-credit-management/data/constants.js

src/components/learner-credit-management/data/utils.js

src/components/learner-credit-management/cards/data/useCourseCardMetadata.jsx

src/components/learner-credit-management/AssignmentDetailsTableCell.jsx

src/components/learner-credit-management/data/utils.js

src/components/learner-credit-management/data/constants.js

src/components/learner-credit-management/data/utils.js

adamstankiewicz · 2024-09-17T15:19:48Z

__mocks__/react-instantsearch-dom.jsx

      start: mockCurrentStartDate,
      end: dayjs(mockCurrentStartDate).add(1, 'year').toISOString(),
      enroll_by: dayjs(mockCurrentStartDate).unix(),


nit: curious whether we can either rename this mock date to be more generic or have separate variables to use for each related field.

As is, the mockCurrentStartDate is being used for end/enroll_by dates, too which could be a bit misleading/confusing.

adamstankiewicz · 2024-09-17T15:20:15Z

src/components/learner-credit-management/data/constants.js

@@ -111,6 +111,9 @@ export const START_DATE_DEFAULT_TO_TODAY_THRESHOLD_DAYS = 14;
 // Default empty content_price value
 export const EMPTY_CONTENT_PRICE_VALUE = 0;

+//


nit: empty comment

adamstankiewicz · 2024-09-17T15:23:01Z

src/components/learner-credit-management/data/utils.js

+        ),
+      );
+    }
+    if (hasCourseStarted(enrollBy) && isLateRedemptionAllowed) {


nit: Should we be using a function hasCourseStarted with an enrollBy date (seems somewhat misleading/confusing)? Perhaps this util function should be renamed to be more generic, so it's not tied explicitly to "start" dates based on its name.

pwnage101 · 2024-09-17T16:41:59Z

src/components/learner-credit-management/data/utils.js

+      isEligibleForEnrollment = dayjs(enrollBy).isBefore(
+        Math.min(
+          dayjs(subsidyExpirationDatetime).subtract(MAX_ALLOWABLE_REFUND_THRESHOLD_DAYS, 'days').toDate(),
+          dayjs().add(DAYS_UNTIL_ASSIGNMENT_ALLOCATION_EXPIRATION, 'days').toDate(),
+        ),
+      );


You sure just changing this to min is sufficient? The docstring was updated to match this logic, but still the desired outcome is not explicitly stated. If the goal is as I described it previously, then I think this is still incorrect because it guarantees that learners ALWAYS have less than 90 days to accept an assignment.

src/components/learner-credit-management/data/utils.js

adamstankiewicz · 2024-09-17T19:44:01Z

src/components/learner-credit-management/data/utils.js

- *  - If hasEnrollBy, we return assignments with enroll before the soonest of the two date: The subsidy expiration
- *    date - refund threshold OR today offset by the 90-day allocation threshold for an assignment denoted as
- *    isEligibleForEnrollment
+ *  - If hasEnrollBy, we return assignments with enroll before the soonest date: The subsidy expiration


nit: enroll -> enrollBy (deferred)

src/components/learner-credit-management/data/utils.js

brobro10000 force-pushed the hu/ent-8878 branch 4 times, most recently from 5473422 to 6411917 Compare September 5, 2024 16:35

brobro10000 marked this pull request as ready for review September 5, 2024 16:51

brobro10000 force-pushed the hu/ent-8878 branch 4 times, most recently from 25667e8 to e1e7ec9 Compare September 6, 2024 15:06

adamstankiewicz reviewed Sep 11, 2024

View reviewed changes

brobro10000 force-pushed the hu/ent-8878 branch 6 times, most recently from 201ce8b to 7a8f7ca Compare September 12, 2024 18:55

adamstankiewicz reviewed Sep 12, 2024

View reviewed changes

brobro10000 force-pushed the hu/ent-8878 branch 4 times, most recently from ec4be07 to f75e9d1 Compare September 16, 2024 18:23

pwnage101 reviewed Sep 16, 2024

View reviewed changes

adamstankiewicz reviewed Sep 16, 2024

View reviewed changes

adamstankiewicz approved these changes Sep 16, 2024

View reviewed changes

src/components/learner-credit-management/cards/data/useCourseCardMetadata.jsx Outdated Show resolved Hide resolved

brobro10000 force-pushed the hu/ent-8878 branch from becf1b4 to 783a5de Compare September 16, 2024 20:52

pwnage101 reviewed Sep 16, 2024

View reviewed changes

src/components/learner-credit-management/AssignmentDetailsTableCell.jsx Outdated Show resolved Hide resolved

pwnage101 reviewed Sep 16, 2024

View reviewed changes

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

brobro10000 force-pushed the hu/ent-8878 branch from 2cd1ccc to 063cd1a Compare September 16, 2024 21:06

pwnage101 reviewed Sep 16, 2024

View reviewed changes

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

brobro10000 added 7 commits September 17, 2024 10:08

chore: fix tests

cb95c12

feat: update enroll-by date threshold to consider refund threshold

07991f3

feat: update modal allocation help text enroll by date

3f054f6

feat: adds price range for lc variant based assignments

0d18f45

chore: PR feedback

4b39fc5

chore: PR feedback 2

a2495ea

chore: PR feedback 3

765c86d

brobro10000 force-pushed the hu/ent-8878 branch 2 times, most recently from a3687dd to d1b71e3 Compare September 17, 2024 14:21

adamstankiewicz approved these changes Sep 17, 2024

View reviewed changes

src/components/learner-credit-management/data/constants.js Outdated Show resolved Hide resolved

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

brobro10000 force-pushed the hu/ent-8878 branch from d1b71e3 to 9a899a3 Compare September 17, 2024 15:00

adamstankiewicz reviewed Sep 17, 2024

View reviewed changes

feat: explicitly handle late enrollment case

027fee7

brobro10000 force-pushed the hu/ent-8878 branch from 9a899a3 to 027fee7 Compare September 17, 2024 15:37

adamstankiewicz approved these changes Sep 17, 2024

View reviewed changes

pwnage101 reviewed Sep 17, 2024

View reviewed changes

chore: refine enrollby and start date logic

eba4828

brobro10000 force-pushed the hu/ent-8878 branch from d810af3 to eba4828 Compare September 17, 2024 17:24

adamstankiewicz approved these changes Sep 17, 2024

View reviewed changes

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

pwnage101 approved these changes Sep 17, 2024

View reviewed changes

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

chore: PR feedback

938303b

brobro10000 force-pushed the hu/ent-8878 branch from bdf9629 to 938303b Compare September 17, 2024 18:29

pwnage101 reviewed Sep 17, 2024

View reviewed changes

src/components/learner-credit-management/data/utils.js Outdated Show resolved Hide resolved

fix: remove 90 day check for enrollment threshold

b5f6cd3

adamstankiewicz reviewed Sep 17, 2024

View reviewed changes

adamstankiewicz approved these changes Sep 17, 2024

View reviewed changes

brobro10000 merged commit 5619bad into master Sep 17, 2024
6 checks passed

brobro10000 deleted the hu/ent-8878 branch September 17, 2024 19:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: assign by course run implementation #1292

feat: assign by course run implementation #1292

brobro10000 commented Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 5, 2024 •

edited

Loading

adamstankiewicz Sep 11, 2024

brobro10000 Sep 11, 2024 •

edited

Loading

adamstankiewicz Sep 12, 2024

adamstankiewicz Sep 11, 2024

adamstankiewicz Sep 12, 2024

adamstankiewicz Sep 12, 2024

adamstankiewicz Sep 12, 2024

adamstankiewicz Sep 12, 2024

brobro10000 Sep 12, 2024

pwnage101 Sep 16, 2024

adamstankiewicz Sep 17, 2024

adamstankiewicz Sep 17, 2024

adamstankiewicz Sep 17, 2024 •

edited

Loading

pwnage101 Sep 17, 2024 •

edited

Loading

adamstankiewicz Sep 17, 2024

	// Start date threshold to default to today days, sets start date to today if course start date is beyond this value
	// When the start date is before this number of days before today, display the alternate start date (fixed to today).

feat: assign by course run implementation #1292

feat: assign by course run implementation #1292

Conversation

brobro10000 commented Sep 3, 2024 • edited Loading

For all changes

Only if submitting a visual change

codecov bot commented Sep 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

brobro10000 Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamstankiewicz Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

pwnage101 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brobro10000 commented Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 5, 2024 •

edited

Loading

brobro10000 Sep 11, 2024 •

edited

Loading

adamstankiewicz Sep 17, 2024 •

edited

Loading

pwnage101 Sep 17, 2024 •

edited

Loading