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

Add an action method for creating a distance request #24385

Merged
merged 21 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/libs/TransactionUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,23 @@ Onyx.connect({
* @param {String} [originalTransactionID]
* @param {String} [merchant]
* @param {Object} [receipt]
* @param {String} [existingTransactionID] When creating a distance request, an empty transaction has already been created with a transactionID. In that case, the transaction here needs to have it's transactionID match what was already generated.
* @returns {Object}
*/
function buildOptimisticTransaction(amount, currency, reportID, comment = '', source = '', originalTransactionID = '', merchant = CONST.REPORT.TYPE.IOU, receipt = {}) {
function buildOptimisticTransaction(
amount,
currency,
reportID,
comment = '',
source = '',
originalTransactionID = '',
merchant = CONST.REPORT.TYPE.IOU,
receipt = {},
existingTransactionID = null,
) {
// transactionIDs are random, positive, 64-bit numeric strings.
// Because JS can only handle 53-bit numbers, transactionIDs are strings in the front-end (just like reportActionID)
const transactionID = NumberUtils.rand64();
const transactionID = existingTransactionID || NumberUtils.rand64();

const commentJSON = {comment};
if (source) {
Expand Down
164 changes: 133 additions & 31 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ function buildOnyxDataForMoneyRequest(
) {
const optimisticData = [
{
// Use SET for new reports because it doesn't exist yet, is faster and we need the data to be available when we navigate to the chat page
onyxMethod: isNewChatReport ? Onyx.METHOD.SET : Onyx.METHOD.MERGE,
onyxMethod: Onyx.METHOD.MERGE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we removing this condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are being changed because:

  1. Set and merge shouldn't be mixed. It will lead to undefined behavior of the order in which subscribers are updated (ie. you end up seeing a little flash of incorrect data as amounts go from $0 to whatever was set in the request).
  2. Using set just because "it's faster" is not a reason to use set

I checked with @luacmartins who originally made the change and we agreed it was OK to use merge for everything for now.

key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
value: {
...chatReport,
Expand All @@ -112,7 +111,7 @@ function buildOnyxDataForMoneyRequest(
},
},
{
onyxMethod: isNewIOUReport ? Onyx.METHOD.SET : Onyx.METHOD.MERGE,
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`,
value: {
...iouReport,
Expand All @@ -122,20 +121,20 @@ function buildOnyxDataForMoneyRequest(
},
},
{
onyxMethod: Onyx.METHOD.SET,
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`,
value: transaction,
},
{
onyxMethod: isNewChatReport ? Onyx.METHOD.SET : Onyx.METHOD.MERGE,
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
...(isNewChatReport ? {[chatCreatedAction.reportActionID]: chatCreatedAction} : {}),
[reportPreviewAction.reportActionID]: reportPreviewAction,
},
},
{
onyxMethod: isNewIOUReport ? Onyx.METHOD.SET : Onyx.METHOD.MERGE,
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport.reportID}`,
value: {
...(isNewIOUReport ? {[iouCreatedAction.reportActionID]: iouCreatedAction} : {}),
Expand Down Expand Up @@ -302,19 +301,33 @@ function buildOnyxDataForMoneyRequest(
}

/**
* Request money from another user
* Gathers all the data needed to make a money request. It attempts to find existing reports, iouReports, and receipts. If it doesn't find them, then
* it creates optimistic versions of them and uses those instead
*
* @param {Object} report
* @param {Number} amount - always in the smallest unit of the currency
* @param {String} currency
* @param {String} payeeEmail
* @param {Number} payeeAccountID
* @param {Object} participant
* @param {String} comment
* @param {Number} amount
* @param {String} currency
* @param {Number} payeeAccountID
* @param {String} payeeEmail
* @param {Object} [receipt]
*
* @returns {Object} data
* @returns {String} data.payerEmail
* @returns {Object} data.iouReport
* @returns {Object} data.chatReport
* @returns {Object} data.transaction
* @returns {Object} data.iouAction
* @returns {Object} data.createdChatReportActionID
* @returns {Object} data.createdIOUReportActionID
* @returns {Object} data.reportPreviewAction
* @returns {Object} data.onyxData
* @returns {Object} data.onyxData.optimisticData
* @returns {Object} data.onyxData.successData
* @returns {Object} data.onyxData.failureData
* @param {String} [existingTransactionID]
*/
function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, participant, comment, receipt = undefined) {
function getMoneyRequestInformation(report, participant, comment, amount, currency, payeeAccountID, payeeEmail, receipt = undefined, existingTransactionID = null) {
const payerEmail = OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login);
const payerAccountID = Number(participant.accountID);
const isPolicyExpenseChat = participant.isPolicyExpenseChat;
Expand Down Expand Up @@ -364,7 +377,7 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
receiptObject.source = receipt.source;
receiptObject.state = CONST.IOU.RECEIPT_STATE.SCANREADY;
}
const optimisticTransaction = TransactionUtils.buildOptimisticTransaction(
const transaction = TransactionUtils.buildOptimisticTransaction(
ReportUtils.isExpenseReport(iouReport) ? -amount : amount,
currency,
iouReport.reportID,
Expand All @@ -373,6 +386,7 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
'',
undefined,
receiptObject,
existingTransactionID,
);

// STEP 4: Build optimistic reportActions. We need:
Expand All @@ -383,18 +397,25 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
// Note: The CREATED action for the IOU report must be optimistically generated before the IOU action so there's no chance that it appears after the IOU action in the chat
const optimisticCreatedActionForChat = ReportUtils.buildOptimisticCreatedReportAction(payeeEmail);
const optimisticCreatedActionForIOU = ReportUtils.buildOptimisticCreatedReportAction(payeeEmail);
const optimisticIOUAction = ReportUtils.buildOptimisticIOUReportAction(
const iouAction = ReportUtils.buildOptimisticIOUReportAction(
CONST.IOU.REPORT_ACTION_TYPE.CREATE,
amount,
currency,
comment,
[participant],
optimisticTransaction.transactionID,
transaction.transactionID,
'',
iouReport.reportID,
receiptObject,
);

let reportPreviewAction = isNewIOUReport ? null : ReportActionsUtils.getReportPreviewAction(chatReport.reportID, iouReport.reportID);
if (reportPreviewAction) {
reportPreviewAction = ReportUtils.updateReportPreview(iouReport, reportPreviewAction, comment);
} else {
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment);
}

// Add optimistic personal details for participant
const optimisticPersonalDetailListAction = isNewChatReport
? {
Expand All @@ -407,28 +428,108 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
}
: undefined;

let reportPreviewAction = isNewIOUReport ? null : ReportActionsUtils.getReportPreviewAction(chatReport.reportID, iouReport.reportID);
if (reportPreviewAction) {
reportPreviewAction = ReportUtils.updateReportPreview(iouReport, reportPreviewAction, comment);
} else {
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment);
}

// STEP 5: Build Onyx Data
const [optimisticData, successData, failureData] = buildOnyxDataForMoneyRequest(
chatReport,
iouReport,
optimisticTransaction,
transaction,
optimisticCreatedActionForChat,
optimisticCreatedActionForIOU,
optimisticIOUAction,
iouAction,
optimisticPersonalDetailListAction,
reportPreviewAction,
isNewChatReport,
isNewIOUReport,
);

// STEP 6: Make the request
return {
payerEmail,
iouReport,
chatReport,
transaction,
iouAction,
createdChatReportActionID: isNewChatReport ? optimisticCreatedActionForChat.reportActionID : 0,
createdIOUReportActionID: isNewIOUReport ? optimisticCreatedActionForIOU.reportActionID : 0,
reportPreviewAction,
onyxData: {
optimisticData,
successData,
failureData,
},
};
}

/**
* Requests money based on a distance (eg. mileage from a map)
*
* @param {Object} report
* @param {String} payeeEmail
* @param {Number} payeeAccountID
* @param {Object} participant
* @param {String} comment
* @param {Object[]} waypoints
* @param {String} waypoints[].address required and must be non empty
* @param {String} [waypoints[].lat] optional
* @param {String} [waypoints[].lng] optional
* @param {String} created
* @param {String} [transactionID]
*/
function createDistanceRequest(report, payeeEmail, payeeAccountID, participant, comment, waypoints, created, transactionID) {
const {payerEmail, iouReport, chatReport, transaction, iouAction, createdChatReportActionID, createdIOUReportActionID, reportPreviewAction, onyxData} = getMoneyRequestInformation(
report,
participant,
comment,
0,
'USD',
payeeAccountID,
payeeEmail,
null,
Comment on lines +496 to +500
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused #26518. We shouldn't have passed in static values like 0 in here if optimistic creation of these requests was supported.

transactionID,
);

API.write(
'CreateDistanceRequest',
{
debtorEmail: payerEmail,
comment,
iouReportID: iouReport.reportID,
chatReportID: chatReport.reportID,
transactionID: transaction.transactionID,
reportActionID: iouAction.reportActionID,
createdChatReportActionID,
createdIOUReportActionID,
reportPreviewReportActionID: reportPreviewAction.reportActionID,
waypoints,
created,
},
onyxData,
);
}

/**
* Request money from another user
*
* @param {Object} report
* @param {Number} amount - always in the smallest unit of the currency
* @param {String} currency
* @param {String} payeeEmail
* @param {Number} payeeAccountID
* @param {Object} participant
* @param {String} comment
* @param {Object} [receipt]
*/
function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, participant, comment, receipt = undefined) {
const {payerEmail, iouReport, chatReport, transaction, iouAction, createdChatReportActionID, createdIOUReportActionID, reportPreviewAction, onyxData} = getMoneyRequestInformation(
report,
participant,
comment,
amount,
currency,
payeeAccountID,
payeeEmail,
receipt,
);

API.write(
'RequestMoney',
{
Expand All @@ -438,14 +539,14 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
comment,
iouReportID: iouReport.reportID,
chatReportID: chatReport.reportID,
transactionID: optimisticTransaction.transactionID,
reportActionID: optimisticIOUAction.reportActionID,
createdChatReportActionID: isNewChatReport ? optimisticCreatedActionForChat.reportActionID : 0,
createdIOUReportActionID: isNewIOUReport ? optimisticCreatedActionForIOU.reportActionID : 0,
transactionID: transaction.transactionID,
reportActionID: iouAction.reportActionID,
createdChatReportActionID,
createdIOUReportActionID,
reportPreviewReportActionID: reportPreviewAction.reportActionID,
receipt,
},
{optimisticData, successData, failureData},
onyxData,
);
resetMoneyRequestInfo();
Navigation.dismissModal(chatReport.reportID);
Expand Down Expand Up @@ -1622,6 +1723,7 @@ function navigateToNextPage(iou, iouType, reportID, report) {
}

export {
createDistanceRequest,
editMoneyRequest,
deleteMoneyRequest,
splitBill,
Expand Down
Loading