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.

Sign up for GitHub

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

Jump to bottom

Refactor IOU.requestMoney function #52231

Merged
merged 12 commits into from
Nov 22, 2024
Merged

Refactor IOU.requestMoney function #52231

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1fd0aae
refactor requestMoney function
mkzie2 Nov 8, 2024
91caa85
revert change
mkzie2 Nov 8, 2024
6fc82cf
fix test
mkzie2 Nov 8, 2024
6f46d31
Merge branch 'main' into mkzie2-issue/50368
mkzie2 Nov 11, 2024
e1fbc35
merge main
mkzie2 Nov 20, 2024
9bd88a2
resolve conflict
mkzie2 Nov 20, 2024
b5ad2c1
remove change
mkzie2 Nov 20, 2024
04bcb99
Merge branch 'main' into mkzie2-issue/50368
mkzie2 Nov 21, 2024
8d99484
merge param into one param
mkzie2 Nov 21, 2024
4e737b3
Merge branch 'main' into mkzie2-issue/50368
mkzie2 Nov 22, 2024
d0918fe
rename param
mkzie2 Nov 22, 2024
e08c53a
rename transactionData
mkzie2 Nov 22, 2024
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
78 changes: 51 additions & 27 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,37 @@ type GPSPoint = {
long: number;
};

type RequestMoneyInformation = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you group many params into 2 objects: RequestMoneyInformation, RequestMoneyTransactionData? (Why not 1 or 3...?)

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 split transaction data into one group and others into another one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @mkzie2,

I understand your idea that RequestMoneyTransactionData should contain the params same as Onyx Transaction. For other params we can store in RequestMoneyInformation

Copy link
Contributor

Choose a reason for hiding this comment

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

I like splitting things up, but I envisioned it slightly different. How about a RequestMoneyParams type which contains all fields from RequestMoneyInformation, then within it we have transactionData: RequestMoneyTransactionData?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm leaning towards a Params suffix vs Data 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.

@neil-marcellini So we will only have one param and in this param we will have some sub param is a object like transactionData, policyData,... 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.

@neil-marcellini Please help to take a look at my comment above when you have a chance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@neil-marcellini Bump on the above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that's right

report: OnyxEntry<OnyxTypes.Report>;
payeeEmail: string | undefined;
payeeAccountID: number;
participant: Participant;
mkzie2 marked this conversation as resolved.
Show resolved Hide resolved
policy?: OnyxEntry<OnyxTypes.Policy>;
policyTagList?: OnyxEntry<OnyxTypes.PolicyTagLists>;
policyCategories?: OnyxEntry<OnyxTypes.PolicyCategories>;
mkzie2 marked this conversation as resolved.
Show resolved Hide resolved
gpsPoints?: GPSPoint;
action?: IOUAction;
reimbursible?: boolean;
};

type RequestMoneyTransactionData = {
attendees: Attendee[] | undefined;
amount: number;
currency: string;
comment?: string;
receipt?: Receipt;
category?: string;
tag?: string;
taxCode?: string;
taxAmount?: number;
billable?: boolean;
merchant: string;
created: string;
actionableWhisperReportActionID?: string;
linkedTrackedExpenseReportAction?: OnyxTypes.ReportAction;
linkedTrackedExpenseReportID?: string;
};

let allPersonalDetails: OnyxTypes.PersonalDetailsList = {};
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
Expand Down Expand Up @@ -3566,33 +3597,26 @@ function shareTrackedExpense(
/**
* Submit expense to another user
*/
function requestMoney(
report: OnyxEntry<OnyxTypes.Report>,
amount: number,
attendees: Attendee[] | undefined,
currency: string,
created: string,
merchant: string,
payeeEmail: string | undefined,
payeeAccountID: number,
participant: Participant,
comment: string,
receipt: Receipt | undefined,
category?: string,
tag?: string,
taxCode = '',
taxAmount = 0,
billable?: boolean,
policy?: OnyxEntry<OnyxTypes.Policy>,
policyTagList?: OnyxEntry<OnyxTypes.PolicyTagLists>,
policyCategories?: OnyxEntry<OnyxTypes.PolicyCategories>,
gpsPoints?: GPSPoint,
action?: IOUAction,
actionableWhisperReportActionID?: string,
linkedTrackedExpenseReportAction?: OnyxTypes.ReportAction,
linkedTrackedExpenseReportID?: string,
reimbursible?: boolean,
) {
function requestMoney(requestMoneyInformation: RequestMoneyInformation, requestMoneyTransactionData: RequestMoneyTransactionData) {
const {report, payeeEmail, payeeAccountID, participant, policy, policyTagList, policyCategories, gpsPoints, action, reimbursible} = requestMoneyInformation;
const {
amount,
currency,
merchant,
comment = '',
receipt,
category,
tag,
taxCode = '',
taxAmount = 0,
billable,
created,
attendees,
actionableWhisperReportActionID,
linkedTrackedExpenseReportAction,
linkedTrackedExpenseReportID,
} = requestMoneyTransactionData;

// If the report is iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function
const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
const currentChatReport = isMoneyRequestReport ? ReportUtils.getReportOrDraftReport(report?.chatReportID) : report;
Expand Down
24 changes: 13 additions & 11 deletions src/pages/iou/request/step/IOURequestStepAmount.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,17 +211,19 @@ function IOURequestStepAmount({
if (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.REQUEST) {
playSound(SOUNDS.DONE);
IOU.requestMoney(
report,
backendAmount,
transaction?.attendees,
currency,
transaction?.created ?? '',
CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
participants.at(0) ?? {},
'',
{},
{
report,
participant: participants.at(0) ?? {},
payeeEmail: currentUserPersonalDetails.login,
payeeAccountID: currentUserPersonalDetails.accountID,
},
{
amount: backendAmount,
currency,
created: transaction?.created ?? '',
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
attendees: transaction?.attendees,
},
);
return;
}
Expand Down
53 changes: 28 additions & 25 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,32 +238,35 @@ function IOURequestStepConfirmation({
if (!participant) {
return;
}

IOU.requestMoney(
report,
transaction.amount,
transaction.attendees,
transaction.currency,
transaction.created,
transaction.merchant,
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
participant,
trimmedComment,
receiptObj,
transaction.category,
transaction.tag,
transactionTaxCode,
transactionTaxAmount,
transaction.billable,
policy,
policyTags,
policyCategories,
gpsPoints,
action,
transaction.actionableWhisperReportActionID,
transaction.linkedTrackedExpenseReportAction,
transaction.linkedTrackedExpenseReportID,
{
report,
payeeEmail: currentUserPersonalDetails.login,
payeeAccountID: currentUserPersonalDetails.accountID,
participant,
policy,
policyTagList: policyTags,
policyCategories,
gpsPoints,
action,
},
{
amount: transaction.amount,
attendees: transaction.attendees,
currency: transaction.currency,
created: transaction.created,
merchant: transaction.merchant,
comment: trimmedComment,
receipt: receiptObj,
category: transaction.category,
tag: transaction.tag,
taxCode: transactionTaxCode,
taxAmount: transactionTaxAmount,
billable: transaction.billable,
actionableWhisperReportActionID: transaction.actionableWhisperReportActionID,
linkedTrackedExpenseReportAction: transaction.linkedTrackedExpenseReportAction,
linkedTrackedExpenseReportID: transaction.linkedTrackedExpenseReportID,
},
);
},
[report, transaction, transactionTaxCode, transactionTaxAmount, currentUserPersonalDetails.login, currentUserPersonalDetails.accountID, policy, policyTags, policyCategories, action],
Expand Down
64 changes: 32 additions & 32 deletions src/pages/iou/request/step/IOURequestStepScan/index.native.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,17 +256,20 @@ function IOURequestStepScan({
);
} else {
IOU.requestMoney(
report,
0,
transaction?.attendees,
transaction?.currency ?? 'USD',
transaction?.created ?? '',
'',
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
participant,
'',
receipt,
{
report,
payeeEmail: currentUserPersonalDetails.login,
payeeAccountID: currentUserPersonalDetails.accountID,
participant,
},
{
amount: 0,
attendees: transaction?.attendees,
currency: transaction?.currency ?? 'USD',
created: transaction?.created ?? '',
merchant: '',
receipt,
},
);
}
},
Expand Down Expand Up @@ -353,28 +356,25 @@ function IOURequestStepScan({
);
} else {
IOU.requestMoney(
report,
0,
transaction?.attendees,
transaction?.currency ?? 'USD',
transaction?.created ?? '',
'',
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
participant,
'',
receipt,
'',
'',
'',
0,
false,
policy,
{},
{},
{
lat: successData.coords.latitude,
long: successData.coords.longitude,
report,
payeeEmail: currentUserPersonalDetails.login,
payeeAccountID: currentUserPersonalDetails.accountID,
participant,
policy,
gpsPoints: {
lat: successData.coords.latitude,
long: successData.coords.longitude,
},
},
{
amount: 0,
attendees: transaction?.attendees,
currency: transaction?.currency ?? 'USD',
created: transaction?.created ?? '',
merchant: '',
receipt,
billable: false,
},
);
}
Expand Down
64 changes: 32 additions & 32 deletions src/pages/iou/request/step/IOURequestStepScan/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,17 +285,20 @@ function IOURequestStepScan({
);
} else {
IOU.requestMoney(
report,
0,
transaction?.attendees,
transaction?.currency ?? 'USD',
transaction?.created ?? '',
'',
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
participant,
'',
receipt,
{
report,
payeeEmail: currentUserPersonalDetails.login,
payeeAccountID: currentUserPersonalDetails.accountID,
participant,
},
{
amount: 0,
attendees: transaction?.attendees,
currency: transaction?.currency ?? 'USD',
created: transaction?.created ?? '',
merchant: '',
receipt,
},
);
}
},
Expand Down Expand Up @@ -383,28 +386,25 @@ function IOURequestStepScan({
);
} else {
IOU.requestMoney(
report,
0,
transaction?.attendees,
transaction?.currency ?? 'USD',
transaction?.created ?? '',
'',
currentUserPersonalDetails.login,
currentUserPersonalDetails.accountID,
participant,
'',
receipt,
'',
'',
'',
0,
false,
policy,
{},
{},
{
lat: successData.coords.latitude,
long: successData.coords.longitude,
report,
payeeEmail: currentUserPersonalDetails.login,
payeeAccountID: currentUserPersonalDetails.accountID,
participant,
policy,
gpsPoints: {
lat: successData.coords.latitude,
long: successData.coords.longitude,
},
},
{
amount: 0,
attendees: transaction?.attendees,
currency: transaction?.currency ?? 'USD',
created: transaction?.created ?? '',
merchant: '',
receipt,
billable: false,
},
);
}
Expand Down
Loading
Loading