[#38536] - [5.2] MediaManager: improve error handling
- Pending
- Medium
- Build: 4.2-dev
- # 38536
- Diff
- Fedik:mediamanager-erroring
User tests: Successful: Unsuccessful:
Pull Request for Issue #38068 and #38530 .
Summary of Changes
Improve error handling in MediaManager
Testing Instructions
Apply patch. Run npm install.
Try upload something not allowed in to mediamanager.
Actual result BEFORE applying this Pull Request
General error Unable to upload file.
Expected result AFTER applying this Pull Request
General error Unable to upload file.
Then multiple detailed errors:
Or Illegal mime type detected or This file type is not supported (depend from medimanager config)
Or for svg: The file looks suspicious, therefore cannot be uploaded
Documentation Changes Required
nope
@brianteeman please review an updated string.
Votes
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Administration com_media NPM Change Language & Strings |
| Labels |
Added:
Language Change
NPM Resource Changed
?
|
||
Thank you for providing a better solution!
even better would be to display the error message from the svgsanitizer which explains exactly why the file was rejected
I have added documentation about uploading SVG files. You can find it at https://docs.joomla.org/J4.x:Media:_Uploading_SVG_files. It may need refining and additional content. You are welcome to pitch in, thank you!
IT does what it says but I wonder why we have the two messages. The second one makes the first one redundant?
The script render all messages that was sent from PHP backend.
In your example, I guess the first one comes from a controller, and second one from a validator.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38536.
Before the fix, the script was rendered only first message, and rest were ignored.
In your example, I guess the first one comes from a controller, and second one from a validator
The first message is from the Exception https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/filesystem/local/src/Adapter/LocalAdapter.php#L772 , additional messages are Joomla system messages generated by calling $app->enqueueMessage from canUpload from MediaHelper https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Helper/MediaHelper.php#L200
Still the first error message is redundant as Brian said. Is it OK to make the Exception https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/filesystem/local/src/Adapter/LocalAdapter.php#L772 with empty message, and have the Joomla system messages rendered for this case ? It is kind of workaround but more user friendly.
Is it OK to make the Exception with empty message
I do not think.
I think the root issue that $helper->canUpload() should return boolean and cannot throw an exception, so someone decided to put all in to $app->enqueueMessage . Not perfect, but working.
I have tested this item
Not perfect but work.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38536.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38536.
@AmyKrizanWang please do not update your comment. It is nothing to do with this issue.
This pull request has been automatically rebased to 4.3-dev.
| Labels |
Added:
?
bug
|
||
| Status | Ready to Commit | ⇒ | Pending |
Pleas test it again, there was a complex conflict, I had to redo changes.
| Labels |
Added:
PR-4.3-dev
?
Removed: ? ? |
||
This pull request has been automatically rebased to 4.4-dev.
| Title |
|
||||||
@AmyKrizanWang please do not update your comment. It is nothing to do with this issue.
comment removed
This pull request has been automatically rebased to 5.2-dev.
| Title |
|
||||||
Nice one as it will catch on every request the error messages.