Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new UpdateMoneyRequestDate API endpoint #30737

Merged
merged 13 commits into from
Dec 13, 2023
68 changes: 54 additions & 14 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -645,18 +645,17 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/* // This one too */

* @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 editDistanceMoneyRequest(transactionID, transactionThreadReportID, transactionChanges) {
function getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, onlyIncludeChangedFields) {
const optimisticData = [];
const successData = [];
const failureData = [];
Expand All @@ -676,11 +675,15 @@ function editDistanceMoneyRequest(transactionID, transactionThreadReportID, tran
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 = onlyIncludeChangedFields ? _.pick(transactionDetails, _.keys(transactionChanges)) : transactionDetails;

const params = {
...transactionDetails,
...dataToIncludeInParams,
reportID: iouReport.reportID,
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
Expand Down Expand Up @@ -740,12 +743,13 @@ function editDistanceMoneyRequest(transactionID, transactionThreadReportID, tran
}

// Optimistically modify the transaction
const optimisticTransaction = onlyIncludeChangedFields ? _.pick(updatedTransaction, _.keys(transactionChanges)) : updatedTransaction;
optimisticData.push({
// We need to use SET method to save updated waypoint instead MERGE method to avoid wrong update of waypoints. More detail: https://github.com/Expensify/App/issues/30290#issuecomment-1778957070
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
...updatedTransaction,
...optimisticTransaction,
pendingFields,
isLoading: _.has(transactionChanges, 'waypoints'),
errorFields: null,
Expand Down Expand Up @@ -790,7 +794,42 @@ function editDistanceMoneyRequest(transactionID, transactionThreadReportID, tran
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);
}

/**
Expand Down Expand Up @@ -1971,7 +2010,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);
}
Expand Down Expand Up @@ -3012,6 +3051,7 @@ export {
setMoneyRequestReceipt,
setUpDistanceTransaction,
navigateToNextPage,
updateMoneyRequestDate,
replaceReceipt,
detachReceipt,
getIOUReportID,
Expand Down
29 changes: 16 additions & 13 deletions src/pages/EditRequestPage.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 transactionCreated = TransactionUtils.getCreated(transaction);
const fieldToEdit = lodashGet(route, ['params', 'field'], '');

// For now, it always defaults to the first tag of the policy
Expand Down Expand Up @@ -123,6 +120,19 @@ function EditRequestPage({report, route, parentReport, policyCategories, policyT
Navigation.dismissModal(report.reportID);
}

const saveCreated = useCallback(
({created: newCreated}) => {
// If the value hasn't changed, don't request to save changes on the server and just close the modal
if (newCreated === TransactionUtils.getCreated(transaction)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming: we're fine with identity comparison here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it should be fine. Thanks for confirming!

Navigation.dismissModal();
return;
}
IOU.updateMoneyRequestDate(transaction.transactionID, report.reportID, newCreated);
Navigation.dismissModal();
},
[transaction, report],
);

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) {
return (
<EditRequestDescriptionPage
Expand All @@ -142,15 +152,8 @@ function EditRequestPage({report, route, parentReport, policyCategories, policyT
if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE) {
return (
<EditRequestCreatedPage
defaultCreated={transactionCreated}
onSubmit={(transactionChanges) => {
// In case the date hasn't been changed, do not make the API request.
if (transactionChanges.created === transactionCreated) {
Navigation.dismissModal();
return;
}
editMoneyRequest(transactionChanges);
}}
defaultCreated={TransactionUtils.getCreated(transaction)}
onSubmit={saveCreated}
/>
);
}
Expand Down
12 changes: 3 additions & 9 deletions src/pages/EditSplitBillPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ function EditSplitBillPage({route, transaction, draftTransaction, report}) {
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 (
Expand All @@ -93,13 +93,7 @@ function EditSplitBillPage({route, transaction, draftTransaction, report}) {
return (
<EditRequestCreatedPage
defaultCreated={transactionCreated}
defaultAmount={transactionAmount}
reportID={reportID}
onSubmit={(transactionChanges) => {
setDraftSplitTransaction({
created: transactionChanges.created,
});
}}
onSubmit={setDraftSplitTransaction}
/>
);
}
Expand Down