From f40baf866ee52747a6d6195172b44a923d7058ef Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Mon, 30 Oct 2023 15:58:31 -0600 Subject: [PATCH 1/8] Add a DRY method for returning the params and onyx data --- src/libs/actions/IOU.js | 60 ++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 2d471a0ca26c..10b95846efad 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -664,18 +664,16 @@ function createDistanceRequest(report, participant, comment, created, transactio } /** - * Edits an existing distance request - * * @param {String} transactionID * @param {Number} transactionThreadReportID * @param {Object} transactionChanges - * @param {String} [transactionChanges.created] - * @param {Number} [transactionChanges.amount] - * @param {Object} [transactionChanges.comment] - * @param {Object} [transactionChanges.waypoints] - * + * @param {String} [transactionChanges.created] // Present when updated the date field + * @param {Boolean} onlyIncludeChangedFieldsInAPIParams + * // When 'true', then the returned params will only include the transaction details for the fields that were changed. + * // When `false`, then the returned params will include all the transaction details, regardless of which fields were changed. + * // This setting is necessary while the UpdateDistanceRequest API is refactored to be fully 1:1:1 in https://github.com/Expensify/App/issues/28358 */ -function updateDistanceRequest(transactionID, transactionThreadReportID, transactionChanges) { +function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, onlyIncludeChangedFieldsInAPIParams) { const optimisticData = []; const successData = []; const failureData = []; @@ -695,11 +693,14 @@ function updateDistanceRequest(transactionID, transactionThreadReportID, transac const updatedTransaction = TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport); const transactionDetails = ReportUtils.getTransactionDetails(updatedTransaction); + // This needs to be a JSON string since we're sending this to the MapBox API + transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints); + + const dataToIncludeInParams = onlyIncludeChangedFieldsInAPIParams ? _.pick(transactionDetails, _.keys(transactionChanges)) : transactionDetails; + const params = { - ...transactionDetails, + ...dataToIncludeInParams, transactionID, - // This needs to be a JSON string since we're sending this to the MapBox API - waypoints: JSON.stringify(transactionDetails.waypoints), }; // Step 3: Build the modified expense report actions @@ -808,7 +809,42 @@ function updateDistanceRequest(transactionID, transactionThreadReportID, transac value: iouReport, }); - API.write('UpdateDistanceRequest', params, {optimisticData, successData, failureData}); + return { + params, + onyxData: {optimisticData, successData, failureData}, + }; +} + +/** + * Updates the created date of a money request + * + * @param {String} transactionID + * @param {Number} transactionThreadReportID + * @param {String} val + */ +function updateMoneyRequestDate(transactionID, transactionThreadReportID, val) { + const transactionChanges = { + created: val, + }; + const {params, onyxData} = getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, true); + API.write('UpdateMoneyRequestDate', params, onyxData); +} + +/** + * Edits an existing distance request + * + * @param {String} transactionID + * @param {Number} transactionThreadReportID + * @param {Object} transactionChanges + * @param {String} [transactionChanges.created] + * @param {Number} [transactionChanges.amount] + * @param {Object} [transactionChanges.comment] + * @param {Object} [transactionChanges.waypoints] + * + */ +function updateDistanceRequest(transactionID, transactionThreadReportID, transactionChanges) { + const {params, onyxData} = getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, false); + API.write('UpdateDistanceRequest', params, onyxData); } /** From 44619ea29df4b5a86c651c4ce822752d20e68c95 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 3 Nov 2023 13:23:00 -0600 Subject: [PATCH 2/8] Remove unused props and simplify callback --- src/pages/EditSplitBillPage.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/pages/EditSplitBillPage.js b/src/pages/EditSplitBillPage.js index 1342d9297d3e..e8e352469d17 100644 --- a/src/pages/EditSplitBillPage.js +++ b/src/pages/EditSplitBillPage.js @@ -63,10 +63,10 @@ function EditSplitBillPage({route, transaction, draftTransaction}) { Navigation.navigate(ROUTES.SPLIT_BILL_DETAILS.getRoute(reportID, reportActionID)); } - function setDraftSplitTransaction(transactionChanges) { + const setDraftSplitTransaction = (transactionChanges) => { IOU.setDraftSplitTransaction(transaction.transactionID, transactionChanges); navigateBackToSplitDetails(); - } + }; if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) { return ( @@ -85,13 +85,7 @@ function EditSplitBillPage({route, transaction, draftTransaction}) { return ( { - setDraftSplitTransaction({ - created: transactionChanges.created, - }); - }} + onSubmit={setDraftSplitTransaction} /> ); } From e8ba7406d4322b1e6ca555fb07d5a4213347995f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 3 Nov 2023 13:41:31 -0600 Subject: [PATCH 3/8] Call new method for updating request date --- src/libs/actions/IOU.js | 2 ++ src/pages/EditRequestPage.js | 28 +++++++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 30e8bd8324f0..9bf3c59ca0cc 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -673,6 +673,7 @@ function createDistanceRequest(report, participant, comment, created, transactio * // When 'true', then the returned params will only include the transaction details for the fields that were changed. * // When `false`, then the returned params will include all the transaction details, regardless of which fields were changed. * // This setting is necessary while the UpdateDistanceRequest API is refactored to be fully 1:1:1 in https://github.com/Expensify/App/issues/28358 + * @returns {object} */ function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, onlyIncludeChangedFieldsInAPIParams) { const optimisticData = []; @@ -3018,6 +3019,7 @@ export { setUpDistanceTransaction, navigateToNextPage, updateDistanceRequest, + updateMoneyRequestDate, replaceReceipt, detachReceipt, getIOUReportID, diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 302b7d35a1c9..e2f18ea73b70 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -1,7 +1,7 @@ import lodashGet from 'lodash/get'; import lodashValues from 'lodash/values'; import PropTypes from 'prop-types'; -import React, {useEffect, useMemo} from 'react'; +import React, {useCallback, useEffect, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; import categoryPropTypes from '@components/categoryPropTypes'; @@ -92,7 +92,7 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, const defaultCurrency = lodashGet(route, 'params.currency', '') || transactionCurrency; // Take only the YYYY-MM-DD value - const transactionCreated = TransactionUtils.getCreated(transaction); + const originalCreated = TransactionUtils.getCreated(transaction); const fieldToEdit = lodashGet(route, ['params', 'field'], ''); // For now, it always defaults to the first tag of the policy @@ -132,6 +132,19 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, Navigation.dismissModal(report.reportID); } + const saveCreated = useCallback( + ({created: newValue}) => { + // If the value hasn't changed, don't request to save changes on the server and just close the modal + if (newValue === originalCreated) { + Navigation.dismissModal(); + return; + } + IOU.updateMoneyRequestDate(transaction.transactionID, report.reportID, newValue); + Navigation.dismissModal(); + }, + [transaction, report, originalCreated], + ); + if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) { return ( { - // In case the date hasn't been changed, do not make the API request. - if (transactionChanges.created === transactionCreated) { - Navigation.dismissModal(); - return; - } - editMoneyRequest(transactionChanges); - }} + defaultCreated={originalCreated} + onSubmit={saveCreated} /> ); } From ec8857a1b27a862c7cec191e786bbce3d46ad2e8 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 9 Nov 2023 09:59:36 -0700 Subject: [PATCH 4/8] Pass the reportID to the API endpoint --- src/libs/actions/IOU.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 2390bdf6d40a..1cd8da969c98 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -706,6 +706,7 @@ function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, t const params = { ...dataToIncludeInParams, + reportID: iouReport.reportID, transactionID, }; From 92934ef377a76eb48b84fe08cbe20b1d54c0f029 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 9 Nov 2023 10:49:30 -0700 Subject: [PATCH 5/8] Only include changed fields in optimistic data --- src/libs/actions/IOU.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 1cd8da969c98..9751b342108f 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -673,13 +673,13 @@ function createDistanceRequest(report, participant, comment, created, transactio * @param {Number} transactionThreadReportID * @param {Object} transactionChanges * @param {String} [transactionChanges.created] // Present when updated the date field - * @param {Boolean} onlyIncludeChangedFieldsInAPIParams + * @param {Boolean} onlyIncludeChangedFields * // When 'true', then the returned params will only include the transaction details for the fields that were changed. * // When `false`, then the returned params will include all the transaction details, regardless of which fields were changed. * // This setting is necessary while the UpdateDistanceRequest API is refactored to be fully 1:1:1 in https://github.com/Expensify/App/issues/28358 * @returns {object} */ -function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, onlyIncludeChangedFieldsInAPIParams) { +function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, onlyIncludeChangedFields) { const optimisticData = []; const successData = []; const failureData = []; @@ -702,7 +702,7 @@ function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, t // This needs to be a JSON string since we're sending this to the MapBox API transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints); - const dataToIncludeInParams = onlyIncludeChangedFieldsInAPIParams ? _.pick(transactionDetails, _.keys(transactionChanges)) : transactionDetails; + const dataToIncludeInParams = onlyIncludeChangedFields ? _.pick(transactionDetails, _.keys(transactionChanges)) : transactionDetails; const params = { ...dataToIncludeInParams, @@ -767,11 +767,12 @@ function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, t } // Optimistically modify the transaction + const optimisticTransaction = onlyIncludeChangedFields ? _.pick(updatedTransaction, _.keys(transactionChanges)) : updatedTransaction; optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, value: { - ...updatedTransaction, + ...optimisticTransaction, pendingFields, isLoading: _.has(transactionChanges, 'waypoints'), errorFields: null, From 16161b7016996d5f85e9dd72104b460bca23470e Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 1 Dec 2023 11:06:34 -0700 Subject: [PATCH 6/8] Simplify how the created field is accessed --- src/libs/actions/IOU.js | 3 +-- src/pages/EditRequestPage.js | 13 +++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 70538fefdc93..4324714b4d98 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -2042,7 +2042,7 @@ function editRegularMoneyRequest(transactionID, transactionThreadReportID, trans */ function editMoneyRequest(transaction, transactionThreadReportID, transactionChanges) { if (TransactionUtils.isDistanceRequest(transaction)) { - editDistanceMoneyRequest(transaction.transactionID, transactionThreadReportID, transactionChanges); + updateDistanceRequest(transaction.transactionID, transactionThreadReportID, transactionChanges); } else { editRegularMoneyRequest(transaction.transactionID, transactionThreadReportID, transactionChanges); } @@ -3083,7 +3083,6 @@ export { setMoneyRequestReceipt, setUpDistanceTransaction, navigateToNextPage, - updateDistanceRequest, updateMoneyRequestDate, replaceReceipt, detachReceipt, diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 2e5b9bddc947..2bdf3d19f16a 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -85,9 +85,6 @@ function EditRequestPage({report, route, parentReport, policyCategories, policyT } = ReportUtils.getTransactionDetails(transaction); const defaultCurrency = lodashGet(route, 'params.currency', '') || transactionCurrency; - - // Take only the YYYY-MM-DD value - const originalCreated = TransactionUtils.getCreated(transaction); const fieldToEdit = lodashGet(route, ['params', 'field'], ''); // For now, it always defaults to the first tag of the policy @@ -124,16 +121,16 @@ function EditRequestPage({report, route, parentReport, policyCategories, policyT } const saveCreated = useCallback( - ({created: newValue}) => { + ({created: newCreated}) => { // If the value hasn't changed, don't request to save changes on the server and just close the modal - if (newValue === originalCreated) { + if (newCreated === TransactionUtils.getCreated(transaction)) { Navigation.dismissModal(); return; } - IOU.updateMoneyRequestDate(transaction.transactionID, report.reportID, newValue); + IOU.updateMoneyRequestDate(transaction.transactionID, report.reportID, newCreated); Navigation.dismissModal(); }, - [transaction, report, originalCreated], + [transaction, report], ); if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) { @@ -155,7 +152,7 @@ function EditRequestPage({report, route, parentReport, policyCategories, policyT if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE) { return ( ); From 35b91ca68866097c53fcc0ffaa41b969f7b3f643 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 7 Dec 2023 09:39:17 -0700 Subject: [PATCH 7/8] Remove nested comments --- src/libs/actions/IOU.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 97e38626d2e0..aaa4a325a64b 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -650,9 +650,9 @@ function createDistanceRequest(report, participant, comment, created, transactio * @param {Object} transactionChanges * @param {String} [transactionChanges.created] // Present when updated the date field * @param {Boolean} onlyIncludeChangedFields - * // When 'true', then the returned params will only include the transaction details for the fields that were changed. - * // When `false`, then the returned params will include all the transaction details, regardless of which fields were changed. - * // This setting is necessary while the UpdateDistanceRequest API is refactored to be fully 1:1:1 in https://github.com/Expensify/App/issues/28358 + * When 'true', then the returned params will only include the transaction details for the fields that were changed. + * When `false`, then the returned params will include all the transaction details, regardless of which fields were changed. + * This setting is necessary while the UpdateDistanceRequest API is refactored to be fully 1:1:1 in https://github.com/Expensify/App/issues/28358 * @returns {object} */ function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, onlyIncludeChangedFields) { From 90ada1c4c7fec7ec737c4d0e68dde05135340954 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 12 Dec 2023 09:05:32 -0700 Subject: [PATCH 8/8] Update IOU.js --- src/libs/actions/IOU.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index aaa4a325a64b..919273d3c04a 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -648,7 +648,7 @@ function createDistanceRequest(report, participant, comment, created, transactio * @param {String} transactionID * @param {Number} transactionThreadReportID * @param {Object} transactionChanges - * @param {String} [transactionChanges.created] // Present when updated the date field + * @param {String} [transactionChanges.created] Present when updated the date field * @param {Boolean} onlyIncludeChangedFields * When 'true', then the returned params will only include the transaction details for the fields that were changed. * When `false`, then the returned params will include all the transaction details, regardless of which fields were changed.