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

fix: sort licenses by whether they are current in ascending order before further processing #1154

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

adamstankiewicz
Copy link
Member

@adamstankiewicz adamstankiewicz commented Aug 16, 2024

Bug 🐛

(ENT-9373)

tl;dr; Some subscription licenses are getting treated as expired in the UX, even though they have activated license in a current subscription plan:

image

Learners who have an activated subscription license at the time a subscription plan renewal was processed (i.e., where their activated license is copied to the renewal plan), end up incorrectly seeing their activated license as expired for the current subscription plan.

In this case of a processed renewal for a learner who had an activated license, the learner-licenses API begins returning the activated renewal license first in the list of licenses tied to the authenticated learner. As a result, the fetchSubscriptions logic which parses the list of subscription licenses ends up extracting the first activated subscription license returned by the API, aka the activated renewal license, not the current license.

Given this, because the determined subscription license to use throughout the UX ends up being the renewal license, the plan's isCurrent flag is false, triggering the expiration experience.

Resolution 💪

Sort the returned list of subscription licenses from the learner-licenses API to prioritize the current licenses when grouping licenses by status. By doing so, we will ensure the extracted license from fetchSubscriptions will be a current license, if one exists.

Alternative approaches to consider

  • Introduce query parameter to learner-licenses to exclude upcoming subscription plans. We currently pass current_plans_only: false to ensure we include expired plans to support the expiration UX, but this pulls in upcoming/scheduled plans as a result.
  • Introduce query parameter to learner-licenses to order the response list of licenses by whether the associated subscription plan isCurrent.

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 Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.06%. Comparing base (8dafafc) to head (bbf5814).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1154   +/-   ##
=======================================
  Coverage   88.05%   88.06%           
=======================================
  Files         394      394           
  Lines        8338     8343    +5     
  Branches     2008     2010    +2     
=======================================
+ Hits         7342     7347    +5     
  Misses        954      954           
  Partials       42       42           

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

@adamstankiewicz adamstankiewicz merged commit f0d0c7e into master Aug 16, 2024
7 checks passed
@adamstankiewicz adamstankiewicz deleted the ags/subs-renewal-license-hotfix branch August 16, 2024 13:39
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