Skip to content

Commit cb41497

Browse files
robertjchenrobertjchen
authored
Merge pull request Expensify#85635 from Expensify/claude-fixStaleWorkspaceRestriction
[Payment due @truph01] Clear stale billing grace period optimistically on restricted action page
2 parents d1dbbcb + 09e4767 commit cb41497

3 files changed

Lines changed: 223 additions & 13 deletions

File tree

‎src/libs/actions/Subscription.ts‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type {OnyxUpdate} from 'react-native-onyx';
1+
import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx';
22
import Onyx from 'react-native-onyx';
33
import * as API from '@libs/API';
44
import type {CancelBillingSubscriptionParams, UpdateSubscriptionAddNewUsersAutomaticallyParams, UpdateSubscriptionAutoRenewParams, UpdateSubscriptionTypeParams} from '@libs/API/parameters';
@@ -7,34 +7,62 @@ import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils';
77
import CONST from '@src/CONST';
88
import type {FeedbackSurveyOptionID, SubscriptionType} from '@src/CONST';
99
import ONYXKEYS from '@src/ONYXKEYS';
10+
import type {BillingGraceEndPeriod} from '@src/types/onyx';
1011
import type {OnyxData} from '@src/types/onyx/Request';
1112

1213
/**
1314
* Fetches data when the user opens the SubscriptionSettingsPage
15+
* @param currentGracePeriods - The current billing grace period collection. If provided and non-empty,
16+
* all entries will be optimistically cleared to handle stale cache when
17+
* billing was resolved (e.g. owner changed). On failure, previous values are restored.
1418
*/
15-
function openSubscriptionPage() {
16-
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA>> = [
19+
function openSubscriptionPage(currentGracePeriods?: OnyxCollection<BillingGraceEndPeriod>) {
20+
type SubscriptionOnyxKeys = typeof ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA | typeof ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END;
21+
22+
const optimisticData: Array<OnyxUpdate<SubscriptionOnyxKeys>> = [
1723
{
1824
onyxMethod: Onyx.METHOD.SET,
1925
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
2026
value: true,
2127
},
2228
];
23-
const successData: Array<OnyxUpdate<typeof ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA>> = [
29+
30+
const successData: Array<OnyxUpdate<SubscriptionOnyxKeys>> = [
2431
{
2532
onyxMethod: Onyx.METHOD.SET,
2633
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
2734
value: false,
2835
},
2936
];
30-
const failureData: Array<OnyxUpdate<typeof ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA>> = [
37+
38+
const failureData: Array<OnyxUpdate<SubscriptionOnyxKeys>> = [
3139
{
3240
onyxMethod: Onyx.METHOD.SET,
3341
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
3442
value: false,
3543
},
3644
];
3745

46+
// Clear ALL billing grace period keys optimistically. If the server still has active
47+
// grace periods, they will be restored from the response. This clears the entire
48+
// collection rather than a single owner's key, so stale entries from previous owners
49+
// are also evicted. On failure, previous values are restored.
50+
if (currentGracePeriods) {
51+
const keys = Object.keys(currentGracePeriods) as Array<`${typeof ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END}${string}`>;
52+
for (const key of keys) {
53+
optimisticData.push({
54+
onyxMethod: Onyx.METHOD.SET,
55+
key,
56+
value: null,
57+
});
58+
failureData.push({
59+
onyxMethod: Onyx.METHOD.SET,
60+
key,
61+
value: currentGracePeriods[key] ?? null,
62+
});
63+
}
64+
}
65+
3866
API.read(READ_COMMANDS.OPEN_SUBSCRIPTION_PAGE, null, {optimisticData, successData, failureData});
3967
}
4068

‎src/pages/RestrictedAction/Workspace/WorkspaceRestrictedActionPage.tsx‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, {useEffect} from 'react';
1+
import React, {useEffect, useRef} from 'react';
22
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
33
import useNetwork from '@hooks/useNetwork';
44
import useOnyx from '@hooks/useOnyx';
@@ -30,20 +30,35 @@ function WorkspaceRestrictedActionPage({
3030
const policy = usePolicy(policyID);
3131
const styles = useThemeStyles();
3232
const [isLoadingSubscriptionData] = useOnyx(ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA);
33+
34+
// Watch billing NVPs so the component re-renders when fresh data arrives from the server.
35+
const [userBillingGracePeriods] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END);
36+
const [ownerBillingGracePeriodEnd] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END);
37+
38+
// Track grace periods in a ref so openSubscriptionPage can roll back on failure
39+
// without adding the collection to effect dependencies (which would re-trigger the fetch
40+
// on every optimistic update).
41+
const gracePeriodsRef = useRef(userBillingGracePeriods);
42+
useEffect(() => {
43+
gracePeriodsRef.current = userBillingGracePeriods;
44+
}, [userBillingGracePeriods]);
45+
3346
const {isOffline} = useNetwork({
34-
onReconnect: () => openSubscriptionPage(),
47+
onReconnect: () => openSubscriptionPage(gracePeriodsRef.current),
3548
});
3649

3750
// Fetch fresh billing NVPs from the server on mount.
3851
// The cached billing data may be stale, causing the restriction to persist
3952
// even after the workspace owner has resolved their billing issue.
53+
// Skip when offline since the API call won't go through and the optimistic
54+
// clear would incorrectly lift the restriction.
4055
useEffect(() => {
41-
openSubscriptionPage();
42-
}, []);
43-
44-
// Watch billing NVPs so the component re-renders when fresh data arrives from the server.
45-
const [userBillingGracePeriods] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END);
46-
const [ownerBillingGracePeriodEnd] = useOnyx(ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END);
56+
if (isOffline) {
57+
return;
58+
}
59+
openSubscriptionPage(gracePeriodsRef.current);
60+
// eslint-disable-next-line react-hooks/exhaustive-deps
61+
}, [isOffline]);
4762

4863
// Navigate back if the fresh server data shows the restriction no longer applies.
4964
useEffect(() => {

‎tests/actions/SubscriptionTest.ts‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import Onyx from 'react-native-onyx';
2+
import {read} from '@libs/API';
3+
import {READ_COMMANDS} from '@libs/API/types';
4+
import ONYXKEYS from '@src/ONYXKEYS';
5+
import type {BillingGraceEndPeriod} from '@src/types/onyx';
6+
import {openSubscriptionPage} from '../../src/libs/actions/Subscription';
7+
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
8+
9+
jest.mock('@libs/API');
10+
const mockRead = jest.mocked(read);
11+
12+
describe('actions/Subscription', () => {
13+
beforeAll(() => {
14+
Onyx.init({
15+
keys: ONYXKEYS,
16+
});
17+
});
18+
19+
beforeEach(() => {
20+
jest.clearAllMocks();
21+
22+
return Onyx.clear().then(waitForBatchedUpdates);
23+
});
24+
25+
afterEach(() => {
26+
jest.restoreAllMocks();
27+
});
28+
29+
describe('openSubscriptionPage', () => {
30+
it('should call API.read with loading optimistic/success/failure data when no grace periods are provided', () => {
31+
openSubscriptionPage();
32+
33+
expect(mockRead).toHaveBeenCalledWith(
34+
READ_COMMANDS.OPEN_SUBSCRIPTION_PAGE,
35+
null,
36+
expect.objectContaining({
37+
optimisticData: [
38+
{
39+
onyxMethod: Onyx.METHOD.SET,
40+
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
41+
value: true,
42+
},
43+
],
44+
successData: [
45+
{
46+
onyxMethod: Onyx.METHOD.SET,
47+
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
48+
value: false,
49+
},
50+
],
51+
failureData: [
52+
{
53+
onyxMethod: Onyx.METHOD.SET,
54+
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
55+
value: false,
56+
},
57+
],
58+
}),
59+
);
60+
});
61+
62+
it('should clear all grace period keys optimistically and restore on failure', () => {
63+
const gracePeriod1: BillingGraceEndPeriod = {value: 1700000000};
64+
const gracePeriod2: BillingGraceEndPeriod = {value: 1700099999};
65+
const key1 = `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END}11111`;
66+
const key2 = `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END}22222`;
67+
68+
openSubscriptionPage({
69+
[key1]: gracePeriod1,
70+
[key2]: gracePeriod2,
71+
});
72+
73+
expect(mockRead).toHaveBeenCalledWith(
74+
READ_COMMANDS.OPEN_SUBSCRIPTION_PAGE,
75+
null,
76+
expect.objectContaining({
77+
optimisticData: expect.arrayContaining([
78+
{
79+
onyxMethod: Onyx.METHOD.SET,
80+
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
81+
value: true,
82+
},
83+
{
84+
onyxMethod: Onyx.METHOD.SET,
85+
key: key1,
86+
value: null,
87+
},
88+
{
89+
onyxMethod: Onyx.METHOD.SET,
90+
key: key2,
91+
value: null,
92+
},
93+
]),
94+
failureData: expect.arrayContaining([
95+
{
96+
onyxMethod: Onyx.METHOD.SET,
97+
key: ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA,
98+
value: false,
99+
},
100+
{
101+
onyxMethod: Onyx.METHOD.SET,
102+
key: key1,
103+
value: gracePeriod1,
104+
},
105+
{
106+
onyxMethod: Onyx.METHOD.SET,
107+
key: key2,
108+
value: gracePeriod2,
109+
},
110+
]),
111+
}),
112+
);
113+
});
114+
115+
it('should handle undefined values in the grace period collection by rolling back to null', () => {
116+
const key1 = `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_USER_BILLING_GRACE_PERIOD_END}33333`;
117+
118+
openSubscriptionPage({
119+
[key1]: undefined,
120+
});
121+
122+
expect(mockRead).toHaveBeenCalledWith(
123+
READ_COMMANDS.OPEN_SUBSCRIPTION_PAGE,
124+
null,
125+
expect.objectContaining({
126+
failureData: expect.arrayContaining([
127+
{
128+
onyxMethod: Onyx.METHOD.SET,
129+
key: key1,
130+
value: null,
131+
},
132+
]),
133+
}),
134+
);
135+
});
136+
137+
it('should not include grace period data when collection is undefined', () => {
138+
openSubscriptionPage(undefined);
139+
140+
const call = mockRead.mock.calls.at(0);
141+
const onyxData = call?.at(2) as Parameters<typeof mockRead>[2];
142+
143+
// optimisticData should only have the loading key
144+
expect(onyxData?.optimisticData).toHaveLength(1);
145+
expect(onyxData?.optimisticData?.[0]?.key).toBe(ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA);
146+
147+
// failureData should only have the loading key
148+
expect(onyxData?.failureData).toHaveLength(1);
149+
expect(onyxData?.failureData?.[0]?.key).toBe(ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA);
150+
});
151+
152+
it('should not include grace period data when collection is empty', () => {
153+
openSubscriptionPage({});
154+
155+
const call = mockRead.mock.calls.at(0);
156+
const onyxData = call?.at(2) as Parameters<typeof mockRead>[2];
157+
158+
// optimisticData should only have the loading key
159+
expect(onyxData?.optimisticData).toHaveLength(1);
160+
expect(onyxData?.optimisticData?.[0]?.key).toBe(ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA);
161+
162+
// failureData should only have the loading key
163+
expect(onyxData?.failureData).toHaveLength(1);
164+
expect(onyxData?.failureData?.[0]?.key).toBe(ONYXKEYS.IS_LOADING_SUBSCRIPTION_DATA);
165+
});
166+
});
167+
});

0 commit comments

Comments
 (0)