[HOLD for payment 2023-11-07] [$500] Scan - App displays 'Split' in title on back from scan request money request

[kbecciv]

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

---

Action Performed:

1. Open the app
2. Click on plus and click request money->scan
3. Select any file
4. Observe now it displays 'Manual' in header, select any user
5. Click on back and observe that now it displays 'Split' in header even though there is no option to add other user to split

Expected Result:

App should not display 'Split' in header when coming back from scan request as we don't allow adding user for split in scan request

Actual Result:

App displays 'Split' in header when coming back from scan request even though we don't allow adding user for split in scan request

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.77.2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

ios.native.split.in.title.on.back.mov

android.chrome.split.in.title.on.back.mp4

ios.safari.split.in.title.on.back.mov

mac.desktop.split.in.title.on.back.mov

mac.chrome.split.in.title.on.back.mov

windows.chrome.split.in.header.issue.mp4

Recording.4840.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696334987842689

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011c86077fd238d8d9
• Upwork Job ID: 1709312814973755392
• Last Price Increase: 2023-10-10
• Automatic offers:
• fedirjh | Reviewer | 27263280
• DylanDylann | Contributor | 27263283
• dhanashree-sawant | Reporter | 27263284

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[neonbhai]

Proposal

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

Header incorrectly shows Split for receipt requests

What is the root cause of that problem?

We don't check if the request is receipt while setting the header here:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Line 63 in 81a6a52

setHeaderTitle(_.isEmpty(iou.participants) ? translate('tabSelector.manual') : translate('iou.split'));

We are missing check for Scan Request while setting Header.

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

Add condition to keep the Header title as Manual if isScanRequest is true.

And only change the title to Split when isSplitRequest is true.

Header.Manual.webm

What alternative solutions did you explore? (Optional)

Alternatively, we can change the Scan flow to reflect that this is a Scan request.

To achieve this, in this useEffect:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Line 57 in 81a6a52

useEffect(() => {

We will set the header title to 'Scan' if the isScanRequest is true

This will achieve the original goal of reflecting the request type (Tab Name) on the header:

Scan.Request.Header.webm

[ZhenjaHorbach]

Proposal

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

App displays 'Split' in title on back from scan request money request to request money participants page

What is the root cause of that problem?

The text depends on the selected participant or not
When selected we have split
When non selected we have manual

In this case, when we select a participant, we update the condition that indicates that we need to use split and here it does not take into account what kind of request
Scan or not scan

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

We can add the new condition for title

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 57 to 64 in 81a6a52

	useEffect(() => {
	if (isDistanceRequest) {
	setHeaderTitle(translate('common.distance'));
	return;
	}
	setHeaderTitle(_.isEmpty(iou.participants) ? translate('tabSelector.manual') : translate('iou.split'));
	}, [iou.participants, isDistanceRequest, translate]);

Like

        if (isScanRequest) {
            setHeaderTitle(translate('tabSelector.manual'));
            return;
        }

Screen.Recording.2023-10-03.at.23.21.04.mov

What alternative solutions did you explore? (Optional)

NA

[vadymbokatov]

Proposal

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

title of page changes from manual to split on back from scan request money

What is the root cause of that problem?

the root cause of the problem is this condition:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 57 to 64 in 81a6a52

	useEffect(() => {
	if (isDistanceRequest) {
	setHeaderTitle(translate('common.distance'));
	return;
	}
	setHeaderTitle(_.isEmpty(iou.participants) ? translate('tabSelector.manual') : translate('iou.split'));
	}, [iou.participants, isDistanceRequest, translate]);

on back from scan request money page the iou.participants. array is no longer empty so the condition is false and the title changes.

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

the setHeader condition should change to
setHeaderTitle(!_.isEmpty(iou.participants) && isSplitRequest ?translate('iou.split') :translate('tabSelector.manual') );

[DylanDylann]

Proposal

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

1. App displays 'Split' in title on back from scan request money
2. App displays 'Split' in title on back from manual request money

What is the root cause of that problem?

Let's see the logic of header message

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 57 to 63 in 7f063c7

	useEffect(() => {
	if (isDistanceRequest) {
	setHeaderTitle(translate('common.distance'));
	return;
	}
	setHeaderTitle(_.isEmpty(iou.participants) ? translate('tabSelector.manual') : translate('iou.split'));

In manual request and scan request, we are using _.isEmpty(iou.participants) as a condition for header message. It is incorrect. Because in manual request and Scan request, when clicking any option iou.participants also be updated

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

1. In Scan Request: There are no options to split the bill, the header title should be fixed

if (isScanRequest) {
            setHeaderTitle(translate('tabSelector.scan'));     
            return;
}

We also do the same thing on Confirmation Page (MoneyRequestConfirmPage)

2. In Manual request
   We need to display 'Split' in 2 cases as confirmed here

Because in both manual request and split request we both update iou.participants, we should not use iou.participants as condition to display split or manual

I suggest we should create a new state called selectedParticipant. We only update this state if the user adds new option to the selections list (not update if the user selects the option). Then we will use this state instead of iou.participants

Result

Screen.Recording.2023-10-05.at.12.38.51.mp4

[DylanDylann]

@shawnborton We are displaying "Manual" in the header message of Scan Request. Is it expected?
Note that:
The header message is Distance in Distance Request
The header message is Manual in Manual Request
The header message is Manual in Scan Request --> my question here

Screen-Recording-2023-10-04-at-14.19.33.mp4

[shawnborton]

Hmmm @JmillsExpensify @trjExpensify thoughts on that one?

[trjExpensify]

The header message is Manual in Scan Request --> my question here

I think this should be Scan, not Manual.

[trjExpensify]

Then wrt to when Split is in the header:

• on the participants selector screen when you click Split on any of the rows to enter that multi-select flow
• on the split confirmation page thereafter

[DylanDylann]

UPdated proposal

[trjExpensify]

Hey @DylanDylann in your vid can you make sure to go through to the confirmation page in the Scan flow to ensure it's correct there as well?

Also on this:

In Scan Request: There are no options to split the bill, the header title should be fixed

There will be an option to split the bill in scan receipts real soon, so let's not assume that isn't on the way. CC: @youssef-lr

[DylanDylann]

@trjExpensify

Hey @DylanDylann in your vid can you make sure to go through to the confirmation page in the Scan flow to ensure it's correct there as well?

Yeah, updated video

Screen.Recording.2023-10-05.at.20.47.30.1.mp4

[trjExpensify]

Huh, it might be because it's late, but why is it now blank in that vid? It should read Scan?

[trjExpensify]

@fedirjh can you review these proposals please? Thanks!

[fedirjh]

@trjExpensify Should we cover both the manual and scan in this issue?

For the manual request, not only does the header change to Split, but also the confirm button is updated:

CleanShot.2023-10-06.at.21.41.56.mp4

[neonbhai]

This behavior is a flaw in the request flow.

Reason:

The Split header on Participants page is set with this line:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Line 63 in c0fe505

setHeaderTitle(_.isEmpty(iou.participants) ? translate('tabSelector.manual') : translate('iou.split'));

This means when Participants are selected and we are still on the Participants page, the page assumes we are going to split.

The way this is designed, if the user comes back, looks like they want to add participants, which triggers split flow.

• To enter manual flow, user has to directly click on the participant.
• To enter split flow, user has to stay on the page with a participant selected (by choosing the Split button (optionSelectorButton))

Image

The Participant options selector only shows confirmButton if there is at least 1 participant selected. As the component is now assuming we are going to the split flow. (The confirm button only allows to split)

The manual and split will have to be redesigned, since this assumption will get in the way.

Split showing on Confirmation page is a symptom of this problem, we should probably not hack it with another participants list.

The problem is we cannot start a manual request from the participants page in a 1 participant selected state.

How to solve this?

• Redesign the flow, to allow user to request with 1 participant + split with 1 participant when we come back. I suggest a dropdown button menu allowing to choose between split and manual request
• Another solution is when going back from Confirmation with 1 participant, we can remove the participant. This will allow the user to again directly click on the target recipient, without adding to split. This lets them go to the Manual Confirm page, with the right heading (aligning the app with the user's intention)

[trjExpensify]

Hm, interesting. So basically, in the instance of one person being tapped for a 1:1 request, when coming back to the participants page we remember the selection, and because a participant is selected, it's recognised as being in the split flow because that's the flow where you select participants. I also agree the Add to split green button is super confusing here when navigating back.

01EwtCZf16.mp4

when going back from Confirmation with 1 participant, we can remove the participant.

I think I agree we should remove the participant in this case. I can understand not in the split case, because you might be going back to add someone else you forgot to the split.

CC: @JmillsExpensify @shawnborton @dannymcclain here's another weird byproduct of this combined request/split participant selector thang. Curious for your take!

[melvin-bot]

📣 @fedirjh 🎉 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]

📣 @DylanDylann 🎉 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 📖

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[trjExpensify]

Wahoo! Let's get the PR going :)

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.93-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Update request money header title #29936

If no regressions arise, payment will be issued on 2023-11-07. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment offer (Reviewer)
• @DylanDylann requires payment offer (Contributor)
• @dhanashree-sawant requires payment offer (Reporter)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[fedirjh]

BugZero Checklist:

• Link to the PR: Simplify Global Create menu #25564
• Link to comment: Simplify Global Create menu #25564 (comment)
• Link to discussion: No need to add anything to the checklist. The PR is very complicated, so it's possible that these bugs were not noticed.
• Determine if we should create a regression test for this bug: Yes

Regression Test Proposal

Scan request:

1. Open the app
2. Click on plus and click request money -> scan
3. Select any file
4. Verify it displays Scan in the header.
5. Select any user, you will be taken to a confirmation page.
6. Click on back to return to the participant selector page.
7. Verify that it displays Scan in the header

Manual/Split request:

1. Open the app
2. Click on plus and click request money -> manual
3. Enter amount and press next
4. Verify it displays Manual in the header.
5. Select any user, you will be taken to a confirmation page.
6. Click on back to return to the participant selector page.
7. Verify that it displays Manual in the header
8. Click the split button
9. Verify that the header message is changed to Split

• Do we agree 👍 or 👎

[melvin-bot]

@amyevans, @trjExpensify, @fedirjh, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@amyevans, @trjExpensify, @fedirjh, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick!

[trjExpensify]

Confirming payments as follows:

$500 to @DylanDylann for the fix
$500 to @fedirjh for C+
$50 to @dhanashree-sawant for the bug report

PR took more than 3 days, so #urgency bonus doesn't apply.

[trjExpensify]

Thanks for those, @fedirjh! Created an issue for Applause.

[trjExpensify]

Settled up with you all. Thanks!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.