diff --git a/src/libs/ReportActionsUtils.js b/src/libs/ReportActionsUtils.js index bb67b2fb39b6..02c6a3de011c 100644 --- a/src/libs/ReportActionsUtils.js +++ b/src/libs/ReportActionsUtils.js @@ -173,6 +173,46 @@ 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, @@ -181,4 +221,5 @@ export { getMostRecentIOUReportActionID, isDeletedAction, isConsecutiveActionMadeByPreviousActor, + filterReportActionIDKeyedOnyxUpdates, }; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 78faeaa3e3a9..9329e098914e 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -63,7 +63,8 @@ function getReportChannelName(reportID) { function subscribeToReportCommentPushNotifications() { PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, onyxData}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); - Onyx.update(onyxData); + const filteredOnyxData = ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(onyxData); + Onyx.update(filteredOnyxData); }); // Open correct report when push notification is clicked diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index 09e7d6873084..62f9c6bd1072 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -285,8 +285,9 @@ function subscribeToUserEvents() { // Receive any relevant Onyx updates from the server PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.ONYX_API_UPDATE, currentUserAccountID, (pushJSON) => { SequentialQueue.getCurrentRequest().then(() => { - Onyx.update(pushJSON); - triggerNotifications(pushJSON); + const filteredOnyxUpdate = ReportActionsUtils.filterReportActionIDKeyedOnyxUpdates(pushJSON); + Onyx.update(filteredOnyxUpdate); + triggerNotifications(filteredOnyxUpdate); }); }); diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 0918dda3ec40..8ad282d2f3cc 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -18,6 +18,7 @@ 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'); @@ -65,8 +66,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, @@ -74,7 +75,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: ACTION_ID, + sequenceNumber: 1, shouldShow: true, }; @@ -131,7 +132,7 @@ describe('actions/Report', () => { key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, value: { [clientID]: null, - [ACTION_ID]: actionWithoutLoading, + 1: actionWithoutLoading, }, }, ]); @@ -145,7 +146,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[ACTION_ID]; + const resultAction = reportActions[1]; // Verify that our action is no longer in the loading state expect(resultAction.pendingAction).not.toBeDefined(); @@ -354,9 +355,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 8431555a8985..e8e008df3336 100644 --- a/tests/unit/ReportActionsUtilsTest.js +++ b/tests/unit/ReportActionsUtilsTest.js @@ -1,5 +1,6 @@ import CONST from '../../src/CONST'; import * as ReportActionsUtils from '../../src/libs/ReportActionsUtils'; +import ONYXKEYS from '../../src/ONYXKEYS'; describe('ReportActionsUtils', () => { describe('getSortedReportActions', () => { @@ -192,4 +193,167 @@ 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); + }); + }); });