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

EZP-30029: As a Developer I'd like API for bulk loading Content Info Items #2529

Merged
merged 1 commit into from
Feb 19, 2019

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented Jan 19, 2019

Question Answer
JIRA issue EZP-30029
New feature yes
Target version master
BC breaks no
Tests pass yes
Doc needed maybe

In order to optimize logic in RelationList/ParameterProvider, and other places eventually.

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

Design:
public function loadContentInfoList(array $contentIds, bool $filterOnUserPermissions = true): iterable;

REVIEW NOTE*

  • I'm not sold on adding bool $filterOnUserPermissions param myself, but it aligns with SearchService, and it also fits well with loadContentListByContentInfo which explicitly does not check permissions as it expects that to already have been done on provided content info.

@andrerom

This comment has been minimized.

@adamwojs

This comment has been minimized.

@andrerom

This comment has been minimized.

@adamwojs

This comment has been minimized.

@andrerom
Copy link
Contributor Author

andrerom commented Jan 31, 2019

@adamwojs / @alongosz Removed $filterOnUserPermissions and added some missing test coverage.

@andrerom
Copy link
Contributor Author

Merging as this is only for sanity checks anyway and hence will be tested as part of 2.5 certification, also I need this for upcoming PR also to master.

@andrerom andrerom merged commit f2dfe9c into master Feb 19, 2019
@andrerom andrerom deleted the bulk_load_content_info_api branch February 19, 2019 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants