[4.0] refactor the media adapters

[dgrammatiko]

Pull Request for Issue #26750 (comment).

Summary of Changes

This is a proposal to change the adapters from ie: local-0 to ie: local-images as sated #26750 (comment).
Probably if this is ok (I think the second part would be better if it's md5($name)) would be merged to #33724 for easier testing?

@joomdonation @Fedik @bembelimen your thoughts here?

Testing Instructions

Goto: administrator/index.php?option=com_plugins&view=plugins&filters=[type=filesystem]
Edit FileSystem - Local plugin by adding a new path, eg installation
Check the media manager
Re-edit the previous plugin by changing the order of the folders (first installation, second images)
Check again the media manager still operates fine

Also, edit any article (or create a new one):

• with tinMCE try to upload an image with drag and drop
• try to select an image for intro or full position

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

There shouldn't be any difference for end-users. This PR just ensures that the adapters are not index related.

Documentation Changes Required

[richard67]

@dgrammatiko The linter is still a bit unhappy: https://ci.joomla.org/joomla/joomla-cms/44184/1/21 :

/********/src/administrator/components/com_media/resources/scripts/store/state.es6.js
  13:15  error  'key' is assigned a value but never used  no-unused-vars

[dgrammatiko]

@dgrammatiko The linter is still a bit unhappy

That was an easy fix but this PR is breaking B/C. (Anyways I was asked if I could help to transition from the index-based naming to this new name-based, so I guess this is already justified)

[Fedik]

looks good, if it works ;)

I not remember where exactly, in past I seen places where second part of the adapter is cast to integer,
I mean stuff like:

$blabla = (int) $part[1];

[dgrammatiko]

I not remember where exactly, in past I seen places where second part of the adapter is cast to integer,

A quick search returned no casting (int) results in com_media or the plugin

[Fedik]

A quick search returned no casting

Maybe I mixed something, thanks for checking

[joomdonation]

Almost there. To make it works well (hard to explain), we will have to change this block of code to:

foreach ($directories as $directoryEntity)
{
	if ($directoryEntity->directory)
	{
		$directoryPath = JPATH_ROOT . '/' . $directoryEntity->directory;
		$directoryPath = rtrim($directoryPath) . '/';

		$adapter = new \Joomla\Plugin\Filesystem\Local\Adapter\LocalAdapter(
			$directoryPath, $directoryEntity->directory
		);

		$adapters[$adapter->getAdapterName()] = $adapter;
	}
}

Now, the name of the adapters have this format: local-images, local-sampledata
images, sampledata is returned by $adapter->getAdapterName() method, so later, to allow the system to find the right adapter from path in the request, we will have to use $adapter->getAdapterName() as key name of the item.

[joomdonation]

I hope this is understandable. Hard to explain but it is still not clear, I will try to find a better way to explain. I only see this problem while checking to see why dropbox adapter does not work after this change.

[dgrammatiko]

Done

[joomdonation]

The parameters name of function here is a bit confusing. How about changing it to const getDrives = (adapterNames, provider). In the server side, we call local, dropbox as providers, so I think we should make the same change here to make it consistent.

[dgrammatiko]

I'm not sure if we need to change anything here as the object (client-side) remains the same:

[dgrammatiko]

Ok, done, arguments renamed

[joomdonation]

Thanks @dgrammatiko. All good. I will now run a final test.

[joomdonation]

Sorry. Could you please do a final change https://github.com/joomla/joomla-cms/blob/4.0-dev/layouts/joomla/form/field/media.php#L107 ? Change local-0 to local-images

Later it will be changed anyway. But this is to avoid when your PR merged, people use nightly build won't get error while trying to select an image for article.

[richard67]

I have tested this item ✅ successfully on 04fa696

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34069.}

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34069.}

[joomdonation]

@rdeutz @wilsonge Could we have this PR merged ? I have an open RB PR which depends on this one and want to have it finished ASAP.

[Fedik]

Thanks!

[brianteeman]

I got this error a few times but its gone now

[dgrammatiko]

There's a sessionStorage entry, so unless you do some update without killing the session then this shouldn't be a problem

[dgrammatiko]

Yeah, but I think it's highly unlikely to hit all these requirements at once...

[brianteeman]

Highly unlikely as in two people already did

[richard67]

Is there anything we can do against it?

[dgrammatiko]

Is there anything we can do against it?

We could empty the sessionStorage key before the update, but it won't cover all the updade paths

[dgrammatiko]

But we can ask people to log out after the update and re-login (in case they used com_media right before the update). No code needed