-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-12-12] [$250] Invoices - No sound when paying invoice #52792
Comments
Triggered auto assignment to @MitchExpensify ( |
Edited by proposal-police: This proposal was edited at 2024-11-19 19:02:17 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Invoices - No sound when paying invoice What is the root cause of that problem?We aren't calling App/src/components/ReportActionItem/ReportPreview.tsx Lines 204 to 226 in 2fe1a06
What changes do you think we should make in order to solve the problem?We should play sound What alternative solutions did you explore? (Optional)Result |
Edited by proposal-police: This proposal was edited at 2024-11-19 19:15:29 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.No sound when paying invoice What is the root cause of that problem?The sounds work as expected when paying someone, but they do not trigger for invoices. This is because the sound is played when selecting the payment type App/src/components/SettlementButton/index.tsx Line 217 in 2fe1a06
But no sound is played when the button is pressed for the "Pay Elsewhere" option in invoices because here we are not playing any sound App/src/components/SettlementButton/index.tsx Line 156 in 2fe1a06
App/src/components/SettlementButton/index.tsx Line 140 in 2fe1a06
What changes do you think we should make in order to solve the problem?We should add App/src/components/SettlementButton/index.tsx Line 156 in 2fe1a06
App/src/components/SettlementButton/index.tsx Line 140 in 2fe1a06
We can do something like this
Optional: We can play sound of our choice. For example What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.Pressing the invoice pay button doesn't play a sound. What is the root cause of that problem?For non-invoice, when we press the pay button, it will call App/src/components/SettlementButton/index.tsx Lines 216 to 219 in 9368566
But for invoice, it will show a popover menu which when we select the individual > pay elsewhere option, it only calls the SettlementButton App/src/components/SettlementButton/index.tsx Lines 129 to 143 in 9368566
What changes do you think we should make in order to solve the problem?Similar to #50572, I suggested to play the sound inside the pay function, such as payMoneyRequest, payInvoice, sendMoneyElsewhere, and sendMoneyWithWallet. We will follow the same pattern as completing a task. If we want to handle this for payInvoice, then we can only do it for payInvoice. Lines 408 to 409 in 517ffaa
If we want to apply to all pay, then we need to remove the play sound from some places like in SettlementButton to not have duplicated sound. |
Job added to Upwork: https://www.upwork.com/jobs/~021859385613425085004 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane ( |
@rushatgabhane, @MitchExpensify Huh... This is 4 days overdue. Who can take care of this? |
i like @bernhardoj's proposal it makes sense to play the sounds in an api action more than doing it in ui component 🎀 👀 🎀 |
Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
PR is ready cc: @rushatgabhane |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.71-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-12-12. 🎊 For reference, here are some details about the assignees on this issue:
|
@rushatgabhane @MitchExpensify @rushatgabhane The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test Proposal Template
Regression Test ProposalPrecondition:Test:Do we agree 👍 or 👎 |
cc @rushatgabhane To complete the C+ steps above |
Payment Summary
BugZero Checklist (@MitchExpensify)
|
All payments coming through NewDot, closing |
Sorry, reopening - @rushatgabhane please complete the BZ steps |
Requested in ND. |
$250 approved for @bernhardoj |
@rushatgabhane, @mountiny, @MitchExpensify, @bernhardoj Whoops! This issue is 2 days overdue. Let's get this updated quick! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.64-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
Issue reported by: Applause - Internal Team
Action Performed:
Precondition:
Expected Result:
There should be sound when paying invoice
Actual Result:
There is no sound when paying invoice
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6669814_1732038805706.Screen_Recording_20241120_014752_Chrome.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @MitchExpensifyThe text was updated successfully, but these errors were encountered: