[HOLD for payment 2023-09-27] [$500] Web - Request Money – The user proceeds to next section on clicking in empty space next to container.

[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. Click FAB (Plus sign).
2. Select the "Request Money" option.
3. Enter the desired amount and click on the "Next" button.
4. Choose the user with whom the money request will be made.
5. Click on the "Request" button.
6. This action will cause an IOU container to appear in the relevant chat.
7. It can be observed that clicking on the empty space on the right side of the container also triggers the user to proceed to the next section.

Expected Result:

The user should only be able to proceed to the next section after clicking directly on the container.

Actual Result:

The user is able to proceed to the next section by clicking on the empty space next to the container.

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.62.0
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

Bug.4.mp4

Recording.4204.mp4

Expensify/Expensify Issue URL:
Issue reported by: @usmantariq96
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693281334993079

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0186657d1865495449
• Upwork Job ID: 1699022151071711232
• Last Price Increase: 2023-09-12
• Automatic offers:
• getusha | Contributor | 26635253

[melvin-bot]

Triggered auto assignment to @MitchExpensify (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]

Job added to Upwork: https://www.upwork.com/jobs/~0186657d1865495449

[melvin-bot]

Current assignee @MitchExpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[MitchExpensify]

Let's fix this in general when clicking outside of any clearly defined container (E.g. A Request)

[getusha]

Proposal

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

Clicking outside request money item navigates to next section.

What is the root cause of that problem?

In

App/src/components/ReportActionItem/MoneyRequestPreview.js

Lines 322 to 333 in b95c1dd

	return (
	<PressableWithFeedback
	onPress={props.onPreviewPressed}
	onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
	onPressOut={() => ControlSelection.unblock()}
	onLongPress={showContextMenu}
	accessibilityLabel={props.isBillSplit ? props.translate('iou.split') : props.translate('iou.cash')}
	accessibilityHint={CurrencyUtils.convertToDisplayString(requestAmount, requestCurrency)}
	>
	{childContainer}
	</PressableWithFeedback>
	);

We are wrapping childContainer with a pressable component to trigger the actions onPress, which will be applied to the whole report item (full width) including outside the container.

App/src/components/ReportActionItem/MoneyRequestPreview.js

Lines 220 to 222 in b95c1dd

	const childContainer = (
	<View>
	<OfflineWithFeedback

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

We should only wrap the following container child components to avoid the actions being triggered when clicking outside the container. we will wrap it with PressableWithFeedback component and it's properties and apply it based on the condition of props.onPreviewPressed

App/src/components/ReportActionItem/MoneyRequestPreview.js

Lines 232 to 312 in b95c1dd

	{hasReceipt && (
	<ReportActionItemImages
	images={[ReceiptUtils.getThumbnailAndImageURIs(props.transaction.receipt.source, props.transaction.filename || '')]}
	isHovered={isScanning}
	/>
	)}
	{_.isEmpty(props.transaction) ? (
	<MoneyRequestSkeletonView />
	) : (
	<View style={styles.moneyRequestPreviewBoxText}>
	<View style={[styles.flexRow]}>
	<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
	<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20]}>{getPreviewHeaderText()}</Text>
	{Boolean(getSettledMessage()) && (
	<>
	<Icon
	src={Expensicons.DotIndicator}
	width={4}
	height={4}
	additionalStyles={[styles.mr1, styles.ml1]}
	/>
	<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20]}>{getSettledMessage()}</Text>
	</>
	)}
	</View>
	{hasFieldErrors && (
	<Icon
	src={Expensicons.DotIndicator}
	fill={colors.red}
	/>
	)}
	</View>
	<View style={[styles.flexRow]}>
	<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
	<Text style={styles.textHeadline}>{getDisplayAmountText()}</Text>
	{ReportUtils.isSettled(props.iouReport.reportID) && !props.isBillSplit && (
	<View style={styles.defaultCheckmarkWrapper}>
	<Icon
	src={Expensicons.Checkmark}
	fill={themeColors.iconSuccessFill}
	/>
	</View>
	)}
	</View>
	{props.isBillSplit && (
	<View style={styles.moneyRequestPreviewBoxAvatar}>
	<MultipleAvatars
	icons={participantAvatars}
	shouldStackHorizontally
	size="small"
	isHovered={props.isHovered}
	shouldUseCardBackground
	/>
	</View>
	)}
	</View>
	{shouldShowMerchant && (
	<View style={[styles.flexRow]}>
	<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20, styles.breakWord]}>{requestMerchant}</Text>
	</View>
	)}
	<View style={[styles.flexRow]}>
	<View style={[styles.flex1]}>
	{!isCurrentUserManager && props.shouldShowPendingConversionMessage && (
	<Text style={[styles.textLabel, styles.colorMuted, styles.mt1]}>{props.translate('iou.pendingConversionMessage')}</Text>
	)}
	{shouldShowDescription && <Text style={[styles.mt1, styles.colorMuted]}>{description}</Text>}
	</View>
	{props.isBillSplit && !_.isEmpty(participantAccountIDs) && (
	<Text style={[styles.textLabel, styles.colorMuted, styles.ml1]}>
	{props.translate('iou.amountEach', {
	amount: CurrencyUtils.convertToDisplayString(
	IOUUtils.calculateAmount(isPolicyExpenseChat ? 1 : participantAccountIDs.length - 1, requestAmount, requestCurrency),
	requestCurrency,
	),
	})}
	</Text>
	)}
	</View>
	</View>
	)}

Solution 2:

Let's fix this in general when clicking outside of any clearly defined container (E.g. A Request)

In order to fix this issue in general we should apply the same styling to the pressable as the container which makes sure the pressable to not go out of the container boundaries.

1. Apply styles.moneyRequestPreviewBox to PressableWithFeedback here

App/src/components/ReportActionItem/MoneyRequestPreview.js

Lines 322 to 333 in b95c1dd

	return (
	<PressableWithFeedback
	onPress={props.onPreviewPressed}
	onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
	onPressOut={() => ControlSelection.unblock()}
	onLongPress={showContextMenu}
	accessibilityLabel={props.isBillSplit ? props.translate('iou.split') : props.translate('iou.cash')}
	accessibilityHint={CurrencyUtils.convertToDisplayString(requestAmount, requestCurrency)}
	>
	{childContainer}
	</PressableWithFeedback>
	);

2. Apply styles.reportPreviewBox to PressableWithFeedback here

App/src/components/ReportActionItem/ReportPreview.js

Line 180 in 3fe7ab1

style={[styles.flexRow, styles.justifyContentBetween]}

Result

Screen.Recording.2023-09-05.at.3.08.56.PM.mov

### What alternative solutions did you explore? (Optional)

[eVoloshchak]

I think we can proceed with @getusha's proposal, the fix is straignt-forward and tests well

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

📣 @eVoloshchak Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

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

📣 @usmantariq96 We're missing your Upwork ID to automatically send you an offer for the Reporter role.
Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

[grgia]

Assigned, thank you!

[getusha]

@eVoloshchak based on this comment i think we also have to fix it for any defined container, which makes us apply the fix on ReportPreview too, it will work fine on MoneyRequestPreview, but noticed weird bug on iOS native where the contents inside the container won't appear. so i updated my proposal which applies for all defined containers (Solution 2). what do you think?

[getusha]

@eVoloshchak @grgia raised the PR!

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @getusha got assigned: 2023-09-12 16:48:37 Z
• when the PR got merged: 2023-09-18 03:39:58 UTC
• days elapsed: 3

On to the next one 🚀

[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.

[getusha]

@eVoloshchak @grgia i raise a PR fixing the above regression #27684

[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.71-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix report preview from being clickable on empty space #27465

If no regressions arise, payment will be issued on 2023-09-27. 🎊

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:

• @eVoloshchak does not require payment (Eligible for Manual Requests)
• @getusha requires payment offer (Contributor)
• @usmantariq96 requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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:

• [@eVoloshchak] The PR that introduced the bug has been identified. Link to the PR:
• [@eVoloshchak] 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:
• [@eVoloshchak] 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:
• [@eVoloshchak] Determine if we should create a regression test for this bug.
• [@eVoloshchak] 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.
• [@MitchExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[MitchExpensify]

Set a reminder to pay on 09/27. Based on this regression, I believe this to be the payment summary but let me know if I'm mistaken:

Payment summary:

Reporter: $250 Upwork @usmantariq96
C: $250 Upwork @getusha
C+: $250 NewDot @eVoloshchak

[eVoloshchak]

@MitchExpensify, wasn't this originally $500? In that case, regression would knock it down to $250

[MitchExpensify]

Ah yes! The fairly recent change in pricing caught me. Thanks for the assist @eVoloshchak

[getusha]

If a PR causes a regression at any point within the regression period (starting when the code is merged and ending 168 hours (that's 7 days) after being deployed to production):

@MitchExpensify Based on the guidelines will the penalty apply to regressions before being deployed to production?

[MitchExpensify]

Yes it appears so based on the Guidelines you shared

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: I'm unable to pinpoint the exact PR that caused this. Looks like that's how it was implemented initially in Make tapping the entire IOU preview component open the IOU Details modal #3541, but the PR is over 2 years old, I can't verify that
• 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: N/A
• 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: It's a fairly simple bug, I don't think an additional discussion is needed
• Determine if we should create a regression test for this bug.
  Is it easy to test for this bug? Yes
  Is the bug related to an important user flow? No
  Is it an impactful bug? No
  The bug has a very low impact, doesn't prevent user from using the app, I don't think a regression test is needed here

[MitchExpensify]

Payment summary:

Reporter: $250 Upwork @usmantariq96 - Please accept the Upwork off I just sent, thank you!
C: $250 Upwork @getusha - PAID
C+: $250 NewDot @eVoloshchak

[usmantariq96]

@MitchExpensify offer accepted
Thanks

[MitchExpensify]

Paid and contract ended! Closing out as @eVoloshchak will receive payment via NewDot money request

[JmillsExpensify]

$250 payment for @eVoloshchak approved based on BZ summary.