Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/hooks/useNavigationTabBarIndicatorChecks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ function useNavigationTabBarIndicatorChecks(): NavigationTabBarChecksResult {
const [billingDisputePending] = useOnyx(ONYXKEYS.NVP_PRIVATE_BILLING_DISPUTE_PENDING, {canBeMissing: true});
const [retryBillingFailed] = useOnyx(ONYXKEYS.SUBSCRIPTION_RETRY_BILLING_STATUS_FAILED, {canBeMissing: true});
const [billingStatus] = useOnyx(ONYXKEYS.NVP_PRIVATE_BILLING_STATUS, {canBeMissing: true});
const [amountOwed = 0] = useOnyx(ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED, {canBeMissing: true});
const [ownerBillingGraceEndPeriod] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END, {canBeMissing: true});
const [session] = useOnyx(ONYXKEYS.SESSION, {canBeMissing: true});
const [allDomainErrors] = useOnyx(ONYXKEYS.COLLECTION.DOMAIN_ERRORS, {canBeMissing: true});
Expand Down Expand Up @@ -79,6 +80,7 @@ function useNavigationTabBarIndicatorChecks(): NavigationTabBarChecksResult {
retryBillingFailed,
fundList,
billingStatus,
amountOwed,
ownerBillingGraceEndPeriod,
),
[CONST.INDICATOR_STATUS.HAS_REIMBURSEMENT_ACCOUNT_ERRORS]: Object.keys(reimbursementAccount?.errors ?? {}).length > 0,
Expand All @@ -99,6 +101,7 @@ function useNavigationTabBarIndicatorChecks(): NavigationTabBarChecksResult {
retryBillingFailed,
fundList,
billingStatus,
amountOwed,
ownerBillingGraceEndPeriod,
),
[CONST.INDICATOR_STATUS.HAS_PARTIALLY_SETUP_BANK_ACCOUNT_INFO]: hasPartiallySetupBankAccount(bankAccountList),
Expand Down
32 changes: 18 additions & 14 deletions src/libs/SubscriptionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,34 +53,34 @@
};

let currentUserAccountID = -1;
Onyx.connect({

Check warning on line 56 in src/libs/SubscriptionUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
currentUserAccountID = value?.accountID ?? CONST.DEFAULT_NUMBER_ID;
},
});

let amountOwed: OnyxEntry<number>;
let privateAmountOwed: OnyxEntry<number>;
Onyx.connect({

Check warning on line 64 in src/libs/SubscriptionUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED,
callback: (value) => (amountOwed = value),
callback: (value) => (privateAmountOwed = value),
});

let ownerBillingGraceEndPeriod: OnyxEntry<number>;
Onyx.connect({

Check warning on line 70 in src/libs/SubscriptionUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END,
callback: (value) => (ownerBillingGraceEndPeriod = value),
});

let userBillingGraceEndPeriodCollection: OnyxCollection<BillingGraceEndPeriod>;
Onyx.connect({

Check warning on line 76 in src/libs/SubscriptionUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END,
callback: (value) => (userBillingGraceEndPeriodCollection = value),
waitForCollectionCallback: true,
});

let allPolicies: OnyxCollection<Policy>;
Onyx.connect({

Check warning on line 83 in src/libs/SubscriptionUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY,
callback: (value) => (allPolicies = value),
waitForCollectionCallback: true,
Expand Down Expand Up @@ -115,22 +115,22 @@
/**
* @returns The amount owed by the workspace owner.
*/
function getAmountOwed(): number {
return amountOwed ?? 0;
function getAmountOwed(amountOwed?: OnyxEntry<number>): number {
return amountOwed ?? privateAmountOwed ?? 0;
}

/**
* @returns Whether there is an amount owed by the workspace owner.
*/
function hasAmountOwed(): boolean {
return !!amountOwed;
return !!privateAmountOwed;
}

/**
* @returns Whether there is a card authentication error.
*/
function hasCardAuthenticatedError(stripeCustomerId: OnyxEntry<StripeCustomerID>) {
return stripeCustomerId?.status === 'authentication_required' && getAmountOwed() === 0;
function hasCardAuthenticatedError(stripeCustomerId: OnyxEntry<StripeCustomerID>, amountOwed: number) {
return stripeCustomerId?.status === 'authentication_required' && amountOwed === 0;
Comment on lines +132 to +133

Choose a reason for hiding this comment

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

P2 Badge Treat missing amountOwed as 0 for auth-required check

Because callers now pass amountOwed directly from useOnyx(..., {canBeMissing: true}), this argument can be null when the key is missing. The strict comparison amountOwed === 0 will then be false, so users with stripeCustomerId.status === 'authentication_required' and no stored amount owed will not see the authentication-required error/red dot. Previously this path used getAmountOwed() which coerces null/undefined to 0, so the regression only appears after this change. Consider normalizing with getAmountOwed(amountOwed) or amountOwed ?? 0 before the comparison.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we already added the default value when getting it from useOnyx

}

/**
Expand All @@ -144,7 +144,7 @@
* @returns Whether there is a card expired error.
*/
function hasCardExpiredError(billingStatus: OnyxEntry<BillingStatus>) {
return billingStatus?.declineReason === 'expired_card' && amountOwed !== 0;
return billingStatus?.declineReason === 'expired_card' && privateAmountOwed !== 0;
}

/**
Expand Down Expand Up @@ -284,6 +284,7 @@
retryBillingFailed: boolean | undefined,
fundList: OnyxEntry<FundList>,
billingStatus: OnyxEntry<BillingStatus>,
amountOwed: number,
ownerBillingGraceEndPeriodParam?: OnyxEntry<number>,
): SubscriptionStatus | undefined {
if (hasOverdueGracePeriod(ownerBillingGraceEndPeriodParam)) {
Expand Down Expand Up @@ -330,7 +331,7 @@
}

// 6. Card not authenticated
if (hasCardAuthenticatedError(stripeCustomerId)) {
if (hasCardAuthenticatedError(stripeCustomerId, amountOwed)) {
return {
status: PAYMENT_STATUS.CARD_AUTHENTICATION_REQUIRED,
isError: true,
Expand Down Expand Up @@ -390,10 +391,12 @@
retryBillingFailed: boolean | undefined,
fundList: OnyxEntry<FundList>,
billingStatus: OnyxEntry<BillingStatus>,
amountOwed: number,
ownerBillingGraceEndPeriodParam?: OnyxEntry<number>,
): boolean {
return (
getSubscriptionStatus(stripeCustomerId, retryBillingSuccessful, billingDisputePending, retryBillingFailed, fundList, billingStatus, ownerBillingGraceEndPeriodParam)?.isError ?? false
getSubscriptionStatus(stripeCustomerId, retryBillingSuccessful, billingDisputePending, retryBillingFailed, fundList, billingStatus, amountOwed, ownerBillingGraceEndPeriodParam)
?.isError ?? false
);
}

Expand All @@ -408,11 +411,12 @@
retryBillingFailed: boolean | undefined,
fundList: OnyxEntry<FundList>,
billingStatus: OnyxEntry<BillingStatus>,
amountOwed: number,
ownerBillingGraceEndPeriodParam?: OnyxEntry<number>,
): boolean {
return (
getSubscriptionStatus(stripeCustomerId, retryBillingSuccessful, billingDisputePending, retryBillingFailed, fundList, billingStatus, ownerBillingGraceEndPeriodParam)?.isError ===
false
getSubscriptionStatus(stripeCustomerId, retryBillingSuccessful, billingDisputePending, retryBillingFailed, fundList, billingStatus, amountOwed, ownerBillingGraceEndPeriodParam)
?.isError === false
);
}

Expand Down Expand Up @@ -525,8 +529,8 @@
if (
isPolicyOwner(policy, currentUserAccountID) &&
ownerBillingGraceEndPeriod &&
amountOwed !== undefined &&
amountOwed > 0 &&
privateAmountOwed !== undefined &&
privateAmountOwed > 0 &&
isAfter(currentDate, fromUnixTime(ownerBillingGraceEndPeriod))
) {
return true;
Expand Down
12 changes: 11 additions & 1 deletion src/pages/settings/InitialSettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ function InitialSettingsPage({currentUserPersonalDetails}: InitialSettingsPagePr
const [billingDisputePending] = useOnyx(ONYXKEYS.NVP_PRIVATE_BILLING_DISPUTE_PENDING, {canBeMissing: true});
const [retryBillingFailed] = useOnyx(ONYXKEYS.SUBSCRIPTION_RETRY_BILLING_STATUS_FAILED, {canBeMissing: true});
const [billingStatus] = useOnyx(ONYXKEYS.NVP_PRIVATE_BILLING_STATUS, {canBeMissing: true});
const [amountOwed = 0] = useOnyx(ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED, {canBeMissing: true});
const [ownerBillingGraceEndPeriod] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END, {canBeMissing: true});
const {shouldUseNarrowLayout} = useResponsiveLayout();
const network = useNetwork();
Expand Down Expand Up @@ -276,7 +277,16 @@ function InitialSettingsPage({currentUserPersonalDetails}: InitialSettingsPagePr
screenName: SCREENS.SETTINGS.SUBSCRIPTION.ROOT,
brickRoadIndicator:
!!privateSubscription?.errors ||
hasSubscriptionRedDotError(stripeCustomerId, retryBillingSuccessful, billingDisputePending, retryBillingFailed, fundList, billingStatus, ownerBillingGraceEndPeriod)
hasSubscriptionRedDotError(
stripeCustomerId,
retryBillingSuccessful,
billingDisputePending,
retryBillingFailed,
fundList,
billingStatus,
amountOwed,
ownerBillingGraceEndPeriod,
)
? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR
: undefined,
badgeText: freeTrialText,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ function CardSection() {
const [billingDisputePending] = useOnyx(ONYXKEYS.NVP_PRIVATE_BILLING_DISPUTE_PENDING, {canBeMissing: true});
const [userBillingFundID] = useOnyx(ONYXKEYS.NVP_BILLING_FUND_ID, {canBeMissing: true});
const [billingStatusOnyx] = useOnyx(ONYXKEYS.NVP_PRIVATE_BILLING_STATUS, {canBeMissing: true});
const [amountOwed = 0] = useOnyx(ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED, {canBeMissing: true});
const [ownerBillingGraceEndPeriod] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END, {canBeMissing: true});
const requestRefund = useCallback(() => {
requestRefundByUser();
Expand Down Expand Up @@ -102,6 +103,7 @@ function CardSection() {
billingStatus: billingStatusOnyx,
creditCardEyesIcon: illustrations.CreditCardEyes,
fundList,
amountOwed,
ownerBillingGraceEndPeriod,
}),
);
Expand All @@ -127,6 +129,7 @@ function CardSection() {
billingStatus: billingStatusOnyx,
creditCardEyesIcon: illustrations.CreditCardEyes,
fundList,
amountOwed,
ownerBillingGraceEndPeriod,
}),
);
Expand All @@ -142,6 +145,7 @@ function CardSection() {
billingStatusOnyx,
illustrations.CreditCardEyes,
fundList,
amountOwed,
ownerBillingGraceEndPeriod,
]);

Expand Down Expand Up @@ -234,7 +238,7 @@ function CardSection() {
large
/>
)}
{hasCardAuthenticatedError(privateStripeCustomerID) && (
{hasCardAuthenticatedError(privateStripeCustomerID, amountOwed) && (
<CardSectionButton
text={translate('subscription.cardSection.authenticatePayment')}
isDisabled={isOffline || !billingStatus?.isAuthenticationRequired}
Expand Down
7 changes: 4 additions & 3 deletions src/pages/settings/Subscription/CardSection/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as Expensicons from '@components/Icon/Expensicons';
import type {LocaleContextProps} from '@components/LocaleContextProvider';
import {convertAmountToDisplayString} from '@libs/CurrencyUtils';
import DateUtils from '@libs/DateUtils';
import {getAmountOwed, getSubscriptionStatus, PAYMENT_STATUS} from '@libs/SubscriptionUtils';
import {getSubscriptionStatus, PAYMENT_STATUS} from '@libs/SubscriptionUtils';
import CONST from '@src/CONST';
import type {StripeCustomerID} from '@src/types/onyx';
import type BillingStatus from '@src/types/onyx/BillingStatus';
Expand Down Expand Up @@ -35,6 +35,7 @@ type GetBillingStatusProps = {
billingStatus: OnyxEntry<BillingStatus>;
creditCardEyesIcon?: IconAsset;
fundList: OnyxEntry<FundList>;
amountOwed: number;
ownerBillingGraceEndPeriod: OnyxEntry<number>;
};

Expand All @@ -50,18 +51,18 @@ function getBillingStatus({
creditCardEyesIcon,
fundList,
ownerBillingGraceEndPeriod,
amountOwed,
}: GetBillingStatusProps): BillingStatusResult | undefined {
const cardEnding = (accountData?.cardNumber ?? '')?.slice(-4);

const amountOwed = getAmountOwed();

const subscriptionStatus = getSubscriptionStatus(
stripeCustomerId,
retryBillingSuccessful,
billingDisputePending,
retryBillingFailed,
fundList,
billingStatus,
amountOwed,
ownerBillingGraceEndPeriod,
);

Expand Down
14 changes: 14 additions & 0 deletions tests/unit/CardsSectionUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: undefined,
}),
).toBeUndefined();
Expand All @@ -125,6 +126,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand Down Expand Up @@ -163,6 +165,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -189,6 +192,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -215,6 +219,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -241,6 +246,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -267,6 +273,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -293,6 +300,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -319,6 +327,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -345,6 +354,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -365,6 +375,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -391,6 +402,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -417,6 +429,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand All @@ -443,6 +456,7 @@ describe('CardSectionUtils', () => {
creditCardEyesIcon,
fundList: undefined,
billingStatus: undefined,
amountOwed: AMOUNT_OWED,
ownerBillingGraceEndPeriod: GRACE_PERIOD_DATE,
}),
).toEqual({
Expand Down
Loading
Loading