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

Improve creating expenses on Collect with Instant Submit #36388

Merged
merged 27 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9bc5b38
Allow requesting money from processing reports in Collect with Instan…
youssef-lr Feb 12, 2024
34917e5
Add new expenses to processing reports
youssef-lr Feb 13, 2024
99d1542
Set optimisitc report as submitted if instant submit is turned on
youssef-lr Feb 13, 2024
4fc82e7
Do not create a new report preview if we have a processing report alr…
youssef-lr Feb 13, 2024
f165755
Merge branch 'main' into youssef_instant_submit_collect
youssef-lr Feb 17, 2024
771eef6
Merge branch 'main' into youssef_instant_submit_collect
youssef-lr Feb 22, 2024
4eb5cbf
Add support for splitBill
youssef-lr Feb 23, 2024
9587d19
Add help method for deciding if we can add transactions to a report
youssef-lr Feb 28, 2024
a2675fe
Fix conflicts and mini refactor on how we decide to create new reports
youssef-lr Feb 29, 2024
72ddbc8
Add comment in function definition
youssef-lr Feb 29, 2024
f5a5959
Clean up code in MoneyRequestHeader
youssef-lr Feb 29, 2024
5be0b3f
DRY up code in shouldBuildOptimisticMoneyRequestReport
youssef-lr Feb 29, 2024
206cd78
Lint
youssef-lr Feb 29, 2024
f0a99eb
Fix test
youssef-lr Feb 29, 2024
933e0d8
Fix test for real now
youssef-lr Feb 29, 2024
902f45c
Apply suggestions from code review
youssef-lr Feb 29, 2024
9cccb7e
Merge branch 'main' into youssef_instant_submit_collect
youssef-lr Feb 29, 2024
267de9d
Update tests
youssef-lr Mar 1, 2024
4e9ccc9
Fix test
youssef-lr Mar 1, 2024
c0371b2
Rename variables
youssef-lr Mar 3, 2024
9eb9b97
Merge branch 'main' into youssef_instant_submit_collect
youssef-lr Mar 3, 2024
8496f5d
Fix calls of functions I missed renaming
youssef-lr Mar 3, 2024
ee17782
Apply suggestions from code review
youssef-lr Mar 7, 2024
6a464b3
Fix bug, use isMoneyRequestReport
youssef-lr Mar 7, 2024
1777c68
Cleanup
youssef-lr Mar 7, 2024
9d9caf6
Rename variable
youssef-lr Mar 7, 2024
3fcd191
Fix wrong helper method used
youssef-lr Mar 7, 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
2 changes: 1 addition & 1 deletion src/components/MoneyRequestHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function MoneyRequestHeader({session, parentReport, report, parentReportAction,
const canHoldOrUnholdRequest = !isSettled && !isApproved && !isDeletedParentAction;

// If the report supports adding transactions to it, then it also supports deleting transactions from it.
const canDeleteRequest = isActionOwner && ReportUtils.canAddTransactionsToMoneyRequest(moneyRequestReport) && !isDeletedParentAction;
const canDeleteRequest = isActionOwner && ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) && !isDeletedParentAction;

const changeMoneyRequestStatus = () => {
if (isOnHold) {
Expand Down
22 changes: 10 additions & 12 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,16 +1255,16 @@ function getChildReportNotificationPreference(reportAction: OnyxEntry<ReportActi
* - report is a draft
* - report is a processing expense report and its policy has Instant reporting frequency
*/
function canAddTransactionsToMoneyRequest(report: OnyxEntry<Report>): boolean {
if (!isIOUReport(report) && !isExpenseReport(report)) {
function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry<Report>): boolean {
if (!isMoneyRequest(moneyRequestReport)) {
Copy link
Contributor

@rlinoz rlinoz Mar 7, 2024

Choose a reason for hiding this comment

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

isMoneyRequest only check for IOU reports, and I think this is making the button not show up inside the expense report (step 7 of the tests).

I think you will need to update this to use isMoneyRequestReport as well

Suggested change
if (!isMoneyRequest(moneyRequestReport)) {
if (!isMoneyRequest(moneyRequestReport) && !isMoneyRequestReport(moneyRequestReport)) {

Copy link
Contributor

Choose a reason for hiding this comment

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

This might fix the failing tests as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

woops just a mistake, will update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if (!isMoneyRequest(moneyRequestReport) && !isMoneyRequestReport(moneyRequestReport))

It should only be isMoneyRequestReport here, because we can't add transactions to a transaction :D

return false;
}

if (isReportApproved(report) || isSettled(report?.reportID)) {
if (isReportApproved(moneyRequestReport) || isSettled(moneyRequestReport?.reportID)) {
return false;
}

if (isGroupPolicy(report) && isProcessingReport(report) && !PolicyUtils.isInstantSubmitEnabled(getPolicy(report?.policyID))) {
if (isGroupPolicy(moneyRequestReport) && isProcessingReport(moneyRequestReport) && !PolicyUtils.isInstantSubmitEnabled(getPolicy(moneyRequestReport?.policyID))) {
return false;
}

Expand All @@ -1285,14 +1285,13 @@ function canDeleteReportAction(reportAction: OnyxEntry<ReportAction>, reportID:
// For now, users cannot delete split actions
const isSplitAction = reportAction?.originalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT;

if (isSplitAction || isSettled(String(reportAction?.originalMessage?.IOUReportID)) || (!isEmptyObject(report) && isReportApproved(report))) {
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 removed this logic as it's already handled in canAddOrDeleteTransactions

if (isSplitAction) {
return false;
}

if (isActionOwner) {
if (!isEmptyObject(report) && isPaidGroupPolicyExpenseReport(report)) {
// If the report supports adding transactions to it, then it also supports deleting transactions from it.
return canAddTransactionsToMoneyRequest(report);
if (!isEmptyObject(report) && isMoneyRequestReport(report)) {
return canAddOrDeleteTransactions(report);
}
return true;
}
Expand Down Expand Up @@ -2974,7 +2973,6 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa

const isInstantSubmitEnabled = PolicyUtils.isInstantSubmitEnabled(policy);

// Define the state and status of the report based on whether the policy is free or paid
const stateNum = isInstantSubmitEnabled ? CONST.REPORT.STATE_NUM.SUBMITTED : CONST.REPORT.STATE_NUM.OPEN;
const statusNum = isInstantSubmitEnabled ? CONST.REPORT.STATUS_NUM.SUBMITTED : CONST.REPORT.STATUS_NUM.OPEN;

Expand Down Expand Up @@ -4315,7 +4313,7 @@ function canRequestMoney(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>, o
// User can request money in any IOU report, unless paid, but user can only request money in an expense report
// which is tied to their workspace chat.
if (isMoneyRequestReport(report)) {
const canAddTransactions = canAddTransactionsToMoneyRequest(report);
const canAddTransactions = canAddOrDeleteTransactions(report);
return isGroupPolicy(report) ? isOwnPolicyExpenseChat && canAddTransactions : canAddTransactions;
}

Expand Down Expand Up @@ -5093,7 +5091,7 @@ function canBeAutoReimbursed(report: OnyxEntry<Report>, policy: OnyxEntry<Policy
- we have one but we can't add more transactions to it due to: report is approved or settled, or report is processing and policy isn't on Instant submit reporting frequency
*/
function shouldCreateNewMoneyRequestReport(existingIOUReport: OnyxEntry<Report> | undefined | null, chatReport: OnyxEntry<Report> | null): boolean {
return !existingIOUReport || hasIOUWaitingOnCurrentUserBankAccount(chatReport) || !canAddTransactionsToMoneyRequest(existingIOUReport);
return !existingIOUReport || hasIOUWaitingOnCurrentUserBankAccount(chatReport) || !canAddOrDeleteTransactions(existingIOUReport);
}

export {
Expand Down Expand Up @@ -5298,7 +5296,7 @@ export {
canEditRoomVisibility,
canEditPolicyDescription,
getPolicyDescriptionText,
canAddTransactionsToMoneyRequest,
canAddOrDeleteTransactions,
shouldCreateNewMoneyRequestReport,
};

Expand Down
Loading