Skip to content
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 2023-12-04] [SMARTSCAN] MEDIUM: Allow users to re-download receipt if upload fails #28884

Closed
Gonals opened this issue Oct 5, 2023 · 34 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff SmartScan Wave5-free-submitters

Comments

@Gonals
Copy link
Contributor

Gonals commented Oct 5, 2023

PROBLEM
If a user uploads a receipt directly from the camera and then throws away the physical receipt, that receipt is gone forever if upload fails.

SOLUTION
As described in the design, allow the user to download any failed receipts

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011b654f3591a45ce6
  • Upwork Job ID: 1709727112645718016
  • Last Price Increase: 2023-10-05
@Gonals Gonals added Weekly KSv2 Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff labels Oct 5, 2023
@Gonals Gonals self-assigned this Oct 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 5, 2023

Job added to Upwork: https://www.upwork.com/jobs/~011b654f3591a45ce6

@melvin-bot
Copy link

melvin-bot bot commented Oct 5, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @0xmiroslav (Internal)

@trjExpensify
Copy link
Contributor

👋 Alberto, how are you getting on here?

@isagoico was adding a few of the manual tests from the Scan Receipt doc, and it prompted me to come here. This is pretty whack at the mo:

image Clicking the preview goes to a full page blocking form: image

Compared to the doc mock:

image

So just a few additional questions:

  1. Can we make sure the error messages also get updated with this change to be more discerning than "unexpected"?
  2. In the original design we were told we couldn't preview, so we went with uploading attachment..., but it seems like we've figured out an empty state thumbnail for the report preview component instead now. I'm not adverse to keeping that as is, but wanted to double check!
  3. Can we make sure we don't run into this "hmm.. it's not there" thing that makes it feel real broken? If you can't access the request transaction view in this state (to say, detach the receipt and fill out the details manually), we should just not make it clickable.

Thanks!

@melvin-bot melvin-bot bot added the Overdue label Oct 13, 2023
@dylanexpensify
Copy link
Contributor

@Gonals mind giving us an update today? 🙇‍♂️

@Gonals
Copy link
Contributor Author

Gonals commented Oct 16, 2023

Yep. I haven't started on this yet, as I was OOO Thursday/Friday.

Once I'm caught up and done with the dailies, this comes after 😁

@Gonals
Copy link
Contributor Author

Gonals commented Oct 17, 2023

Code ready! I'll do cleanup and screenshots tomorrow!

@Gonals Gonals added the Reviewing Has a PR in review label Oct 18, 2023
@Gonals
Copy link
Contributor Author

Gonals commented Oct 18, 2023

In review!

@dylanexpensify dylanexpensify changed the title [Wave 4 Polish] Allow users to re-download receipt if upload fails [Wave 4 Polish] MEDIUM: Allow users to re-download receipt if upload fails Oct 27, 2023
@dylanexpensify dylanexpensify changed the title [Wave 4 Polish] MEDIUM: Allow users to re-download receipt if upload fails [SMARTSCAN] MEDIUM: Allow users to re-download receipt if upload fails Nov 7, 2023
@dylanexpensify dylanexpensify added the SmartScan Wave5-free-submitters label Nov 7, 2023
@melvin-bot melvin-bot bot removed the Weekly KSv2 label Nov 13, 2023
Copy link

melvin-bot bot commented Nov 13, 2023

This issue has not been updated in over 15 days. @Gonals, @0xmiroslav eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@melvin-bot melvin-bot bot added the Monthly KSv2 label Nov 13, 2023
@trjExpensify
Copy link
Contributor

Putting this back to daily, let's close out this PR ASAP, it's so close.

@trjExpensify trjExpensify added Daily KSv2 and removed Monthly KSv2 labels Nov 13, 2023
Copy link

melvin-bot bot commented Nov 21, 2023

@Gonals, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

@Gonals
Copy link
Contributor Author

Gonals commented Nov 21, 2023

Hopefully merging today!

Copy link

melvin-bot bot commented Nov 24, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Nov 24, 2023
@melvin-bot melvin-bot bot added the Monthly KSv2 label Jan 1, 2024
@trjExpensify
Copy link
Contributor

Putting back on daily, awaiting a secondary PR review from @NikkiWines.

@trjExpensify trjExpensify added Daily KSv2 and removed Monthly KSv2 labels Jan 3, 2024
Copy link

melvin-bot bot commented Jan 10, 2024

@Gonals, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Jan 12, 2024

@Gonals, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Jan 16, 2024

@Gonals, @0xmiroslav 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@situchan
Copy link
Contributor

Please assign me here. This is ready for payment

@garrettmknight garrettmknight added the Bug Something is broken. Auto assigns a BugZero manager. label Jan 16, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@dylanexpensify
Copy link
Contributor

@situchan assigned

@dylanexpensify
Copy link
Contributor

@Gonals what's left to do here? Just payments?

@0xmiros
Copy link
Contributor

0xmiros commented Jan 18, 2024

yes payment

@dylanexpensify
Copy link
Contributor

Payment summary:

  • Contributor+: @0xmiroslav $500
  • Contributor+: @situchan $500

Please request!

@dylanexpensify
Copy link
Contributor

@situchan remind me, are you paid in ND yet?

@situchan
Copy link
Contributor

@dylanexpensify not yet

@mallenexpensify mallenexpensify removed the Reviewing Has a PR in review label Jan 24, 2024
@mallenexpensify
Copy link
Contributor

Contributor+: @0xmiroslav $500
Contributor+: @situchan $500

@dylanexpensify for payment confirmations, can you please us a format that both includes if the contributor is paid or is owed money (paid as in Upwork, owed as in waiting for NewDot payment) and also include where they are paid and owed? Upwork or NewDot. Below is the TextExpander I use, I leave the second blank since many C+ are paid via Newdot.

Contributor: @ paid $ via Upwork
Contributor+: @

Removed the Reviewing label here too

@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2024
@dylanexpensify
Copy link
Contributor

sounds good, thanks @mallenexpensify!

@melvin-bot melvin-bot bot removed the Overdue label Jan 29, 2024
@dylanexpensify
Copy link
Contributor

@situchan sent invite for this job!

@dylanexpensify
Copy link
Contributor

offer sent!

@dylanexpensify
Copy link
Contributor

done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff SmartScan Wave5-free-submitters
Projects
No open projects
Development

No branches or pull requests

7 participants