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

fix: uploads files after validation #4218

Merged
merged 2 commits into from
Dec 4, 2023
Merged

Conversation

JessChowdhury
Copy link
Member

Description

Closes #4214

Upload files were being written to local storage prior to the beforeChange hooks and field validation, as a result uploads that fail validation are still being saved. Moving the upload function to after validation fixes this issue.

  • I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Existing test suite passes locally with my changes

Sorry, something went wrong.

@denolfe
Copy link
Member

denolfe commented Nov 20, 2023

Can you run through and make sure that the mimetype detection still works as expected?

I recall having the save to disk early in the process because it was required for the detection. We've since moved to detecting it from the buffer, but I think some targeted testing would be good 👍

@JessChowdhury
Copy link
Member Author

@denolfe all appears to be working correctly - I manually checked with different file types and I've added an E2E test to this PR.

Copy link
Member

@denolfe denolfe left a comment

Choose a reason for hiding this comment

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

Thank you for adding a test 👍

@JarrodMFlesch
Copy link
Contributor

@denolfe I also do not see an issue with this. Are there more things we should test before merging?

@denolfe
Copy link
Member

denolfe commented Nov 21, 2023

Testing w/ plugin-cloud-storage would be a good thing, but I don't think absolutely necessary.

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.

file still saved even when mimeType validation fails
3 participants