Skip to content

Commit 48916e2

Browse files
GonalsGonals
authored
Merge pull request Expensify#86851 from DylanDylann/refactor-66580-p2
Part 2: Remove Onyx.connect() for the key: ONYXKEYS.PERSONAL_DETAILS_LIST in src/libs/actions/Policy/Policy.ts
2 parents e38712f + ab775e2 commit 48916e2

6 files changed

Lines changed: 152 additions & 3 deletions

File tree

‎src/libs/actions/Policy/Policy.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ type DeleteWorkspaceActionParams = {
383383
localeCompare: LocaleContextProps['localeCompare'];
384384
hasDeleteWorkspaceExpensifyCardsError?: boolean;
385385
currentUserAccountID: number;
386+
accountIDToLogin: Record<number, string>;
386387
};
387388

388389
/**
@@ -404,6 +405,7 @@ function deleteWorkspace(params: DeleteWorkspaceActionParams) {
404405
personalPolicyID,
405406
hasDeleteWorkspaceExpensifyCardsError,
406407
currentUserAccountID,
408+
accountIDToLogin,
407409
} = params;
408410

409411
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
@@ -546,7 +548,7 @@ function deleteWorkspace(params: DeleteWorkspaceActionParams) {
546548
// Announce & admin chats have FAKE owners, but expense chats w/ users do have owners.
547549
let emailClosingReport: string = CONST.POLICY.OWNER_EMAIL_FAKE;
548550
if (!!ownerAccountID && ownerAccountID !== CONST.POLICY.OWNER_ACCOUNT_ID_FAKE) {
549-
emailClosingReport = deprecatedAllPersonalDetails?.[ownerAccountID]?.login ?? '';
551+
emailClosingReport = accountIDToLogin[ownerAccountID] ?? '';
550552
}
551553
const optimisticClosedReportAction = ReportUtils.buildOptimisticClosedReportAction(emailClosingReport, policyName, currentUserAccountID, CONST.REPORT.ARCHIVE_REASON.POLICY_DELETED);
552554
optimisticData.push({

‎src/pages/workspace/WorkspaceOverviewPage.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import CONST from '@src/CONST';
7272
import ONYXKEYS from '@src/ONYXKEYS';
7373
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
7474
import type SCREENS from '@src/SCREENS';
75+
import {accountIDToLoginSelector} from '@src/selectors/PersonalDetails';
7576
import {ownerPoliciesSelector} from '@src/selectors/Policy';
7677
import {reimbursementAccountErrorSelector} from '@src/selectors/ReimbursementAccount';
7778
import {isEmptyObject} from '@src/types/utils/EmptyObject';
@@ -185,6 +186,7 @@ function WorkspaceOverviewPage({policyDraft, policy: policyProp, route}: Workspa
185186
const [isLeaveModalOpen, setIsLeaveModalOpen] = useState(false);
186187
const [session] = useOnyx(ONYXKEYS.SESSION);
187188
const personalDetails = usePersonalDetails();
189+
const [accountIDToLogin] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: accountIDToLoginSelector(reportsToArchive)});
188190
const [isCannotLeaveWorkspaceModalOpen, setIsCannotLeaveWorkspaceModalOpen] = useState(false);
189191
const privateSubscription = usePrivateSubscription();
190192
const accountID = currentUserPersonalDetails?.accountID;
@@ -271,6 +273,7 @@ function WorkspaceOverviewPage({policyDraft, policy: policyProp, route}: Workspa
271273
personalPolicyID,
272274
hasDeleteWorkspaceExpensifyCardsError,
273275
currentUserAccountID: accountID,
276+
accountIDToLogin: accountIDToLogin ?? {},
274277
});
275278
if (isOffline) {
276279
setIsDeleteModalOpen(false);

‎src/pages/workspace/WorkspacesListPage.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import CONST from '@src/CONST';
7373
import ONYXKEYS from '@src/ONYXKEYS';
7474
import ROUTES from '@src/ROUTES';
7575
import type SCREENS from '@src/SCREENS';
76+
import {accountIDToLoginSelector} from '@src/selectors/PersonalDetails';
7677
import {ownerPoliciesSelector} from '@src/selectors/Policy';
7778
import {reimbursementAccountErrorSelector} from '@src/selectors/ReimbursementAccount';
7879
import type {Policy as PolicyType} from '@src/types/onyx';
@@ -202,6 +203,7 @@ function WorkspacesListPage() {
202203
policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyIDToDelete}`]?.workspaceAccountID);
203204
const hasExpensifyCard = !!policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyIDToDelete}`]?.areExpensifyCardsEnabled && !isEmptyObject(cardsList);
204205
const personalDetails = usePersonalDetails();
206+
const [accountIDToLogin] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: accountIDToLoginSelector(reportsToArchive)});
205207
const [isLeaveModalOpen, setIsLeaveModalOpen] = useState(false);
206208
const [isCannotLeaveWorkspaceModalOpen, setIsCannotLeaveWorkspaceModalOpen] = useState(false);
207209
const [policyIDToLeave, setPolicyIDToLeave] = useState<string>();
@@ -242,6 +244,7 @@ function WorkspacesListPage() {
242244
personalPolicyID,
243245
hasDeleteWorkspaceExpensifyCardsError,
244246
currentUserAccountID: currentUserPersonalDetails.accountID,
247+
accountIDToLogin: accountIDToLogin ?? {},
245248
});
246249
if (isOffline) {
247250
setIsDeleteModalOpen(false);

‎src/selectors/PersonalDetails.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
11
import type {OnyxEntry} from 'react-native-onyx';
2-
import type {PersonalDetailsList} from '@src/types/onyx';
2+
import CONST from '@src/CONST';
3+
import type {PersonalDetailsList, Report} from '@src/types/onyx';
34

45
const personalDetailsSelector = (accountID: number) => (personalDetailsList: OnyxEntry<PersonalDetailsList>) => personalDetailsList?.[accountID];
56

67
const personalDetailsLoginSelector = (accountID: number) => (personalDetailsList: OnyxEntry<PersonalDetailsList>) => personalDetailsList?.[accountID]?.login;
78

8-
export {personalDetailsSelector, personalDetailsLoginSelector};
9+
const accountIDToLoginSelector = (reportsToArchive: Report[]) => (personalDetailsList: OnyxEntry<PersonalDetailsList>) => {
10+
const map: Record<number, string> = {};
11+
for (const report of reportsToArchive) {
12+
const {ownerAccountID} = report;
13+
if (ownerAccountID && ownerAccountID !== CONST.POLICY.OWNER_ACCOUNT_ID_FAKE && personalDetailsList?.[ownerAccountID]?.login) {
14+
map[ownerAccountID] = personalDetailsList[ownerAccountID].login;
15+
}
16+
}
17+
return map;
18+
};
19+
20+
export {personalDetailsSelector, personalDetailsLoginSelector, accountIDToLoginSelector};

‎tests/actions/IOUTest.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10148,6 +10148,7 @@ describe('actions/IOU', () => {
1014810148
lastUsedPaymentMethods: undefined,
1014910149
localeCompare,
1015010150
currentUserAccountID: CARLOS_ACCOUNT_ID,
10151+
accountIDToLogin: {},
1015110152
});
1015210153
}
1015310154
return waitForBatchedUpdates();

‎tests/actions/PolicyTest.ts‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2846,6 +2846,7 @@ describe('actions/Policy', () => {
28462846
lastUsedPaymentMethods: undefined,
28472847
localeCompare: TestHelper.localeCompare,
28482848
currentUserAccountID: ESH_ACCOUNT_ID,
2849+
accountIDToLogin: {},
28492850
});
28502851

28512852
await waitForBatchedUpdates();
@@ -2945,6 +2946,7 @@ describe('actions/Policy', () => {
29452946
lastUsedPaymentMethods: undefined,
29462947
localeCompare: TestHelper.localeCompare,
29472948
currentUserAccountID: ESH_ACCOUNT_ID,
2949+
accountIDToLogin: {},
29482950
});
29492951

29502952
await waitForBatchedUpdates();
@@ -3002,6 +3004,7 @@ describe('actions/Policy', () => {
30023004
lastUsedPaymentMethods: undefined,
30033005
localeCompare: TestHelper.localeCompare,
30043006
currentUserAccountID: ESH_ACCOUNT_ID,
3007+
accountIDToLogin: {},
30053008
});
30063009
await waitForBatchedUpdates();
30073010

@@ -3040,6 +3043,7 @@ describe('actions/Policy', () => {
30403043
lastUsedPaymentMethods: undefined,
30413044
localeCompare: TestHelper.localeCompare,
30423045
currentUserAccountID: ESH_ACCOUNT_ID,
3046+
accountIDToLogin: {},
30433047
});
30443048
await waitForBatchedUpdates();
30453049

@@ -3080,6 +3084,7 @@ describe('actions/Policy', () => {
30803084
lastUsedPaymentMethods: undefined,
30813085
localeCompare: TestHelper.localeCompare,
30823086
currentUserAccountID: ESH_ACCOUNT_ID,
3087+
accountIDToLogin: {},
30833088
});
30843089
await waitForBatchedUpdates();
30853090

@@ -3095,6 +3100,129 @@ describe('actions/Policy', () => {
30953100

30963101
expect(lastAccessedWorkspacePolicyIDAfterDelete).toBe(lastAccessedWorkspacePolicyID);
30973102
});
3103+
3104+
it('should use accountIDToLogin to resolve owner email in closed report action', async () => {
3105+
const ownerAccountID = 42;
3106+
const ownerLogin = 'owner@example.com';
3107+
const fakePolicy = createRandomPolicy(0);
3108+
const fakeReport = {
3109+
...createRandomReport(0, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT),
3110+
ownerAccountID,
3111+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
3112+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
3113+
policyName: fakePolicy.name,
3114+
};
3115+
3116+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);
3117+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
3118+
3119+
Policy.deleteWorkspace({
3120+
policies: {[`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`]: fakePolicy},
3121+
policyID: fakePolicy.id,
3122+
personalPolicyID: undefined,
3123+
activePolicyID: undefined,
3124+
policyName: fakePolicy.name,
3125+
lastAccessedWorkspacePolicyID: undefined,
3126+
policyCardFeeds: undefined,
3127+
reportsToArchive: [fakeReport],
3128+
transactionViolations: undefined,
3129+
reimbursementAccountError: undefined,
3130+
lastUsedPaymentMethods: undefined,
3131+
localeCompare: TestHelper.localeCompare,
3132+
currentUserAccountID: ESH_ACCOUNT_ID,
3133+
accountIDToLogin: {[ownerAccountID]: ownerLogin},
3134+
});
3135+
3136+
await waitForBatchedUpdates();
3137+
3138+
// Then the closed report action should contain the owner's login from accountIDToLogin
3139+
const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${fakeReport.reportID}` as const);
3140+
const closedAction = Object.values(reportActions ?? {}).find((action) => action && 'actionName' in action && action.actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED);
3141+
expect(closedAction).toBeDefined();
3142+
const message = closedAction && 'message' in closedAction && Array.isArray(closedAction.message) ? closedAction.message.at(0) : undefined;
3143+
expect(message?.text).toBe(ownerLogin);
3144+
});
3145+
3146+
it('should use fake owner email when ownerAccountID is fake', async () => {
3147+
const fakePolicy = createRandomPolicy(0);
3148+
const fakeReport = {
3149+
...createRandomReport(0, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT),
3150+
ownerAccountID: CONST.POLICY.OWNER_ACCOUNT_ID_FAKE,
3151+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
3152+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
3153+
policyName: fakePolicy.name,
3154+
};
3155+
3156+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);
3157+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
3158+
3159+
Policy.deleteWorkspace({
3160+
policies: {[`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`]: fakePolicy},
3161+
policyID: fakePolicy.id,
3162+
personalPolicyID: undefined,
3163+
activePolicyID: undefined,
3164+
policyName: fakePolicy.name,
3165+
lastAccessedWorkspacePolicyID: undefined,
3166+
policyCardFeeds: undefined,
3167+
reportsToArchive: [fakeReport],
3168+
transactionViolations: undefined,
3169+
reimbursementAccountError: undefined,
3170+
lastUsedPaymentMethods: undefined,
3171+
localeCompare: TestHelper.localeCompare,
3172+
currentUserAccountID: ESH_ACCOUNT_ID,
3173+
accountIDToLogin: {},
3174+
});
3175+
3176+
await waitForBatchedUpdates();
3177+
3178+
// Then the closed report action should use the fake owner email
3179+
const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${fakeReport.reportID}` as const);
3180+
const closedAction = Object.values(reportActions ?? {}).find((action) => action && 'actionName' in action && action.actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED);
3181+
expect(closedAction).toBeDefined();
3182+
const message = closedAction && 'message' in closedAction && Array.isArray(closedAction.message) ? closedAction.message.at(0) : undefined;
3183+
expect(message?.text).toBe(CONST.POLICY.OWNER_EMAIL_FAKE);
3184+
});
3185+
3186+
it('should fall back to empty string when accountIDToLogin has no entry for ownerAccountID', async () => {
3187+
const ownerAccountID = 99;
3188+
const fakePolicy = createRandomPolicy(0);
3189+
const fakeReport = {
3190+
...createRandomReport(0, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT),
3191+
ownerAccountID,
3192+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
3193+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
3194+
policyName: fakePolicy.name,
3195+
};
3196+
3197+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);
3198+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
3199+
3200+
Policy.deleteWorkspace({
3201+
policies: {[`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`]: fakePolicy},
3202+
policyID: fakePolicy.id,
3203+
personalPolicyID: undefined,
3204+
activePolicyID: undefined,
3205+
policyName: fakePolicy.name,
3206+
lastAccessedWorkspacePolicyID: undefined,
3207+
policyCardFeeds: undefined,
3208+
reportsToArchive: [fakeReport],
3209+
transactionViolations: undefined,
3210+
reimbursementAccountError: undefined,
3211+
lastUsedPaymentMethods: undefined,
3212+
localeCompare: TestHelper.localeCompare,
3213+
currentUserAccountID: ESH_ACCOUNT_ID,
3214+
accountIDToLogin: {},
3215+
});
3216+
3217+
await waitForBatchedUpdates();
3218+
3219+
// Then the closed report action should have an empty string for the email
3220+
const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${fakeReport.reportID}` as const);
3221+
const closedAction = Object.values(reportActions ?? {}).find((action) => action && 'actionName' in action && action.actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED);
3222+
expect(closedAction).toBeDefined();
3223+
const message = closedAction && 'message' in closedAction && Array.isArray(closedAction.message) ? closedAction.message.at(0) : undefined;
3224+
expect(message?.text).toBe('');
3225+
});
30983226
});
30993227

31003228
describe('leaveWorkspace', () => {

0 commit comments

Comments
 (0)