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

Refactor payIOU #12739

Merged
merged 46 commits into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
24e9948
Add SendMoneyElsewhere request
youssef-lr Nov 15, 2022
913ffbc
Update logic in sendMoneyElsewhere
youssef-lr Nov 16, 2022
2ad8370
Cleanup
youssef-lr Nov 16, 2022
d3311f4
Use the new SendMoneyElseWhere command from IOUModal
youssef-lr Nov 16, 2022
2b31e3f
Fix param type
youssef-lr Nov 16, 2022
5374b91
Allow sending money while offline
youssef-lr Nov 17, 2022
3f4a973
Wrap IOUAction in OfflineWithFeedback
youssef-lr Nov 18, 2022
2bac5e7
Reverse wrapping IOUAction in OfflineWithFeedback
youssef-lr Nov 21, 2022
1928a62
Add PayPalMe flow for sending money
youssef-lr Nov 21, 2022
a3ffc0d
Create separate method for PayPal flow
youssef-lr Nov 21, 2022
2c48c72
Add PayMoneyRequest command
youssef-lr Nov 22, 2022
6e72edc
Improve comment
youssef-lr Nov 22, 2022
f6d293a
Add JSDocs to the new methods
youssef-lr Nov 22, 2022
2776c3a
Add clarifying comment
youssef-lr Nov 22, 2022
1c3a97b
Merge branch 'main' into youssef_refactor_payiou
youssef-lr Nov 23, 2022
3784707
Merge branch 'main' into youssef_refactor_payiou
youssef-lr Nov 24, 2022
4f16c9e
Use a seperate function for each payment method
youssef-lr Nov 25, 2022
8bbd68c
Add generic error messaged to failed payIOU request
youssef-lr Nov 28, 2022
53bff3f
Allow errored sendMoney report actions to be deleted
youssef-lr Nov 29, 2022
ae4a358
Fix style
youssef-lr Nov 29, 2022
927b86b
Merge branch 'main' into youssef_refactor_payiou
youssef-lr Nov 29, 2022
781f867
Update generic error message
youssef-lr Nov 29, 2022
b27c956
Fix navigation to report after settling up
youssef-lr Nov 30, 2022
dfcfb07
Fix settlment button visible after settling up offline
youssef-lr Nov 30, 2022
36d4382
Make code more readable
youssef-lr Nov 30, 2022
0880140
Use correct chatReportID for navigation
youssef-lr Nov 30, 2022
6b483c6
Add navigation to PayPal.me in PayMoneyRequest
youssef-lr Nov 30, 2022
6313845
Renaming
youssef-lr Dec 1, 2022
cc3df11
Fix typo
youssef-lr Dec 1, 2022
8e425f1
Delete deprecated methods
youssef-lr Dec 1, 2022
e318467
Merge branch 'main' into youssef_refactor_payiou
youssef-lr Dec 1, 2022
ddbe821
Use appropriate error message
youssef-lr Dec 2, 2022
05bfe3a
Cleanup
youssef-lr Dec 2, 2022
04af650
Apply requested changes
youssef-lr Dec 2, 2022
8272d1d
Fix typo
youssef-lr Dec 2, 2022
b457ad0
Deprecate REPORT_IOUS Onyx key
youssef-lr Dec 2, 2022
39d2884
Fix IOUBadge changing when we send money
youssef-lr Dec 2, 2022
2de2def
Keep deprecated payWithWallet as it should be removed in a different PR
youssef-lr Dec 2, 2022
281d32b
Update report's lastActionCreated in IOU flows
youssef-lr Dec 2, 2022
7bba84b
Fix typo
youssef-lr Dec 2, 2022
4becf7f
Fix optimistic IOU payment messages
youssef-lr Dec 3, 2022
01aa33d
Fix method argument
youssef-lr Dec 7, 2022
3ec3625
Small improvement
youssef-lr Dec 7, 2022
1b15023
Remove unneeded code
youssef-lr Dec 7, 2022
e7d78ba
Fix IOU Preview still showing after failed pay request
youssef-lr Dec 8, 2022
eedccb1
Use SET in sendMoney to save the iouReport
youssef-lr Dec 8, 2022
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
2 changes: 1 addition & 1 deletion src/components/SettlementButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class SettlementButton extends React.Component {
>
{triggerKYCFlow => (
<ButtonWithMenu
isDisabled={this.props.isDisabled || this.props.network.isOffline}
isDisabled={this.props.isDisabled}
isLoading={this.props.isLoading}
onPress={(event, iouPaymentType) => {
if (iouPaymentType === CONST.IOU.PAYMENT_TYPE.EXPENSIFY) {
Expand Down
9 changes: 6 additions & 3 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -669,25 +669,28 @@ function buildOptimisticReportAction(sequenceNumber, text, file) {
* @param {String} chatReportID - Report ID of the chat where the IOU is.
* @param {String} currency - IOU currency.
* @param {String} locale - Locale where the IOU is created
* @param {Boolean} isSendingMoney - If we send money the IOU should be created as settled
*
* @returns {Object}
*/
function buildOptimisticIOUReport(ownerEmail, userEmail, total, chatReportID, currency, locale) {
function buildOptimisticIOUReport(ownerEmail, userEmail, total, chatReportID, currency, locale, isSendingMoney = false) {
const formattedTotal = NumberFormatUtils.format(locale,
total, {
style: 'currency',
currency,
});

// stateNum 2: report is settled (submitted)
return {
cachedTotal: formattedTotal,
chatReportID,
currency,
hasOutstandingIOU: true,
hasOutstandingIOU: !isSendingMoney,
mountiny marked this conversation as resolved.
Show resolved Hide resolved
managerEmail: userEmail,
ownerEmail,
reportID: generateReportID(),
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: 1,
stateNum: isSendingMoney ? 2 : 1,
mountiny marked this conversation as resolved.
Show resolved Hide resolved
total,
};
}
Expand Down
307 changes: 307 additions & 0 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Onyx from 'react-native-onyx';
import _ from 'underscore';
import lodashGet from 'lodash/get';
import Str from 'expensify-common/lib/str';
import CONST from '../../CONST';
import ONYXKEYS from '../../ONYXKEYS';
import ROUTES from '../../ROUTES';
Expand Down Expand Up @@ -722,11 +723,317 @@ function payIOUReport({
return promiseWithHandlers;
}

/**
* @param {Object} report
* @param {Number} amount
* @param {String} currency
* @param {String} comment
* @param {String} paymentMethodType
* @param {String} managerEmail - Email of the person sending the money
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
* @param {Object} recipient - The user receiving the money
* @returns {Object}
*/
function getSendMoneyParams(report, amount, currency, comment, paymentMethodType, managerEmail, recipient) {
mountiny marked this conversation as resolved.
Show resolved Hide resolved
const newIOUReportDetails = JSON.stringify({
amount,
currency,
requestorEmail: recipient.login,
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
comment,
idempotencyKey: Str.guid(),
});

const recipientEmail = OptionsListUtils.addSMSDomainIfPhoneNumber(recipient.login);
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
let chatReport = lodashGet(report, 'reportID', null) ? report : null;
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
let isNewChat = false;
if (!chatReport) {
chatReport = ReportUtils.getChatByParticipants([recipientEmail]);
sketchydroide marked this conversation as resolved.
Show resolved Hide resolved
}
if (!chatReport) {
chatReport = ReportUtils.buildOptimisticChatReport([recipientEmail]);
isNewChat = true;
}
const optimisticIOUReport = ReportUtils.buildOptimisticIOUReport(recipientEmail, managerEmail, amount, chatReport.reportID, currency, preferredLocale, true);
const newSequenceNumber = Report.getMaxSequenceNumber(chatReport.reportID) + 1;
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved

const optimisticPaidReportAction = ReportUtils.buildOptimisticIOUReportAction(
newSequenceNumber,
CONST.IOU.REPORT_ACTION_TYPE.PAY,
amount,
currency,
comment,
[recipient],
CONST.IOU.PAYMENT_TYPE.ELSEWHERE,
mountiny marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

@youssef-lr This should be paymentMethodType, shouldnt it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct.

'',
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the problem is that we generate the iouReportID before for the iouReport, but what we could still do is to add here the new generatated IOUTransactionID instead of passing in empty string. Then if it is type PAY and has transactionID defined it is sending money.

Just unsure if we should keep adding more optional parameters here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me give this a try.

optimisticIOUReport.reportID,
);

// First, add data that will be used in all cases
const optimisticData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
value: {
...chatReport,
lastVisitedTimestamp: Date.now(),
lastReadSequenceNumber: newSequenceNumber,
maxSequenceNumber: newSequenceNumber,
lastMessageText: optimisticPaidReportAction.message[0].text,
lastMessageHtml: optimisticPaidReportAction.message[0].html,
hasOutstandingIOU: false,
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
iouReportID: optimisticIOUReport.reportID,
},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticPaidReportAction.sequenceNumber]: {
...optimisticPaidReportAction,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
},
},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
key: `${ONYXKEYS.COLLECTION.REPORT_IOUS}${optimisticIOUReport.reportID}`,
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
value: optimisticIOUReport,
},
];

const successData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticPaidReportAction.sequenceNumber]: {
pendingAction: null,
},
},
},
];

const failureData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticPaidReportAction.sequenceNumber]: {
...optimisticPaidReportAction,
pendingAction: null,
},
},
},
];

// Now, let's add the data we need just when we are creating a new chat report
if (isNewChat) {
const optimisticCreateAction = ReportUtils.buildOptimisticCreatedReportAction(recipientEmail);

// Change the method to 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
optimisticData[0].onyxMethod = CONST.ONYX.METHOD.SET;
optimisticData[0].value = {
...optimisticData[0].value,
pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD},
};
optimisticData[1].onyxMethod = CONST.ONYX.METHOD.SET;
optimisticData[1].value = {
...optimisticCreateAction,
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
...optimisticData[1].value,
};
optimisticData[2].onyxMethod = CONST.ONYX.METHOD.SET;
}

return {
params: {
iouReportID: optimisticIOUReport.reportID,
chatReportID: chatReport.reportID,
paidReportActionID: optimisticPaidReportAction.reportActionID,
paymentMethodType,
transactionID: optimisticPaidReportAction.originalMessage.IOUTransactionID,
clientID: optimisticPaidReportAction.sequenceNumber,
newIOUReportDetails,
},
optimisticData,
successData,
failureData,
};
}

/**
* @param {Object} chatReport
* @param {Object} iouReport
* @param {Object} recipient
* @param {String} paymentMethodType
* @returns {Object}
*/
function getPayMoneyRequestParams(chatReport, iouReport, recipient, paymentMethodType) {
const newSequenceNumber = Report.getMaxSequenceNumber(chatReport.reportID) + 1;

const optimisticPaidReportAction = ReportUtils.buildOptimisticIOUReportAction(
newSequenceNumber,
CONST.IOU.REPORT_ACTION_TYPE.PAY,
iouReport.total,
iouReport.currency,
'',
[recipient],
paymentMethodType,
'',
iouReport.reportID,
);

// First, add data that will be used in all cases
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
const optimisticData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
value: {
...chatReport,
lastVisitedTimestamp: Date.now(),
lastReadSequenceNumber: newSequenceNumber,
maxSequenceNumber: newSequenceNumber,
lastMessageText: optimisticPaidReportAction.message[0].text,
lastMessageHtml: optimisticPaidReportAction.message[0].html,
hasOutstandingIOU: false,
iouReportID: iouReport.reportID,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that since hasOutstandingIOU = false, we should also clear iouReportID since that report is closed and we need to create a new iouReport when we send the the next IOU.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets also maybe merge this PR into this branch https://github.com/Expensify/App/pull/12759/files, we are waiting there for Web E deploy to production, but it could be handy here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only change left, what's the decision here?

Copy link
Contributor Author

@youssef-lr youssef-lr Dec 7, 2022

Choose a reason for hiding this comment

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

@mountiny I can see that PR just got merged, but I'm not sure I need that logic in SendMoney and payMoneyRequest.

For SendMoney, we are always creating an IOUReport and then settling up. For payMoneyRequest, we always know we have an IOUReport to settle. What do you think?

@luacmartins clearing the iouReportID makes sense to me as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

but I'm not sure I need that logic in SendMoney and payMoneyRequest.

I agree that that logic doesn't seem necessary here.

clearing the iouReportID makes sense to me as well.

Yea, let's clear the iouReportID

},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticPaidReportAction.sequenceNumber]: {
...optimisticPaidReportAction,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
},
},
},
{
onyxMethod: CONST.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.

@youssef-lr same here, we have paid this up and we included all the data about the iou report in here so we can use set can we not?

Suggested change
onyxMethod: CONST.ONYX.METHOD.MERGE,
onyxMethod: CONST.ONYX.METHOD.SET,

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should use merge if the IOUReport is already stored in Onyx. The difference between this and sendMoney is that sendMoney creates an IOUReport that doesn't yet exist in Onyx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, here we already have an iouReport that we're settling up.

key: `${ONYXKEYS.COLLECTION.REPORT_IOUS}${iouReport.reportID}`,
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
value: iouReport,
},
];

const successData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticPaidReportAction.sequenceNumber]: {
pendingAction: null,
},
},
},
];

const failureData = [
{
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticPaidReportAction.sequenceNumber]: {
...optimisticPaidReportAction,
pendingAction: null,
},
},
},
];

return {
params: {
iouReportID: iouReport.reportID,
paidReportActionID: optimisticPaidReportAction.reportActionID,
paymentMethodType,
clientID: optimisticPaidReportAction.sequenceNumber,
},
optimisticData,
successData,
failureData,
};
}

/**
* @param {Object} report
* @param {Number} amount
* @param {String} currency
* @param {String} comment
* @param {String} managerEmail - Email of the person sending the money
* @param {Object} recipient - The user receiving the money
*/
function sendMoneyElsewhere(report, amount, currency, comment, managerEmail, recipient) {
const {
params, optimisticData, successData, failureData,
} = getSendMoneyParams(report, amount, currency, comment, CONST.IOU.PAYMENT_TYPE.ELSEWHERE, managerEmail, recipient);

API.write('SendMoneyElsewhere', params, {optimisticData, successData, failureData});

Navigation.navigate(ROUTES.getReportRoute(params.chatReportID));
}

/**
* @param {Object} report
* @param {Number} amount
* @param {String} currency
* @param {String} comment
* @param {String} managerEmail - Email of the person sending the money
* @param {Object} recipient - The user receiving the money
*/
function sendMoneyViaPaypal(report, amount, currency, comment, managerEmail, recipient) {
const {
params, optimisticData, successData, failureData,
} = getSendMoneyParams(report, amount, currency, comment, CONST.IOU.PAYMENT_TYPE.PAYPAL_ME, managerEmail, recipient);

API.write('SendMoneyViaPaypal', params, {optimisticData, successData, failureData});

Navigation.navigate(ROUTES.getReportRoute(params.chatReportID));

asyncOpenURL(Promise.resolve(), buildPayPalPaymentUrl(amount, recipient.payPalMeAddress, currency));
}

/**
* @param {Object} chatReport
* @param {Object} iouReport
* @param {Object} recipient
*/
function payMoneyRequestElsewhere(chatReport, iouReport, recipient) {
const {
params, optimisticData, successData, failureData,
} = getPayMoneyRequestParams(
chatReport, iouReport, recipient, CONST.IOU.PAYMENT_TYPE.ELSEWHERE,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
} = getPayMoneyRequestParams(
chatReport, iouReport, recipient, CONST.IOU.PAYMENT_TYPE.ELSEWHERE,
);
} = getPayMoneyRequestParams(chatReport, iouReport, recipient, CONST.IOU.PAYMENT_TYPE.ELSEWHERE);


API.write('PayMoneyRequestElsewhere', params, {optimisticData, successData, failureData});

Navigation.navigate(ROUTES.getReportRoute(params.chatReportID));
}

/**
* @param {Object} chatReport
* @param {Object} iouReport
* @param {Object} recipient
*/
function payMoneyRequestViaPaypal(chatReport, iouReport, recipient) {
const {
params, optimisticData, successData, failureData,
} = getPayMoneyRequestParams(
chatReport, iouReport, recipient, CONST.IOU.PAYMENT_TYPE.PAYPAL_ME,
);
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved

API.write('PayMoneyRequestViaPaypal', params, {optimisticData, successData, failureData});

Navigation.navigate(ROUTES.getReportRoute(params.chatReportID));
}

export {
cancelMoneyRequest,
splitBill,
splitBillAndOpenReport,
requestMoney,
payIOUReport,
youssef-lr marked this conversation as resolved.
Show resolved Hide resolved
sendMoneyElsewhere,
sendMoneyViaPaypal,
payMoneyRequestElsewhere,
payMoneyRequestViaPaypal,
setIOUSelectedCurrency,
};
14 changes: 14 additions & 0 deletions src/libs/actions/ReportActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,21 @@ function clearReportActionErrors(reportID, sequenceNumber) {
});
}

/**
* This method clears the errors for a chat where send money action was done
* @param {String} chatReportID
* @param {String} reportActionID
*/
function clearSendMoneyErrors(chatReportID, reportActionID) {
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}`, {
[reportActionID]: {
errors: null,
},
});
}

export {
clearReportActionErrors,
deleteOptimisticReportAction,
clearSendMoneyErrors,
};
Loading