-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[5.3] Ensure isMediaFile defaults to all supported formats when mediatypes is not specified #43915
base: 5.3-dev
Are you sure you want to change the base?
Conversation
…ty string, and reformatting for line length.
Reopened against 5.2-dev after accidentally closing #43640 because I'm an idiot. 🤦♂️ This PR incorporates feedback from @bembelimen in this comment. In the scenario where
|
Using PR or not got: "Unable to upload file." Test System Information: Sample Data – Testing |
@mattelkins-bluefrontier could you elaborate on #3 of the testing instruction ("In a custom component or module, call \Joomla\Component\Media\Administrator\Model\FileModel::getFileInformation('local-images:/file_example_MP4_480_1_5MG.mp4');")? How do you do that? This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43915. |
This PR requires coding a custom module or component and not a mere easy PR test. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43915. |
Please correct Your testing instructions, calling the methode |
I have tested this item ✅ successfully on 72f667f This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43915. |
That means this is a breaking change then doesnt it? |
I revoke my statement that it cannot be called up statically, it does work. I just tested it again successfully with the static call. |
@degobbis thanks for clarifying that |
Many thanks for all of your comments, and especially to @degobbis for the example module. I've updated the PR description accordingly. My apologies for the confusion around getFileInformation() - it is not a static method in the FileModel class. This was my mistake - I used the PHPDoc FQSEN in the line of code I provided. |
This pull request has been automatically rebased to 5.3-dev. |
Summary of Changes
Testing Instructions
\Joomla\Component\Media\Administrator\Model\FileModel->getFileInformation('local-images:/file_example_MP4_480_1_5MG.mp4');
Actual result BEFORE applying this Pull Request
With mediatypes not present or empty in the input object, calling FileModel::getFileInformation() with a path to anything other than an image file will generate an InvalidPathException.
The exception is thrown by ApiModel::getFile(), and will also be thrown if this function is called directly with a separate adapter and path.
The exception is triggered due to the fact that ApiModel::isMediaFile() will return false for a non-image file if mediatypes is not present or is empty in the input object. Without mediatypes, allowed formats will be limited to images only, rather than images, audio files, videos and documents.
Expected result AFTER applying this Pull Request
FileModel::getFileInformation() (and ApiModel::getFile()) will return an object containing file information for image and non-image files, assuming the referenced file exists at the location specified by the path.
(The namespace for all classes mentioned above is: Joomla\Component\Media\Administrator\Model.)
Link to documentations
No documentation changes for docs.joomla.org needed.
No documentation changes for manual.joomla.org needed.