From dd3a8bee1e36e53ec4145165a5fe19a4ce29d321 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Sat, 10 Aug 2024 16:49:34 -0400 Subject: [PATCH 1/3] Fix double OpenReport call when comment linking, enable test --- jest/setup.ts | 20 ++++++++++++++++++++ src/pages/home/ReportScreen.tsx | 3 +-- tests/ui/PaginationTest.tsx | 33 ++++++++++++++++++++++++--------- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/jest/setup.ts b/jest/setup.ts index 19e20a6d03958..51385ad19e454 100644 --- a/jest/setup.ts +++ b/jest/setup.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-classes-per-file */ import '@shopify/flash-list/jestSetup'; import 'react-native-gesture-handler/jestSetup'; import type * as RNKeyboardController from 'react-native-keyboard-controller'; @@ -78,3 +79,22 @@ jest.mock('@src/libs/actions/Timing', () => ({ start: jest.fn(), end: jest.fn(), })); + +// This makes FlatList render synchronously for easier testing. +jest.mock( + '@react-native/virtualized-lists/Interaction/Batchinator', + () => + class SyncBachinator { + #callback: () => void; + + constructor(callback: () => void) { + this.#callback = callback; + } + + schedule() { + this.#callback(); + } + + dispose() {} + }, +); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 709dbc1c06858..f178734e96b04 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -522,10 +522,9 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro useEffect(() => { // Call OpenReport only if we are not linking to a message or the report is not available yet - if (isLoadingReportOnyx || (reportActionIDFromRoute && report.reportID && isLinkedMessagePageReady)) { + if (isLoadingReportOnyx || reportActionIDFromRoute) { return; } - fetchReportIfNeeded(); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [isLoadingReportOnyx]); diff --git a/tests/ui/PaginationTest.tsx b/tests/ui/PaginationTest.tsx index 277f6b88e78f0..44ad43e699535 100644 --- a/tests/ui/PaginationTest.tsx +++ b/tests/ui/PaginationTest.tsx @@ -309,9 +309,7 @@ describe('Pagination', () => { expect(getReportActions()).toHaveLength(18); }); - // Currently broken on main by https://github.com/Expensify/App/pull/42582. - // TODO: Investigate and re-enable. - it.skip('opens a chat and load newer messages', async () => { + it('opens a chat and load newer messages', async () => { mockOpenReport(5, '5'); mockGetNewerActions(5); @@ -325,26 +323,43 @@ describe('Pagination', () => { }); // ReportScreen relies on the onLayout event to receive updates from onyx. triggerListLayout(); + await waitForBatchedUpdatesWithAct(); - expect(getReportActions()).toHaveLength(5); + // Here we have 5 messages from the initial OpenReport and 5 from the initial GetNewerActions. + expect(getReportActions()).toHaveLength(10); // There is 1 extra call here because of the comment linking report. TestHelper.expectAPICommandToHaveBeenCalled('OpenReport', 2); TestHelper.expectAPICommandToHaveBeenCalledWith('OpenReport', 1, {reportID: REPORT_ID, reportActionID: '5'}); TestHelper.expectAPICommandToHaveBeenCalled('GetOlderActions', 0); - TestHelper.expectAPICommandToHaveBeenCalled('GetNewerActions', 0); + TestHelper.expectAPICommandToHaveBeenCalledWith('GetNewerActions', 0, {reportID: REPORT_ID, reportActionID: '5'}); + // Simulate the maintainVisibleContentPosition scroll adjustment, so it is now possible to scroll down more. + scrollToOffset(500); scrollToOffset(0); await waitForBatchedUpdatesWithAct(); TestHelper.expectAPICommandToHaveBeenCalled('OpenReport', 2); TestHelper.expectAPICommandToHaveBeenCalled('GetOlderActions', 0); - TestHelper.expectAPICommandToHaveBeenCalled('GetNewerActions', 1); - TestHelper.expectAPICommandToHaveBeenCalledWith('GetNewerActions', 0, {reportID: REPORT_ID, reportActionID: '5'}); + TestHelper.expectAPICommandToHaveBeenCalled('GetNewerActions', 2); + TestHelper.expectAPICommandToHaveBeenCalledWith('GetNewerActions', 1, {reportID: REPORT_ID, reportActionID: '10'}); + + // We now have 15 messages. 5 from the initial OpenReport and 10 from the 2 GetNewerActions calls. + expect(getReportActions()).toHaveLength(15); + // Simulate the backend returning no new messages to simulate reaching the start of the chat. + mockGetNewerActions(0); + + scrollToOffset(500); + scrollToOffset(0); await waitForBatchedUpdatesWithAct(); - // We now have 10 messages. 5 from the initial OpenReport and 5 from GetNewerActions. - expect(getReportActions()).toHaveLength(10); + TestHelper.expectAPICommandToHaveBeenCalled('OpenReport', 2); + TestHelper.expectAPICommandToHaveBeenCalled('GetOlderActions', 0); + TestHelper.expectAPICommandToHaveBeenCalled('GetNewerActions', 3); + TestHelper.expectAPICommandToHaveBeenCalledWith('GetNewerActions', 2, {reportID: REPORT_ID, reportActionID: '15'}); + + // We still have 15 messages. 5 from the initial OpenReport and 10 from the 2 GetNewerActions calls. + expect(getReportActions()).toHaveLength(15); }); }); From f533f7981a64d37d02fceaa3d0a10c9abe10d0bc Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Wed, 28 Aug 2024 14:13:42 -0400 Subject: [PATCH 2/3] Check if jest mock is the problem --- jest/setup.ts | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/jest/setup.ts b/jest/setup.ts index 51385ad19e454..cb6fb34369afb 100644 --- a/jest/setup.ts +++ b/jest/setup.ts @@ -81,20 +81,20 @@ jest.mock('@src/libs/actions/Timing', () => ({ })); // This makes FlatList render synchronously for easier testing. -jest.mock( - '@react-native/virtualized-lists/Interaction/Batchinator', - () => - class SyncBachinator { - #callback: () => void; - - constructor(callback: () => void) { - this.#callback = callback; - } - - schedule() { - this.#callback(); - } - - dispose() {} - }, -); +// jest.mock( +// '@react-native/virtualized-lists/Interaction/Batchinator', +// () => +// class SyncBachinator { +// #callback: () => void; + +// constructor(callback: () => void) { +// this.#callback = callback; +// } + +// schedule() { +// this.#callback(); +// } + +// dispose() {} +// }, +// ); From 3d1d8783f6f945c727d124b249100f30c10d2153 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Wed, 28 Aug 2024 14:34:20 -0400 Subject: [PATCH 3/3] Bring back flatlist mock --- jest/setup.ts | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/jest/setup.ts b/jest/setup.ts index cb6fb34369afb..51385ad19e454 100644 --- a/jest/setup.ts +++ b/jest/setup.ts @@ -81,20 +81,20 @@ jest.mock('@src/libs/actions/Timing', () => ({ })); // This makes FlatList render synchronously for easier testing. -// jest.mock( -// '@react-native/virtualized-lists/Interaction/Batchinator', -// () => -// class SyncBachinator { -// #callback: () => void; - -// constructor(callback: () => void) { -// this.#callback = callback; -// } - -// schedule() { -// this.#callback(); -// } - -// dispose() {} -// }, -// ); +jest.mock( + '@react-native/virtualized-lists/Interaction/Batchinator', + () => + class SyncBachinator { + #callback: () => void; + + constructor(callback: () => void) { + this.#callback = callback; + } + + schedule() { + this.#callback(); + } + + dispose() {} + }, +);