EZP-30414: Relation fields aren't resolved #30
Conversation
|
@bdunogier is this ready for QA? |
| } | ||
|
|
||
| $contentItems = $this->contentLoader->find(new Query( | ||
| ['filter' => new Query\Criterion\ContentId($destinationContentIds)] |
There was a problem hiding this comment.
Tested this with Solr enabled and it will throw an exception when $destinationContentIds is an empty array, a suggestion would be to add the following before "$contentItems = ..."
if(empty($destinationContentIds)) { return $multiple ? [] : null; }
There was a problem hiding this comment.
Thank you, very good remark @crosa7. It should be fixed now.
bdunogier
force-pushed
the
object_relation_resolver_fix
branch
from
5505643 to
41a1e0f
Compare
|
@lserwatka Not yet. I have extracted the perf optimization commit, as it is not ready. I'll check @crosa7's feedback, and I'll mark it as ready for review (friday). |
bdunogier
force-pushed
the
object_relation_resolver_fix
branch
from
41a1e0f to
6cbe7cf
Compare
Always run a multiple search, and return the results depending on the multiple flag.
bdunogier
force-pushed
the
object_relation_resolver_fix
branch
from
6cbe7cf to
045fd7f
Compare
| @@ -136,24 +136,23 @@ public function resolveDomainFieldValue(Content $content, $fieldDefinitionIdenti | |||
|
|
|||
| public function resolveDomainRelationFieldValue(Field $field, $multiple = false) | |||
| { | |||
| if (!$field->value instanceof FieldType\RelationList\Value) { | |||
| throw new UserError("$field->fieldTypeIdentifier is not a RelationList field value"); | |||
| if ($field->value instanceof FieldType\RelationList\Value) { | |||
There was a problem hiding this comment.
Should it also work with ImageAsset fieldtype? It's also a Relation field if I'm not mistaken.
Also I'd extract the essence from this method and split into multiple methods for Relation, RelationList, ImageAssets to avoid nasty if statements.
There was a problem hiding this comment.
About Image Asset, no, not really. It does load the related asset, indeed, but does extra things, and doesn't have much in common with relations overall (see ImageAssetFieldResolver).
I'll see what I can do to make it more readable.
There was a problem hiding this comment.
There. I have moved the ID extraction to its own method. I could break it down into two, one for Relation, one for RelationList, but imho it is good enough that way. What do you think @webhdx ?
lserwatka
changed the title
The
RelationFieldDefinitionMapperis used for bothezobjectrelationandezobjectrelationlist. The resolver is also the same, but was not correctly updated forezobjectrelation, making it return null in every case for it.The resolver has been changed to always fetch multiple items, and return one or an array depending on the multiple flag.