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

feat: integrate redemption into learner credit audit upgrade modal #1118

Merged
merged 19 commits into from
Jul 22, 2024

Conversation

adamstankiewicz
Copy link
Member

@adamstankiewicz adamstankiewicz commented Jul 16, 2024

Description

Integrates the EnrollModal (enrollment/upgrade confirmation modal used on Dashboard and Course page routes) with the Learner Credit redemption API, such that the audit enrollment is upgraded to verified and the learner is redirected to courseware.

The behavior of EnrollModal for non-Learner Credit eligible upgrades (i.e., coupon code, enterprise offer) is functionally unchanged.

Demo

Successful Learner Credit redemption for audit -> verified upgrade

LC_audit_verified_upgrade_success.mov

For all changes

  • Ensure adequate tests are in place (or reviewed existing tests cover changes)
  • Ensure English strings are marked for translation. See documentation for more details.

Only if submitting a visual change

  • Ensure to attach screenshots
  • Ensure to have UX team confirm screenshots

Copy link

codecov bot commented Jul 16, 2024

Codecov Report

Attention: Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.01%. Comparing base (ffeea85) to head (f00e8a9).

Files Patch % Lines
...e-enrollments/course-cards/UpgradeCourseButton.jsx 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1118      +/-   ##
==========================================
+ Coverage   86.90%   87.01%   +0.10%     
==========================================
  Files         388      388              
  Lines        8035     8064      +29     
  Branches     1967     1939      -28     
==========================================
+ Hits         6983     7017      +34     
+ Misses       1003      998       -5     
  Partials       49       49              

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

@adamstankiewicz adamstankiewicz marked this pull request as ready for review July 16, 2024 20:10
Copy link
Member

@brobro10000 brobro10000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just 1 additional nit and a question

src/components/course/EnrollModal.jsx Outdated Show resolved Hide resolved
Comment on lines 239 to 250
return {
...defaultReturn,
subsidyForCourse: applicableSubsidy,
redeem: async (e) => {
e?.preventDefault();
await sendEnterpriseTrackEventWithDelay(
enterpriseCustomer.uuid,
'edx.ui.enterprise.learner_portal.course.upgrade_button.subscription_license.clicked',
);
global.location.assign(applicableSubsidy.redemptionUrl);
},
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[curious] Was this return not supposed to set hasUpgradeAndConfirm to true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, audit -> verified upgrade with a subscription license does not use a distinct "Upgrade" CTA or have a confirmation modal so hasUpgradeAndConfirm: false from the defaultReturn is returned here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[addtl context] In the subscription license upgrade case, both the course title hyperlink and the "Start course" / "Resume" button CTA links through the Data Sharing Consent flow , without an explicit upgrade confirmation.

@adamstankiewicz adamstankiewicz merged commit a9026e3 into master Jul 22, 2024
7 checks passed
@adamstankiewicz adamstankiewicz deleted the ags/ent-9066-pt2 branch July 22, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants