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

feat: show error upon FileReader failure #121

Merged
merged 10 commits into from
Aug 12, 2020
Merged

Conversation

mantariksh
Copy link
Contributor

@mantariksh mantariksh commented Aug 11, 2020

Problem

We have a high frequency of "Required but no attachment content" validation errors from attachment fields, meaning the user and the frontend think that the attachment was uploaded successfully, but the backend discovers that the attachment content is actually missing. These errors occur almost exclusively on Android phones.

Closes #23

Solution

The error occurs when the user attempts to upload a file directly from Google Drive. The UI behaves as if the file has been uploaded correctly, but in fact FileReader has failed silently because we have not set an error handler on it.

The ideal solution would be to fix file uploads from Google Drive, but this is unlikely to be straightforward and likely to introduce a lot of special cases in the code since Google Drive files clearly behave differently from files in the local file system.

Hence this solution does two things:

  1. Adds an error handler which prompts the user to download their file first if they are using Google Drive.
  2. Only shows that the file was uploaded successfully if the file was saved as a blob successfully. This makes sense because the upload is really only successful after we have set vm.field.file. It also prevents situations where the UI first behaves as if the attachment was successful, then suddenly flips to an error when FileReader.onerror is called.

I also took the opportunity to introduce a few small refactors to simplify the attachment component.

Screenshots

Screenshot 2020-08-11 at 10 48 47 PM

Tests

  • Upload an attachment from Google Drive on an Android phone. The error should show as per the screenshot.
  • On both desktop and mobile, upload a valid attachment. The UI should behave correctly. Make sure that this works for .zip files.
  • On both desktop and mobile, upload an attachment which exceeds the maximum allowed size. You should see the correct error message, and you should subsequently be able to upload a valid attachment and submit the form.
  • On both desktop and mobile, upload an attachment of an invalid file type (e.g. .js). You should see the correct error message, and you should subsequently be able to upload a valid attachment and submit the form.

@mantariksh
Copy link
Contributor Author

@syan-syan seeking your approval on the error wording pls

Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i made some minor suggestions, lgtm otherwise!

@mantariksh mantariksh merged commit 4169133 into develop Aug 12, 2020
@r00dgirl
Copy link
Contributor

@mantariksh sorry late but its ok!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate possible bug
3 participants