feat: use discount info endpoint for streak discount information (#1763) - #7
Merged
Merged
Conversation
…nedx#1763) * feat: use discount info endpoint for streak discount information * feat: pass course run key to discount code info call * feat: move changes behind a flag * fix: use async IIFE inside useEffect * fix: fix line length * fix: remove default value in dev * fix: improve coverage by adding conditional test based on env value * refactor: move logic inside function * refactor: move functions to utils * fix: ignore merge config
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new discount information endpoint for determining streak discount percentages, providing a fallback mechanism to the existing ecommerce-based discount calculation. The change allows the application to use a dedicated discount code service when available, while maintaining compatibility with the previous ecommerce API approach.
Key changes:
- Added support for
DISCOUNT_CODE_INFO_URLconfiguration option - Implemented new
getDiscountCodePercentagefunction as primary discount calculation method - Refactored existing discount logic to use the new endpoint when available, falling back to ecommerce API
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/shared/streak-celebration/utils.jsx |
Added new discount calculation functions and exports |
src/shared/streak-celebration/StreakCelebrationModal.jsx |
Refactored discount calculation to use new endpoint with fallback logic |
src/shared/streak-celebration/StreakCelebrationModal.test.jsx |
Added test for new discount endpoint functionality |
src/index.jsx |
Added DISCOUNT_CODE_INFO_URL to configuration |
.env, .env.development, .env.test |
Added DISCOUNT_CODE_INFO_URL environment variable |
Comments suppressed due to low confidence (1)
src/shared/streak-celebration/StreakCelebrationModal.jsx:169
- The istanbul ignore comment suggests this configuration code is not covered by tests. Consider adding test coverage for the configuration merging logic to ensure environment variables are properly handled.
{ queryingDiscount && (
| const result = await getAuthenticatedHttpClient().get(url); | ||
| const { isApplicable, discountPercentage } = camelCaseObject(result).data; | ||
|
|
||
| return isApplicable ? +discountPercentage : 0; |
There was a problem hiding this comment.
[nitpick] Using the unary plus operator (+) for type conversion is less clear than explicit conversion methods. Consider using Number(discountPercentage) or parseFloat(discountPercentage) for better readability.
Suggested change
| return isApplicable ? +discountPercentage : 0; | |
| return isApplicable ? Number(discountPercentage) : 0; |
jansenk
approved these changes
Aug 6, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proxied from @NawfalAhmed, selectively merging openedx#1763 from openedx upstream.