-
Notifications
You must be signed in to change notification settings - Fork 16
IBX-10458: Implemented new Content Type search PHP API #633
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
base: 4.6
Are you sure you want to change the base?
Conversation
701a49c
to
5c19221
Compare
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.
Let's focus first on establishing final shape of the contracts.
src/lib/Persistence/Legacy/Content/Type/Gateway/CriterionHandler/ContentTypeGroupIds.php
Outdated
Show resolved
Hide resolved
src/contracts/Repository/Values/ContentType/Query/Criterion/Identifiers.php
Outdated
Show resolved
Hide resolved
src/contracts/Repository/Values/ContentType/Query/Criterion/ContentTypeGroupIds.php
Outdated
Show resolved
Hide resolved
src/contracts/Repository/Values/ContentType/Query/Criterion/ContainsFieldDefinitionIds.php
Outdated
Show resolved
Hide resolved
src/contracts/Repository/Values/ContentType/Query/ContentTypeQuery.php
Outdated
Show resolved
Hide resolved
src/contracts/Repository/Values/ContentType/Query/CriterionHandlerInterface.php
Show resolved
Hide resolved
b3df6c4
to
0859ce5
Compare
src/contracts/Persistence/Content/Type/CriterionHandlerInterface.php
Outdated
Show resolved
Hide resolved
tests/integration/Core/Repository/ContentTypeService/FindContentTypesTest.php
Show resolved
Hide resolved
src/contracts/Persistence/Content/Type/CriterionHandlerInterface.php
Outdated
Show resolved
Hide resolved
src/lib/Persistence/Legacy/Content/Type/Gateway/CriterionVisitor/CriterionVisitor.php
Outdated
Show resolved
Hide resolved
d25bd08
to
7242b33
Compare
foreach ($query->getSortClauses() as $sortClause) { | ||
$column = sprintf('c.%s', $sortClause->target); | ||
$joinedQueryBuilder->addOrderBy($column, $this->getQuerySortingDirection($sortClause->direction)); | ||
} |
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.
Why is sorting done on the second query, not on the one where you actually acquire IDs?
When done in the 2nd query, assuming that LIMIT caused later results to be discarded, the order of results will be incorrect.
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.
Isn't it the opposite? When I sort in the first query, I get sorted ids, correct. But do I have a guarantee that results in second query are sorted when fetched with WHERE IN
and without a sort clause? Tests show that I need to sort the second query.
/** | ||
* @param list<int>|int $value | ||
*/ | ||
public function __construct($value) |
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.
Is there an easy way to make it either array or int and not both? Would that provide to many limitations?
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.
We could potentially create second property and type it properly, but I'm not sure if that won't overcomplicate stuff
src/lib/Persistence/Legacy/Content/Type/Gateway/CriterionHandler/ContainsFieldDefinitionId.php
Outdated
Show resolved
Hide resolved
src/lib/Persistence/Legacy/Content/Type/Gateway/CriterionHandler/ContentTypeGroupId.php
Outdated
Show resolved
Hide resolved
$queryBuilder | ||
->select( | ||
[ | ||
'c.id AS ezcontentclass_id', |
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.
remember that we also changed those aliases - either, if possible, rename them now to ibexa_
or if those relay on exsiting mappers - remember to do it on mergeup.
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.
They rely on the mappers, but I'll keep in mind to change it during mergup, thanks!
f96e9a3
to
69eebef
Compare
69eebef
to
6d12b78
Compare
|
Caution
Table/column names/aliases mergup.
Description:
The
ContentTypeService
contract has been extended with a new method:The
ContentTypeQuery
object receives the following arguments:And the available criterions are:
ContainsFieldDefinitionIds
(joins field definitions and validates if a content type contains them)ContentTypeGroupIds
(joins content type group assignment table and searches for provided group ids)Identifiers
(content type identifiers)Ids
(content type ids)IsSystem
(joins content type group table via content type group assignment table and validates if group is system or not)LogicalAnd
LogicalOr
LogicalNot
There are also available the following sort clauses:
Ibexa\Contracts\Core\Repository\Values\ContentType\Query\SortClause\Id
Ibexa\Contracts\Core\Repository\Values\ContentType\Query\SortClause\Identifiers
Examplary query:
For QA:
Documentation: