Skip to content

Commit 5fe14a7

Browse files
authored
Merge pull request #89464 from shubham1206agra/refactor-lockAccount
Refactor: isolate lockAccount from ONYXKEYS.SESSION Onyx data
2 parents b4103e0 + eb97f0d commit 5fe14a7

3 files changed

Lines changed: 36 additions & 48 deletions

File tree

src/libs/actions/User.ts

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,6 @@ import {openOldDotLink} from './Link';
6767
import {showReportActionNotification} from './Report';
6868
import {resendValidateCode as sessionResendValidateCode} from './Session';
6969

70-
let currentUserAccountID: number = CONST.DEFAULT_NUMBER_ID;
71-
Onyx.connect({
72-
key: ONYXKEYS.SESSION,
73-
callback: (value) => {
74-
currentUserAccountID = value?.accountID ?? CONST.DEFAULT_NUMBER_ID;
75-
},
76-
});
77-
7870
type DomainOnyxUpdate =
7971
| OnyxUpdate<`${typeof ONYXKEYS.COLLECTION.DOMAIN}${string}`>
8072
| OnyxUpdate<`${typeof ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${string}`>
@@ -674,7 +666,7 @@ function isBlockedFromConcierge(blockedFromConciergeNVP: OnyxEntry<BlockedFromCo
674666

675667
function triggerNotifications<TKey extends OnyxKey>(
676668
onyxUpdates: Array<OnyxServerUpdate<TKey>>,
677-
currentUserAccountIDParam: number,
669+
currentUserAccountID: number,
678670
currentUserEmail: string,
679671
reportAttributes?: ReportAttributesDerivedValue['reports'],
680672
) {
@@ -689,27 +681,27 @@ function triggerNotifications<TKey extends OnyxKey>(
689681
for (const action of reportActions) {
690682
if (action) {
691683
// They aren't connected to a UI anywhere, it's OK to use currentUserEmail
692-
showReportActionNotification(reportID, action, currentUserAccountIDParam, currentUserEmail, reportAttributes);
684+
showReportActionNotification(reportID, action, currentUserAccountID, currentUserEmail, reportAttributes);
693685
}
694686
}
695687
}
696688
}
697689

698-
const isChannelMuted = (reportId: string, currentUserAccountIDParam: number) =>
690+
const isChannelMuted = (reportId: string, currentUserAccountID: number) =>
699691
new Promise((resolve) => {
700692
// We use `connectWithoutView` here since this connection is non-reactive in nature.
701693
const connection = Onyx.connectWithoutView({
702694
key: `${ONYXKEYS.COLLECTION.REPORT}${reportId}`,
703695
callback: (report) => {
704696
Onyx.disconnect(connection);
705-
const notificationPreference = report?.participants?.[currentUserAccountIDParam]?.notificationPreference;
697+
const notificationPreference = report?.participants?.[currentUserAccountID]?.notificationPreference;
706698

707699
resolve(!notificationPreference || notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE || ReportUtils.isHiddenForCurrentUser(notificationPreference));
708700
},
709701
});
710702
});
711703

712-
function playSoundForMessageType<TKey extends OnyxKey>(pushJSON: Array<OnyxServerUpdate<TKey>>, currentUserAccountIDParam: number, currentUserEmail: string) {
704+
function playSoundForMessageType<TKey extends OnyxKey>(pushJSON: Array<OnyxServerUpdate<TKey>>, currentUserAccountID: number, currentUserEmail: string) {
713705
const reportActionsOnly = pushJSON.filter((update) => update.key?.includes('reportActions_'));
714706
// "reportActions_5134363522480668" -> "5134363522480668"
715707
const reportID = reportActionsOnly
@@ -720,7 +712,7 @@ function playSoundForMessageType<TKey extends OnyxKey>(pushJSON: Array<OnyxServe
720712
return;
721713
}
722714

723-
isChannelMuted(reportID, currentUserAccountIDParam).then((isSoundMuted) => {
715+
isChannelMuted(reportID, currentUserAccountID).then((isSoundMuted) => {
724716
if (isSoundMuted) {
725717
return;
726718
}
@@ -799,13 +791,13 @@ function playSoundForMessageType<TKey extends OnyxKey>(pushJSON: Array<OnyxServe
799791
let pongHasBeenMissed = false;
800792
let lastPingSentTimestamp = Date.now();
801793
let lastPongReceivedTimestamp = Date.now();
802-
function subscribeToPusherPong(currentUserAccountIDParam: number) {
794+
function subscribeToPusherPong(currentUserAccountID: number) {
803795
// If there is no user accountID yet (because the app isn't fully setup yet), the channel can't be subscribed to so return early
804-
if (!currentUserAccountIDParam) {
796+
if (!currentUserAccountID) {
805797
return;
806798
}
807799

808-
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.PONG, currentUserAccountIDParam.toString(), (pushJSON) => {
800+
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.PONG, currentUserAccountID.toString(), (pushJSON) => {
809801
Log.info(`[Pusher PINGPONG] Received a PONG event from the server`, false, pushJSON);
810802
lastPongReceivedTimestamp = Date.now();
811803

@@ -879,7 +871,7 @@ function checkForLatePongReplies() {
879871

880872
let pingPusherIntervalID: ReturnType<typeof setInterval>;
881873
let checkForLatePongRepliesIntervalID: ReturnType<typeof setInterval>;
882-
function initializePusherPingPong(currentUserAccountIDParam: number) {
874+
function initializePusherPingPong(currentUserAccountID: number) {
883875
// Only run the ping pong from the leader client
884876
if (!ActiveClientManager.isClientTheLeader()) {
885877
Log.info("[Pusher PINGPONG] Not starting PING PONG because this instance isn't the leader client");
@@ -890,7 +882,7 @@ function initializePusherPingPong(currentUserAccountIDParam: number) {
890882

891883
// Subscribe to the pong event from Pusher. Unfortunately, there is no way of knowing when the client is actually subscribed
892884
// so there could be a little delay before the client is actually listening to this event.
893-
subscribeToPusherPong(currentUserAccountIDParam);
885+
subscribeToPusherPong(currentUserAccountID);
894886

895887
// If things are initializing again (which is fine because it will reinitialize each time Pusher authenticates), clear the old intervals
896888
if (pingPusherIntervalID) {
@@ -916,15 +908,15 @@ function initializePusherPingPong(currentUserAccountIDParam: number) {
916908
* Handles the newest events from Pusher where a single mega multipleEvents contains
917909
* an array of singular events all in one event
918910
*/
919-
function subscribeToUserEvents(currentUserAccountIDParam: number, currentUserEmail: string, getReportAttributes?: () => ReportAttributesDerivedValue['reports'] | undefined) {
911+
function subscribeToUserEvents(currentUserAccountID: number, currentUserEmail: string, getReportAttributes?: () => ReportAttributesDerivedValue['reports'] | undefined) {
920912
// If we don't have the user's accountID yet (because the app isn't fully setup yet) we can't subscribe so return early
921-
if (!currentUserAccountIDParam) {
913+
if (!currentUserAccountID) {
922914
return;
923915
}
924916

925917
// Handles the mega multipleEvents from Pusher which contains an array of single events.
926918
// Each single event is passed to PusherUtils in order to trigger the callbacks for that event
927-
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.MULTIPLE_EVENTS, currentUserAccountIDParam.toString(), (pushJSON) => {
919+
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.MULTIPLE_EVENTS, currentUserAccountID.toString(), (pushJSON) => {
928920
const pushEventData = pushJSON;
929921
// If this is not the main client, we shouldn't process any data received from pusher.
930922
if (!ActiveClientManager.isClientTheLeader()) {
@@ -949,7 +941,7 @@ function subscribeToUserEvents(currentUserAccountIDParam: number, currentUserEma
949941
// See https://github.com/Expensify/App/issues/57961 for more details
950942
const debouncedPlaySoundForMessageType = debounce(
951943
(pushJSONMessage: AnyOnyxServerUpdate[]) => {
952-
playSoundForMessageType(pushJSONMessage, currentUserAccountIDParam, currentUserEmail);
944+
playSoundForMessageType(pushJSONMessage, currentUserAccountID, currentUserEmail);
953945
},
954946
CONST.TIMING.PLAY_SOUND_MESSAGE_DEBOUNCE_TIME,
955947
{trailing: true},
@@ -962,7 +954,7 @@ function subscribeToUserEvents(currentUserAccountIDParam: number, currentUserEma
962954
return SequentialQueue.getCurrentRequest().then(() => {
963955
// If we don't have the currentUserAccountID (user is logged out) or this is not the
964956
// main client we don't want to update Onyx with data from Pusher
965-
if (!currentUserAccountIDParam) {
957+
if (!currentUserAccountID) {
966958
return;
967959
}
968960
if (!ActiveClientManager.isClientTheLeader()) {
@@ -971,7 +963,7 @@ function subscribeToUserEvents(currentUserAccountIDParam: number, currentUserEma
971963
}
972964

973965
const onyxUpdatePromise = Onyx.update(pushJSON).then(() => {
974-
triggerNotifications(pushJSON, currentUserAccountIDParam, currentUserEmail, getReportAttributes?.());
966+
triggerNotifications(pushJSON, currentUserAccountID, currentUserEmail, getReportAttributes?.());
975967
});
976968

977969
// Return a promise when Onyx is done updating so that the OnyxUpdatesManager can properly apply all
@@ -987,7 +979,7 @@ function subscribeToUserEvents(currentUserAccountIDParam: number, currentUserEma
987979
return Promise.resolve();
988980
});
989981

990-
initializePusherPingPong(currentUserAccountIDParam);
982+
initializePusherPingPong(currentUserAccountID);
991983
}
992984

993985
/**
@@ -1232,13 +1224,13 @@ function updateTheme(theme: ValueOf<typeof CONST.THEME>, shouldGoBack = true) {
12321224
/**
12331225
* Sets a custom status
12341226
*/
1235-
function updateCustomStatus(currentUserAccountIDParam: number, status: Status) {
1227+
function updateCustomStatus(currentUserAccountID: number, status: Status) {
12361228
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.PERSONAL_DETAILS_LIST>> = [
12371229
{
12381230
onyxMethod: Onyx.METHOD.MERGE,
12391231
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
12401232
value: {
1241-
[currentUserAccountIDParam]: {
1233+
[currentUserAccountID]: {
12421234
status,
12431235
},
12441236
},
@@ -1255,13 +1247,13 @@ function updateCustomStatus(currentUserAccountIDParam: number, status: Status) {
12551247
/**
12561248
* Clears the custom status
12571249
*/
1258-
function clearCustomStatus(currentUserAccountIDParam: number) {
1250+
function clearCustomStatus(currentUserAccountID: number) {
12591251
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.PERSONAL_DETAILS_LIST>> = [
12601252
{
12611253
onyxMethod: Onyx.METHOD.MERGE,
12621254
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
12631255
value: {
1264-
[currentUserAccountIDParam]: {
1256+
[currentUserAccountID]: {
12651257
status: null, // Clearing the field
12661258
},
12671259
},
@@ -1365,7 +1357,7 @@ function setShouldShowBranchNameInTitle(value: boolean) {
13651357
Onyx.set(ONYXKEYS.SHOULD_SHOW_BRANCH_NAME_IN_TITLE, value);
13661358
}
13671359

1368-
function lockAccount(accountID?: number, domainAccountID?: number, domainName?: string) {
1360+
function lockAccount(currentUserAccountID: number, accountID: number | undefined, domainAccountID: number | undefined, domainName: string | undefined) {
13691361
let domainOptimisticData: DomainOnyxUpdate[] = [];
13701362
let domainFailureData: DomainOnyxUpdate[] = [];
13711363
let domainSuccessData: DomainOnyxUpdate[] = [];

src/pages/settings/Security/LockAccount/LockAccountPageBase.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ import Button from '@components/Button';
44
import HeaderPageLayout from '@components/HeaderPageLayout';
55
import {ModalActions} from '@components/Modal/Global/ModalContext';
66
import useConfirmModal from '@hooks/useConfirmModal';
7+
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
78
import useLocalize from '@hooks/useLocalize';
89
import useNetwork from '@hooks/useNetwork';
9-
import useOnyx from '@hooks/useOnyx';
1010
import useThemeStyles from '@hooks/useThemeStyles';
1111
import type {LockAccountOnyxKey} from '@userActions/User';
1212
import {lockAccount} from '@userActions/User';
13-
import ONYXKEYS from '@src/ONYXKEYS';
1413
import type Response from '@src/types/onyx/Response';
1514

1615
type BaseLockAccountComponentProps = {
@@ -37,12 +36,12 @@ function LockAccountPageBase({
3736
const styles = useThemeStyles();
3837
const {isOffline} = useNetwork();
3938
const [isLoading, setIsLoading] = useState(false);
40-
const [session] = useOnyx(ONYXKEYS.SESSION);
39+
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
4140

4241
const {showConfirmModal} = useConfirmModal();
4342

4443
const handleReportSuspiciousActivity = async () => {
45-
if (!accountID && session?.accountID === -1) {
44+
if (!accountID && !currentUserPersonalDetails.accountID) {
4645
return;
4746
}
4847
const modalResult = await showConfirmModal({
@@ -61,7 +60,7 @@ function LockAccountPageBase({
6160
}
6261

6362
setIsLoading(true);
64-
const response = await lockAccount(accountID, domainAccountID, domainName);
63+
const response = await lockAccount(currentUserPersonalDetails.accountID, accountID, domainAccountID, domainName);
6564
setIsLoading(false);
6665

6766
handleLockRequestFinish(response);

tests/actions/UserTest.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -590,9 +590,10 @@ describe('actions/User', () => {
590590
});
591591

592592
describe('lockAccount', () => {
593+
const currentUserAccountID = 999;
593594
it('should execute with explicit accountID ', async () => {
594595
const accountID = 123456;
595-
UserActions.lockAccount(accountID);
596+
UserActions.lockAccount(currentUserAccountID, accountID, undefined, undefined);
596597

597598
await waitForBatchedUpdates();
598599

@@ -608,30 +609,26 @@ describe('actions/User', () => {
608609
});
609610

610611
it('should execute without accountID (uses current user)', async () => {
611-
UserActions.lockAccount();
612+
UserActions.lockAccount(currentUserAccountID, undefined, undefined, undefined);
612613
await waitForBatchedUpdates();
613614

614-
expect(mockAPI.makeRequestWithSideEffects).toHaveBeenCalledWith(
615-
SIDE_EFFECT_REQUEST_COMMANDS.LOCK_ACCOUNT,
616-
expect.objectContaining({accountID: expect.any(Number) as number}),
617-
expect.any(Object),
618-
);
615+
expect(mockAPI.makeRequestWithSideEffects).toHaveBeenCalledWith(SIDE_EFFECT_REQUEST_COMMANDS.LOCK_ACCOUNT, {accountID: currentUserAccountID}, expect.any(Object));
619616
});
620617

621618
it('should pass domainAccountID and domainName as params when provided', async () => {
622619
const accountID = 100;
623620
const domainAccountID = 200;
624621
const domainName = 'expensify.com';
625622

626-
UserActions.lockAccount(accountID, domainAccountID, domainName);
623+
UserActions.lockAccount(currentUserAccountID, accountID, domainAccountID, domainName);
627624
await waitForBatchedUpdates();
628625

629626
expect(mockAPI.makeRequestWithSideEffects).toHaveBeenCalledWith(SIDE_EFFECT_REQUEST_COMMANDS.LOCK_ACCOUNT, {accountID, domainAccountID, domainName}, expect.any(Object));
630627
});
631628

632629
describe('when locking current user (no accountID or matching currentUserAccountID)', () => {
633630
it('should include correct ACCOUNT optimistic, success, and failure data', async () => {
634-
UserActions.lockAccount();
631+
UserActions.lockAccount(currentUserAccountID, undefined, undefined, undefined);
635632
await waitForBatchedUpdates();
636633

637634
const calls = (mockAPI.makeRequestWithSideEffects as jest.Mock).mock.calls;
@@ -675,7 +672,7 @@ describe('actions/User', () => {
675672
});
676673

677674
it('should NOT include domain-related onyx data', async () => {
678-
UserActions.lockAccount();
675+
UserActions.lockAccount(currentUserAccountID, undefined, undefined, undefined);
679676
await waitForBatchedUpdates();
680677

681678
const calls = (mockAPI.makeRequestWithSideEffects as jest.Mock).mock.calls;
@@ -704,7 +701,7 @@ describe('actions/User', () => {
704701
const userLockKey = `${CONST.DOMAIN.PRIVATE_LOCKED_ACCOUNT_PREFIX}${accountID}`;
705702

706703
it('should include correct domain optimistic, success, and failure data', async () => {
707-
UserActions.lockAccount(accountID, domainAccountID, domainName);
704+
UserActions.lockAccount(currentUserAccountID, accountID, domainAccountID, domainName);
708705
await waitForBatchedUpdates();
709706

710707
const calls = (mockAPI.makeRequestWithSideEffects as jest.Mock).mock.calls;
@@ -773,7 +770,7 @@ describe('actions/User', () => {
773770
await Onyx.merge(ONYXKEYS.SESSION, {accountID: 999, email: 'admin@expensify.com'});
774771
await waitForBatchedUpdates();
775772

776-
UserActions.lockAccount(accountID, domainAccountID, domainName);
773+
UserActions.lockAccount(currentUserAccountID, accountID, domainAccountID, domainName);
777774
await waitForBatchedUpdates();
778775

779776
const calls = (mockAPI.makeRequestWithSideEffects as jest.Mock).mock.calls;

0 commit comments

Comments
 (0)