-
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
[HOLD for payment 2024-03-20] [$500] IOU – Edited amount in a receipt scan failed is not visible in IOU preview. #34418
Comments
Job added to Upwork: https://www.upwork.com/jobs/~016702df242a6ed127 |
Triggered auto assignment to @dylanexpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.What is the root cause of that problem?Currently, we are handle amount, receiptScanning, receiptMissingDetail in this function App/src/components/ReportActionItem/MoneyRequestPreview.js Lines 229 to 243 in dc748de
So, at a time we only display one of them What changes do you think we should make in order to solve the problem?As new expected, we allow to display amount along with error message. To do that Firstlt, we should move this code App/src/components/ReportActionItem/MoneyRequestPreview.js Lines 234 to 240 in dc748de
to new function like this
The new variable And then here
we should add a condition to display the amount (only display the amount, if it is valuable)
Then in here
Add new element to display message like this
Because we added a new element, we need to update the style here
to
|
ProposalPlease re-state the problem that we are trying to solve in this issue.IOU – Edited amount in a receipt scan failed is not visible in IOU preview. What is the root cause of that problem?getDisplayAmountText function returns the text "Receipt missing details" if there missing required fields . And the return value is used to display the amount What changes do you think we should make in order to solve the problem?We can replace this condition with below code. But with this solution the "Missing Receipt Details" text will be visible only when the amount is missing and if the merchant is missing the text will not be visible
But I think that the text "Missing Receipt Details" should be visible if any of the required parameters is missing. What alternative solutions did you explore? (Optional)
Screen.Recording.2024-01-12.at.17.11.49.mov |
Proposal |
Will review asap. |
Thanks for the proposals, but does anyone have any insight as to why this is only happening in workspace chats, it seems to work fine in a 1-1 chat. |
This comment was marked as duplicate.
This comment was marked as duplicate.
@Ollyws Updated proposal I think the actual result in OP is correct, the error should be shown because the merchant field is required and It is empty. |
Thanks @DylanDylann! cc @Ollyws to review update |
Because the report type is different in workspace and 1:1 chats. This condition is I think before updating our proposal we need clarify what is the correct behavior . Is there any documentation which describes how should display the preview for each case ? Or we need more detailed expected result . Should the text "Missing Receipt Details" be displayed when the amount exists but the merchant is empty ? |
Thank you both for looking into it. |
@Ollyws I think is still needs be fixed because current behavior is following. If the merchant exists and the amount does not, the preview displays both the error message and the merchant. But if the amount exists and the merchant does not, only an error message is displayed. |
I think we should ping an internal engineer to confirm it |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@Ollyws shall we get |
Yes but just to confirm the desired behaviour, I'm not sure it necessarily has to be an internal issue. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
PR merged! Pending regression period then payments |
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. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.51-3 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-03-20. 🎊 For reference, here are some details about the assignees on this issue:
|
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:
|
Payment coming up! |
BugZero Checklist:
I wouldn't call this a bug, it was originally just part of the design to replace the amount with
N/A
N/A
Yes as it's a new feature.
Regression Test Proposal
Do we agree 👍 or 👎 |
@dylanexpensify Could you send the payment please ? |
Payment summary:
|
@Ollyws can you accept offer? @shahinyan11 you've been paid! |
@DylanDylann I don't see any offer, I think it may have expired. Apologies! |
I meant to direct #34418 (comment) at @dylanexpensify, that's the second time I've done that now 😆 |
Ahh, let me get you a new one @Ollyws! |
@dylanexpensify Thanks but that seems to just be a job rather than an offer. |
Yeah, if you can just apply to it 😄 |
Done! |
Offer sent!
…On Tue, Mar 26, 2024 at 10:59 AM Olly ***@***.***> wrote:
Done!
—
Reply to this email directly, view it on GitHub
<#34418 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOKVCAHIMBJH4BHM6AQE7G3Y2FBH5AVCNFSM6AAAAABBX3GNISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRQGEZDCOBVGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Accepted, thanks! |
Done! |
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: v1.4.24-7
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
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
Action Performed:
Expected Result:
Edited amount in a receipt scan failed is visible in IOU preview
Actual Result:
Edited amount in a receipt scan failed is not visible in IOU preview. When edit merchant only in a receipt scan failed – merchant name is visible in IOU preview.
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6339893_1705049915252.Edited_amount_in_a_receipt_scan_failed_is_not_visible_in_IOU_preview.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: