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

!!!FEATURE: Support asset collection count #3806

Open
wants to merge 2 commits into
base: 9.0
Choose a base branch
from

Conversation

sorenmalling
Copy link
Contributor

@sorenmalling sorenmalling commented May 24, 2022

Upgrade instructions

The SupportsCollectionInterface have a new method countByAssetCollection. If your AssetProxy implements that interface, you must include that method in your implementation

Review instructions

  • Checkout this PR
  • Add images to a certain collection
  • Add images from your AssetProxy to a collection
  • See the number adjust accordingly

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked wit !!! and have upgrade-instructions

@sorenmalling
Copy link
Contributor Author

For reference, this is a extension of the functionality created for tags

#3451

@bwaidelich
Copy link
Member

bwaidelich commented Jun 10, 2022

Isn't the master branch supposed to be readonly by now (so this should target 8.1 instead) ? – well, or probably 9.0 even since this is a breaking change?

@sorenmalling
Copy link
Contributor Author

Isn't the master branch supposed to be readonly by now (so this should target 8.1 instead) ? – well, or probably 9.0 even since this is a breaking change?

@bwaidelich Let me know, and I'll change it :)

@bwaidelich
Copy link
Member

@sorenmalling According to #3713 the master branch will be renamed to 8.1. So if this is a breaking change it should be against 9.0

@sorenmalling sorenmalling changed the base branch from master to 9.0 August 16, 2022 07:51
@Sebobo
Copy link
Member

Sebobo commented Jul 27, 2024

I fear this could get slow without further optimisation in case of many asset collections.
A native query that does all of this in one query would better. Similar to what I did in the Media.UI Flowpack/media-ui@7bf8e96

@kdambekalns kdambekalns self-requested a review January 28, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants