Validation upload file #4524
Validation upload file #4524
Conversation
Add a simple wraper function for the various file validation methods. This method makes it easier to wrap up the various file validators into one method. Refs #4456
This makes validating forms where there is an optional file much easier.
|
These changes could easily be backported to 2.6 if people are interested. |
| if (!static::uploadError($file, $options['optional'])) { | ||
| return false; | ||
| } | ||
| if ($options['optional'] && (int)$file['error'] === UPLOAD_ERR_NO_FILE) { |
There was a problem hiding this comment.
I think for any other type of upload error the validator should return false.
There was a problem hiding this comment.
Doh, I missed that uploadError() check is already done above.
|
Nice! looks good to me 👍 |
|
Awesome! This is going to bring everybody a lot of joy. Also I check for extension of the uploaded file. That's quite common isn't it? So it might as well be in this wrapper. For reference, extensions were mentioned in #4456. Any particular reason you left it out? |
|
@Phally Extension checking isn't really useful as they can be easily faked. |
|
While having a single method to handle all file upload related validations is nice I would personally prefer separate methods for each. That why I can set seperate error messages for each and user can be notified of the exact problem. |
|
@ADmad Yes it can be faked, but that isn't its purpose. The purpose is to tell the user they might have selected the wrong file or exported something in the wrong format. Maybe that's a good argument not to put it in this method so you can separate the error message as you said. Though for completeness I still feel it should be in this wrapper. |
|
@Phally Fair enough, extension checking is more user centric. |
|
I think extension checking is secondary to mimetype checking, which is included. One drawback to this validation method is you cannot give fine grained error messages. However, if you need a simple pass/fail it couldnbe quite useful. |
|
@ADmad All the part needed to build fine grained validation are still there. It is a bit tricky to build optional file uploads which was one of the goals for ths method. |
|
@markstory Indeed they are, so I am cool with merging this. |
|
@markstory Can you please backport this validation to 2.x. |
|
@vanquang9387 You're welcome to open a pull request with the backport. I'm pretty strapped for time right now. |
|
It's a bit difficult to me. Will give it a try 😊 |
Add
Validation::uploadedFile()as a simple wrapper around the various file upload validation methods. It also makes it easier to deal with optional files which was something that came up in #4456.