From d699153f661f6ef1e05e7d84de7795bf9f7becf2 Mon Sep 17 00:00:00 2001 From: Chirag Chandrakant Salian Date: Mon, 23 Jan 2023 13:55:06 -0800 Subject: [PATCH] Merge pull request #14484 from Expensify/revert-14455-Rory-PickyReportActionsOnyxUpdates Revert "[No QA] Filter out reportActionID-keyed Onyx updates" (cherry picked from commit 40e4baf6daae51dbbdeb91fa168c0f924b99489b) --- src/libs/ReportActionsUtils.js | 41 ------- src/libs/actions/Report.js | 3 +- src/libs/actions/User.js | 5 +- tests/actions/ReportTest.js | 11 +- tests/unit/ReportActionsUtilsTest.js | 164 --------------------------- 5 files changed, 8 insertions(+), 216 deletions(-) diff --git a/src/libs/ReportActionsUtils.js b/src/libs/ReportActionsUtils.js index 02c6a3de011c..bb67b2fb39b6 100644 --- a/src/libs/ReportActionsUtils.js +++ b/src/libs/ReportActionsUtils.js @@ -173,46 +173,6 @@ function getLastVisibleMessageText(reportID, actionsToMerge = {}) { return ReportUtils.formatReportLastMessageText(messageText); } -/** - * Given an array of Onyx updates, this function will filter out any which attempt to update reportActions data using reportActionID as the key. - * It is a TEMPORARY measure while we migrate the shape of reportActions Onyx data. Additional context: - * - * - In GitHub: https://github.com/Expensify/App/issues/14452 - * - In Slack: https://expensify.slack.com/archives/C04DC6LU2UB/p1674243288556829?thread_ts=1674242808.964579&cid=C04DC6LU2UB - * - * @param {Array} onyxUpdates – each Onyx update typically has shape: {onyxMethod: string, key: string, value: *} - * @returns {Array} - */ -function filterReportActionIDKeyedOnyxUpdates(onyxUpdates) { - return _.reduce( - onyxUpdates, - (memo, onyxUpdate) => { - if (!onyxUpdate.key.startsWith(ONYXKEYS.COLLECTION.REPORT_ACTIONS)) { - memo.push(onyxUpdate); - return memo; - } - - const newValue = {}; - _.each(onyxUpdate.value, (reportAction, key) => { - if (reportAction && reportAction.reportActionID === key) { - return; - } - newValue[key] = reportAction; - }); - - if (_.isEmpty(newValue)) { - return memo; - } - - // eslint-disable-next-line no-param-reassign - onyxUpdate.value = newValue; - memo.push(onyxUpdate); - return memo; - }, - [], - ); -} - export { getSortedReportActions, filterReportActionsForDisplay, @@ -221,5 +181,4 @@ export { getMostRecentIOUReportActionID, isDeletedAction, isConsecutiveActionMadeByPreviousActor, - filterReportActionIDKeyedOnyxUpdates, }; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9329e098914e..78faeaa3e3a9 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -63,8 +63,7 @@ function getReportChannelName(reportID) { function subscribeToReportCommentPushNotifications() { PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, onyxData}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); - const filteredOnyxData = ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(onyxData); - Onyx.update(filteredOnyxData); + Onyx.update(onyxData); }); // Open correct report when push notification is clicked diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index 62f9c6bd1072..09e7d6873084 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -285,9 +285,8 @@ function subscribeToUserEvents() { // Receive any relevant Onyx updates from the server PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.ONYX_API_UPDATE, currentUserAccountID, (pushJSON) => { SequentialQueue.getCurrentRequest().then(() => { - const filteredOnyxUpdate = ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(pushJSON); - Onyx.update(filteredOnyxUpdate); - triggerNotifications(filteredOnyxUpdate); + Onyx.update(pushJSON); + triggerNotifications(pushJSON); }); }); diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 8ad282d2f3cc..0918dda3ec40 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -18,7 +18,6 @@ import * as PersistedRequests from '../../src/libs/actions/PersistedRequests'; import * as User from '../../src/libs/actions/User'; import * as ReportUtils from '../../src/libs/ReportUtils'; import DateUtils from '../../src/libs/DateUtils'; -import * as NumberUtils from '../../src/libs/NumberUtils'; jest.mock('../../src/libs/actions/Report', () => { const originalModule = jest.requireActual('../../src/libs/actions/Report'); @@ -66,8 +65,8 @@ describe('actions/Report', () => { const TEST_USER_ACCOUNT_ID = 1; const TEST_USER_LOGIN = 'test@test.com'; const REPORT_ID = 1; + const ACTION_ID = 1; const REPORT_ACTION = { - reportActionID: NumberUtils.rand64(), actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, actorAccountID: TEST_USER_ACCOUNT_ID, actorEmail: TEST_USER_LOGIN, @@ -75,7 +74,7 @@ describe('actions/Report', () => { avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png', message: [{type: 'COMMENT', html: 'Testing a comment', text: 'Testing a comment'}], person: [{type: 'TEXT', style: 'strong', text: 'Test User'}], - sequenceNumber: 1, + sequenceNumber: ACTION_ID, shouldShow: true, }; @@ -132,7 +131,7 @@ describe('actions/Report', () => { key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, value: { [clientID]: null, - 1: actionWithoutLoading, + [ACTION_ID]: actionWithoutLoading, }, }, ]); @@ -146,7 +145,7 @@ describe('actions/Report', () => { // Verify there is only one action and our optimistic comment has been removed expect(_.size(reportActions)).toBe(1); - const resultAction = reportActions[1]; + const resultAction = reportActions[ACTION_ID]; // Verify that our action is no longer in the loading state expect(resultAction.pendingAction).not.toBeDefined(); @@ -355,9 +354,9 @@ describe('actions/Report', () => { onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, value: { - [_.toArray(reportActions)[0].clientID]: null, [_.toArray(reportActions)[1].clientID]: null, [_.toArray(reportActions)[2].clientID]: null, + [_.toArray(reportActions)[3].clientID]: null, 2: { ...USER_1_BASE_ACTION, message: [{type: 'COMMENT', html: 'Current User Comment 1', text: 'Current User Comment 1'}], diff --git a/tests/unit/ReportActionsUtilsTest.js b/tests/unit/ReportActionsUtilsTest.js index e8e008df3336..8431555a8985 100644 --- a/tests/unit/ReportActionsUtilsTest.js +++ b/tests/unit/ReportActionsUtilsTest.js @@ -1,6 +1,5 @@ import CONST from '../../src/CONST'; import * as ReportActionsUtils from '../../src/libs/ReportActionsUtils'; -import ONYXKEYS from '../../src/ONYXKEYS'; describe('ReportActionsUtils', () => { describe('getSortedReportActions', () => { @@ -193,167 +192,4 @@ describe('ReportActionsUtils', () => { expect(result).toStrictEqual(input); }); }); - - describe('filterReportActionIDKeyedOnyxUpdates', () => { - it('should not error with an empty value', () => { - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates()).toStrictEqual([]); - }); - - it('should not error with an empty array', () => { - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates([])).toStrictEqual([]); - }); - - it('should not affect any Onyx update not including reportActions data', () => { - const onyxUpdates = [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.POLICY}1234`, - value: { - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - errors: null, - }, - }, - ]; - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(onyxUpdates)).toStrictEqual(onyxUpdates); - }); - - it('should not affect a sequenceNumber-keyed Onyx update', () => { - const onyxUpdates = [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}1234`, - value: { - reportID: 1234, - }, - }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}1234`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - ]; - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(onyxUpdates)).toStrictEqual(onyxUpdates); - }); - - it('should reject a reportActionID-keyed Onyx update', () => { - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates([ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}1234`, - value: { - 54321: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - ])).toStrictEqual([]); - }); - - it('should work with multiple reportActions Onyx updates', () => { - const onyxUpdates = [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}1234`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}4321`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - ]; - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(onyxUpdates)).toStrictEqual(onyxUpdates); - }); - - it('should filter out reportAction Onyx updates keyed by reportActionID, leaving those keyed by sequenceNumber in place', () => { - const input = [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}1234`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - 54321: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}4321`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - 54321: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - ]; - - const expectedOutput = [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}1234`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}4321`, - value: { - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - ]; - - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(input)).toStrictEqual(expectedOutput); - }); - - it('should not filter out the removal of any clientID-keyed reportAction', () => { - const onyxUpdates = [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}4321`, - value: { - 123135135: null, - 1: { - sequenceNumber: 1, - reportActionID: '54321', - }, - }, - }, - ]; - expect(ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(onyxUpdates)).toStrictEqual(onyxUpdates); - }); - }); });