-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[ON HOLD for Payment 2/24/25] [$250] Per diem - Quantity field shows error after selecting a subrate #55399
Comments
Triggered auto assignment to @CortneyOfstad ( |
Triggered auto assignment to @youssef-lr ( |
💬 A slack conversation has been started in #expensify-open-source |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
@CortneyOfstad 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. |
NAB, per diem is behind a beta |
@youssef-lr You can take this external. |
@youssef-lr, @CortneyOfstad Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@youssef-lr any update? Thanks! |
Job added to Upwork: https://www.upwork.com/jobs/~021883939320560472623 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Per diem - Quantity field shows error after selecting a subrate What is the root cause of that problem?When we press on the subrate menu the text input is blurred and this will set the App/src/components/Form/FormProvider.tsx Line 374 in 6118a1f
So when we select the subrate that will invoke the validate and field required error will be set here App/src/pages/iou/request/step/IOURequestStepSubrate.tsx Lines 123 to 124 in 6118a1f
because the text input is considered as touched as explained this will show the error App/src/components/Form/FormProvider.tsx Lines 159 to 164 in 6118a1f
What changes do you think we should make in order to solve the problem?The behaviour is correct because if a user blurs from a text input without inputting a value then that is considered as touched so error will be shown as they have avoided inputting a required value. Because the subrate is the first input in the page to solve the bug on this issue we should avoid auto focusing the quantity text input when the page opens and optionally focus when the user has selected a subrate(optionally we can also additionally focus when the subrate modal is dismissed even without selecting a value).
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?N / A What alternative solutions did you explore? (Optional) |
@mananjadhav @youssef-lr @CortneyOfstad 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! |
@youssef-lr @CortneyOfstad Can you confirm if the desired behavior is correct? The proposal mentions the behavior is expected. @FitseTLT While I haven't tested the issue, can you confirm why is it not reproducible on production, but is on staging? |
@mananjadhav This is because validation on change was only enabled in #54760
|
@FitseTLT's proposal makes sense, subject to verifying the expected behavior. 🎀 👀 🎀 C+ reviewed. |
Current assignee @youssef-lr is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
@Gonals Can you please check the proposal too here just to cover all bases here? |
📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
The issue was just assigned yesterday. PR will be up soon. @FitseTLT any ETA on the PR? |
PR is being actively worked on 👍 |
Waiting for a review from @youssef-lr |
@CortneyOfstad The PR was deployed to production yesterday but the payout date isn't updated. |
@mananjadhav I sent you an offer to the job here — can you let me know once you accept? Also, can you confirm if a regression test is needed? |
@CortneyOfstad I'll be raising the request on NewDot. I've declined the offer on Upwork. Posting the checklist in the next comment. |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test ProposalPrecondition:
Test:
Do we agree 👍 or 👎 |
Payment Summary@FitseTLT — paid $250 via Upwork Regression Test |
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.87-0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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
Device used: Mac 15.0 / Chrome
App Component: Money Requests
Action Performed:
Precondition:
Expected Result:
Quantity field will not show error after selecting a subrate (production behavior).
Actual Result:
Quantity field shows error after selecting a subrate.
Workaround:
Unknown
Platforms:
Screenshots/Videos
bug.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @mananjadhavThe text was updated successfully, but these errors were encountered: