[$250] Expense - Payment sound triggered prematurely when submitting a held expense.

[lanitochka17]

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: 9.0.46-5
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Create a workspace
3. Navigate to the workspace chat
4. Submit an expense
5. Right-click on the submitted expense and select hold
6. Click the green "Pay" button
7. Observe that the payment sound is triggered and an additional "Confirm payment amount" modal appears

Expected Result:

For this case the sound indicating a payment should be triggered after the payment is successfully completed. If the expense is on hold, the sound should be delayed until the confirmation is made and finalized

Actual Result:

The payment sound is triggered when the green pay button is clicked, even though the payment is not yet finalized

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6630541_1728553595243.soundd.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021846013804908725875
• Upwork Job ID: 1846013804908725875
• Last Price Increase: 2024-10-22
• Automatic offers:
• mkzie2 | Contributor | 104560763

Issue Owner

Current Issue Owner: @abdulrahuman5196

[melvin-bot]

Triggered auto assignment to @RachCHopkins (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.

[mkzie2]

Edited by proposal-police: This proposal was edited at 2023-10-10T10:00:00.

Proposal

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

The payment sound is triggered when the green pay button is clicked, even though the payment is not yet finalized

What is the root cause of that problem?

We're play sound when after selecting payment type in SettlementButton regardless of the issue is on hold or not:

App/src/components/SettlementButton/index.tsx

Line 210 in 420e780

playSound(SOUNDS.SUCCESS);

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

We should not play sound in SettlementButton when the expense is on hold but play it after confirming in the hold modal instead.

1. Introduce shouldPlaySound in SettlementButton to determine whether to play sound or not
2. In ReportPreview and MoneyReportHeader, pass shouldPlaySound={isAnyTransactionOnHold}, or hasHeldExpenses:

App/src/components/MoneyReportHeader.tsx

Line 146 in 5b38516

const isAnyTransactionOnHold = ReportUtils.hasHeldExpenses(moneyRequestReport?.reportID);

App/src/components/ReportActionItem/ReportPreview.tsx

Lines 204 to 205 in 5b38516

	} else if (ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
	setIsHoldMenuVisible(true);

3. Play the sound when confirming the hold modal if we PAY the expense:

App/src/components/ProcessMoneyReportHoldMenu.tsx

Line 69 in c0d26c5

IOU.payMoneyRequest(paymentType, chatReport, moneyRequestReport, full);

[etCoderDysto]

Proposal

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

Payment sound triggered prematurely when submitting a held expense.

What is the root cause of that problem?

When user clicks pay button while request is on hold we call playSound(SOUNDS.SUCCESS); before calling confirmPayment (onPress(iouPaymentType);)

App/src/components/SettlementButton/index.tsx

Lines 210 to 211 in 517ffaa

	playSound(SOUNDS.SUCCESS);
	onPress(iouPaymentType);

Here since request is onhold, instead of calling IOU.payMoneyRequest, hold modal will be opened here

App/src/components/ReportActionItem/ReportPreview.tsx

Lines 199 to 217 in 517ffaa

	const confirmPayment = useCallback(
	(type: PaymentMethodType | undefined, payAsBusiness?: boolean) => {
	if (!type) {
	return;
	}
	setPaymentType(type);
	setRequestType(CONST.IOU.REPORT_ACTION_TYPE.PAY);
	if (isDelegateAccessRestricted) {
	setIsNoDelegateAccessMenuVisible(true);
	} else if (ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
	setIsHoldMenuVisible(true);
	} else if (chatReport && iouReport) {
	setIsPaidAnimationRunning(true);
	HapticFeedback.longPress();
	if (ReportUtils.isInvoiceReport(iouReport)) {
	IOU.payInvoice(type, chatReport, iouReport, payAsBusiness);
	} else {
	IOU.payMoneyRequest(type, chatReport, iouReport);
	}

Hence the sound is played but not followed by settling the request

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

1. Here we should check if the request is held before playing the sound

if (!ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
            playSound(SOUNDS.SUCCESS);
        }

App/src/components/SettlementButton/index.tsx

Lines 210 to 211 in 517ffaa

	playSound(SOUNDS.SUCCESS);
	onPress(iouPaymentType);

2. Play the sound in HoldMenu when user clicks pay button

playSound(SOUNDS.SUCCESS);
IOU.payMoneyRequest(paymentType, chatReport, moneyRequestReport, full);

App/src/components/ProcessMoneyReportHoldMenu.tsx

Line 76 in 517ffaa

IOU.payMoneyRequest(paymentType, chatReport, moneyRequestReport, full);

What alternative solutions did you explore? (Optional)

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-10-10 14:11:07 UTC.

Proposal

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

Pressing the pay button plays the sound even though a confirm modal shows.

What is the root cause of that problem?

We always play the sound without checking whether there is a held expenses or not.

App/src/components/SettlementButton/index.tsx

Lines 210 to 211 in 517ffaa

	playSound(SOUNDS.SUCCESS);
	onPress(iouPaymentType);

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

Instead of playing the sound when pressing the SettlementButton, we can play it inside the pay function, such as payMoneyRequest, payInvoice, sendMoneyElsewhere, and sendMoneyWithWallet. We will follow the same pattern as completing a task.

App/src/libs/actions/Task.ts

Lines 408 to 409 in 517ffaa

	playSound(SOUNDS.SUCCESS);
	API.write(WRITE_COMMANDS.COMPLETE_TASK, parameters, {optimisticData, successData, failureData});

[RachCHopkins]

Easy to repro. It sounds when you hit the green button, not when you confirm.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[RachCHopkins]

@abdulrahuman5196 we have a number of early proposals here - do any look good to you?

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

@RachCHopkins, @abdulrahuman5196 Still overdue 6 days?! Let's take care of this!

[abdulrahuman5196]

checking now

[abdulrahuman5196]

Checking again

[abdulrahuman5196]

@mkzie2 's proposal here #50572 (comment) looks good and works well. One suggestion for the PR would be to try and see if we can avoid introducing a new param in the settlement button which is already bloated, instead we can check if the expense is on hold inside the settlement button itself. I can see we already have chatReport there which we can re-use.
All proposals here are almost the same, so going with the first proposal.

🎀 👀 🎀
C+ Reviewed

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

All proposals here are almost the same, so going with the first proposal.

@abdulrahuman5196 I don't think my proposal is almost the same as the others. My proposal suggesting to follow the same pattern as creating task, which doesn't require any conditional logic.

cc: @carlosmiceli

[carlosmiceli]

@abdulrahuman5196 After looking at @bernhardoj's proposal, I agree that it's not the same. What's the case for the one you selected?

@mkzie2 please let's hold a bit on the PR 🙏

[abdulrahuman5196]

I don't think my proposal is almost the same as the others. My proposal suggesting to follow the same pattern as creating task, which doesn't require any conditional logic.

@bernhardoj Could you kindly elaborate more on your expected changes? Still not sure if different, Maybe I could have misunderstood the proposal.

[melvin-bot]

@carlosmiceli @RachCHopkins @abdulrahuman5196 @mkzie2 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[bernhardoj]

With my solution, first we will remove the playSound from SettlementButton.

App/src/components/SettlementButton/index.tsx

Lines 211 to 212 in 66112d1

	playSound(SOUNDS.SUCCESS);
	onPress(iouPaymentType);

Then, play the sound inside IOU.payMoneyRequest (or payInvoice, sendMoneyElsewhere, and sendMoneyWithWallet).

App/src/libs/actions/IOU.ts

Lines 7658 to 7675 in 66112d1

	function payMoneyRequest(paymentType: PaymentMethodType, chatReport: OnyxTypes.Report, iouReport: OnyxEntry<OnyxTypes.Report>, full = true) {
	if (chatReport.policyID && SubscriptionUtils.shouldRestrictUserBillableActions(chatReport.policyID)) {
	Navigation.navigate(ROUTES.RESTRICTED_ACTION.getRoute(chatReport.policyID));
	return;
	}
	const paymentSelected = paymentType === CONST.IOU.PAYMENT_TYPE.VBBA ? CONST.IOU.PAYMENT_SELECTED.BBA : CONST.IOU.PAYMENT_SELECTED.PBA;
	completePaymentOnboarding(paymentSelected);
	const recipient = {accountID: iouReport?.ownerAccountID ?? -1};
	const {params, optimisticData, successData, failureData} = getPayMoneyRequestParams(chatReport, iouReport, recipient, paymentType, full);
	// For now, we need to call the PayMoneyRequestWithWallet API since PayMoneyRequest was not updated to work with
	// Expensify Wallets.
	const apiCommand = paymentType === CONST.IOU.PAYMENT_TYPE.EXPENSIFY ? WRITE_COMMANDS.PAY_MONEY_REQUEST_WITH_WALLET : WRITE_COMMANDS.PAY_MONEY_REQUEST;
	API.write(apiCommand, params, {optimisticData, successData, failureData});
	}

So, we don't play the sound when the button is pressed, but when the pay request is made.

[abdulrahuman5196]

I agree its not the same as the accepted proposal. I think I had initially misunderstood the proposal.
Anyways I will give a check on this again and update with the final result today.

[abdulrahuman5196]

I have checked @bernhardoj proposal. And IMO, the concern I have is, removing playSound(SOUNDS.SUCCESS); from SettlementButton and adding it to places where we would make the API call would cause us to add the playSound in multiple places in IOU functions which would be a duplicate code. And I am not a fan of having UX interaction like playSound to happen from API calling place which would break the UX and network separation.

So I still think the accepted proposal is better. @carlosmiceli. But anyways do let us know your thoughts as well.

[carlosmiceli]

Understood, @abdulrahuman5196, I like your logic here, I just wanted to confirm that proposals were distinct and a clear reasoning behind the selected one. @mkzie2 feel free to proceed and thanks for understanding with the delay.