From a8888b7171e6701859ff7f4560c6f8ad4a589593 Mon Sep 17 00:00:00 2001 From: VH Date: Fri, 3 Nov 2023 20:27:56 +0700 Subject: [PATCH 1/7] Do not call API EditTask if title and description are not changed --- src/libs/actions/Task.js | 4 ++-- src/pages/tasks/TaskDescriptionPage.js | 10 +++++++--- src/pages/tasks/TaskTitlePage.js | 10 +++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/Task.js b/src/libs/actions/Task.js index 959710967881..38d84504afa3 100644 --- a/src/libs/actions/Task.js +++ b/src/libs/actions/Task.js @@ -357,7 +357,7 @@ function reopenTask(taskReport) { * @param {object} report * @param {Object} editedTask */ -function editTaskAndNavigate(report, {title, description}) { +function editTask(report, {title, description}) { // Create the EditedReportAction on the task const editTaskReportAction = ReportUtils.buildOptimisticEditedTaskReportAction(currentUserEmail); @@ -917,7 +917,7 @@ function getTaskReportActionMessage(actionName, reportID, isCreateTaskAction) { export { createTaskAndNavigate, - editTaskAndNavigate, + editTask, editTaskAssigneeAndNavigate, setTitleValue, setDescriptionValue, diff --git a/src/pages/tasks/TaskDescriptionPage.js b/src/pages/tasks/TaskDescriptionPage.js index 5d496fbca6c1..d0aeb3835734 100644 --- a/src/pages/tasks/TaskDescriptionPage.js +++ b/src/pages/tasks/TaskDescriptionPage.js @@ -39,9 +39,13 @@ function TaskDescriptionPage(props) { const submit = useCallback( (values) => { - // Set the description of the report in the store and then call Task.editTaskReport - // to update the description of the report on the server - Task.editTaskAndNavigate(props.report, {description: values.description}); + if (values.description !== props.report.description) { + // Set the description of the report in the store and then call EditTask API + // to update the description of the report on the server + Task.editTask(props.report, {description: values.description}); + } + + Navigation.dismissModal(props.report.reportID); }, [props], ); diff --git a/src/pages/tasks/TaskTitlePage.js b/src/pages/tasks/TaskTitlePage.js index 375a23cc3012..4c111667e094 100644 --- a/src/pages/tasks/TaskTitlePage.js +++ b/src/pages/tasks/TaskTitlePage.js @@ -50,9 +50,13 @@ function TaskTitlePage(props) { const submit = useCallback( (values) => { - // Set the title of the report in the store and then call Task.editTaskReport - // to update the title of the report on the server - Task.editTaskAndNavigate(props.report, {title: values.title}); + if (values.title !== props.report.reportName) { + // Set the title of the report in the store and then call EditTask API + // to update the title of the report on the server + Task.editTask(props.report, {title: values.title}); + } + + Navigation.dismissModal(props.report.reportID); }, [props], ); From 70f661f0b73f41327dc67a85c33c280308185b05 Mon Sep 17 00:00:00 2001 From: VH Date: Fri, 3 Nov 2023 20:29:01 +0700 Subject: [PATCH 2/7] Update description --- src/libs/actions/Task.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Task.js b/src/libs/actions/Task.js index 38d84504afa3..eb22d436abec 100644 --- a/src/libs/actions/Task.js +++ b/src/libs/actions/Task.js @@ -616,9 +616,9 @@ function setAssigneeValue(assigneeEmail, assigneeAccountID, shareDestination, is // This is only needed for creation of a new task and so it should only be stored locally Onyx.merge(ONYXKEYS.TASK, {assignee: assigneeEmail, assigneeAccountID}); - // When we're editing the assignee, we immediately call EditTaskAndNavigate. Since setting the assignee is async, - // the chatReport is not yet set when EditTaskAndNavigate is called. So we return the chatReport here so that - // EditTaskAndNavigate can use it. + // When we're editing the assignee, we immediately call editTaskAssigneeAndNavigate. Since setting the assignee is async, + // the chatReport is not yet set when editTaskAssigneeAndNavigate is called. So we return the chatReport here so that + // editTaskAssigneeAndNavigate can use it. return chatReport; } From 9e7cac008910931c40769635d196a79f444930d1 Mon Sep 17 00:00:00 2001 From: VH Date: Wed, 8 Nov 2023 09:51:20 +0700 Subject: [PATCH 3/7] Remove redudant navigation --- src/libs/actions/Task.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libs/actions/Task.js b/src/libs/actions/Task.js index afa1d2ebc891..b2a9ebd7866c 100644 --- a/src/libs/actions/Task.js +++ b/src/libs/actions/Task.js @@ -433,8 +433,6 @@ function editTask(report, {title, description}) { }, {optimisticData, successData, failureData}, ); - - Navigation.dismissModal(report.reportID); } function editTaskAssigneeAndNavigate(report, ownerAccountID, assigneeEmail, assigneeAccountID = 0, assigneeChatReport = null) { From a7e49733c3786d06fbd85f7600c71d069958bb2c Mon Sep 17 00:00:00 2001 From: VH Date: Tue, 21 Nov 2023 20:00:59 +0700 Subject: [PATCH 4/7] Do not call API EditTaskAssignee if assignee is not changed --- src/libs/actions/Task.js | 12 +++++------- src/pages/tasks/TaskAssigneeSelectorModal.js | 9 ++++++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/Task.js b/src/libs/actions/Task.js index 225737ccbf6b..c7426bb470c4 100644 --- a/src/libs/actions/Task.js +++ b/src/libs/actions/Task.js @@ -435,7 +435,7 @@ function editTask(report, {title, description}) { ); } -function editTaskAssigneeAndNavigate(report, ownerAccountID, assigneeEmail, assigneeAccountID = 0, assigneeChatReport = null) { +function editTaskAssignee(report, ownerAccountID, assigneeEmail, assigneeAccountID = 0, assigneeChatReport = null) { // Create the EditedReportAction on the task const editTaskReportAction = ReportUtils.buildOptimisticEditedTaskReportAction(currentUserEmail); const reportName = report.reportName.trim(); @@ -520,8 +520,6 @@ function editTaskAssigneeAndNavigate(report, ownerAccountID, assigneeEmail, assi }, {optimisticData, successData, failureData}, ); - - Navigation.dismissModal(report.reportID); } /** @@ -623,9 +621,9 @@ function setAssigneeValue(assigneeEmail, assigneeAccountID, shareDestination, is // This is only needed for creation of a new task and so it should only be stored locally Onyx.merge(ONYXKEYS.TASK, {assignee: assigneeEmail, assigneeAccountID}); - // When we're editing the assignee, we immediately call editTaskAssigneeAndNavigate. Since setting the assignee is async, - // the chatReport is not yet set when editTaskAssigneeAndNavigate is called. So we return the chatReport here so that - // editTaskAssigneeAndNavigate can use it. + // When we're editing the assignee, we immediately call editTaskAssignee. Since setting the assignee is async, + // the chatReport is not yet set when editTaskAssignee is called. So we return the chatReport here so that + // editTaskAssignee can use it. return chatReport; } @@ -937,7 +935,7 @@ function getTaskReportActionMessage(actionName, reportID, isCreateTaskAction) { export { createTaskAndNavigate, editTask, - editTaskAssigneeAndNavigate, + editTaskAssignee, setTitleValue, setDescriptionValue, setTaskReport, diff --git a/src/pages/tasks/TaskAssigneeSelectorModal.js b/src/pages/tasks/TaskAssigneeSelectorModal.js index a817b4f94e9b..6ad08e36be7c 100644 --- a/src/pages/tasks/TaskAssigneeSelectorModal.js +++ b/src/pages/tasks/TaskAssigneeSelectorModal.js @@ -196,10 +196,13 @@ function TaskAssigneeSelectorModal(props) { // Check to see if we're editing a task and if so, update the assignee if (report) { - const assigneeChatReport = Task.setAssigneeValue(option.login, option.accountID, props.route.params.reportID, OptionsListUtils.isCurrentUser(option)); + if (option.accountID !== report.managerID) { + const assigneeChatReport = Task.setAssigneeValue(option.login, option.accountID, props.route.params.reportID, OptionsListUtils.isCurrentUser(option)); - // Pass through the selected assignee - Task.editTaskAssigneeAndNavigate(report, props.session.accountID, option.login, option.accountID, assigneeChatReport); + // Pass through the selected assignee + Task.editTaskAssignee(report, props.session.accountID, option.login, option.accountID, assigneeChatReport); + } + return Navigation.dismissModal(report.reportID); } }; From 4204fc21bf7d70cbe5cb5254bbd487f8a3e11de7 Mon Sep 17 00:00:00 2001 From: VH Date: Tue, 21 Nov 2023 20:08:34 +0700 Subject: [PATCH 5/7] Fix prettier --- src/pages/tasks/TaskAssigneeSelectorModal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/tasks/TaskAssigneeSelectorModal.js b/src/pages/tasks/TaskAssigneeSelectorModal.js index 6ad08e36be7c..0dd2b65480a0 100644 --- a/src/pages/tasks/TaskAssigneeSelectorModal.js +++ b/src/pages/tasks/TaskAssigneeSelectorModal.js @@ -200,7 +200,7 @@ function TaskAssigneeSelectorModal(props) { const assigneeChatReport = Task.setAssigneeValue(option.login, option.accountID, props.route.params.reportID, OptionsListUtils.isCurrentUser(option)); // Pass through the selected assignee - Task.editTaskAssignee(report, props.session.accountID, option.login, option.accountID, assigneeChatReport); + Task.editTaskAssignee(report, props.session.accountID, option.login, option.accountID, assigneeChatReport); } return Navigation.dismissModal(report.reportID); } From 745f12ea1f3e2282dcc1ab70ecaa2413b98d2b6a Mon Sep 17 00:00:00 2001 From: VH Date: Sun, 26 Nov 2023 16:40:54 +0700 Subject: [PATCH 6/7] Normalize CRLF before compare description --- src/libs/StringUtils.ts | 11 ++++++++++- src/pages/tasks/TaskDescriptionPage.js | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libs/StringUtils.ts b/src/libs/StringUtils.ts index f1dd5a06e828..b80bb4be2c42 100644 --- a/src/libs/StringUtils.ts +++ b/src/libs/StringUtils.ts @@ -63,4 +63,13 @@ function removeInvisibleCharacters(value: string): string { return result.trim(); } -export default {sanitizeString, isEmptyString, removeInvisibleCharacters}; +/** + * Replace all CRLF with LF + * @param value - The input string + * @returns The string with all CRLF replaced with LF + */ +function normalizeCRLF(value: string | null): string { + return value?.replace(/\r\n/g, '\n'); +} + +export default {sanitizeString, isEmptyString, removeInvisibleCharacters, normalizeCRLF}; diff --git a/src/pages/tasks/TaskDescriptionPage.js b/src/pages/tasks/TaskDescriptionPage.js index c123d3c5b98c..6901f2562ee3 100644 --- a/src/pages/tasks/TaskDescriptionPage.js +++ b/src/pages/tasks/TaskDescriptionPage.js @@ -15,6 +15,7 @@ import * as Browser from '@libs/Browser'; import compose from '@libs/compose'; import Navigation from '@libs/Navigation/Navigation'; import * as ReportUtils from '@libs/ReportUtils'; +import StringUtils from '@libs/StringUtils'; import updateMultilineInputRange from '@libs/UpdateMultilineInputRange'; import withReportOrNotFound from '@pages/home/report/withReportOrNotFound'; import reportPropTypes from '@pages/reportPropTypes'; @@ -42,7 +43,8 @@ function TaskDescriptionPage(props) { const submit = useCallback( (values) => { - if (values.description !== props.report.description) { + // props.report.description might contain CRLF from the server + if (StringUtils.normalizeCRLF(values.description) !== StringUtils.normalizeCRLF(props.report.description)) { // Set the description of the report in the store and then call EditTask API // to update the description of the report on the server Task.editTask(props.report, {description: values.description}); From bfe61c247ef35e8f9e9571304a33b2555ad534c6 Mon Sep 17 00:00:00 2001 From: VH Date: Sun, 26 Nov 2023 16:45:11 +0700 Subject: [PATCH 7/7] Fix typescript --- src/libs/StringUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/StringUtils.ts b/src/libs/StringUtils.ts index b80bb4be2c42..2fb918c7a233 100644 --- a/src/libs/StringUtils.ts +++ b/src/libs/StringUtils.ts @@ -68,7 +68,7 @@ function removeInvisibleCharacters(value: string): string { * @param value - The input string * @returns The string with all CRLF replaced with LF */ -function normalizeCRLF(value: string | null): string { +function normalizeCRLF(value?: string): string | undefined { return value?.replace(/\r\n/g, '\n'); }