-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add interface for ContentRichEntityRepository #268
base: 0.9
Are you sure you want to change the base?
Add interface for ContentRichEntityRepository #268
Conversation
e1e76e1
to
b112b18
Compare
Pull Request Test Coverage Report for Build 10318437173Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prokyonn what do you think about the interface?
* @template T of ContentRichEntityInterface | ||
* | ||
* @phpstan-type ContentRichEntityRepositoryFilters array{ | ||
* ids: array<string>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this may is a ids
or uuids
so not sure about how we handle this. Should we go with identifiers
@Prokyonn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ContentRichEntityInterface has a getId()
:
public function getId(); |
getId
in the interface keep ids
and may also just rename Artice:uuid
to Article::id
even if its a uuid. /cc @chirimoya
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
* locale?: string|null, | ||
* stage?: string|null, | ||
* categoryIds?: int[], | ||
* categoryKeys?: string[], | ||
* categoryOperator?: 'AND'|'OR', | ||
* tagIds?: int[], | ||
* tagNames?: string[], | ||
* tagOperator?: 'AND'|'OR', | ||
* templateKeys?: string[], | ||
* loadGhost?: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
);
b112b18
to
d22638b
Compare
d22638b
to
b4cc47b
Compare
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
DimensionContentRepository
which we sadly use at a lot of cases.