-
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 2021-12-27] $500 - solution + reporting bonus. Android/iOS - User is able to upload a video of >50mb - Reported by: @akshayasalvi #5918
Comments
Triggered auto assignment to @cead22 ( |
Triggered auto assignment to @Beamanator ( |
It doesn't look like the user is actually able to upload the video successfully, but the reproduction steps are clear and we should show an error messages instead of endless spinners |
N6-hold ended, and changing to Weekly instead of daily b/c it's not super high priority. Will look into it later this week. |
Looking more into this later this week |
Not finding much time to work on this so far this week 🙈 |
@Beamanator I was retesting this and it looks like I closed this in another PR. (#5900) This commit fixed it. Earlier it basically failed to fetch the size for videos and allowed the user to upload. |
Hmm can you test again on Android or iOS? I believe I reproduced the issue a week ago, which is even after the PR you mention was merged to production |
I'll try again to reproduce this week in case it actually was fixed before |
@Beamanator So I was able to reproduce this. Problem is when the files are picked from photo gallery we are not getting the size parameter. It turns out thats a limitation from If I upload the video using add Document API then I do get Here's my proposal to fix this:
|
Triggered auto assignment to @parasharrajat ( |
Current assignee @Beamanator is eligible for the Exported assigner, not assigning anyone new. |
here is my proposal to fix this issue
} |
Thanks for the proposal @FoolCoder, but I've actually already assigned someone to this job (see this comment) As for your proposal, please keep in mind that we currently don't use |
Triggered auto assignment to @stephanieelliott ( |
@stephanieelliott i'm OOO quite a bit over next week moving house, so handing this one off while I"m away thanks! |
PR is on staging. |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.21-1 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 2021-12-27. 🎊 |
paid $500 - solution ($250) + reporting bonus ($250) |
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:
Expected Result:
There should be a error message about the upload limit of 50mb
Actual Result:
User is able to upload the >50mb video
Workaround:
None needed.
Platform:
Where is this issue occurring?
Version Number: 1.1.8-2
Reproducible in staging?: Yes
Reproducible in production?: Yes
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
android-bug_5S3TO7ec.mp4
Expensify/Expensify Issue URL:
Issue reported by: @akshayasalvi
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1634322848367700
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: