Update to fix view on image validation failure #10009
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds `validate` method to image validator. Replaces existing image validation calls with a single `validate` call in the `upload` method. Validation errors are now propagated from `upload` to the image upload handler. When a validation error occurs, the `covers/add` template is rendered with an error message
cdrini
requested changes
Nov 12, 2024
| @@ -88,18 +96,10 @@ def upload(self, key, i): | |||
| olid = key.split("/")[-1] | |||
|
|
|||
| if i.file is not None and hasattr(i.file, 'file'): | |||
| file_data = i.file.file | |||
There was a problem hiding this comment.
I'm not sure we want to access i.file.file multiple times; I wonder if it might have side effects / not be cached. Having validate take the filename and the file data seems like a quick way to avoid it, or we can try to find a way to verify that calling it multiple times is ok.
| data = self.upload(key, i) | ||
| try: | ||
| data = self.upload(key, i) | ||
| except ValueError as e: |
There was a problem hiding this comment.
Ah, this except now covers more code than before, and ValueError is a somewhat generic error that can also be thrown by native python. I think we now might need to make a custom ImageValidationError to avoid accidentally masking unrelated bugs/ValueErrors.
cdrini
added
the
Needs: Submitter Input
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #9966
Fixes "Unable to render this page" message being rendered in cover modal when image validation fails.
Technical
Adds
validatemethod to image validator. Replaces existing image validation calls with a singlevalidatecall in theuploadmethod.Validation errors are now propagated from
uploadto the image upload handler. When a validation error occurs, thecovers/addtemplate is rendered with an error message.Testing
Attempt to upload an invalid image to the covers service. Expect to see a generic error message.
An invalid image has at least one of the following properties:
Screenshot
Stakeholders