-
Notifications
You must be signed in to change notification settings - Fork 31
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: display refunds in spent table when backed by transactions API #1091
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1091 +/- ##
==========================================
- Coverage 84.66% 84.60% -0.07%
==========================================
Files 475 475
Lines 9934 9924 -10
Branches 2070 2076 +6
==========================================
- Hits 8411 8396 -15
- Misses 1482 1486 +4
- Partials 41 42 +1 ☔ View full report in Codecov by Sentry. |
courseListPrice: PropTypes.number.isRequired, | ||
enrollmentDate: PropTypes.string.isRequired, | ||
courseProductLine: PropTypes.string.isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[inform] "Product" column is no longer included in this table.
f1ca9c7
to
8786422
Compare
8786422
to
70ad071
Compare
70ad071
to
c532abe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
practicing my rejections :). Couple comments, otherwise LGTM!
@@ -83,7 +86,7 @@ const useOfferRedemptions = ( | |||
} | |||
let data; | |||
let transformedTableResults; | |||
if (budgetId && shouldFetchSubsidyTransactions) { | |||
if (budgetId && isTopDownAssignmentEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just this?
if (budgetId && isTopDownAssignmentEnabled) { | |
if (shouldFetchSubsidyTransactions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
if (budgetId !== null) { | ||
options.budgetId = budgetId; | ||
options[isTopDownAssignmentEnabled ? 'subsidyAccessPolicyUuid' : 'budgetId'] = budgetId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this key off shouldFetchSubsidyTransactions instead?
options[isTopDownAssignmentEnabled ? 'subsidyAccessPolicyUuid' : 'budgetId'] = budgetId; | |
options[shouldFetchSubsidyTransactions ? 'subsidyAccessPolicyUuid' : 'budgetId'] = budgetId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could; I will update.
Description
Ticket: ENT-7644 | Figma
Summary:
transactions
API via enterprise-subsidy.transactions
API.?search_all
when viewing a enterprise offer backed by the analytics API (it was passing?search
as it was were communicating totransactions
API instead).useBudgetDetailActivityOverview
was calling the analytics API for spent transactions whereas the "Spent" table was pulling from thetransactions
(enterprise-subsidy) API. These should always rely on the same data source.-
to indicate that balance was drawn down (as seen in Figma spec and consistent with "Assigned" table).For all changes
Only if submitting a visual change