Merge pull request #6043 from Expensify/marcaaron-shouldProcessImmedi…

…ately

Fix duplicate comments posting related to `Log` calls from inside `processNetworkRequestQueue()`

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);

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();

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);
+            });
+    });
 });

tests/utils/TestHelper.js

@@ -60,21 +60,21 @@ 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,
             personalDetailsList,
         }));
 
     fetchPersonalDetails();
-    return waitForPromisesToResolve();
+    return waitForPromisesToResolve()
+        .then(() => {
+            HttpUtils.xhr = originalXHR;
+        });
 }
 
 export {