From aac44cb03663c956c262f4e67b7b812febf73166 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 7 Jun 2022 13:57:23 -1000 Subject: [PATCH 01/18] Add SystemInlineMessage and remove error handlers --- src/components/InlineSystemMessage.js | 7 +- src/libs/actions/Report.js | 209 +++++++++------------- src/pages/home/report/ReportActionItem.js | 5 + src/styles/utilities/spacing.js | 4 + 4 files changed, 99 insertions(+), 126 deletions(-) diff --git a/src/components/InlineSystemMessage.js b/src/components/InlineSystemMessage.js index 9f3b124db8e8..0b6645fe255b 100644 --- a/src/components/InlineSystemMessage.js +++ b/src/components/InlineSystemMessage.js @@ -9,7 +9,11 @@ import Icon from './Icon'; const propTypes = { /** Error to display */ - message: PropTypes.string.isRequired, + message: PropTypes.string, +}; + +const defaultProps = { + message: '', }; const InlineSystemMessage = (props) => { @@ -25,5 +29,6 @@ const InlineSystemMessage = (props) => { }; InlineSystemMessage.propTypes = propTypes; +InlineSystemMessage.defaultProps = defaultProps; InlineSystemMessage.displayName = 'InlineSystemMessage'; export default InlineSystemMessage; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 8a909fdb1a26..53c64ec22c30 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -10,7 +10,6 @@ import * as Pusher from '../Pusher/pusher'; import LocalNotification from '../Notification/LocalNotification'; import PushNotification from '../Notification/PushNotification'; import * as PersonalDetails from './PersonalDetails'; -import * as User from './User'; import Navigation from '../Navigation/Navigation'; import * as ActiveClientManager from '../ActiveClientManager'; import Visibility from '../Visibility'; @@ -27,6 +26,7 @@ import * as ReportActions from './ReportActions'; import Growl from '../Growl'; import * as Localize from '../Localize'; import PusherUtils from '../PusherUtils'; +import * as API from '../API'; let currentUserEmail; let currentUserAccountID; @@ -536,51 +536,6 @@ function updateReportActionMessage(reportID, sequenceNumber, message) { }); } -/** - * Updates a report in the store with a new report action - * - * @param {Number} reportID - * @param {Object} reportAction - * @param {String} [notificationPreference] On what cadence the user would like to be notified - */ -function updateReportWithNewAction( - reportID, - reportAction, - notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, -) { - const messageText = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED - ? lodashGet(reportAction, 'originalMessage.html', '') - : lodashGet(reportAction, ['message', 0, 'text'], ''); - - const updatedReportObject = { - // Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged - reportID, - maxSequenceNumber: reportAction.sequenceNumber, - notificationPreference, - lastMessageTimestamp: reportAction.timestamp, - lastMessageText: ReportUtils.formatReportLastMessageText(messageText), - lastActorEmail: reportAction.actorEmail, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); - - const reportActionsToMerge = {}; - if (reportAction.clientID) { - // Remove the optimistic action from the report since we are about to replace it - // with the real one (which has the true sequenceNumber) - reportActionsToMerge[reportAction.clientID] = null; - } - - // Add the action into Onyx - reportActionsToMerge[reportAction.sequenceNumber] = { - ...reportAction, - isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})), - loading: false, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); -} - /** * Updates a report in Onyx with a new pinned state. * @@ -615,21 +570,6 @@ function subscribeToUserEvents() { return; } - // Live-update a report's actions when a 'report comment' event is received. - PusherUtils.subscribeToPrivateUserChannelEvent( - Pusher.TYPE.REPORT_COMMENT, - currentUserAccountID, - pushJSON => updateReportWithNewAction(pushJSON.reportID, pushJSON.reportAction, pushJSON.notificationPreference), - ); - - // Live-update a report's actions when a 'chunked report comment' event is received. - PusherUtils.subscribeToPrivateUserChannelEvent( - Pusher.TYPE.REPORT_COMMENT_CHUNK, - currentUserAccountID, - pushJSON => updateReportWithNewAction(pushJSON.reportID, pushJSON.reportAction, pushJSON.notificationPreference), - true, - ); - // Live-update a report's actions when an 'edit comment' event is received. PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.REPORT_COMMENT_EDIT, currentUserAccountID, @@ -655,7 +595,37 @@ function subscribeToUserEvents() { function subscribeToReportCommentPushNotifications() { PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, reportAction}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); - updateReportWithNewAction(reportID, reportAction); + const messageText = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED + ? lodashGet(reportAction, 'originalMessage.html', '') + : lodashGet(reportAction, ['message', 0, 'text'], ''); + + const updatedReportObject = { + // Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged + reportID, + maxSequenceNumber: reportAction.sequenceNumber, + notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, + lastMessageTimestamp: reportAction.timestamp, + lastMessageText: ReportUtils.formatReportLastMessageText(messageText), + lastActorEmail: reportAction.actorEmail, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); + + const reportActionsToMerge = {}; + if (reportAction.clientID) { + // Remove the optimistic action from the report since we are about to replace it + // with the real one (which has the true sequenceNumber) + reportActionsToMerge[reportAction.clientID] = null; + } + + // Add the action into Onyx + reportActionsToMerge[reportAction.sequenceNumber] = { + ...reportAction, + isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})), + loading: false, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); }); // Open correct report when push notification is clicked @@ -951,12 +921,12 @@ function addAction(reportID, text, file) { : htmlForNewComment.replace(/((]*>)+)/gi, ' ').replace(/<[^>]*>?/gm, ''); // Update the report in Onyx to have the new sequence number - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + const optimisticReport = { maxSequenceNumber: newSequenceNumber, lastMessageTimestamp: moment().unix(), lastMessageText: ReportUtils.formatReportLastMessageText(textForNewComment), lastActorEmail: currentUserEmail, - }); + }; // Generate a clientID so we can save the optimistic action to storage with the clientID as key. Later, we will // remove the optimistic action when we add the real action created in the server. We do this because it's not @@ -972,75 +942,64 @@ function addAction(reportID, text, file) { // Store the optimistic action ID on the report the comment was added to. It will be removed later when refetching // report actions in order to clear out any stuck actions (i.e. actions where the client never received a Pusher // event, for whatever reason, from the server with the new action data - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - optimisticReportActionIDs: [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID], - }); + optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID]; // Optimistically add the new comment to the store before waiting to save it to the server - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [optimisticReportActionID]: { - actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, - actorEmail: currentUserEmail, - actorAccountID: currentUserAccountID, - person: [ - { - style: 'strong', - text: myPersonalDetails.displayName || currentUserEmail, - type: 'TEXT', - }, - ], - automatic: false, - - // Use the client generated ID as a optimistic action ID so we can remove it later - sequenceNumber: optimisticReportActionID, - clientID: optimisticReportActionID, - avatar: myPersonalDetails.avatar, - timestamp: moment().unix(), - message: [ - { - type: CONST.REPORT.MESSAGE.TYPE.COMMENT, - html: htmlForNewComment, - text: textForNewComment, - }, - ], - isFirstItem: false, - isAttachment, - attachmentInfo, - loading: true, - shouldShow: true, - }, - }); + const optimisticReportActions = {}; + + optimisticReportActions[optimisticReportActionID] = { + actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, + actorEmail: currentUserEmail, + actorAccountID: currentUserAccountID, + person: [ + { + style: 'strong', + text: myPersonalDetails.displayName || currentUserEmail, + type: 'TEXT', + }, + ], + automatic: false, - DeprecatedAPI.Report_AddComment({ + // Use the client generated ID as a optimistic action ID so we can remove it later + sequenceNumber: optimisticReportActionID, + clientID: optimisticReportActionID, + avatar: myPersonalDetails.avatar, + timestamp: moment().unix(), + message: [ + { + type: CONST.REPORT.MESSAGE.TYPE.COMMENT, + html: htmlForNewComment, + text: textForNewComment, + }, + ], + isFirstItem: false, + isAttachment, + attachmentInfo, + loading: true, + shouldShow: true, + }; + + const parameters = { reportID, file, reportComment: commentText, clientID: optimisticReportActionID, - persist: true, - }) - .then((response) => { - if (response.jsonCode === 408) { - Growl.error(Localize.translateLocal('reportActionCompose.fileUploadFailed')); - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [optimisticReportActionID]: null, - }); - console.error(response.message); - return; - } - - if (response.jsonCode === 666 && reportID === conciergeChatReportID) { - Growl.error(Localize.translateLocal('reportActionCompose.blockedFromConcierge')); - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { - [optimisticReportActionID]: null, - }); - - // The fact that the API is returning this error means the BLOCKED_FROM_CONCIERGE nvp in the user details has changed since the last time we checked, so let's update - User.getUserDetails(); - return; - } + }; - updateReportWithNewAction(reportID, response.reportAction); - }); + API.write('AddComment', parameters, { + optimisticData: [ + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: optimisticReport, + }, + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + value: optimisticReportActions, + }, + ], + }); } /** diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index ac04ba8a5e5b..0fca2a6f68c9 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -26,6 +26,8 @@ import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu' import * as ContextMenuActions from './ContextMenu/ContextMenuActions'; import {withReportActionsDrafts} from '../../../components/OnyxProvider'; import RenameAction from '../../../components/ReportActionItem/RenameAction'; +import InlineSystemMessage from '../../../components/InlineSystemMessage'; +import styles from '../../../styles/styles'; const propTypes = { /** The ID of the report this action is on. */ @@ -202,6 +204,9 @@ class ReportActionItem extends Component { )} + + + ); } diff --git a/src/styles/utilities/spacing.js b/src/styles/utilities/spacing.js index 6ba2830de7bf..246f2a0da47b 100644 --- a/src/styles/utilities/spacing.js +++ b/src/styles/utilities/spacing.js @@ -117,6 +117,10 @@ export default { marginLeft: 32, }, + ml10: { + marginLeft: 40, + }, + mln5: { marginLeft: -20, }, From ae0e541dd58448614868c76741e8f873ed6f2453 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 7 Jun 2022 14:01:26 -1000 Subject: [PATCH 02/18] Fix up optimisticReportActions --- src/libs/actions/Report.js | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 53c64ec22c30..365ea24df37c 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -945,38 +945,38 @@ function addAction(reportID, text, file) { optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID]; // Optimistically add the new comment to the store before waiting to save it to the server - const optimisticReportActions = {}; - - optimisticReportActions[optimisticReportActionID] = { - actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, - actorEmail: currentUserEmail, - actorAccountID: currentUserAccountID, - person: [ - { - style: 'strong', - text: myPersonalDetails.displayName || currentUserEmail, - type: 'TEXT', - }, - ], - automatic: false, - - // Use the client generated ID as a optimistic action ID so we can remove it later - sequenceNumber: optimisticReportActionID, - clientID: optimisticReportActionID, - avatar: myPersonalDetails.avatar, - timestamp: moment().unix(), - message: [ - { - type: CONST.REPORT.MESSAGE.TYPE.COMMENT, - html: htmlForNewComment, - text: textForNewComment, - }, - ], - isFirstItem: false, - isAttachment, - attachmentInfo, - loading: true, - shouldShow: true, + const optimisticReportActions = { + [optimisticReportActionID]: { + actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, + actorEmail: currentUserEmail, + actorAccountID: currentUserAccountID, + person: [ + { + style: 'strong', + text: myPersonalDetails.displayName || currentUserEmail, + type: 'TEXT', + }, + ], + automatic: false, + + // Use the client generated ID as a optimistic action ID so we can remove it later + sequenceNumber: optimisticReportActionID, + clientID: optimisticReportActionID, + avatar: myPersonalDetails.avatar, + timestamp: moment().unix(), + message: [ + { + type: CONST.REPORT.MESSAGE.TYPE.COMMENT, + html: htmlForNewComment, + text: textForNewComment, + }, + ], + isFirstItem: false, + isAttachment, + attachmentInfo, + loading: true, + shouldShow: true, + }, }; const parameters = { From 984f41e9e6a27d38dd8c6d1b12a4cca356fc26e6 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 7 Jun 2022 14:07:56 -1000 Subject: [PATCH 03/18] revert updateReportWithNewAction change for now --- src/libs/actions/Report.js | 77 +++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 365ea24df37c..957c962d4079 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -536,6 +536,51 @@ function updateReportActionMessage(reportID, sequenceNumber, message) { }); } +/** + * Updates a report in the store with a new report action + * + * @param {Number} reportID + * @param {Object} reportAction + * @param {String} [notificationPreference] On what cadence the user would like to be notified + */ +function updateReportWithNewAction( + reportID, + reportAction, + notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, +) { + const messageText = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED + ? lodashGet(reportAction, 'originalMessage.html', '') + : lodashGet(reportAction, ['message', 0, 'text'], ''); + + const updatedReportObject = { + // Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged + reportID, + maxSequenceNumber: reportAction.sequenceNumber, + notificationPreference, + lastMessageTimestamp: reportAction.timestamp, + lastMessageText: ReportUtils.formatReportLastMessageText(messageText), + lastActorEmail: reportAction.actorEmail, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); + + const reportActionsToMerge = {}; + if (reportAction.clientID) { + // Remove the optimistic action from the report since we are about to replace it + // with the real one (which has the true sequenceNumber) + reportActionsToMerge[reportAction.clientID] = null; + } + + // Add the action into Onyx + reportActionsToMerge[reportAction.sequenceNumber] = { + ...reportAction, + isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})), + loading: false, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); +} + /** * Updates a report in Onyx with a new pinned state. * @@ -595,37 +640,7 @@ function subscribeToUserEvents() { function subscribeToReportCommentPushNotifications() { PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, reportAction}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); - const messageText = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED - ? lodashGet(reportAction, 'originalMessage.html', '') - : lodashGet(reportAction, ['message', 0, 'text'], ''); - - const updatedReportObject = { - // Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged - reportID, - maxSequenceNumber: reportAction.sequenceNumber, - notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, - lastMessageTimestamp: reportAction.timestamp, - lastMessageText: ReportUtils.formatReportLastMessageText(messageText), - lastActorEmail: reportAction.actorEmail, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); - - const reportActionsToMerge = {}; - if (reportAction.clientID) { - // Remove the optimistic action from the report since we are about to replace it - // with the real one (which has the true sequenceNumber) - reportActionsToMerge[reportAction.clientID] = null; - } - - // Add the action into Onyx - reportActionsToMerge[reportAction.sequenceNumber] = { - ...reportAction, - isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})), - loading: false, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); + updateReportWithNewAction(reportID, reportAction); }); // Open correct report when push notification is clicked From 4b4bcebb728574a6a5e10e0b2ab13b19dc719a86 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 7 Jun 2022 14:09:58 -1000 Subject: [PATCH 04/18] use const --- src/CONST.js | 6 ++++++ src/libs/actions/Report.js | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/CONST.js b/src/CONST.js index 5b1b82da5904..7e510d2962f7 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -701,6 +701,12 @@ const CONST = { // There's a limit of 60k characters in Auth - https://github.com/Expensify/Auth/blob/198d59547f71fdee8121325e8bc9241fc9c3236a/auth/lib/Request.h#L28 MAX_COMMENT_LENGTH: 60000, + + ONYX: { + METHOD: { + MERGE: 'merge', + }, + }, }; export default CONST; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 957c962d4079..e93bf11de3ce 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1004,12 +1004,12 @@ function addAction(reportID, text, file) { API.write('AddComment', parameters, { optimisticData: [ { - onyxMethod: 'merge', + onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: optimisticReport, }, { - onyxMethod: 'merge', + onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, value: optimisticReportActions, }, From 0ea7e2d8f37a8aef06bfdab45ff89d5508174979 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 8 Jun 2022 08:02:02 -1000 Subject: [PATCH 05/18] use Onyx.update() for Airship --- src/libs/actions/Report.js | 49 ++------------------------------------ 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index e93bf11de3ce..14a1ec9677e2 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -536,51 +536,6 @@ function updateReportActionMessage(reportID, sequenceNumber, message) { }); } -/** - * Updates a report in the store with a new report action - * - * @param {Number} reportID - * @param {Object} reportAction - * @param {String} [notificationPreference] On what cadence the user would like to be notified - */ -function updateReportWithNewAction( - reportID, - reportAction, - notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, -) { - const messageText = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED - ? lodashGet(reportAction, 'originalMessage.html', '') - : lodashGet(reportAction, ['message', 0, 'text'], ''); - - const updatedReportObject = { - // Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged - reportID, - maxSequenceNumber: reportAction.sequenceNumber, - notificationPreference, - lastMessageTimestamp: reportAction.timestamp, - lastMessageText: ReportUtils.formatReportLastMessageText(messageText), - lastActorEmail: reportAction.actorEmail, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); - - const reportActionsToMerge = {}; - if (reportAction.clientID) { - // Remove the optimistic action from the report since we are about to replace it - // with the real one (which has the true sequenceNumber) - reportActionsToMerge[reportAction.clientID] = null; - } - - // Add the action into Onyx - reportActionsToMerge[reportAction.sequenceNumber] = { - ...reportAction, - isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})), - loading: false, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); -} - /** * Updates a report in Onyx with a new pinned state. * @@ -638,9 +593,9 @@ function subscribeToUserEvents() { * Setup reportComment push notification callbacks. */ function subscribeToReportCommentPushNotifications() { - PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, reportAction}) => { + PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, onyxData}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); - updateReportWithNewAction(reportID, reportAction); + Onyx.update(onyxData); }); // Open correct report when push notification is clicked From 5759035d480a790a733c4b131dcd7662570835f9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 8 Jun 2022 08:54:14 -1000 Subject: [PATCH 06/18] Fix jest tests --- tests/actions/ReportTest.js | 39 ++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index f0a4fda3c6a0..8c34b20ecc57 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -14,6 +14,7 @@ import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import * as TestHelper from '../utils/TestHelper'; import Log from '../../src/libs/Log'; import * as PersistedRequests from '../../src/libs/actions/PersistedRequests'; +import * as User from '../../src/libs/actions/User'; describe('actions/Report', () => { beforeAll(() => { @@ -76,7 +77,7 @@ describe('actions/Report', () => { // Set up Onyx with some test user data return TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN) .then(() => { - Report.subscribeToUserEvents(); + User.subscribeToUserEvents(); return waitForPromisesToResolve(); }) .then(() => TestHelper.fetchPersonalDetailsForTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN, { @@ -98,19 +99,35 @@ describe('actions/Report', () => { // Store the generated clientID so that we can send it with our mock Pusher update clientID = resultAction.sequenceNumber; - expect(resultAction.message).toEqual(REPORT_ACTION.message); expect(resultAction.person).toEqual(REPORT_ACTION.person); expect(resultAction.loading).toEqual(true); - }) - .then(() => { + // We subscribed to the Pusher channel above and now we need to simulate a reportComment action // Pusher event so we can verify that action was handled correctly and merged into the reportActions. const channel = Pusher.getChannel(`private-encrypted-user-accountID-1${CONFIG.PUSHER.SUFFIX}`); - channel.emit(Pusher.TYPE.REPORT_COMMENT, { - reportID: REPORT_ID, - reportAction: {...REPORT_ACTION, clientID}, - }); + channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, + value: { + reportID: REPORT_ID, + maxSequenceNumber: 1, + notificationPreference: 'always', + lastMessageTimestamp: 0, + lastMessageText: 'Testing a comment', + lastActorEmail: TEST_USER_LOGIN, + }, + }, + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + value: { + [clientID]: null, + [ACTION_ID]: _.without(resultAction, 'loading'), + }, + }, + ]); // Once a reportComment event is emitted to the Pusher channel we should see the comment get processed // by the Pusher callback and added to the storage so we must wait for promises to resolve again and @@ -124,7 +141,7 @@ describe('actions/Report', () => { const resultAction = reportActions[ACTION_ID]; // Verify that our action is no longer in the loading state - expect(resultAction.loading).toEqual(false); + expect(resultAction.loading).not.toBeDefined(); }); }); @@ -211,9 +228,9 @@ describe('actions/Report', () => { return waitForPromisesToResolve(); }) .then(() => { - // THEN only ONE call to Report_AddComment will happen + // THEN only ONE call to AddComment will happen const URL_ARGUMENT_INDEX = 0; - const reportAddCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('Report_AddComment')); + const reportAddCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('AddComment')); expect(reportAddCommentCalls.length).toBe(1); }); }); From 05d540dfb409e582c4a844e3faab795e41cdf51d Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 8 Jun 2022 09:26:00 -1000 Subject: [PATCH 07/18] Fix styles --- src/pages/home/report/ReportActionItem.js | 2 +- src/styles/styles.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 0fca2a6f68c9..00dbb7c6832d 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -204,7 +204,7 @@ class ReportActionItem extends Component { )} - + diff --git a/src/styles/styles.js b/src/styles/styles.js index 58efa6b4cf73..38e833d20304 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -1802,6 +1802,10 @@ const styles = { ...{borderRadius: variables.componentBorderRadiusSmall}, }, + reportActionSystemMessageContainer: { + marginLeft: 42, + }, + reportDetailsTitleContainer: { ...flex.dFlex, ...flex.flexColumn, @@ -2530,7 +2534,7 @@ const styles = { color: themeColors.textSupporting, fontSize: variables.fontSizeLabel, fontFamily: fontFamily.GTA, - marginLeft: 4, + marginLeft: 6, }, }; From 631932fa7c1edc5cb7a666e4f123828756713aee Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 8 Jun 2022 10:05:46 -1000 Subject: [PATCH 08/18] Update everything to use isPending instead of loading --- src/libs/actions/Report.js | 13 ++++++++++++- src/libs/actions/ReportActions.js | 2 +- src/pages/home/report/ReportActionItem.js | 5 +++-- src/pages/home/report/ReportActionItemMessage.js | 4 ++-- src/pages/home/report/ReportActionItemSingle.js | 2 +- src/pages/home/report/reportActionPropTypes.js | 3 +-- tests/README.md | 2 +- tests/actions/ReportTest.js | 4 ++-- 8 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 14a1ec9677e2..8cf4d588817f 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -944,7 +944,7 @@ function addAction(reportID, text, file) { isFirstItem: false, isAttachment, attachmentInfo, - loading: true, + isPending: true, shouldShow: true, }, }; @@ -969,6 +969,17 @@ function addAction(reportID, text, file) { value: optimisticReportActions, }, ], + failureData: [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + value: { + [optimisticReportActionID]: { + isPending: false, + }, + }, + }, + ], }); } diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index 31d84afca71c..4ae46b60191d 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -33,7 +33,7 @@ Onyx.connect({ const reportID = CollectionUtils.extractCollectionItemID(key); const actionsArray = _.toArray(actions); reportActions[reportID] = actionsArray; - const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.loading); + const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.isPending); const mostRecentAction = actionsArray[mostRecentNonLoadingActionIndex]; if (!mostRecentAction || _.isUndefined(mostRecentAction.sequenceNumber)) { return; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 00dbb7c6832d..a7bf94d4c871 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -24,7 +24,7 @@ import canUseTouchScreen from '../../../libs/canUseTouchscreen'; import MiniReportActionContextMenu from './ContextMenu/MiniReportActionContextMenu'; import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu'; import * as ContextMenuActions from './ContextMenu/ContextMenuActions'; -import {withReportActionsDrafts} from '../../../components/OnyxProvider'; +import {withNetwork, withReportActionsDrafts} from '../../../components/OnyxProvider'; import RenameAction from '../../../components/ReportActionItem/RenameAction'; import InlineSystemMessage from '../../../components/InlineSystemMessage'; import styles from '../../../styles/styles'; @@ -175,7 +175,7 @@ class ReportActionItem extends Component { hovered || this.state.isContextMenuActive || this.props.draftMessage, - this.props.action.isPending || this.props.action.error, + (this.props.network.isOffline && this.props.action.isPending) || this.props.action.error, )} > {!this.props.displayAsGroup @@ -216,6 +216,7 @@ ReportActionItem.defaultProps = defaultProps; export default compose( withWindowDimensions, + withNetwork(), withReportActionsDrafts({ propName: 'draftMessage', transformValue: (drafts, props) => { diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index c1eb02a8326d..afdc8244d6e5 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -22,7 +22,7 @@ const propTypes = { }; const ReportActionItemMessage = (props) => { - const isUnsent = props.network.isOffline && props.action.loading; + const isUnsent = props.network.isOffline && props.action.isPending; return ( @@ -32,7 +32,7 @@ const ReportActionItemMessage = (props) => { fragment={fragment} isAttachment={props.action.isAttachment} attachmentInfo={props.action.attachmentInfo} - loading={props.action.loading} + loading={props.action.isPending} /> ))} diff --git a/src/pages/home/report/ReportActionItemSingle.js b/src/pages/home/report/ReportActionItemSingle.js index bc3c0ddcab38..3c6b39509de8 100644 --- a/src/pages/home/report/ReportActionItemSingle.js +++ b/src/pages/home/report/ReportActionItemSingle.js @@ -92,7 +92,7 @@ const ReportActionItemSingle = (props) => { fragment={fragment} tooltipText={props.action.actorEmail} isAttachment={props.action.isAttachment} - isLoading={props.action.loading} + isLoading={props.action.isPending} isSingleLine /> ))} diff --git a/src/pages/home/report/reportActionPropTypes.js b/src/pages/home/report/reportActionPropTypes.js index c4400d36c3ab..e75ae7794357 100644 --- a/src/pages/home/report/reportActionPropTypes.js +++ b/src/pages/home/report/reportActionPropTypes.js @@ -24,8 +24,7 @@ export default { IOUTransactionID: PropTypes.string, }), - /** If the reportAction is pending, that means we have not yet sent the Report_AddComment command to the server. - This should most often occur when the client is offline. */ + /** Whether we have received a response back from the server */ isPending: PropTypes.bool, /** Error message that's come back from the server. */ diff --git a/tests/README.md b/tests/README.md index a166fd22b5f4..bd8e55d3d438 100644 --- a/tests/README.md +++ b/tests/README.md @@ -69,7 +69,7 @@ describe('actions/Report', () => { // the comment we left and it will be in a loading state because // it's an "optimistic comment" expect(action.message[0].text).toBe('Hello!'); - expect(action.loading).toBe(true); + expect(action.isPending).toBe(true); }); }); }); diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 8c34b20ecc57..f755d4f08fb2 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -101,7 +101,7 @@ describe('actions/Report', () => { clientID = resultAction.sequenceNumber; expect(resultAction.message).toEqual(REPORT_ACTION.message); expect(resultAction.person).toEqual(REPORT_ACTION.person); - expect(resultAction.loading).toEqual(true); + expect(resultAction.isPending).toEqual(true); // We subscribed to the Pusher channel above and now we need to simulate a reportComment action // Pusher event so we can verify that action was handled correctly and merged into the reportActions. @@ -141,7 +141,7 @@ describe('actions/Report', () => { const resultAction = reportActions[ACTION_ID]; // Verify that our action is no longer in the loading state - expect(resultAction.loading).not.toBeDefined(); + expect(resultAction.isPending).not.toBeDefined(); }); }); From 665b93b5e1ca4ffe34044213bdf80a6dafeb82b9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 8 Jun 2022 14:47:26 -1000 Subject: [PATCH 09/18] remove unused spacing --- src/styles/utilities/spacing.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/styles/utilities/spacing.js b/src/styles/utilities/spacing.js index 246f2a0da47b..6ba2830de7bf 100644 --- a/src/styles/utilities/spacing.js +++ b/src/styles/utilities/spacing.js @@ -117,10 +117,6 @@ export default { marginLeft: 32, }, - ml10: { - marginLeft: 40, - }, - mln5: { marginLeft: -20, }, From 33eb904c1a06e5865846acaa32014e77c5c0f8a1 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 9 Jun 2022 12:08:55 -1000 Subject: [PATCH 10/18] remove unused stuff --- src/libs/actions/Report.js | 58 -------------------------------------- 1 file changed, 58 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 880fd8a8a257..1d77e5821d0c 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -533,64 +533,6 @@ function updateReportActionMessage(reportID, sequenceNumber, message) { }); } -/** - * Updates a report in the store with a new report action - * - * @param {Number} reportID - * @param {Object} reportAction - * @param {String} [notificationPreference] On what cadence the user would like to be notified - */ -function updateReportWithNewAction( - reportID, - reportAction, - notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, -) { - const messageHtml = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED - ? lodashGet(reportAction, 'originalMessage.html', '') - : lodashGet(reportAction, ['message', 0, 'html'], ''); - - const parser = new ExpensiMark(); - const messageText = parser.htmlToText(messageHtml); - - const updatedReportObject = { - // Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged - reportID, - maxSequenceNumber: reportAction.sequenceNumber, - notificationPreference, - lastMessageTimestamp: reportAction.timestamp, - lastMessageText: ReportUtils.formatReportLastMessageText(messageText), - lastActorEmail: reportAction.actorEmail, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); - - const reportActionsToMerge = {}; - if (reportAction.clientID) { - // Remove the optimistic action from the report since we are about to replace it - // with the real one (which has the true sequenceNumber) - reportActionsToMerge[reportAction.clientID] = null; - } - - // Add the action into Onyx - reportActionsToMerge[reportAction.sequenceNumber] = { - ...reportAction, - isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})), - loading: false, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); -} - -/** - * Updates a report in Onyx with a new pinned state. - * - * @param {Number} reportID - * @param {Boolean} isPinned - */ -function updateReportPinnedState(reportID, isPinned) { - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {isPinned}); -} - /** * Get the private pusher channel name for a Report. * From d7985411fe3de45999c76fcda1985185cf36dc74 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 10 Jun 2022 10:57:14 -1000 Subject: [PATCH 11/18] Remove throttled timezone update call --- src/libs/DateUtils.js | 35 ++++++++++++++---- src/libs/actions/Report.js | 39 ++++++++++++++------ src/pages/home/report/ReportActionCompose.js | 3 -- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/libs/DateUtils.js b/src/libs/DateUtils.js index 791bd9e8c81c..256c9938f805 100644 --- a/src/libs/DateUtils.js +++ b/src/libs/DateUtils.js @@ -115,21 +115,38 @@ function startCurrentDateUpdater() { }); } -/* - * Updates user's timezone, if their timezone is set to automatic and - * is different from current timezone +/** + * @returns {Object} */ -function updateTimezone() { +function getCurrentTimezone() { const currentTimezone = moment.tz.guess(true); if (timezone.automatic && timezone.selected !== currentTimezone) { - PersonalDetails.setPersonalDetails({timezone: {...timezone, selected: currentTimezone}}); + return {...timezone, selected: currentTimezone}; } + return timezone; } /* - * Returns a version of updateTimezone function throttled by 5 minutes + * Updates user's timezone, if their timezone is set to automatic and + * is different from current timezone + */ +function updateTimezone() { + PersonalDetails.setPersonalDetails({timezone: getCurrentTimezone()}); +} + +// Used to throttle updates to the timezone when necessary +let lastUpdatedTimezoneTime = moment(); + +/** + * @returns {Boolean} */ -const throttledUpdateTimezone = _.throttle(() => updateTimezone(), 1000 * 60 * 5); +function canUpdateTimezone() { + return lastUpdatedTimezoneTime.isBefore(moment().subtract(5, 'minutes')); +} + +function setTimezoneUpdated() { + lastUpdatedTimezoneTime = moment(); +} /** * @namespace DateUtils @@ -139,8 +156,10 @@ const DateUtils = { timestampToDateTime, startCurrentDateUpdater, updateTimezone, - throttledUpdateTimezone, getLocalMomentFromTimestamp, + getCurrentTimezone, + canUpdateTimezone, + setTimezoneUpdated, }; export default DateUtils; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 1d77e5821d0c..3494c1c9c935 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -26,6 +26,7 @@ import * as ReportActions from './ReportActions'; import Growl from '../Growl'; import * as Localize from '../Localize'; import PusherUtils from '../PusherUtils'; +import DateUtils from '../DateUtils'; let currentUserEmail; let currentUserAccountID; @@ -937,19 +938,33 @@ function addAction(reportID, text, file) { clientID: optimisticReportActionID, }; + const optimisticData = [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: optimisticReport, + }, + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + value: optimisticReportActions, + }, + ]; + + // Update the timezone if it's been 5 minutes from the last time the user added a comment + if (DateUtils.canUpdateTimezone()) { + const timezone = DateUtils.getCurrentTimezone(); + parameters.timezone = timezone; + optimisticData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS, + value: {timezone}, + }); + DateUtils.setTimezoneUpdated(); + } + API.write('AddComment', parameters, { - optimisticData: [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - value: optimisticReport, - }, - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, - value: optimisticReportActions, - }, - ], + optimisticData, failureData: [ { onyxMethod: CONST.ONYX.METHOD.MERGE, diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index d7ebdb896a05..ccaf04cc7618 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -36,7 +36,6 @@ import ReportActionComposeFocusManager from '../../../libs/ReportActionComposeFo import participantPropTypes from '../../../components/participantPropTypes'; import ParticipantLocalTime from './ParticipantLocalTime'; import {withPersonalDetails} from '../../../components/OnyxProvider'; -import DateUtils from '../../../libs/DateUtils'; import * as User from '../../../libs/actions/User'; import Tooltip from '../../../components/Tooltip'; import EmojiPickerButton from '../../../components/EmojiPicker/EmojiPickerButton'; @@ -422,8 +421,6 @@ class ReportActionCompose extends React.Component { return; } - DateUtils.throttledUpdateTimezone(); - this.props.onSubmit(trimmedComment); this.updateComment(''); this.setTextInputShouldClear(true); From a7174be75831f51ce9a4bbb18ebe610669a04f85 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 10 Jun 2022 11:01:58 -1000 Subject: [PATCH 12/18] Update both my details and personal details --- src/libs/actions/Report.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 3494c1c9c935..025a2eb05491 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -957,9 +957,14 @@ function addAction(reportID, text, file) { parameters.timezone = timezone; optimisticData.push({ onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS, + key: ONYXKEYS.MY_PERSONAL_DETAILS, value: {timezone}, }); + optimisticData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS, + value: {[currentUserEmail]: timezone}, + }); DateUtils.setTimezoneUpdated(); } From 7ba89c1d72a549ab7a89535e0b43a8a6bb4f0442 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 13 Jun 2022 08:55:27 -1000 Subject: [PATCH 13/18] stringify --- src/libs/actions/Report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 025a2eb05491..97ac1917c4f7 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -954,7 +954,7 @@ function addAction(reportID, text, file) { // Update the timezone if it's been 5 minutes from the last time the user added a comment if (DateUtils.canUpdateTimezone()) { const timezone = DateUtils.getCurrentTimezone(); - parameters.timezone = timezone; + parameters.timezone = JSON.stringify(timezone); optimisticData.push({ onyxMethod: CONST.ONYX.METHOD.MERGE, key: ONYXKEYS.MY_PERSONAL_DETAILS, From 950ea108bede1993bf49953bbec4feecc39b5930 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 13 Jun 2022 10:03:04 -1000 Subject: [PATCH 14/18] Just keep CreateComment and CreateAttachment separate for now --- src/libs/actions/Report.js | 15 ++++++++++++--- src/pages/home/ReportScreen.js | 2 +- src/pages/home/report/ReportActionCompose.js | 2 +- tests/README.md | 2 +- tests/actions/ReportTest.js | 4 ++-- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 97ac1917c4f7..392de2063c7c 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -855,7 +855,7 @@ function fetchAllReports( * @param {String} text * @param {File} [file] */ -function addAction(reportID, text, file) { +function createComment(reportID, text, file) { // For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database // For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!! const parser = new ExpensiMark(); @@ -968,7 +968,7 @@ function addAction(reportID, text, file) { DateUtils.setTimezoneUpdated(); } - API.write('AddComment', parameters, { + API.write(isAttachment ? 'CreateAttachment' : 'CreateComment', parameters, { optimisticData, failureData: [ { @@ -984,6 +984,14 @@ function addAction(reportID, text, file) { }); } +/** + * @param {Number} reportID + * @param {Object} file + */ +function createAttachment(reportID, file) { + createComment(reportID, '', file); +} + /** * Get the last read sequence number for a report * @param {String|Number} reportID @@ -1484,7 +1492,8 @@ export { fetchChatReportsByIDs, fetchIOUReportByID, fetchIOUReportByIDAndUpdateChatReport, - addAction, + createComment, + createAttachment, updateLastReadActionID, updateNotificationPreference, setNewMarkerPosition, diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index a0cb67ffbbeb..357af0015c4e 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -122,7 +122,7 @@ class ReportScreen extends React.Component { * @param {String} text */ onSubmitComment(text) { - Report.addAction(getReportID(this.props.route), text); + Report.createComment(getReportID(this.props.route), text); } /** diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index ccaf04cc7618..fb33b77c3931 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -462,7 +462,7 @@ class ReportActionCompose extends React.Component { headerTitle={this.props.translate('reportActionCompose.sendAttachment')} onConfirm={(file) => { this.submitForm(); - Report.addAction(this.props.reportID, '', file); + Report.createAttachment(this.props.reportID, file); this.setTextInputShouldClear(false); }} > diff --git a/tests/README.md b/tests/README.md index bd8e55d3d438..5ba979ce11ec 100644 --- a/tests/README.md +++ b/tests/README.md @@ -60,7 +60,7 @@ describe('actions/Report', () => { })); // When we add a new action to that report - addAction(REPORT_ID, 'Hello!'); + Report.createComment(REPORT_ID, 'Hello!'); return waitForPromisesToResolve() .then(() => { const action = reportActions[ACTION_ID]; diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 38edb26efe5f..f85aef0040e3 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -91,7 +91,7 @@ describe('actions/Report', () => { .then(() => { // This is a fire and forget response, but once it completes we should be able to verify that we // have an "optimistic" report action in Onyx. - Report.addAction(REPORT_ID, 'Testing a comment'); + Report.createComment(REPORT_ID, 'Testing a comment'); return waitForPromisesToResolve(); }) .then(() => { @@ -197,7 +197,7 @@ describe('actions/Report', () => { } // And leave a comment on a report - Report.addAction(REPORT_ID, 'Testing a comment'); + Report.createComment(REPORT_ID, 'Testing a comment'); // Then we should expect that there is on persisted request expect(PersistedRequests.getAll().length).toBe(1); From a6ad4854b5e4774920d261ddf35c96ea8cc5f3d0 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 13 Jun 2022 10:05:27 -1000 Subject: [PATCH 15/18] Keep the name save the refactor for later --- src/libs/actions/Report.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 392de2063c7c..faf1d720796e 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -968,7 +968,7 @@ function createComment(reportID, text, file) { DateUtils.setTimezoneUpdated(); } - API.write(isAttachment ? 'CreateAttachment' : 'CreateComment', parameters, { + API.write(isAttachment ? 'CreateAttachmentWithOptionalComment' : 'CreateComment', parameters, { optimisticData, failureData: [ { @@ -988,7 +988,7 @@ function createComment(reportID, text, file) { * @param {Number} reportID * @param {Object} file */ -function createAttachment(reportID, file) { +function createAttachmentWithOptionalComment(reportID, file) { createComment(reportID, '', file); } @@ -1493,7 +1493,7 @@ export { fetchIOUReportByID, fetchIOUReportByIDAndUpdateChatReport, createComment, - createAttachment, + createAttachmentWithOptionalComment, updateLastReadActionID, updateNotificationPreference, setNewMarkerPosition, From 99306f1a932001445ec0291590cd6b4e25baa43c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 13 Jun 2022 10:07:55 -1000 Subject: [PATCH 16/18] Fix test --- tests/actions/ReportTest.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index f85aef0040e3..d418136dbea4 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -208,8 +208,8 @@ describe('actions/Report', () => { .then(() => { // THEN only ONE call to AddComment will happen const URL_ARGUMENT_INDEX = 0; - const reportAddCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('AddComment')); - expect(reportAddCommentCalls.length).toBe(1); + const createCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('CreateComment')); + expect(createCommentCalls.length).toBe(1); }); }); }); From d5131e55b7345eef2c9b758ca78ec4d72b334408 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 16 Jun 2022 10:23:52 -1000 Subject: [PATCH 17/18] Update command names --- src/libs/actions/Report.js | 12 ++++++------ src/pages/home/ReportScreen.js | 2 +- tests/README.md | 2 +- tests/actions/ReportTest.js | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 5eb40af5c5e8..e6196a7f85e7 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -855,7 +855,7 @@ function fetchAllReports( * @param {String} text * @param {File} [file] */ -function createComment(reportID, text, file) { +function addComment(reportID, text, file) { // For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database // For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!! const parser = new ExpensiMark(); @@ -968,7 +968,7 @@ function createComment(reportID, text, file) { DateUtils.setTimezoneUpdated(); } - API.write(isAttachment ? 'CreateAttachmentWithOptionalComment' : 'CreateComment', parameters, { + API.write(isAttachment ? 'AddAttachmentWithOptionalComment' : 'AddComment', parameters, { optimisticData, failureData: [ { @@ -988,8 +988,8 @@ function createComment(reportID, text, file) { * @param {Number} reportID * @param {Object} file */ -function createAttachmentWithOptionalComment(reportID, file) { - createComment(reportID, '', file); +function addAttachmentWithOptionalComment(reportID, file) { + addComment(reportID, '', file); } /** @@ -1500,8 +1500,8 @@ export { fetchChatReportsByIDs, fetchIOUReportByID, fetchIOUReportByIDAndUpdateChatReport, - createComment, - createAttachmentWithOptionalComment, + addComment, + addAttachmentWithOptionalComment, updateLastReadActionID, updateNotificationPreference, setNewMarkerPosition, diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index ae338598472c..968ba296db4b 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -136,7 +136,7 @@ class ReportScreen extends React.Component { * @param {String} text */ onSubmitComment(text) { - Report.createComment(getReportID(this.props.route), text); + Report.addComment(getReportID(this.props.route), text); } /** diff --git a/tests/README.md b/tests/README.md index 5ba979ce11ec..1ed12dad161f 100644 --- a/tests/README.md +++ b/tests/README.md @@ -60,7 +60,7 @@ describe('actions/Report', () => { })); // When we add a new action to that report - Report.createComment(REPORT_ID, 'Hello!'); + Report.addComment(REPORT_ID, 'Hello!'); return waitForPromisesToResolve() .then(() => { const action = reportActions[ACTION_ID]; diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 97bad981104a..bd25b2476639 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -91,7 +91,7 @@ describe('actions/Report', () => { .then(() => { // This is a fire and forget response, but once it completes we should be able to verify that we // have an "optimistic" report action in Onyx. - Report.createComment(REPORT_ID, 'Testing a comment'); + Report.addComment(REPORT_ID, 'Testing a comment'); return waitForPromisesToResolve(); }) .then(() => { @@ -197,7 +197,7 @@ describe('actions/Report', () => { } // And leave a comment on a report - Report.createComment(REPORT_ID, 'Testing a comment'); + Report.addComment(REPORT_ID, 'Testing a comment'); // Then we should expect that there is on persisted request expect(PersistedRequests.getAll().length).toBe(1); @@ -208,8 +208,8 @@ describe('actions/Report', () => { .then(() => { // THEN only ONE call to AddComment will happen const URL_ARGUMENT_INDEX = 0; - const createCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('CreateComment')); - expect(createCommentCalls.length).toBe(1); + const addCommentCalls = _.filter(global.fetch.mock.calls, callArguments => callArguments[URL_ARGUMENT_INDEX].includes('AddComment')); + expect(addCommentCalls.length).toBe(1); }); }); }); From 8761aa6c578bc3b47689bbcd3135c7a413af8a02 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 23 Jun 2022 08:08:22 -1000 Subject: [PATCH 18/18] Update to reflect recent standards and Web-E changes --- src/libs/actions/Report.js | 10 +++++----- src/libs/actions/ReportActions.js | 2 +- src/pages/home/report/ReportActionCompose.js | 2 +- src/pages/home/report/ReportActionItem.js | 2 +- src/pages/home/report/ReportActionItemMessage.js | 4 ++-- src/pages/home/report/ReportActionItemSingle.js | 2 +- src/pages/home/report/reportActionPropTypes.js | 2 +- src/styles/StyleUtils.js | 6 +++--- tests/actions/ReportTest.js | 4 ++-- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 94d0be873a67..fff484b3e81c 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -919,7 +919,7 @@ function addComment(reportID, text, file) { isFirstItem: false, isAttachment, attachmentInfo, - isPending: true, + isLoading: true, shouldShow: true, }, }; @@ -961,7 +961,7 @@ function addComment(reportID, text, file) { DateUtils.setTimezoneUpdated(); } - API.write(isAttachment ? 'AddAttachmentWithOptionalComment' : 'AddComment', parameters, { + API.write(isAttachment ? 'AddAttachment' : 'AddComment', parameters, { optimisticData, failureData: [ { @@ -969,7 +969,7 @@ function addComment(reportID, text, file) { key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, value: { [optimisticReportActionID]: { - isPending: false, + isLoading: false, }, }, }, @@ -981,7 +981,7 @@ function addComment(reportID, text, file) { * @param {Number} reportID * @param {Object} file */ -function addAttachmentWithOptionalComment(reportID, file) { +function addAttachment(reportID, file) { addComment(reportID, '', file); } @@ -1494,7 +1494,7 @@ export { fetchIOUReportByID, fetchIOUReportByIDAndUpdateChatReport, addComment, - addAttachmentWithOptionalComment, + addAttachment, updateLastReadActionID, updateNotificationPreference, setNewMarkerPosition, diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index f9fe63558ac7..a069aa6b115c 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -34,7 +34,7 @@ Onyx.connect({ const reportID = CollectionUtils.extractCollectionItemID(key); const actionsArray = _.toArray(actions); reportActions[reportID] = actionsArray; - const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.isPending); + const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.isLoading); const mostRecentAction = actionsArray[mostRecentNonLoadingActionIndex]; if (!mostRecentAction || _.isUndefined(mostRecentAction.sequenceNumber)) { return; diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 680c9b400e32..24be05efec76 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -495,7 +495,7 @@ class ReportActionCompose extends React.Component { headerTitle={this.props.translate('reportActionCompose.sendAttachment')} onConfirm={(file) => { this.submitForm(); - Report.createAttachment(this.props.reportID, file); + Report.addAttachment(this.props.reportID, file); this.setTextInputShouldClear(false); }} > diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index a7bf94d4c871..10e5b58de3cb 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -175,7 +175,7 @@ class ReportActionItem extends Component { hovered || this.state.isContextMenuActive || this.props.draftMessage, - (this.props.network.isOffline && this.props.action.isPending) || this.props.action.error, + (this.props.network.isOffline && this.props.action.isLoading) || this.props.action.error, )} > {!this.props.displayAsGroup diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js index afdc8244d6e5..fd6bb82fa023 100644 --- a/src/pages/home/report/ReportActionItemMessage.js +++ b/src/pages/home/report/ReportActionItemMessage.js @@ -22,7 +22,7 @@ const propTypes = { }; const ReportActionItemMessage = (props) => { - const isUnsent = props.network.isOffline && props.action.isPending; + const isUnsent = props.network.isOffline && props.action.isLoading; return ( @@ -32,7 +32,7 @@ const ReportActionItemMessage = (props) => { fragment={fragment} isAttachment={props.action.isAttachment} attachmentInfo={props.action.attachmentInfo} - loading={props.action.isPending} + loading={props.action.isLoading} /> ))} diff --git a/src/pages/home/report/ReportActionItemSingle.js b/src/pages/home/report/ReportActionItemSingle.js index 3c6b39509de8..80a1bc991905 100644 --- a/src/pages/home/report/ReportActionItemSingle.js +++ b/src/pages/home/report/ReportActionItemSingle.js @@ -92,7 +92,7 @@ const ReportActionItemSingle = (props) => { fragment={fragment} tooltipText={props.action.actorEmail} isAttachment={props.action.isAttachment} - isLoading={props.action.isPending} + isLoading={props.action.isLoading} isSingleLine /> ))} diff --git a/src/pages/home/report/reportActionPropTypes.js b/src/pages/home/report/reportActionPropTypes.js index e75ae7794357..d752047cbb97 100644 --- a/src/pages/home/report/reportActionPropTypes.js +++ b/src/pages/home/report/reportActionPropTypes.js @@ -25,7 +25,7 @@ export default { }), /** Whether we have received a response back from the server */ - isPending: PropTypes.bool, + isLoading: PropTypes.bool, /** Error message that's come back from the server. */ error: PropTypes.string, diff --git a/src/styles/StyleUtils.js b/src/styles/StyleUtils.js index ab37cbe68b72..fe7113bb6314 100644 --- a/src/styles/StyleUtils.js +++ b/src/styles/StyleUtils.js @@ -387,10 +387,10 @@ function getLoginPagePromoStyle() { * Generate the styles for the ReportActionItem wrapper view. * * @param {Boolean} [isHovered] - * @param {Boolean} [isPending] + * @param {Boolean} [isLoading] * @returns {Object} */ -function getReportActionItemStyle(isHovered = false, isPending = false) { +function getReportActionItemStyle(isHovered = false, isLoading = false) { return { display: 'flex', justifyContent: 'space-between', @@ -399,7 +399,7 @@ function getReportActionItemStyle(isHovered = false, isPending = false) { // Warning: Setting this to a non-transparent color will cause unread indicator to break on Android : colors.transparent, - opacity: isPending ? 0.5 : 1, + opacity: isLoading ? 0.5 : 1, cursor: 'default', }; } diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index bd25b2476639..99c2b0fb0e8e 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -101,7 +101,7 @@ describe('actions/Report', () => { clientID = resultAction.sequenceNumber; expect(resultAction.message).toEqual(REPORT_ACTION.message); expect(resultAction.person).toEqual(REPORT_ACTION.person); - expect(resultAction.isPending).toEqual(true); + expect(resultAction.isLoading).toEqual(true); // We subscribed to the Pusher channel above and now we need to simulate a reportComment action // Pusher event so we can verify that action was handled correctly and merged into the reportActions. @@ -141,7 +141,7 @@ describe('actions/Report', () => { const resultAction = reportActions[ACTION_ID]; // Verify that our action is no longer in the loading state - expect(resultAction.isPending).not.toBeDefined(); + expect(resultAction.isLoading).not.toBeDefined(); }); });