Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Accept receipt in request money #23770
Accept receipt in request money #23770
Changes from all commits
89ce236
94ccccc
eafded7
af581b4
1f7f7bd
3715973
991fc6a
7c88271
c075113
0a3cd9b
448813c
43922f7
551af16
b595397
bbeb8cc
71a8481
0c5f0c2
576c4de
601dccc
68afecf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This caused a bug #26940. On Android the File API constructor needs the type to be specified otherwise network uploads would fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are resolving even on failures? Shouldn't we reject?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We resolve with an undefined receipt file and the call to RequestMoney proceeds without the receipt.
I agree that we should eventually reject and display some error to the user, but I didn't see anything about it in the doc. Since this PR is blocking others in this project and we currently don't display errors in MoneyRequestConfirmPage, I think we should keep the scope of this PR smaller and tackle displaying an error as a follow up once we align on what error will be displayed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Fine by me then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✋ Coming from #28347
The transaction creation with receipt upload won't work after uploading the receipt and refreshing the page because the file is removed. So we decided to check the file, if the file is not available then we navigated back the user to the main request money page.