-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Update policyOwnerAmountOwedOverdue billing banner #58910
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
Update policyOwnerAmountOwedOverdue billing banner #58910
Conversation
…mountOwed in English and Spanish translations
CardSectionUtils.getBillingStatus({ | ||
translate, | ||
accountData: defaultCard?.accountData ?? {}, | ||
purchase: purchaseList?.[0], | ||
}), |
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.
Is there a reason we pass just the first purchase?
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.
Yes, accroding to #58067 (comment) we need only the first item for now
@@ -66,15 +67,29 @@ function CardSection() { | |||
Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query})); | |||
}, []); | |||
|
|||
const [billingStatus, setBillingStatus] = useState<BillingStatusResult | undefined>(() => CardSectionUtils.getBillingStatus(translate, defaultCard?.accountData ?? {})); | |||
const [billingStatus, setBillingStatus] = useState<BillingStatusResult | undefined>(() => CardSectionUtils.getBillingStatus({translate, accountData: defaultCard?.accountData ?? {}})); |
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.
Shouldn't we pass the purchase here too?
src/languages/en.ts
Outdated
generalSubtitle: 'Please add a payment card to clear the amount owed.', | ||
subtitle: ({date, purchaseAmountOwed}: BillingBannerOwnerAmountOwedOverdueParams) => | ||
`Your ${date} charge of ${purchaseAmountOwed} could not be processed. Please add a payment card to clear the amount owed.`, |
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.
Can we remove the generalSubtitle
variant and just make the props optional?
…emove getPurchaseDetails function
…de date and amount owed
tests/unit/CardsSectionUtilsTest.ts
Outdated
it('should return POLICY_OWNER_WITH_AMOUNT_OWED_OVERDUE variant with generalSubtitle', () => { | ||
mockGetSubscriptionStatus.mockReturnValue({ | ||
status: PAYMENT_STATUS.POLICY_OWNER_WITH_AMOUNT_OWED_OVERDUE, | ||
}); | ||
|
||
expect(CardSectionUtils.getBillingStatus({translate: translateMock, accountData: ACCOUNT_DATA, purchase: undefined})).toEqual({ | ||
title: 'subscription.billingBanner.policyOwnerAmountOwedOverdue.title', | ||
subtitle: 'subscription.billingBanner.policyOwnerAmountOwedOverdue.generalSubtitle', | ||
isError: true, | ||
isRetryAvailable: true, | ||
}); | ||
}); |
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.
Remove this test (no longer valid)
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.
Many fields are optional, here is sample
{
"amount": 0,
"created": "2025-03-01 03:32:13",
"currency": "USD",
"message": {
"accountManagerAccountID": 0,
"approvedAccountantAccountIDs": [],
"approvedSpend": {
"USD": 0
},
"billableAmount": 2700,
"billablePolicies": {
"0680EF3C70411F57": {
"actorList": "[email protected]",
"approvedSpend": {
"USD": 0
},
"corporate": true,
"expensifyCardSpend": {
"USD": 0
},
"type": "corporate"
},
"8970B100A18A2CD6": {
"actorList": "[email protected]",
"approvedSpend": {
"USD": 0
},
"corporate": true,
"expensifyCardSpend": {
"USD": 0
},
"type": "corporate"
},
"8ECD8C2EBB39933F": {
"actorList": "[email protected]",
"approvedSpend": {
"USD": 0
},
"corporate": true,
"expensifyCardSpend": {
"USD": 0
},
"type": "corporate"
},
"978C2546B3751E81": {
"actorList": "[email protected]",
"approvedSpend": {
"USD": 0
},
"corporate": false,
"expensifyCardSpend": {
"USD": 0
},
"type": "team"
},
"CDD878B939EB631E": {
"actorList": "[email protected]",
"approvedSpend": {
"USD": 0
},
"corporate": true,
"expensifyCardSpend": {
"USD": 0
},
"type": "corporate"
},
"E666A63199B43042": {
"actorList": "[email protected]",
"approvedSpend": {
"USD": 0
},
"corporate": true,
"expensifyCardSpend": {
"USD": 0
},
"type": "corporate"
}
},
"billingType": "failed_2018",
"cardSpendSurchargePercent": 1,
"cashBackAmount": 0,
"cashBackPercentage": 0,
"chatOnlyActorList": "",
"corporateActorCount": 1,
"corporateRevenue": 3600,
"expensifyCardMonthlySpend": 0,
"expensifyCardSpend": {
"USD": 0
},
"freeToTeamMigrationDiscountAmount": 900,
"freeToTeamMigrationDiscountPercent": 25,
"freebieCreditsUsed": 0,
"guideAccountID": 12003194,
"isApprovedAccountant": false,
"isApprovedAccountantClient": false,
"monthlyCorpSubscriptionCost": 1800,
"monthlyTeamSubscriptionCost": 0,
"paidActorCount": 1,
"partnerManagerAccountID": 0,
"perPolicyTotalMembersCount": {
"05E4A0124E314E2D": 1,
"23B249B9FA2A78A3": 2,
"2499B1B92C375F8B": 1,
"4656350AB2539E7A": 1,
"5F5EB0E523187199": 1,
"6AEBBB88BBAF7048": 1,
"762A250743827663": 1,
"978C2546B3751E81": 3,
"C22CACBB604EED41": 1,
"CFD83323ED0FC6A9": 1,
"D15117903706B9B0": 4,
"E5405AABB0C65377": 2,
"FED1806CF392498F": 1
},
"potentialCashBackAmount": 1755,
"potentialCashBackPercentage": 0.01,
"subscription": {
"type": "monthly2018"
},
"teamActorCount": 0,
"teamRevenue": 0,
"totalActorCount": 1,
"totalFreebieCredits": 0,
"totalPlatformSpend": 175500,
"totalRevenue": 2700,
"totalUniqueMembersCount": 6,
"wasDomainBillingUsed": false
},
"purchaseID": 71976932
}
cc @amyevans to confirm which fields are required or we can assume all are optional to avoid accessing a non existing field which may lead to a crash
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.
The top level fields are required (amount, created, currency, message, purchaseID). Anything within message
we should assume is optional and code defensively to avoid crashes - message
is just a JSON string in our database, so the existence of fields is not enforced/guaranteed.
…oved clarity and data management
Reviewer Checklist
Screenshots/VideosiOS: NativeJS bundle issue. NAB |
…cture for improved clarity and maintainability
… on billing failure
PR updated |
@pasyukevich can you please address #58910 (comment) |
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.
Looking good! Mostly just nit picks on the documentation of types
src/types/onyx/PurchaseList.ts
Outdated
|
||
/** Type for a billable policy */ | ||
type BillablePolicy = { | ||
/** List of actors in the policy */ |
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.
/** List of actors in the policy */ | |
/** Comma separated list of emails for members in the policy */ |
PR updated |
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.
Looks great! Thanks! 🚀
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/amyevans in version: 9.1.24-2 🚀
|
@@ -6924,6 +6924,10 @@ const CONST = { | |||
ILLUSTRATION_ASPECT_RATIO: 39 / 22, | |||
|
|||
OFFLINE_INDICATOR_HEIGHT: 25, | |||
|
|||
BILLING: { | |||
TYPE_FAILED_2018: 'typeFailed2018', |
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.
While QAing this on staging I realized this got changed at the last minute to be typeFailed2018
, when the correct value is failed_2018
. I'll open a PR to fix it
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.
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.24-10 🚀
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.24-10 🚀
|
Explanation of Change
Fixed Issues
$ #58067
PROPOSAL:
Tests
mock purchase object
Edit src/pages/settings/Subscription/CardSection/CardSection.tsx
Replace
purchase
withmockPurchase
Update billingBanner with custom
Patch
getSubscriptionStatus
with early return:Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Assign to @amyevans for Internal QA
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop