From 8990790f3677703ed3499646f10f04b64cfee580 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 23 Jun 2022 13:20:14 -1000 Subject: [PATCH 1/6] Sketch some rough refactor changes --- src/libs/actions/Report.js | 179 +++++++++++++++---- src/pages/home/report/ReportActionCompose.js | 36 ++-- 2 files changed, 168 insertions(+), 47 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index fff484b3e81c..023833273312 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -841,14 +841,7 @@ function fetchAllReports( }); } -/** - * Add an action item to a report - * - * @param {Number} reportID - * @param {String} text - * @param {File} [file] - */ -function addComment(reportID, text, file) { +function buildOptimisticReportAction(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(); @@ -856,23 +849,12 @@ function addComment(reportID, text, file) { const isAttachment = _.isEmpty(text) && file !== undefined; const attachmentInfo = isAttachment ? file : {}; - // The new sequence number will be one higher than the highest - const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; - const newSequenceNumber = highestSequenceNumber + 1; const htmlForNewComment = isAttachment ? 'Uploading Attachment...' : commentText; // Remove HTML from text when applying optimistic offline comment const textForNewComment = isAttachment ? '[Attachment]' : parser.htmlToText(htmlForNewComment); - // Update the report in Onyx to have the new sequence number - 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 // safe to assume that this will use the very next sequenceNumber. An action created by another can overwrite that @@ -884,14 +866,9 @@ function addComment(reportID, text, file) { const randomNumber = Math.floor((Math.random() * (999 - 100)) + 100); const optimisticReportActionID = parseInt(`${Date.now()}${randomNumber}`, 10); - // 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 - optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID]; - - // Optimistically add the new comment to the store before waiting to save it to the server - const optimisticReportActions = { - [optimisticReportActionID]: { + return { + commentText, + reportAction: { actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, actorEmail: currentUserEmail, actorAccountID: currentUserAccountID, @@ -923,12 +900,43 @@ function addComment(reportID, text, file) { shouldShow: true, }, }; +} + +/** + * Add an action item to a report + * + * @param {Number} reportID + * @param {String} text + */ +function addComment(reportID, text) { + const {reportAction, commentText} = buildOptimisticReportAction(reportID, text); + + // The new sequence number will be one higher than the highest + const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; + const newSequenceNumber = highestSequenceNumber + 1; + + // Update the report in Onyx to have the new sequence number + const optimisticReport = { + maxSequenceNumber: newSequenceNumber, + lastMessageTimestamp: moment().unix(), + lastMessageText: ReportUtils.formatReportLastMessageText(reportAction.message[0].text), + lastActorEmail: currentUserEmail, + }; + + // 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 + optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), reportAction.clientID]; + + // Optimistically add the new comment to the store before waiting to save it to the server + const optimisticReportActions = { + [reportAction.clientID]: reportAction, + }; const parameters = { reportID, - file, reportComment: commentText, - clientID: optimisticReportActionID, + clientID: reportAction.clientID, }; const optimisticData = [ @@ -961,14 +969,14 @@ function addComment(reportID, text, file) { DateUtils.setTimezoneUpdated(); } - API.write(isAttachment ? 'AddAttachment' : 'AddComment', parameters, { + API.write('AddComment', parameters, { optimisticData, failureData: [ { onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, value: { - [optimisticReportActionID]: { + [reportAction.clientID]: { isLoading: false, }, }, @@ -976,13 +984,118 @@ function addComment(reportID, text, file) { ], }); } +function updateAndGetOptimisticTimezoneData() { + let timezone = '{}'; + const optimisticTimezoneData = []; + + // Update the timezone if it's been 5 minutes from the last time the user added a comment + if (DateUtils.canUpdateTimezone()) { + timezone = JSON.stringify(DateUtils.getCurrentTimezone()); + optimisticTimezoneData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.MY_PERSONAL_DETAILS, + value: {timezone}, + }); + optimisticTimezoneData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS, + value: {[currentUserEmail]: timezone}, + }); + DateUtils.setTimezoneUpdated(); + } + + return { + optimisticTimezoneData, + timezone, + }; +} /** * @param {Number} reportID * @param {Object} file + * @param {String} [text] */ -function addAttachment(reportID, file) { - addComment(reportID, '', file); +function addAttachment(reportID, file, text = '') { + let reportCommentAction; + let reportCommentText; + + if (text) { + const builtAction = buildOptimisticReportAction(reportID, text); + reportCommentAction = builtAction.reportAction; + reportCommentText = builtAction.commentText; + } + + const {reportAction, commentText} = buildOptimisticReportAction(reportID, '', file); + + // AddAttachment can create up to two actions. An attachment and a report comment simultaneously. Therefore we may need a newSequenceNumber that is 1 or 2 larger than the current. + // The new sequence number will be one higher than the highest + const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; + const newSequenceNumber = highestSequenceNumber + (commentText ? 2 : 1); + + // Update the report in Onyx to have the new sequence number + const optimisticReport = { + maxSequenceNumber: newSequenceNumber, + lastMessageTimestamp: moment().unix(), + lastMessageText: ReportUtils.formatReportLastMessageText(reportAction.message[0].text), + lastActorEmail: currentUserEmail, + }; + + // 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 + optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), reportAction.clientID]; + + // We also add the optimistic comment here if we have the optional text + if (text) { + optimisticReport.optimisticReportActionIDs.push(reportCommentAction.clientID); + } + + // Optimistically add the new comment to the store before waiting to save it to the server + const optimisticReportActions = { + [reportAction.clientID]: reportAction, + }; + + if (text) { + optimisticReportActions[reportCommentAction.clientID] = reportCommentAction; + } + + const {optimisticTimezoneData, timezone} = updateAndGetOptimisticTimezoneData(); + const parameters = { + reportID, + reportComment: reportCommentText, + clientID: reportAction.clientID, + file, + timezone, + }; + + 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, + }, + ...optimisticTimezoneData, + ]; + + API.write('AddAttachment', parameters, { + optimisticData, + failureData: [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + value: { + [reportAction.clientID]: { + isLoading: false, + }, + }, + }, + ], + }); } /** diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 24be05efec76..18da26bfdd62 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -430,24 +430,14 @@ class ReportActionCompose extends React.Component { } } - /** - * Add a new comment to this chat - * - * @param {SyntheticEvent} [e] - */ - submitForm(e) { - if (e) { - e.preventDefault(); - } - + prepareCommentAndResetComposer() { const trimmedComment = this.comment.trim(); // Don't submit empty comments or comments that exceed the character limit if (this.state.isCommentEmpty || trimmedComment.length > CONST.MAX_COMMENT_LENGTH) { - return; + return ''; } - this.props.onSubmit(trimmedComment); this.updateComment(''); this.setTextInputShouldClear(true); if (this.props.isComposerFullSize) { @@ -459,6 +449,24 @@ class ReportActionCompose extends React.Component { this.textInput.setNativeProps({selection: {start: 0, end: 0}}); } + /** + * Add a new comment to this chat + * + * @param {SyntheticEvent} [e] + */ + submitForm(e) { + if (e) { + e.preventDefault(); + } + + const comment = this.prepareCommentAndResetComposer(); + if (!comment) { + return; + } + + this.props.onSubmit(comment); + } + render() { // Waiting until ONYX variables are loaded before displaying the component if (_.isEmpty(this.props.personalDetails)) { @@ -494,8 +502,8 @@ class ReportActionCompose extends React.Component { { - this.submitForm(); - Report.addAttachment(this.props.reportID, file); + const comment = this.prepareCommentAndResetComposer(); + Report.addAttachment(this.props.reportID, file, comment); this.setTextInputShouldClear(false); }} > From a4a58a8e092d3ff05b35c8a543f3ba3d0ad201cb Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 23 Jun 2022 13:35:39 -1000 Subject: [PATCH 2/6] First draft at creating two actions at once --- src/libs/actions/Report.js | 80 +++++++++----------- src/pages/home/report/ReportActionCompose.js | 4 + 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 023833273312..e2d49dcf3a3d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -841,6 +841,12 @@ function fetchAllReports( }); } +/** + * @param {Number} reportID + * @param {String} [text] + * @param {File} [file] + * @returns {Object} + */ function buildOptimisticReportAction(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!! @@ -848,7 +854,6 @@ function buildOptimisticReportAction(reportID, text, file) { const commentText = text.length < 10000 ? parser.replace(text) : text; const isAttachment = _.isEmpty(text) && file !== undefined; const attachmentInfo = isAttachment ? file : {}; - const htmlForNewComment = isAttachment ? 'Uploading Attachment...' : commentText; // Remove HTML from text when applying optimistic offline comment @@ -902,6 +907,32 @@ function buildOptimisticReportAction(reportID, text, file) { }; } +function updateAndGetOptimisticTimezoneData() { + let timezone = '{}'; + const optimisticTimezoneData = []; + + // Update the timezone if it's been 5 minutes from the last time the user added a comment + if (DateUtils.canUpdateTimezone()) { + timezone = JSON.stringify(DateUtils.getCurrentTimezone()); + optimisticTimezoneData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.MY_PERSONAL_DETAILS, + value: {timezone}, + }); + optimisticTimezoneData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS, + value: {[currentUserEmail]: timezone}, + }); + DateUtils.setTimezoneUpdated(); + } + + return { + optimisticTimezoneData, + timezone, + }; +} + /** * Add an action item to a report * @@ -933,10 +964,13 @@ function addComment(reportID, text) { [reportAction.clientID]: reportAction, }; + const {optimisticTimezoneData, timezone} = updateAndGetOptimisticTimezoneData(); + const parameters = { reportID, reportComment: commentText, clientID: reportAction.clientID, + timezone, }; const optimisticData = [ @@ -950,25 +984,9 @@ function addComment(reportID, text) { key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, value: optimisticReportActions, }, + ...optimisticTimezoneData, ]; - // 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 = JSON.stringify(timezone); - optimisticData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.MY_PERSONAL_DETAILS, - value: {timezone}, - }); - optimisticData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS, - value: {[currentUserEmail]: timezone}, - }); - DateUtils.setTimezoneUpdated(); - } - API.write('AddComment', parameters, { optimisticData, failureData: [ @@ -984,31 +1002,6 @@ function addComment(reportID, text) { ], }); } -function updateAndGetOptimisticTimezoneData() { - let timezone = '{}'; - const optimisticTimezoneData = []; - - // Update the timezone if it's been 5 minutes from the last time the user added a comment - if (DateUtils.canUpdateTimezone()) { - timezone = JSON.stringify(DateUtils.getCurrentTimezone()); - optimisticTimezoneData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.MY_PERSONAL_DETAILS, - value: {timezone}, - }); - optimisticTimezoneData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS, - value: {[currentUserEmail]: timezone}, - }); - DateUtils.setTimezoneUpdated(); - } - - return { - optimisticTimezoneData, - timezone, - }; -} /** * @param {Number} reportID @@ -1064,6 +1057,7 @@ function addAttachment(reportID, file, text = '') { reportID, reportComment: reportCommentText, clientID: reportAction.clientID, + commentClientID: reportCommentAction.clientID, file, timezone, }; diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js index 18da26bfdd62..4f07e9b0f3fd 100755 --- a/src/pages/home/report/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose.js @@ -430,6 +430,9 @@ class ReportActionCompose extends React.Component { } } + /** + * @returns {String} + */ prepareCommentAndResetComposer() { const trimmedComment = this.comment.trim(); @@ -447,6 +450,7 @@ class ReportActionCompose extends React.Component { // Important to reset the selection on Submit action this.textInput.setNativeProps({selection: {start: 0, end: 0}}); + return trimmedComment; } /** From 4a61536ab14cd39d92ed7bce9e46448cb9cea56d Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 24 Jun 2022 09:22:50 -1000 Subject: [PATCH 3/6] Consolidate logic into single method to avoid duplication --- src/libs/actions/Report.js | 219 ++++++++++++++++--------------------- 1 file changed, 94 insertions(+), 125 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index e2d49dcf3a3d..6073a3258cd5 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -907,159 +907,82 @@ function buildOptimisticReportAction(reportID, text, file) { }; } -function updateAndGetOptimisticTimezoneData() { - let timezone = '{}'; - const optimisticTimezoneData = []; - - // Update the timezone if it's been 5 minutes from the last time the user added a comment - if (DateUtils.canUpdateTimezone()) { - timezone = JSON.stringify(DateUtils.getCurrentTimezone()); - optimisticTimezoneData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.MY_PERSONAL_DETAILS, - value: {timezone}, - }); - optimisticTimezoneData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS, - value: {[currentUserEmail]: timezone}, - }); - DateUtils.setTimezoneUpdated(); - } - - return { - optimisticTimezoneData, - timezone, - }; -} - /** - * Add an action item to a report + * Add up to two report actions to a report. This method can be called for the following situations: + * + * - Adding one comment + * - Adding one attachment + * - Add both a comment and attachment simultaneously * * @param {Number} reportID - * @param {String} text - */ -function addComment(reportID, text) { - const {reportAction, commentText} = buildOptimisticReportAction(reportID, text); - - // The new sequence number will be one higher than the highest - const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; - const newSequenceNumber = highestSequenceNumber + 1; - - // Update the report in Onyx to have the new sequence number - const optimisticReport = { - maxSequenceNumber: newSequenceNumber, - lastMessageTimestamp: moment().unix(), - lastMessageText: ReportUtils.formatReportLastMessageText(reportAction.message[0].text), - lastActorEmail: currentUserEmail, - }; - - // 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 - optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), reportAction.clientID]; - - // Optimistically add the new comment to the store before waiting to save it to the server - const optimisticReportActions = { - [reportAction.clientID]: reportAction, - }; - - const {optimisticTimezoneData, timezone} = updateAndGetOptimisticTimezoneData(); - - const parameters = { - reportID, - reportComment: commentText, - clientID: reportAction.clientID, - timezone, - }; - - 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, - }, - ...optimisticTimezoneData, - ]; - - API.write('AddComment', parameters, { - optimisticData, - failureData: [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, - value: { - [reportAction.clientID]: { - isLoading: false, - }, - }, - }, - ], - }); -} - -/** - * @param {Number} reportID - * @param {Object} file * @param {String} [text] + * @param {Object} [file] */ -function addAttachment(reportID, file, text = '') { +function addActions(reportID, text = '', file) { let reportCommentAction; let reportCommentText; + let attachmentAction; + let commandName = 'AddComment'; if (text) { - const builtAction = buildOptimisticReportAction(reportID, text); - reportCommentAction = builtAction.reportAction; - reportCommentText = builtAction.commentText; + const reportComment = buildOptimisticReportAction(reportID, text); + reportCommentAction = reportComment.reportAction; + reportCommentText = reportComment.commentText; + } + + // When we are adding an attachment we will call AddAttachment + if (file) { + commandName = 'AddAttachment'; + const attachment = buildOptimisticReportAction(reportID, '', file); + attachmentAction = attachment.reportAction; } - const {reportAction, commentText} = buildOptimisticReportAction(reportID, '', file); + // Always prefer the file as the last action over text + const lastAction = attachmentAction || reportCommentAction; - // AddAttachment can create up to two actions. An attachment and a report comment simultaneously. Therefore we may need a newSequenceNumber that is 1 or 2 larger than the current. + // AddActions can create up to two actions. An attachment and a report comment simultaneously. Therefore we may need a newSequenceNumber that is 1 or 2 larger than the current. // The new sequence number will be one higher than the highest const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; - const newSequenceNumber = highestSequenceNumber + (commentText ? 2 : 1); + const newSequenceNumber = highestSequenceNumber + (text && file ? 2 : 1); // Update the report in Onyx to have the new sequence number const optimisticReport = { maxSequenceNumber: newSequenceNumber, lastMessageTimestamp: moment().unix(), - lastMessageText: ReportUtils.formatReportLastMessageText(reportAction.message[0].text), + lastMessageText: ReportUtils.formatReportLastMessageText(lastAction.message[0].text), lastActorEmail: currentUserEmail, }; // 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 - optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), reportAction.clientID]; + optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || [])]; - // We also add the optimistic comment here if we have the optional text if (text) { optimisticReport.optimisticReportActionIDs.push(reportCommentAction.clientID); } + if (file) { + optimisticReport.optimisticReportActionIDs.push(attachmentAction.clientID); + } + // Optimistically add the new comment to the store before waiting to save it to the server - const optimisticReportActions = { - [reportAction.clientID]: reportAction, - }; + const optimisticReportActions = {}; if (text) { optimisticReportActions[reportCommentAction.clientID] = reportCommentAction; } - const {optimisticTimezoneData, timezone} = updateAndGetOptimisticTimezoneData(); + if (file) { + optimisticReportActions[attachmentAction.clientID] = attachmentAction; + } + const parameters = { reportID, reportComment: reportCommentText, - clientID: reportAction.clientID, + clientID: lastAction.clientID, commentClientID: reportCommentAction.clientID, file, - timezone, }; const optimisticData = [ @@ -1073,25 +996,71 @@ function addAttachment(reportID, file, text = '') { key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, value: optimisticReportActions, }, - ...optimisticTimezoneData, ]; - API.write('AddAttachment', parameters, { + // 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 = JSON.stringify(timezone); + optimisticData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.MY_PERSONAL_DETAILS, + value: {timezone}, + }); + optimisticData.push({ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS, + value: {[currentUserEmail]: timezone}, + }); + DateUtils.setTimezoneUpdated(); + } + + const failureDataReportActions = {}; + + if (text) { + failureDataReportActions[reportCommentAction.clientID] = { + isLoading: false, + }; + } + + if (file) { + failureDataReportActions[attachmentAction.clientID] = { + isLoading: false, + }; + } + + API.write(commandName, parameters, { optimisticData, - failureData: [ - { - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, - value: { - [reportAction.clientID]: { - isLoading: false, - }, - }, - }, - ], + failureData: [{ + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + value: failureDataReportActions, + }], }); } +/** + * + * Add an attachment and optional comment. + * + * @param {Number} reportID + * @param {File} file + * @param {String} [text] + */ +function addAttachment(reportID, file, text = '') { + addActions(reportID, text, file); +} + +/** + * Add a single comment to a report + * + * @param {Number} reportID + * @param {String} text + */ +function addComment(reportID, text) { + addActions(reportID, text); +} + /** * Get the last read sequence number for a report * @param {String|Number} reportID From 9b2fd87be337cec92a4239b5e6ce419a14f0ed05 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 24 Jun 2022 09:49:10 -1000 Subject: [PATCH 4/6] Clean up comments --- src/libs/actions/Report.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 6073a3258cd5..87bbbebd373d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -919,8 +919,8 @@ function buildOptimisticReportAction(reportID, text, file) { * @param {Object} [file] */ function addActions(reportID, text = '', file) { + let reportCommentText = ''; let reportCommentAction; - let reportCommentText; let attachmentAction; let commandName = 'AddComment'; @@ -930,8 +930,9 @@ function addActions(reportID, text = '', file) { reportCommentText = reportComment.commentText; } - // When we are adding an attachment we will call AddAttachment if (file) { + // When we are adding an attachment we will call AddAttachment. + // It supports sending an attachment with an optional comment and AddComment supports adding a single text comment only. commandName = 'AddAttachment'; const attachment = buildOptimisticReportAction(reportID, '', file); attachmentAction = attachment.reportAction; @@ -940,10 +941,10 @@ function addActions(reportID, text = '', file) { // Always prefer the file as the last action over text const lastAction = attachmentAction || reportCommentAction; - // AddActions can create up to two actions. An attachment and a report comment simultaneously. Therefore we may need a newSequenceNumber that is 1 or 2 larger than the current. - // The new sequence number will be one higher than the highest + // We need a newSequenceNumber that is n larger than the current depending on how many actions we are adding. + const actionCount = text && file ? 2 : 1; const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; - const newSequenceNumber = highestSequenceNumber + (text && file ? 2 : 1); + const newSequenceNumber = highestSequenceNumber + actionCount; // Update the report in Onyx to have the new sequence number const optimisticReport = { @@ -966,13 +967,11 @@ function addActions(reportID, text = '', file) { optimisticReport.optimisticReportActionIDs.push(attachmentAction.clientID); } - // Optimistically add the new comment to the store before waiting to save it to the server + // Optimistically add the new actions to the store before waiting to save them to the server const optimisticReportActions = {}; - if (text) { optimisticReportActions[reportCommentAction.clientID] = reportCommentAction; } - if (file) { optimisticReportActions[attachmentAction.clientID] = attachmentAction; } From 25c26e814309e9e615c46665996da08fc9d2daf4 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 24 Jun 2022 10:04:37 -1000 Subject: [PATCH 5/6] Clean up --- src/libs/actions/Report.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 87bbbebd373d..7b393c47f060 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1015,17 +1015,16 @@ function addActions(reportID, text = '', file) { } const failureDataReportActions = {}; + const defaultLoadingState = { + isLoading: false, + }; if (text) { - failureDataReportActions[reportCommentAction.clientID] = { - isLoading: false, - }; + failureDataReportActions[reportCommentAction.clientID] = defaultLoadingState; } if (file) { - failureDataReportActions[attachmentAction.clientID] = { - isLoading: false, - }; + failureDataReportActions[attachmentAction.clientID] = defaultLoadingState; } API.write(commandName, parameters, { From 49a93eb5ce9e698e6e3692df7c6146a3a24b4afa Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 13 Jul 2022 09:10:15 -1000 Subject: [PATCH 6/6] Fix bug when adding attachment. Remove deprecated Report_AddComment. --- src/libs/actions/Report.js | 2 +- src/libs/deprecatedAPI.js | 16 ---------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 915d7ad52e8e..36fd0c06eed2 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -961,7 +961,7 @@ function addActions(reportID, text = '', file) { reportID, reportComment: reportCommentText, clientID: lastAction.clientID, - commentClientID: reportCommentAction.clientID, + commentClientID: lodashGet(reportCommentAction, 'clientID', ''), file, }; diff --git a/src/libs/deprecatedAPI.js b/src/libs/deprecatedAPI.js index 32c6659ae051..cffdac6fdbd8 100644 --- a/src/libs/deprecatedAPI.js +++ b/src/libs/deprecatedAPI.js @@ -290,21 +290,6 @@ function RejectTransaction(parameters) { return Network.post(commandName, parameters); } -/** - * @param {Object} parameters - * @param {String} parameters.reportComment - * @param {Number} parameters.reportID - * @param {String} parameters.clientID - * @param {File|Object} [parameters.file] - * @returns {Promise} - */ -function Report_AddComment(parameters) { - const commandName = 'Report_AddComment'; - requireParameters(['reportComment', 'reportID', 'clientID'], - parameters, commandName); - return Network.post(commandName, parameters); -} - /** * @param {Object} parameters * @param {Number} parameters.reportID @@ -873,7 +858,6 @@ export { Plaid_GetLinkToken, Policy_Employees_Merge, RejectTransaction, - Report_AddComment, Report_GetHistory, Report_EditComment, ResendValidateCode,