[POC] Test over the onyx updates

package.json

@@ -181,7 +181,7 @@
     "react-native-localize": "^3.5.4",
     "react-native-nitro-modules": "0.29.4",
     "react-native-nitro-sqlite": "9.2.0",
-    "react-native-onyx": "3.0.43",
+    "react-native-onyx": "git+https://github.com/Expensify/react-native-onyx.git#eff279e9d61550a6fb677e896807778694cae39f",
     "react-native-pager-view": "8.0.0",
     "react-native-pdf": "7.0.2",
     "react-native-permissions": "^5.4.0",

src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx

@@ -1,4 +1,4 @@
-import {useFocusEffect} from '@react-navigation/native';
+import {useIsFocused} from '@react-navigation/native';
 import {hasSeenTourSelector} from '@selectors/Onboarding';
 import {FlashList} from '@shopify/flash-list';
 import type {FlashListRef, ListRenderItemInfo} from '@shopify/flash-list';
@@ -543,20 +543,29 @@ function MoneyRequestReportPreviewContent({
         carouselTransactionsRef.current = carouselTransactions;
     }, [carouselTransactions]);
 
-    useFocusEffect(
-        useCallback(() => {
-            const index = carouselTransactions.findIndex((transaction) => newTransactionIDs?.has(transaction.transactionID));
+    const isFocused = useIsFocused();
+    const isFocusedRef = useRef(isFocused);
 
-            if (index < 0) {
+    useEffect(() => {
+        isFocusedRef.current = isFocused;
+    }, [isFocused]);
+
+    useEffect(() => {
+        const index = carouselTransactions.findIndex((transaction) => newTransactionIDs?.has(transaction.transactionID));
+
+        if (index < 0) {
+            return;
+        }
+        const newTransaction = carouselTransactions.at(index);
+        setTimeout(() => {
+            if (!isFocusedRef.current) {
+                return;
+            }
+            // If the new transaction is not available at the index it was on before the delay, avoid the scrolling
+            // because we are scrolling to either a wrong or unavailable transaction (which can cause crash).
+            if (newTransaction?.transactionID !== carouselTransactionsRef.current.at(index)?.transactionID) {
                 return;
             }
-            const newTransaction = carouselTransactions.at(index);
-            setTimeout(() => {
-                // If the new transaction is not available at the index it was on before the delay, avoid the scrolling
-                // because we are scrolling to either a wrong or unavailable transaction (which can cause crash).
-                if (newTransaction?.transactionID !== carouselTransactionsRef.current.at(index)?.transactionID) {
-                    return;
-                }
 
                 carouselRef.current?.scrollToIndex({
                     index,
@@ -565,9 +574,8 @@ function MoneyRequestReportPreviewContent({
                 });
             }, CONST.ANIMATED_TRANSITION);
 
-            // eslint-disable-next-line react-hooks/exhaustive-deps
-        }, [newTransactionIDs]),
-    );
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [newTransactionIDs]);
 
     const onViewableItemsChanged = useRef(({viewableItems}: {viewableItems: ViewToken[]; changed: ViewToken[]}) => {
         const newIndex = viewableItems.at(0)?.index;

src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx

@@ -1,4 +1,3 @@
-import {useIsFocused} from '@react-navigation/native';
 import type {ListRenderItem} from '@shopify/flash-list';
 import React, {useCallback, useMemo, useRef, useState} from 'react';
 import type {LayoutChangeEvent} from 'react-native';
@@ -122,9 +121,8 @@ function MoneyRequestReportPreview({
         selector: hasOnceLoadedReportActionsSelector,
     });
     const newTransactions = useNewTransactions(hasOnceLoadedReportActions, transactions);
-    const isFocused = useIsFocused();
     // We only want to highlight the new expenses if the screen is focused.
-    const newTransactionIDs = isFocused ? new Set(newTransactions.map((transaction) => transaction.transactionID)) : undefined;
+    const newTransactionIDs = new Set(newTransactions.map((transaction) => transaction.transactionID));
 
     const transactionPreviewContainerStyles = [styles.h100, reportPreviewStyles.transactionPreviewCarouselStyle];
 

tests/actions/ReportTest.ts

@@ -1338,6 +1338,8 @@ describe('actions/Report', () => {
             currentUserAccountID: TEST_USER_ACCOUNT_ID,
         });
 
+        await waitForBatchedUpdates();
+
         // Need the reportActionID to delete the comments
         const newComment = PersistedRequests.getAll().at(1);
         const reportActionID = newComment?.data?.reportActionID as string | undefined;
@@ -1354,16 +1356,8 @@ describe('actions/Report', () => {
 
         await waitForBatchedUpdates();
 
-        await new Promise<void>((resolve) => {
-            const connection = Onyx.connect({
-                key: ONYXKEYS.PERSISTED_REQUESTS,
-                callback: (persistedRequests) => {
-                    Onyx.disconnect(connection);
-                    expect(persistedRequests?.at(0)?.command).toBe(WRITE_COMMANDS.UPDATE_COMMENT);
-                    resolve();
-                },
-            });
-        });
+        const persistedRequests = PersistedRequests.getAll();
+        expect(persistedRequests?.at(0)?.command).toBe(WRITE_COMMANDS.UPDATE_COMMENT);
 
         rerender(originalReport);
         Report.deleteReportComment(originalReport, reportAction, ancestors.current, undefined, undefined, '');
@@ -1832,6 +1826,9 @@ describe('actions/Report', () => {
         const newComment = PersistedRequests.getAll().at(0);
         const reportActionID = newComment?.data?.reportActionID as string | undefined;
         const reportAction = TestHelper.buildTestReportComment(created, TEST_USER_ACCOUNT_ID, reportActionID);
+
+        await waitForBatchedUpdates();
+
         await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true});
 
         // wait for Onyx.connect execute the callback and start processing the queue
@@ -1861,18 +1858,10 @@ describe('actions/Report', () => {
             TEST_USER_ACCOUNT_ID,
         );
 
-        await waitForBatchedUpdates();
-        await new Promise<void>((resolve) => {
-            const connection = Onyx.connect({
-                key: ONYXKEYS.PERSISTED_REQUESTS,
-                callback: (persistedRequests) => {
-                    Onyx.disconnect(connection);
-                    expect(persistedRequests?.at(0)?.command).toBe(WRITE_COMMANDS.ADD_EMOJI_REACTION);
-                    expect(persistedRequests?.at(1)?.command).toBe(WRITE_COMMANDS.REMOVE_EMOJI_REACTION);
-                    resolve();
-                },
-            });
-        });
+        const persistedRequests = PersistedRequests.getAll();
+        expect(persistedRequests?.length).toBe(2);
+        expect(persistedRequests?.at(0)?.command).toBe(WRITE_COMMANDS.ADD_EMOJI_REACTION);
+        expect(persistedRequests?.at(1)?.command).toBe(WRITE_COMMANDS.REMOVE_EMOJI_REACTION);
 
         Report.deleteReportComment(REPORT, reportAction, [], undefined, undefined, '');
 
@@ -2025,6 +2014,7 @@ describe('actions/Report', () => {
         expect(requests?.at(0)?.data?.reportComment).toBe('value3');
 
         await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
+        await waitForBatchedUpdates();
 
         TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.UPDATE_COMMENT, 1);
     });
@@ -2123,6 +2113,8 @@ describe('actions/Report', () => {
         expect(requests?.at(0)?.data?.reportComment).toBe('value3');
 
         await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
+        await waitForBatchedUpdates();
+
         TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.UPDATE_COMMENT, 1);
     });
 

tests/actions/SessionTest.ts

@@ -189,6 +189,8 @@ describe('Session', () => {
 
         await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
 
+        await waitForBatchedUpdates();
+
         expect(getAllPersistedRequests().length).toBe(0);
     });
 
@@ -226,6 +228,8 @@ describe('Session', () => {
 
         await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
 
+        await waitForBatchedUpdates();
+
         expect(getAllPersistedRequests().length).toBe(0);
     });
 

tests/unit/APITest.ts

@@ -436,7 +436,7 @@ describe('APITests', () => {
                 });
 
                 Onyx.set(ONYXKEYS.NETWORK, {isOffline: true});
-                expect(NetworkStore.isOffline()).toBe(false);
+                expect(NetworkStore.isOffline()).toBe(true);
                 expect(NetworkStore.isAuthenticating()).toBe(false);
                 return waitForBatchedUpdates();
             })
@@ -551,7 +551,7 @@ describe('APITests', () => {
                 API.write('MockCommandThree' as WriteCommand, {});
 
                 // THEN the retryable requests should immediately be added to the persisted requests
-                expect(PersistedRequests.getAll().length).toBe(2);
+                expect(PersistedRequests.getLength()).toBe(2);
 
                 // WHEN we wait for the queue to run and finish processing
                 return waitForBatchedUpdates();

tests/unit/OnyxDerivedTest.tsx

@@ -104,6 +104,8 @@ describe('OnyxDerived', () => {
             await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`, mockReport);
             await IntlStore.load(CONST.LOCALES.ES);
 
+            await waitForBatchedUpdates();
+
             const derivedReportAttributes = await OnyxUtils.get(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES);
 
             expect(derivedReportAttributes).toMatchObject({

tests/unit/OptionsListUtilsTest.tsx

@@ -3521,7 +3521,7 @@ describe('OptionsListUtils', () => {
             expect(canCreate).toBe(false);
         });
 
-        it('createOptionList() localization', () => {
+        it('createOptionList() localization', async () => {
             renderLocaleContextProvider();
             // Given a set of reports and personal details
             // When we call createOptionList and extract the reports
@@ -3530,18 +3530,15 @@ describe('OptionsListUtils', () => {
             // Then the returned reports should match the expected values
             expect(reports.at(10)?.subtitle).toBe(`Submits to Mister Fantastic`);
 
-            return (
-                waitForBatchedUpdates()
-                    // When we set the preferred locale to Spanish
-                    .then(() => Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.ES))
-                    .then(() => {
-                        // When we call createOptionList again
-                        const newReports = createOptionList(PERSONAL_DETAILS, CURRENT_USER_ACCOUNT_ID, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS).reports;
-                        // Then the returned reports should change to Spanish
-                        // cspell:disable-next-line
-                        expect(newReports.at(10)?.subtitle).toBe('Se envía a Mister Fantastic');
-                    })
-            );
+            await Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.ES);
+
+            await waitForBatchedUpdates();
+
+            // When we call createOptionList again
+            const newReports = createOptionList(PERSONAL_DETAILS, CURRENT_USER_ACCOUNT_ID, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS).reports;
+            // Then the returned reports should change to Spanish
+            // cspell:disable-next-line
+            expect(newReports.at(10)?.subtitle).toBe('Se envía a Mister Fantastic');
         });
     });
 
@@ -3615,6 +3612,9 @@ describe('OptionsListUtils', () => {
                     '1': getFakeAdvancedReportAction(CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT),
                 },
             });
+
+            await waitForBatchedUpdates();
+
             // When we call createOptionList
             const reports = createOptionList(PERSONAL_DETAILS, CURRENT_USER_ACCOUNT_ID, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS).reports;
             const archivedReport = reports.find((report) => report.reportID === '10');

tests/unit/ReportSecondaryActionUtilsTest.ts

@@ -52,14 +52,16 @@ describe('getSecondaryAction', () => {
     beforeAll(() => {
         Onyx.init({
             keys: ONYXKEYS,
+            initialKeyStates: {
+                [ONYXKEYS.SESSION]: SESSION,
+                [ONYXKEYS.PERSONAL_DETAILS_LIST]: {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS, [APPROVER_ACCOUNT_ID]: {accountID: APPROVER_ACCOUNT_ID, login: APPROVER_EMAIL}},
+            },
         });
     });
 
     beforeEach(async () => {
         jest.clearAllMocks();
         Onyx.clear();
-        await Onyx.merge(ONYXKEYS.SESSION, SESSION);
-        await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS, [APPROVER_ACCOUNT_ID]: {accountID: APPROVER_ACCOUNT_ID, login: APPROVER_EMAIL}});
     });
 
     it('should always return default options', () => {

tests/unit/SequentialQueueTest.ts

@@ -53,10 +53,10 @@ describe('SequentialQueue', () => {
         };
         SequentialQueue.push(requestWithConflictResolution);
         expect(getLength()).toBe(1);
-        // We know there is only one request in the queue, so we can get the first one and verify
-        // that the persisted request is the second one.
-        const persistedRequest = getAll().at(0);
-        expect(persistedRequest?.data?.accountID).toBe(56789);
+        // We know there is only one request and it's ongoing.
+        // We can get it and verify that the ongoing request is the second one.
+        const ongoingRequest = getOngoingRequest();
+        expect(ongoingRequest?.data?.accountID).toBe(56789);
     });
 
     it('should push two requests with conflict resolution and push', () => {
@@ -109,7 +109,9 @@ describe('SequentialQueue', () => {
         };
 
         SequentialQueue.push(requestWithConflictResolution);
-        expect(getLength()).toBe(2);
+
+        const ongoingRequest = getOngoingRequest();
+        expect(ongoingRequest?.data?.accountID).toBe(56789);
     });
 
     it('should replace request request in queue while a similar one is ongoing', async () => {
@@ -175,9 +177,14 @@ describe('SequentialQueue', () => {
 
         expect(getLength()).toBe(4);
         const persistedRequests = getAll();
-        // We know ReconnectApp is at index 1 in the queue, so we can get it to verify
+        const ongoingRequest = getOngoingRequest();
+
+        // The first OpenReport call is ongoing
+        expect(ongoingRequest?.command).toBe('OpenReport');
+
+        // We know ReconnectApp is at index 0 in the queue now, so we can get it to verify
         // that was replaced by the new request.
-        expect(persistedRequests.at(1)?.data?.accountID).toBe(56789);
+        expect(persistedRequests.at(0)?.data?.accountID).toBe(56789);
     });
 
     // need to test a rance condition between processing the next request and then pushing a new request with conflict resolver