Skip to content

Commit

Permalink
Merge pull request #14484 from Expensify/revert-14455-Rory-PickyRepor…
Browse files Browse the repository at this point in the history 
…tActionsOnyxUpdates

Revert "[No QA] Filter out reportActionID-keyed Onyx updates"

(cherry picked from commit 40e4baf)
  • Loading branch information
@chiragsalian @OSBotify
chiragsalian authored and OSBotify committed Jan 23, 2023
1 parent 3496289 commit d699153
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 216 deletions.
41 changes: 0 additions & 41 deletions src/libs/ReportActionsUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -221,5 +181,4 @@ export {
getMostRecentIOUReportActionID,
isDeletedAction,
isConsecutiveActionMadeByPreviousActor,
filterReportActionIDKeyedOnyxUpdates,
};
3 changes: 1 addition & 2 deletions src/libs/actions/Report.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/libs/actions/User.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down
11 changes: 5 additions & 6 deletions tests/actions/ReportTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -66,16 +65,16 @@ describe('actions/Report', () => {
const TEST_USER_ACCOUNT_ID = 1;
const TEST_USER_LOGIN = '[email protected]';
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,
automatic: false,
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,
};

Expand Down Expand Up @@ -132,7 +131,7 @@ describe('actions/Report', () => {
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`,
value: {
[clientID]: null,
1: actionWithoutLoading,
[ACTION_ID]: actionWithoutLoading,
},
},
]);
Expand All @@ -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();
Expand Down Expand Up @@ -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'}],
Expand Down
164 changes: 0 additions & 164 deletions tests/unit/ReportActionsUtilsTest.js
View file
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
});

0 comments on commit d699153

Please sign in to comment.