Add interface for ContentRichEntityRepository - #268
Draft
alexander-schranz wants to merge 4 commits into
Draft
Conversation
alexander-schranz
force-pushed
the
feature/content-rich-entity-repository
branch
from
August 9, 2024 10:58
e1e76e1 to
b112b18
Compare
Pull Request Test Coverage Report for Build 10318437173Details
💛 - Coveralls |
alexander-schranz
left a comment
Member
Author
There was a problem hiding this comment.
@Prokyonn what do you think about the interface?
| * @template T of ContentRichEntityInterface | ||
| * | ||
| * @phpstan-type ContentRichEntityRepositoryFilters array{ | ||
| * ids: array<string>, |
Member
Author
There was a problem hiding this comment.
this may is a ids or uuids so not sure about how we handle this. Should we go with identifiers @Prokyonn
Member
Author
There was a problem hiding this comment.
The ContentRichEntityInterface has a getId():
getId in the interface keep ids and may also just rename Artice:uuid to Article::id even if its a uuid. /cc @chirimoya
Member
There was a problem hiding this comment.
Technically speaking, id is just the abbreviation of identifier 😅
also writing ['identifier' => xx] feels more cumbersome than 'id' => xx 🤔
Although we may use uuid, I think I would still go with id as a string.
Comment on lines
+23
to
+32
| * locale?: string|null, | ||
| * stage?: string|null, | ||
| * categoryIds?: int[], | ||
| * categoryKeys?: string[], | ||
| * categoryOperator?: 'AND'|'OR', | ||
| * tagIds?: int[], | ||
| * tagNames?: string[], | ||
| * tagOperator?: 'AND'|'OR', | ||
| * templateKeys?: string[], | ||
| * loadGhost?: bool, |
Member
Author
There was a problem hiding this comment.
ids / identifiers is the only when which need be implemented the other filters are provided via the QueryEnhancer:
$this->dimensionContentQueryEnhancer->addFilters(
$queryBuilder,
'example',
ExampleDimensionContent::class,
$filters,
$sortBys
);
alexander-schranz
force-pushed
the
feature/content-rich-entity-repository
branch
from
August 9, 2024 11:12
b112b18 to
d22638b
Compare
alexander-schranz
force-pushed
the
feature/content-rich-entity-repository
branch
from
August 9, 2024 11:14
d22638b to
b4cc47b
Compare
Prokyonn
approved these changes
Aug 14, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We require a minimal interface which all content rich entity need to implemented. This interface should then be used to provide basic implementations of:
It should also replace the whole
DimensionContentRepositorywhich we sadly use at a lot of cases.