Skip to content

Commit 37661b0

Browse files
authored
Merge pull request Expensify#80639 from shubham1206agra/refactor-buildOptimisticChangePolicyData
Refactor: isolate buildOptimisticChangePolicyData from Onyx.connect ONYXKEYS.COLLECTION.REPORT
2 parents bc8e27b + 0256f61 commit 37661b0

3 files changed

Lines changed: 79 additions & 42 deletions

File tree

src/libs/actions/Report/index.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5940,6 +5940,7 @@ function navigateToTrainingModal(isChangePolicyTrainingModalDismissed: boolean,
59405940

59415941
function buildOptimisticChangePolicyData(
59425942
report: Report,
5943+
parentReport: OnyxEntry<Report>,
59435944
policy: Policy,
59445945
currentUserAccountID: number,
59455946
email: string,
@@ -6155,7 +6156,6 @@ function buildOptimisticChangePolicyData(
61556156
});
61566157

61576158
// Update the expense chat report
6158-
const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oldWorkspaceChatReportID}`];
61596159
const lastMessageText = getLastVisibleMessage(oldWorkspaceChatReportID, isReportLastVisibleArchived, {
61606160
[oldReportPreviewActionID]: updatedReportPreviewAction as ReportAction,
61616161
})?.lastMessageText;
@@ -6177,7 +6177,7 @@ function buildOptimisticChangePolicyData(
61776177
failureData.push({
61786178
onyxMethod: Onyx.METHOD.MERGE,
61796179
key: `${ONYXKEYS.COLLECTION.REPORT}${oldWorkspaceChatReportID}`,
6180-
value: chatReport,
6180+
value: parentReport,
61816181
});
61826182
}
61836183

@@ -6401,6 +6401,7 @@ function buildOptimisticChangePolicyData(
64016401
*/
64026402
function changeReportPolicy(
64036403
report: Report,
6404+
parentReport: OnyxEntry<Report>,
64046405
policy: Policy,
64056406
accountID: number,
64066407
email: string,
@@ -6416,6 +6417,7 @@ function changeReportPolicy(
64166417

64176418
const {optimisticData, successData, failureData, optimisticReportPreviewAction, optimisticMovedReportAction} = buildOptimisticChangePolicyData(
64186419
report,
6420+
parentReport,
64196421
policy,
64206422
accountID,
64216423
email,
@@ -6441,18 +6443,31 @@ function changeReportPolicy(
64416443
/**
64426444
* Invites the submitter to the new report policy, changes the policy of a report and all its child reports, and moves the report to the new policy's expense chat
64436445
*/
6444-
function changeReportPolicyAndInviteSubmitter(
6445-
report: Report,
6446-
policy: Policy,
6447-
currentUserAccountID: number,
6448-
email: string,
6449-
hasViolationsParam: boolean,
6450-
isChangePolicyTrainingModalDismissed: boolean,
6451-
isASAPSubmitBetaEnabled: boolean,
6452-
employeeList: PolicyEmployeeList | undefined,
6453-
formatPhoneNumber: LocaleContextProps['formatPhoneNumber'],
6454-
isReportLastVisibleArchived: boolean | undefined,
6455-
) {
6446+
function changeReportPolicyAndInviteSubmitter({
6447+
report,
6448+
parentReport,
6449+
policy,
6450+
currentUserAccountID,
6451+
email,
6452+
hasViolationsParam,
6453+
isChangePolicyTrainingModalDismissed,
6454+
isASAPSubmitBetaEnabled,
6455+
employeeList,
6456+
formatPhoneNumber,
6457+
isReportLastVisibleArchived,
6458+
}: {
6459+
report: Report;
6460+
parentReport: OnyxEntry<Report>;
6461+
policy: Policy;
6462+
currentUserAccountID: number;
6463+
email: string;
6464+
hasViolationsParam: boolean;
6465+
isChangePolicyTrainingModalDismissed: boolean;
6466+
isASAPSubmitBetaEnabled: boolean;
6467+
employeeList: PolicyEmployeeList | undefined;
6468+
formatPhoneNumber: LocaleContextProps['formatPhoneNumber'];
6469+
isReportLastVisibleArchived: boolean | undefined;
6470+
}) {
64566471
if (!report.reportID || !policy?.id || report.policyID === policy.id || !isExpenseReport(report) || !report.ownerAccountID) {
64576472
return;
64586473
}
@@ -6486,6 +6501,7 @@ function changeReportPolicyAndInviteSubmitter(
64866501
optimisticMovedReportAction,
64876502
} = buildOptimisticChangePolicyData(
64886503
report,
6504+
parentReport,
64896505
policy,
64906506
currentUserAccountID,
64916507
email,

src/pages/ReportChangeWorkspacePage.tsx

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ function ReportChangeWorkspacePage({report, route}: ReportChangeWorkspacePagePro
5454
const {translate, formatPhoneNumber, localeCompare} = useLocalize();
5555
const reportTransactions = useReportTransactions(reportID);
5656

57+
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`, {canBeMissing: true});
5758
const [policies, fetchStatus] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {canBeMissing: false});
5859
const [reportNextStep] = useOnyx(`${ONYXKEYS.COLLECTION.NEXT_STEP}${reportID}`, {canBeMissing: true});
5960
const [isChangePolicyTrainingModalDismissed = false] = useOnyx(ONYXKEYS.NVP_DISMISSED_PRODUCT_TRAINING, {canBeMissing: true, selector: changePolicyTrainingModalDismissedSelector});
@@ -94,21 +95,23 @@ function ReportChangeWorkspacePage({report, route}: ReportChangeWorkspacePagePro
9495
// eslint-disable-next-line @typescript-eslint/no-deprecated
9596
} else if (isExpenseReport(report) && isPolicyAdmin(policy) && report.ownerAccountID && !isPolicyMember(policy, getLoginByAccountID(report.ownerAccountID))) {
9697
const employeeList = policy?.employeeList;
97-
changeReportPolicyAndInviteSubmitter(
98+
changeReportPolicyAndInviteSubmitter({
9899
report,
100+
parentReport,
99101
policy,
100-
session?.accountID ?? CONST.DEFAULT_NUMBER_ID,
101-
session?.email ?? '',
102-
hasViolations,
102+
currentUserAccountID: session?.accountID ?? CONST.DEFAULT_NUMBER_ID,
103+
email: session?.email ?? '',
104+
hasViolationsParam: hasViolations,
103105
isChangePolicyTrainingModalDismissed,
104106
isASAPSubmitBetaEnabled,
105107
employeeList,
106108
formatPhoneNumber,
107109
isReportLastVisibleArchived,
108-
);
110+
});
109111
} else {
110112
changeReportPolicy(
111113
report,
114+
parentReport,
112115
policy,
113116
session?.accountID ?? CONST.DEFAULT_NUMBER_ID,
114117
session?.email ?? '',
@@ -125,6 +128,7 @@ function ReportChangeWorkspacePage({report, route}: ReportChangeWorkspacePagePro
125128
route.params,
126129
reportID,
127130
report,
131+
parentReport,
128132
formatPhoneNumber,
129133
reportTransactions,
130134
isReportLastVisibleArchived,

tests/actions/ReportTest.ts

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2484,7 +2484,7 @@ describe('actions/Report', () => {
24842484
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${newPolicy.id}`, newPolicy);
24852485

24862486
// When moving to another workspace
2487-
Report.changeReportPolicy(expenseReport, newPolicy, 1, '', true, false, false);
2487+
Report.changeReportPolicy(expenseReport, undefined, newPolicy, 1, '', true, false, false);
24882488
await waitForBatchedUpdates();
24892489

24902490
// Then the expense report should not be archived anymore
@@ -2522,12 +2522,16 @@ describe('actions/Report', () => {
25222522
chatReportID: '2',
25232523
parentReportID: '2',
25242524
};
2525+
const parentReport: OnyxTypes.Report = {
2526+
...createRandomReport(2, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT),
2527+
policyID: '1',
2528+
};
25252529

25262530
const newPolicy = createRandomPolicy(2);
25272531
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${newPolicy.id}`, newPolicy);
25282532

25292533
// When moving to another workspace
2530-
Report.changeReportPolicy(expenseReport, newPolicy, 1, '', false, false, false);
2534+
Report.changeReportPolicy(expenseReport, parentReport, newPolicy, 1, '', false, false, false);
25312535
await waitForBatchedUpdates();
25322536

25332537
// Then the expense report chatReportID and parentReportID should be updated to the new expense chat reportID
@@ -2584,7 +2588,7 @@ describe('actions/Report', () => {
25842588
};
25852589
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${newPolicy.id}`, newPolicy);
25862590

2587-
Report.changeReportPolicy(expenseReport, newPolicy, 1, '', false, false, false);
2591+
Report.changeReportPolicy(expenseReport, undefined, newPolicy, 1, '', false, false, false);
25882592
await waitForBatchedUpdates();
25892593

25902594
const updatedReport = await new Promise<OnyxEntry<OnyxTypes.Report>>((resolve) => {
@@ -2667,7 +2671,7 @@ describe('actions/Report', () => {
26672671
};
26682672
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${newPolicy.id}`, newPolicy);
26692673

2670-
Report.changeReportPolicy(expenseReport, newPolicy, 1, '', false, false, false);
2674+
Report.changeReportPolicy(expenseReport, undefined, newPolicy, 1, '', false, false, false);
26712675
await waitForBatchedUpdates();
26722676

26732677
// Then the report total should correctly include expense (-1000) and refund (+500) = -500
@@ -2738,7 +2742,7 @@ describe('actions/Report', () => {
27382742
};
27392743
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${newPolicy.id}`, newPolicy);
27402744

2741-
Report.changeReportPolicy(expenseReport, newPolicy, 1, '', false, false, false);
2745+
Report.changeReportPolicy(expenseReport, undefined, newPolicy, 1, '', false, false, false);
27422746
await waitForBatchedUpdates();
27432747

27442748
// Then only AUD transaction should contribute to total (-1000), USD is excluded
@@ -2777,20 +2781,21 @@ describe('actions/Report', () => {
27772781
});
27782782

27792783
// When moving to another workspace
2780-
Report.changeReportPolicyAndInviteSubmitter(
2781-
expenseReport,
2782-
createRandomPolicy(Number(2)),
2783-
1,
2784-
'',
2785-
true,
2786-
false,
2787-
false,
2788-
{
2784+
Report.changeReportPolicyAndInviteSubmitter({
2785+
report: expenseReport,
2786+
parentReport: undefined,
2787+
policy: createRandomPolicy(Number(2)),
2788+
currentUserAccountID: 1,
2789+
email: '',
2790+
hasViolationsParam: true,
2791+
isChangePolicyTrainingModalDismissed: false,
2792+
isASAPSubmitBetaEnabled: false,
2793+
employeeList: {
27892794
[adminEmail]: {role: CONST.POLICY.ROLE.ADMIN},
27902795
},
2791-
TestHelper.formatPhoneNumber,
2792-
undefined,
2793-
);
2796+
formatPhoneNumber: TestHelper.formatPhoneNumber,
2797+
isReportLastVisibleArchived: undefined,
2798+
});
27942799
await waitForBatchedUpdates();
27952800

27962801
// Then the expense report should not be archived anymore
@@ -2864,7 +2869,19 @@ describe('actions/Report', () => {
28642869
await waitForBatchedUpdates();
28652870

28662871
// Call changeReportPolicyAndInviteSubmitter
2867-
Report.changeReportPolicyAndInviteSubmitter(expenseReport, newPolicy, 1, '', true, false, false, employeeList, TestHelper.formatPhoneNumber, false);
2872+
Report.changeReportPolicyAndInviteSubmitter({
2873+
report: expenseReport,
2874+
parentReport: undefined,
2875+
policy: newPolicy,
2876+
currentUserAccountID: 1,
2877+
email: '',
2878+
hasViolationsParam: true,
2879+
isChangePolicyTrainingModalDismissed: false,
2880+
isASAPSubmitBetaEnabled: false,
2881+
employeeList,
2882+
formatPhoneNumber: TestHelper.formatPhoneNumber,
2883+
isReportLastVisibleArchived: false,
2884+
});
28682885
await waitForBatchedUpdates();
28692886

28702887
// Simulate network failure
@@ -3051,7 +3068,7 @@ describe('actions/Report', () => {
30513068
type: CONST.REPORT.TYPE.EXPENSE,
30523069
};
30533070
const policy = createRandomPolicy(Number(1));
3054-
Report.buildOptimisticChangePolicyData(report, policy, 1, '', false, true, undefined);
3071+
Report.buildOptimisticChangePolicyData(report, undefined, policy, 1, '', false, true, undefined);
30553072
// eslint-disable-next-line @typescript-eslint/no-deprecated
30563073
expect(buildNextStepNew).toHaveBeenCalledWith({
30573074
report,
@@ -3092,7 +3109,7 @@ describe('actions/Report', () => {
30923109
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction);
30933110
await waitForBatchedUpdates();
30943111

3095-
const {optimisticData, successData, failureData} = Report.buildOptimisticChangePolicyData(report, policy, 1, '', false, true, undefined);
3112+
const {optimisticData, successData, failureData} = Report.buildOptimisticChangePolicyData(report, undefined, policy, 1, '', false, true, undefined);
30963113

30973114
// Find the transaction optimistic data
30983115
const transactionOptimisticData = optimisticData.find((data) => data.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
@@ -3140,7 +3157,7 @@ describe('actions/Report', () => {
31403157
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction);
31413158
await waitForBatchedUpdates();
31423159

3143-
const {optimisticData} = Report.buildOptimisticChangePolicyData(report, policy, 1, '', false, true, undefined);
3160+
const {optimisticData} = Report.buildOptimisticChangePolicyData(report, undefined, policy, 1, '', false, true, undefined);
31443161

31453162
// Should NOT find transaction optimistic data when currencies are the same
31463163
const transactionOptimisticData = optimisticData.find((data) => data.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
@@ -3175,7 +3192,7 @@ describe('actions/Report', () => {
31753192
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction);
31763193
await waitForBatchedUpdates();
31773194

3178-
const {optimisticData} = Report.buildOptimisticChangePolicyData(report, policy, 1, '', false, true, undefined);
3195+
const {optimisticData} = Report.buildOptimisticChangePolicyData(report, undefined, policy, 1, '', false, true, undefined);
31793196

31803197
// Should NOT find transaction optimistic data when transaction matches destination currency
31813198
const transactionOptimisticData = optimisticData.find((data) => data.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
@@ -3223,7 +3240,7 @@ describe('actions/Report', () => {
32233240
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${nonMatchingTransactionID}`, nonMatchingTransaction);
32243241
await waitForBatchedUpdates();
32253242

3226-
const {optimisticData} = Report.buildOptimisticChangePolicyData(report, policy, 1, '', false, true, undefined);
3243+
const {optimisticData} = Report.buildOptimisticChangePolicyData(report, undefined, policy, 1, '', false, true, undefined);
32273244

32283245
// Should NOT find optimistic data for the matching transaction (USD matches USD destination)
32293246
const matchingOptimisticData = optimisticData.find((data) => data.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${matchingTransactionID}`);

0 commit comments

Comments
 (0)