diff --git a/src/libs/Log.js b/src/libs/Log.js index 9db56f5b749..14a32f19865 100644 --- a/src/libs/Log.js +++ b/src/libs/Log.js @@ -21,6 +21,7 @@ let timeout = null; */ function serverLoggingCallback(logger, params) { const requestParams = params; + requestParams.shouldProcessImmediately = false; requestParams.expensifyCashAppVersion = `expensifyCash[${getPlatform()}]${version}`; if (requestParams.parameters) { requestParams.parameters = JSON.stringify(params.parameters); diff --git a/src/libs/Network.js b/src/libs/Network.js index 01592b51f38..aa1fd853ae8 100644 --- a/src/libs/Network.js +++ b/src/libs/Network.js @@ -232,6 +232,14 @@ function processNetworkRequestQueue() { // Process our write queue very often setInterval(processNetworkRequestQueue, 1000); +/** + * @param {Object} request + * @returns {Boolean} + */ +function canProcessRequestImmediately(request) { + return lodashGet(request, 'data.shouldProcessImmediately', true); +} + /** * Perform a queued post request * @@ -243,15 +251,21 @@ setInterval(processNetworkRequestQueue, 1000); */ function post(command, data = {}, type = CONST.NETWORK.METHOD.POST, shouldUseSecure = false) { return new Promise((resolve, reject) => { - // Add the write request to a queue of actions to perform - networkRequestQueue.push({ + const request = { command, data, type, resolve, reject, shouldUseSecure, - }); + }; + + // Add the request to a queue of actions to perform + networkRequestQueue.push(request); + + if (!canProcessRequestImmediately(request)) { + return; + } // Try to fire off the request as soon as it's queued so we don't add a delay to every queued command processNetworkRequestQueue(); diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 4955196288d..718e5f9ef00 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -9,6 +9,7 @@ import {addAction, togglePinnedState, subscribeToUserEvents} from '../../src/lib import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import PushNotification from '../../src/libs/Notification/PushNotification'; import {signInWithTestUser, fetchPersonalDetailsForTestUser} from '../utils/TestHelper'; +import Log from '../../src/libs/Log'; PushNotification.register = () => {}; PushNotification.deregister = () => {}; @@ -165,4 +166,45 @@ describe('actions/Report', () => { expect(reportIsPinned).toEqual(true); }); }); + + it('Should not leave duplicate comments when logger sends packet because of calling process queue while processing the queue', () => { + const TEST_USER_ACCOUNT_ID = 1; + const TEST_USER_LOGIN = 'test@test.com'; + const REPORT_ID = 1; + const LOGGER_MAX_LOG_LINES = 50; + + // GIVEN a test user with initial data + return signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN) + .then(() => fetchPersonalDetailsForTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN, { + [TEST_USER_LOGIN]: { + accountID: TEST_USER_ACCOUNT_ID, + email: TEST_USER_LOGIN, + firstName: 'Test', + lastName: 'User', + }, + })) + .then(() => { + global.fetch = jest.fn() + .mockImplementation(() => Promise.resolve({ + json: () => Promise.resolve({ + jsonCode: 200, + }), + })); + + // WHEN we add 1 less than the max before a log packet is sent + for (let i = 0; i < LOGGER_MAX_LOG_LINES; i++) { + Log.info('Test log info'); + } + + // And leave a comment on a report which will trigger the log packet to be sent in the same call + addAction(REPORT_ID, 'Testing a comment'); + return waitForPromisesToResolve(); + }) + .then(() => { + // THEN only ONE call to Report_AddComment will happen + const URL_ARGUMENT_INDEX = 0; + const reportAddCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('Report_AddComment')); + expect(reportAddCommentCalls.length).toBe(1); + }); + }); }); diff --git a/tests/utils/TestHelper.js b/tests/utils/TestHelper.js index b5e2866c7bf..1328d251bd6 100644 --- a/tests/utils/TestHelper.js +++ b/tests/utils/TestHelper.js @@ -60,13 +60,10 @@ function signInWithTestUser(accountID, login, password = 'Password1', authToken * @returns {Promise} */ function fetchPersonalDetailsForTestUser(accountID, email, personalDetailsList) { - // Mock xhr() + const originalXHR = HttpUtils.xhr; HttpUtils.xhr = jest.fn(); - // Get the personalDetails HttpUtils.xhr - - // fetchPersonalDetails .mockImplementationOnce(() => Promise.resolve({ accountID, email, @@ -74,7 +71,10 @@ function fetchPersonalDetailsForTestUser(accountID, email, personalDetailsList) })); fetchPersonalDetails(); - return waitForPromisesToResolve(); + return waitForPromisesToResolve() + .then(() => { + HttpUtils.xhr = originalXHR; + }); } export {