Updates Requested PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

Summary of Changes

  • If mediatypes is not specified in the input, fallback to an explodable string of all supported formats (0,1,2,3) rather than images only (0).
  • Remove a redundant if statement - count($mediaTypes) is never === 0.

Testing Instructions

  1. Download the following file: Example MP4
  2. Upload the video file in to the root of the media library - by default, the path to the file should be: local-images:/file_example_MP4_480_1_5MG.mp4
  3. In a custom component or module, call \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.

avatar mattelkins-bluefrontier mattelkins-bluefrontier - open - 10 Jun 2024
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 10 Jun 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Jun 2024
Category Administration com_media
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 10 Jun 2024
The description was changed
avatar mattelkins-bluefrontier mattelkins-bluefrontier - edited - 10 Jun 2024
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 10 Jun 2024
The description was changed
avatar mattelkins-bluefrontier mattelkins-bluefrontier - edited - 10 Jun 2024
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 10 Jun 2024
The description was changed
avatar mattelkins-bluefrontier mattelkins-bluefrontier - edited - 10 Jun 2024
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 10 Jun 2024
The description was changed
avatar mattelkins-bluefrontier mattelkins-bluefrontier - edited - 10 Jun 2024
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 17 Jul 2024
The description was changed
avatar mattelkins-bluefrontier mattelkins-bluefrontier - edited - 17 Jul 2024
avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 17 Jul 2024
Labels Added: PR-5.1-dev
avatar bembelimen
bembelimen - comment - 11 Aug 2024

Thanks for the contribution @mattelkins-bluefrontier

I see one issue here: if you remove the if then you have a different behaviour to before when sending an empty string as mediatypes:

Before PR:

after the if you still have all types selected

After PR:

after the if, it's still an empty sting

Reason

$input->get(...) only returns the default value if the value is not set at all, but an empty string counts as not empty with isset.

Also please check the XML if you change default values.

avatar mattelkins-bluefrontier mattelkins-bluefrontier - change - 14 Aug 2024
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2024-08-14 06:59:46
Closed_By mattelkins-bluefrontier
Labels Added: Updates Requested
avatar mattelkins-bluefrontier mattelkins-bluefrontier - close - 14 Aug 2024
avatar mattelkins-bluefrontier
mattelkins-bluefrontier - comment - 14 Aug 2024

Thanks for your feedback, @bembelimen. I'll reopen this against 5.2-dev.

Add a Comment

Login with GitHub to post a comment