-
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
[HIGH][$250] Send invoice - Category is missing in confirmation page but it is present in invoice view #41281
Comments
Triggered auto assignment to @robertjchen ( |
Triggered auto assignment to @sonialiap ( |
👋 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:
|
We think this bug might be related to #wave-collect - Release 1 |
@sonialiap 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. |
Does not appear to be a serious deploy blocker to me and can be addressable by external contributors 👍 |
#vip-billpay related. @danielrvidal could you please help prioritize? |
Given this is core functionality for anyone with a workspace, I'm marking it as CRITICAL as it might be needed in the demos. |
Job added to Upwork: https://www.upwork.com/jobs/~017a0bd1ec84ee8690 |
@Pujan92 can you take care of the checklist here? Thanks! |
@greg-schroeder I completed this earlier here for the issue and it remains same. Let me know if I need to post the same here again. |
@Pujan92 Oops, sorry, I missed that! I was just picking this up for Sasha so I am catching up here. :) That's fine, no need to go again |
Just a moment, here -- @Pujan92 perhaps you can help explain. It seems this was completed back in May/June, but the issue re-surfaced and the same two folks were involved as Contributor/C+. Was this a regression, or was an entirely new solution required? Why was this project basically re-done a couple of months later? If you're able, a quick explanation would be helpful for me! |
@greg-schroeder Our earlier PR changes were partially modified in this PR to fix issue #41931, due to that the issue re-surfaced again. Now while checking I can see that deployblocker was somewhat connected to regression from our PR but we didn't notify and weren't aware of that. So when this issue occurred again we opened it for the external contributors to put proposals to apply a new solution on top of the latest changes. |
Hmm, okay. That is a bit tricky. Let's get a few more eyes on this. Hey @cristipaval - do you mind weighing in on this one? Do you view it as a regression or a separate issue with a unique solution? |
@greg-schroeder back from OOO so can take this back, feel free to stay on or unassign yourself |
@cristipaval bumping Greg's question here #41281 (comment) regarding whether you think this is a recession or a separate issue with a unique solution? |
Cristi is on parental leave. I'm going to review this tomorrow to see if we can close without his input |
@sonialiap Have you made up your mind yet? |
Apologies, I must have accidentally closed the tab and lost the issue. Thanks for the ping |
@youssef-lr could you help assess whether there was a regression here? Christi is on parental leave and I'm getting lost in the code. This issue here was solved with PR 41723, then your PR 41945 changed the solution, this caused the issue to resurface and the GH issue was reopened. A new PR, 47474 was pushed and needs payment. The resurfaced issue was not flagged as a regression. Comment from the C+. Do you think there was a regression caused by #41723 ? |
This is ready for payment, we just need to figure out whether there was a regression. @youssef-lr bumping my question from above in hopes that you can help #41281 (comment) |
@youssef-lr, @Pujan92, @sonialiap, @cristipaval, @gijoe0295 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
It looks like it was a regression indeed @sonialiap |
Thanks for checking, Youssef! @Pujan92 @gijoe0295 because this latest PR was a fix for a regression on this issue, it does not qualify for additional payment. Had this been caught within the regression period then the payment would have been decreased but since it popped up a month later, you received full payment for the first fix and nothing needs to happen at this point. Thank you for quickly addressing the regression! |
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: 1.4.68-0
Reproducible in staging?: Yes
Reproducible in production?: No (New feature)
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
Expensify/Expensify Issue URL:
Issue reported by: Applause Internal Team
Slack conversation:
Action Performed:
Pre condition:Create a new workspace and use to send Invoice
Expected Result:
"Category" row should be present in the confirmation page if it is present in the invoice details page.
Actual Result:
"Category" row is not present in the confirmation page when it is present in the invoice details page.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6466065_1714436349471.category.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @youssef-lrThe text was updated successfully, but these errors were encountered: