-
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
[SMARTSCAN] [HOLD for payment 2023-09-27] [$500] HIGH: Receipt modal closes when API response updates image url #26828
Comments
Triggered auto assignment to @bfitzexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
When the API response updates the image source, the old (local) source will be considered deleted. If it's deleted, we will dismiss the attachment modal. App/src/components/Attachments/AttachmentCarousel/index.js Lines 46 to 49 in 1eafc7d
There is a previous discussion about this behavior here but there is no action taken yet. |
I think we can also just disable the carousel when the image is being uploaded. This can be done by adding |
Thanks for the link @allroundexperts! It seems like we're treating this as a new feature. So I'll remove the bug label, add new feature, mark it external and bump it to weekly for now. |
Current assignee @bfitzexpensify is eligible for the NewFeature assigner, not assigning anyone new. |
Job added to Upwork: https://www.upwork.com/jobs/~019f02e5a5c44adaa8 |
Current assignee @bfitzexpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Modal closes when API response updates the image URL What is the root cause of that problem?In here, we'll dismiss the modal if the source of an image change since we assume that the image was deleted, however it was not true in this case since the image is the same, it's just local source is different from the Expensify source. What changes do you think we should make in order to solve the problem?If the source changes, but another unique identifier of the image (for example the The user in that case will still see the local image, or will see the Expensify-source image being loaded on top when it's available, depending on our preference. What alternative solutions did you explore? (Optional)If we don't want to show the carousel if the image is being uploaded, we can disable |
Still looking at proposals |
@luacmartins do we think we'll have this completed by Fancy week? |
@luacmartins, @bfitzexpensify, @thesahindia, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Still waiting on #27824 |
Same update, still waiting on #27824 |
@luacmartins, @bfitzexpensify, @thesahindia, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Gonna move this to weekly while it's on hold |
Still on hold for the moment |
still waiting on #27824 |
27824 is merged and deployed! What's next here? |
Actually, confirmed - this bug doesn't exist any longer. @bfitzexpensify feel free to complete the remaining steps! |
Sweet. We left this open to address a regression from the initial work done here. Payments here are all complete, the BZ checklist was also completed. Now that that's solved, we can close this one out. |
Hey @bfitzexpensify, can you please comment the payment summary? |
Bump @bfitzexpensify |
Sorry, missed your message the other day. Payment summary: Contributor: @tienifr $500 paid via Upwork |
$500 payment approved for @thesahindia based on comment above. |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Scan
optionVideo:
#26815 (comment)
Expected Result:
Modal should remain open
Actual Result:
Modal closes when API response updates the image URL
Workaround:
Reopen the modal
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number:
Reproducible in staging?:
Reproducible in production?:
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
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: