[PAY 7/9] [$250] Expense - Unable to click Mark as cash button in expense report

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: v9.0.21-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp https://expensify.testrail.io/index.php?/tests/view/4865202
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com

2. Log in to [email protected]

3. Go to workspace chat.

4. Submit two same scan receipts that will match with card transaction.

5. Go to expense report after the scanning for both expenses is complete.

6. Click on Mark as cash button in expense report (not transaction thread).

Expected Result:

User should be able to click on Mark as cash button in the expense report.

Actual Result:

Nothing happens when clicking on Mark as cash button in the expense report.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6573957_1723828838949.20240817_011503.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011e2e93352cb1e755
• Upwork Job ID: 1825845317524560506
• Last Price Increase: 2024-08-20
• Automatic offers:
• mkhutornyi | Reviewer | 103636257
• nkdengineer | Contributor | 103636259

Issue Owner

Current Issue Owner: @mkhutornyi

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

We think that this bug might be related to #wave-collect - Release 2

[IuliiaHerets]

@bfitzexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-08-16 20:01:24 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Unable to click Mark as cash button in expense report

What is the root cause of that problem?

We are early retuning in markAsCash if requestParentReportAction doesn't exist

App/src/components/MoneyReportHeader.tsx

Lines 183 to 186 in d4d5a25

	const markAsCash = useCallback(() => {
	if (!requestParentReportAction) {
	return;
	}

and in this case there two transactions in the report and transactionThreadReportID is undefined MoneyReportHeader

What changes do you think we should make in order to solve the problem?

We should not display the button when transactionThreadReportID doesn't exist because as the mark as cash only is applicable when there is only one transaction (transactionThreadReportID)

App/src/components/MoneyReportHeader.tsx

Lines 320 to 322 in d4d5a25

	{allHavePendingRTERViolation && !shouldUseNarrowLayout && (
	<View style={[styles.pv2]}>
	<Button

App/src/components/MoneyReportHeader.tsx

Lines 368 to 370 in d4d5a25

	)}
	{allHavePendingRTERViolation && shouldUseNarrowLayout && (
	<Button

    const canMarkAsCash = allHavePendingRTERViolation && !!transactionThreadReportID

or

    const canMarkAsCash = allHavePendingRTERViolation && !!requestParentReportAction

and replace all allHavePendingRTERViolation instances with canMarkAsCash as allHavePendingRTERViolation is now being used to identify if the button is displayed in MoneyReportHeader.

What alternative solutions did you explore? (Optional)

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Nothing happens when clicking on Mark as cash button in the expense report.

What is the root cause of that problem?

We do nothing if requestParentReportAction doesn't exist

App/src/components/MoneyReportHeader.tsx

Lines 184 to 186 in 62722e8

	const markAsCash = useCallback(() => {
	if (!requestParentReportAction) {
	return;

and it only exists when the report is the combined report

App/src/components/MoneyReportHeader.tsx

Lines 67 to 69 in 62722e8

	const requestParentReportAction = useMemo(() => {
	if (!reportActions || !transactionThreadReport?.parentReportActionID) {
	return null;

What changes do you think we should make in order to solve the problem?

We should not show the mask as cash button if the report is not the combine report.

As the way we should this button in MoneyRequestHeader here, we should update allHavePendingRTERViolation condition here to only check the transaction here has pending RTER pending violation or not since this is the transaction of combine report and only exists if the report is the combine report.

const allHavePendingRTERViolation = TransactionUtils.allHavePendingRTERViolation([transaction?.transactionID ?? '-1']);

OPTIONAL: we can replace allHavePendingRTERViolation with hasAllPendingRTERViolations

App/src/components/MoneyReportHeader.tsx

Line 110 in 62722e8

const allHavePendingRTERViolation = TransactionUtils.allHavePendingRTERViolation(transactionIDs);

With this, we also reduce the violation of all transactions that aren't necessary since we only want to display this in the combine report

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

@bfitzexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011e2e93352cb1e755

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mkhutornyi (External)

[mkhutornyi]

I think @FitseTLT's solution is enough to fix this bug.

[nkdengineer]

@mkhutornyi I think we should only pass the transaction that is the transaction of the combine report to allHavePendingRTERViolation. Let's imagine a case in which we have many transactions in the expense report, allHavePendingRTERViolation will be called with many transactions if we do this solution. It can take many times and it's unnecessary because in this case we don't need to do that.

[mkhutornyi]

@nkdengineer ok, so your proposed solution is more like optimization beyond the scope of this issue, isn't it?

[nkdengineer]

@nkdengineer ok, so your proposed solution is more like optimization beyond the scope of this issue, isn't it?

@mkhutornyi It's not beyond the scope of this issue, it's the simplest solution and optimization. It is also consistent with the way we show the mark as cash button in MoneyRequestHeader here

[mkhutornyi]

@nkdengineer's proposal looks good to me.
🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Julesssss, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @mkhutornyi 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[trjExpensify]

@bfitzexpensify just noting this is a #wave-control feature, so moving it over to that project.

(It also hit prod 3 days btw).

[bfitzexpensify]

Payments complete. Closing this out.