neos-development-collection
neos-development-collection copied to clipboard
BUGFIX: Show asset count for assets from readonly asset sources
For asset sources that have isReadOnly()
set, instead of the edit action, the show action is resolved by default. But the corresponding template is missing the "usages" button
Fixes: #3989
But one thing i still dont understand. I tested this out with the exact same change as you have in your PR. And in a asset source for example: https://github.com/daniellienert/assetsource-unsplash the asset count was still not showing up 🤔
Did you maybe just reload the show
action instead of going back to the list view and then click the image (to end up in the edit
action) after applying the fix?
instead of going back to the list view and then click the image (to end up in the edit action) after applying the fix?
I'm sure i did exactly this..
@crydotsnake Just to confirm that we don't talk cross purpose: With
I'm sure i did exactly this..
You mean, that you applied the fix, navigated back to the list, then detail view and could not reproduce the fix?
Actually there is no asset
property in those external asset sources I checked (including @crydotsnake example :wink:). And Neos\Media\Domain\Model\AssetSource\AssetProxy\AssetProxyInterface
also doesn't indicate that this would be a given.
As far as I can see {assetProxy.asset}
will only return an object for Neos\Media\Domain\Model\AssetSource\Neos\NeosAssetProxy
.
Actually there is no asset property in those external asset sources I checked
Ah right, good point. The custom implementations have to implement that if they want to support this feature, i.e.
final class SomeAssetProxy implements AssetProxyInterface, HasRemoteOriginalInterface, SupportsIptcMetadataInterface
{
/**
* @Flow\Inject
* @var AssetRepository
*/
protected $assetRepository;
/**
* Return the locally imported asset (if any)
* This is used in order to display the asset usages in the Neos.Media.Browser backend module
*
* @return AssetInterface|null
*/
public function getAsset(): ?AssetInterface
{
$localAssetId = $this->getLocalAssetIdentifier();
if ($localAssetId === null) {
return null;
}
/** @var AssetInterface|null $asset */
$asset = $this->assetRepository->findByIdentifier($localAssetId);
return $asset;
}
// ...
not beautiful at all.. but this way the limitation can be worked around
Thanks @ahaeslich for the clarification! So i assume this has to be fixed in the package itself :)
What would be the best way too fix this in other asset source packages?
Ping
What would be the best way too fix this in other asset source packages?
I don't know.. :-/
I think inUse is evaluated on every call
I'm not sure I understand what you mean by "on every call". This PR just modifies the Show
template, i.e. the display of a single asset
You added the line <f:if condition="{assetProxy.asset.inUse}">
which will call all AssetUsageStrategies.
I don't know how to proceed with this one.. @Sebobo you blocked this due to performance concerns. That's a fair point of course, but I don't see a different easy way to solve #3989 – do you have a suggestion?
I would show the button without checking whether there are actual usages. In a medium sized project it can take up to a minute to calculate the usage. This would be super annoying when one wants to simply view the assets details.
@Sebobo The button shows the usage count. You suggest to change that?
@Sebobo just to make sure that we're on the same track:
Currently, the button shows the usage count like:
Besides, it is not rendered when there are no usages. We could change that to always show the button with a more generic label like "show usages" in any case and then resolve usages only if its clicked.
But: We currently disable the Delete
Button in the footer if the asset is used, so this would have to be changed too..
In a medium sized project it can take up to a minute to calculate the usage
That's really annoying (and we should fix that by integrating something like Wwwision.AssetUsage to the core). But I don't see how that is related to this fix.
So could you please clarify what you suggest me to do to get a +1 (or at least no -1)?
I get back to you on Monday. Have to check the code and Pr again from my computer instead of my phone 🤪
OK now I know again. So the edit action already had this behaviour and had the annoying performance problem with assets that had more than a few usages.
With this change the show action which is also used by the UI editor would then have the same problem. I think at least the show action should remain lightweight and fast and not get slower now. Also this information is not as important there than it is in the editing mode.
I wonder whether we can extend the condition to fulfil both needs 🤔
I'm not 100% sure, but IIRC we display the "show" template for readonly asset sources and the "edit" template for the others. As a result we don't display the usage count for readonly asset sources – which is a really bad limitation.
I need to dig into this once again – but maybe that is already fixed with the new media UI?
Yes the new Media UI handles this by showing the usage count directly on the button if a fast strategy is available and showing them only on demand after the button is pressed if not. Similar behaviour with the delete button.