Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageVariants have no asset collection assigned #3560

Closed
kdambekalns opened this issue Jan 11, 2022 · 6 comments
Closed

ImageVariants have no asset collection assigned #3560

kdambekalns opened this issue Jan 11, 2022 · 6 comments
Assignees

Comments

@kdambekalns
Copy link
Member

kdambekalns commented Jan 11, 2022

Description

When an ImageVariant is created by means of cropping in the Neos UI inspector, it is not assigned to an AssetCollection. This leads to issues if access to assets not being in an asset collection is limited.

Steps to Reproduce

  1. Block access to assets without an assigned asset collection
  2. Have an asset in an asset collection
  3. Create a crop of that asset

Expected behavior

The crop is created and assigned to the node property, becoming visible in the inspector (and the content, once applied.)

Actual behavior

The crop is in fact created, but since it is not accessible (due to step 1 above), it's metadata cannot be fetched (an error notice is shown) and the property will in fact be emptied.

Affected Versions

Neos: 5.3+

@kdambekalns
Copy link
Member Author

For a customer I solved this by using an aspect on ContentController.createImageVariantAction(), which includes a crude fix to solve #3559 through ObjectAccess.setProperty().

A possible proper solution would be: When an ImageVariant is created, have the constructor in https://github.com/neos/neos-development-collection/blob/5.3/Neos.Media/Classes/Domain/Model/ImageVariant.php#L84-L96 set the asset collections to those from the original asset.

@kdambekalns
Copy link
Member Author

Created a draft PR, so we can discuss this solution. Potential issue: When the original asset's asset collections are changed later, should that be kept in sync?

@kdambekalns
Copy link
Member Author

ping… any opinions on the above question?

@robertlemke
Copy link
Member

I agree with the solution. And I also think that it makes sense to keep the collections in sync. Everything else would be error-prone.

@mficzel
Copy link
Member

mficzel commented Mar 22, 2022

If the variants get collections assigned we should keep them in sync with the original asset, otherwise it gets really confusing.

Seems we are avoiding to seperate the variants from the assets. However this would be more of a long term topic.

@ru3fu5z
Copy link
Contributor

ru3fu5z commented Feb 3, 2023

Seems like this is still in issue in 7.3.10
Editors with asset restrictions by assetcollection run in an error, when using cropping images

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants