Merge pull request Expensify#89262 from callstack-internal/perf/preserve-last-full-reconnect-time-on-delegate-switch

Don't fire duplicate ReconnectApp on delegate account switch

Author: mountiny

src/libs/actions/Delegate.ts

@@ -4,6 +4,7 @@ import type {OnyxEntry, OnyxKey, OnyxUpdate} from 'react-native-onyx';
 import * as API from '@libs/API';
 import type {AddDelegateParams as APIAddDelegateParams, RemoveDelegateParams as APIRemoveDelegateParams, UpdateDelegateRoleParams as APIUpdateDelegateRoleParams} from '@libs/API/parameters';
 import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types';
+import DateUtils from '@libs/DateUtils';
 import * as ErrorUtils from '@libs/ErrorUtils';
 import Log from '@libs/Log';
 import {clearPreservedSearchNavigatorStates} from '@libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState';
@@ -42,14 +43,25 @@ const KEYS_TO_PRESERVE_DELEGATE_ACCESS = [
 ];
 
 /**
- * Atomically reset Onyx for a delegate-access transition while keeping IS_LOADING_APP=true
- * so consumers never observe the post-clear state with HAS_LOADED_APP=true and
- * IS_LOADING_APP=undefined. That combination falsely looks like a stuck app and triggers
- * DelegateAccessHandler's recovery effect, producing a duplicate openApp queued behind the
- * explicit openApp the caller is about to make.
+ * Atomically reset Onyx for a delegate-access transition while seeding two values that
+ * subscribers would otherwise misinterpret on the post-clear state and double up calls
+ * the caller is about to make explicitly:
+ *
+ * - IS_LOADING_APP=true: without it, consumers observe HAS_LOADED_APP=true and
+ *   IS_LOADING_APP=undefined together, which looks like a stuck app and triggers
+ *   DelegateAccessHandler's recovery effect, queueing a duplicate openApp.
+ * - LAST_FULL_RECONNECT_TIME=now: subscribeToFullReconnect compares this against the
+ *   server-supplied NVP_RECONNECT_APP_IF_FULL_RECONNECT_BEFORE that lands in OpenApp's
+ *   response.onyxData. Because applyHTTPSOnyxUpdates applies response.onyxData before
+ *   successData, the timestamp would still be empty when the comparison runs, falsely
+ *   triggering a duplicate ReconnectApp. Seeding to `now` short-circuits the subscriber
+ *   until OpenApp's successData refreshes it.
  */
 function clearOnyxForDelegateTransition(): Promise<void> {
-    return Onyx.merge(ONYXKEYS.IS_LOADING_APP, true).then(() => Onyx.clear([...KEYS_TO_PRESERVE_DELEGATE_ACCESS, ONYXKEYS.IS_LOADING_APP]));
+    return Onyx.multiSet({
+        [ONYXKEYS.IS_LOADING_APP]: true,
+        [ONYXKEYS.LAST_FULL_RECONNECT_TIME]: DateUtils.getDBTime(),
+    }).then(() => Onyx.clear([...KEYS_TO_PRESERVE_DELEGATE_ACCESS, ONYXKEYS.IS_LOADING_APP, ONYXKEYS.LAST_FULL_RECONNECT_TIME]));
 }
 
 type WithDelegatedAccess = {

tests/actions/DelegateTest.ts

@@ -8,6 +8,7 @@ import {
     removeDelegate,
     updateDelegateRole,
 } from '@libs/actions/Delegate';
+import DateUtils from '@libs/DateUtils';
 import {pause, resetQueue} from '@libs/Network/SequentialQueue';
 import CONST from '@src/CONST';
 import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager';
@@ -293,5 +294,44 @@ describe('actions/Delegate', () => {
                 });
             });
         });
+
+        it('seeds LAST_FULL_RECONNECT_TIME so subscribeToFullReconnect does not fire a duplicate ReconnectApp before OpenApp successData arrives', async () => {
+            // Simulate the pre-switch state: timestamp is empty, mimicking the post-clear baseline.
+            await Onyx.multiSet({
+                [ONYXKEYS.LAST_FULL_RECONNECT_TIME]: null,
+                [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT,
+            });
+            await waitForBatchedUpdates();
+
+            const before = DateUtils.getDBTime();
+            await clearOnyxForDelegateTransition();
+            await waitForBatchedUpdates();
+
+            // The timestamp must be a non-empty DB time string seeded at-or-after `before`.
+            await new Promise<void>((resolve) => {
+                const conn = Onyx.connect({
+                    key: ONYXKEYS.LAST_FULL_RECONNECT_TIME,
+                    callback: (value) => {
+                        expect(typeof value).toBe('string');
+                        expect(value && value.length > 0).toBe(true);
+                        expect((value ?? '') >= before).toBe(true);
+                        Onyx.disconnect(conn);
+                        resolve();
+                    },
+                });
+            });
+
+            // A non-preserved key should still have been cleared alongside the seed.
+            await new Promise<void>((resolve) => {
+                const conn = Onyx.connect({
+                    key: ONYXKEYS.NVP_PRIORITY_MODE,
+                    callback: (value) => {
+                        expect(value).toBeUndefined();
+                        Onyx.disconnect(conn);
+                        resolve();
+                    },
+                });
+            });
+        });
     });
 });