[4.0] Return sensible default from 'FileSystem - Local' plugin. #34364
Conversation
…dir_fix' into filesystem_local_plugin_default_dir_fix # Conflicts: # plugins/filesystem/local/local.php
I am doubt here, the plugin should use own options. There may be another filesystem plugins.
I not checked how it connected, but probably the folder should come from filesystem plugins only. The path option in the new Media Manager configuration seems does not make sense anymore. |
If I understand it correctly:
So I think it makes sense to use Media Manager's path setting as a default for the
I did check. After changing paths in the Media Manager, it did not work anymore until I selected the proper directory in the
That may well be, but it is there and can be used. As long as that's the case, I think it must behave in a predictable manner. |
The thing that Media Manager supports depend from enabled FileSystem plugins. |
|
I agree with @Fedik . You can install a filesystem plugin e.g. for using Dropbox, and then you can set paths to files and images folders in the media manager options. With the change which is suggested here, the local filesystem plug-in would use the path to a folder on Dropbox as directory, and that would be wrong. |
|
@Fedik @richard67 I'm not sure I understand. Is it possible to have multiple active filesystem plugins? If so, how does the Media Manager distinguish between them, i.e. know which one to use? |
|
@pjdevries Yes, the media manager can use multiple different filesystems. Each one is a base folder in the media manager. See the testing instructions in this PR: #33724 . In media manager you see 2 parent folders where you can navigate to children, the local "Images" and the "Dropbox" folder when doing as described in that PR. |
|
P.S.: Same when e.g. inserting an image into an article: You can use images from the local filesystem as well as from Dropbox when using that dropbox plugin (which is not ready yet and was developed just for testing, but I'm sure sooner or later you will find more such plugins in the extensions directoy). |
| * | ||
| * @copyright (C) 2017 Open Source Matters, Inc. <https://www.joomla.org> | ||
| * @license GNU General Public License version 2 or later; see LICENSE.txt | ||
| * @license GNU General Public License version 2 or later; see |
There was a problem hiding this comment.
please revert this change
There was a problem hiding this comment.
please revert this change
Not so hasty. The issue still exists. If this isn't the right solution, let's think of a better one.
But first some thoughts.
Thanks to @richard67 explanation, I now have a better understanding. I also better understand whu @Fedik says the path option in the new Media Manager configuration is obsolete. I not only tend to agree, but it makes things confusing. Maybe remove them, in which case the original 'images' as the default return value from the Local filesystem plugin makes much more sense.
On the other hand, as long as those paths in the Media Manager configration are there, why not use them the way I proposed. Is it that much different than the original code, returning 'images' as a default? After all, it's just a default. On a fresh Joomla! 4 installation, only the Local filesystem plugin is enabled by default but not yet explicitly configured. Only under those circumstances will the default be returned and returning the Media Manager path seems just as good (or bad) as 'images' :)
But I'm happy to change the default to something more sensible. Any suggestions?
There was a problem hiding this comment.
Not so hasty. The issue still exists. If this isn't the right solution, let's think of a better one.
Apologies to @brianteeman for not looking more closely what he was referring to.
|
I think we need to consider removing |
From what I understand from @richard67's explanation, I think you are right. If Media Manager no longer uses those paths but only the adapter settings, the issue that resulted in this PR vanishes automatically. A default return value of 'images' from the |
Seemed like a good idea to play a bit with multiple adapters, but I get this known error: 'Call to undefined method GuzzleHttp\Utils::chooseHandler()'. |
@pjdevries That's an issue with that plugin which was a quick made thing just for testing. You can find the solution here: #33724 (comment) |
Yes, works! Cool! |
Pull Request for Issue #34358 .
Summary of Changes
Changed the way directory settings are obtained in the
FileSystem - Localplugin. If mo directory has been explicitly chosen yet, return Media Manager's 'Path to 'Path to Files Folder' setting.Testing Instructions
Make sure you have a fresh Joomla! 4.0 installation, or at least one with an unaltered
FileSystem - Localplugin configuration.Change both path's of the Media Manager configuration, i.e. 'Path to Files Folder' and 'Path to Images Folder'.
Open Media Manager.
Beware of caching when fiddling with the Media Manager and its path settings! (see also #34239)
Actual result BEFORE applying this Pull Request
The result depends on proper configuration of the FileSystem - Local plugin. If no directory is chosen, which is the default, the plugin returns [{"directory": "images"}] and the Media Manager shows nothing.
Expected result AFTER applying this Pull Request
A list of files in folder 'Path to Files Folder', if there are any, or an empty folder otherwise.
Documentation Changes Required
It's probably a good thing to document this somewhere, preventing (new) users to get confused.