-
-
Notifications
You must be signed in to change notification settings - Fork 895
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
fix(graphql): nested collection for mongo #6174
Conversation
tests/Fixtures/TestBundle/Document/MultiRelationsNestedPaginated.php
Outdated
Show resolved
Hide resolved
fcce827
to
206a83b
Compare
@soyuka I am done now with the changes you requested. The failing unit tests are not due to my changes (some deprecation notice). From my perspective this PR could be merged. |
@@ -89,7 +89,7 @@ public function addOneToManyRelation(MultiRelationsRelatedDummy $relatedMultiUse | |||
|
|||
public function getNestedCollection(): Collection | |||
{ | |||
return $this->nestedCollection; | |||
return $this->nestedCollection->map(fn ($entry) => ['name' => $entry->name]); |
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.
I'm not fond of this why is this necessary?
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 mapping function was neccessary because mongo ODM behaves different like ORM does. The ORM Entity provides a Collection with each entry being an associative array. The ODM Document on the other hand provides a Collection with each entry being an instance of MultiRelationsNested
/ MultiRelationsNestedPaginated
. So my intention here was to align the Testables here to get the same results.
Without this mapper the case for MultiRelationsNestedPaginated
gets broken in test. If this is not acceptable for you, you may undo this mapping and find another way - I was not able to.
If you can add a documentation for that feature it'd be awesome, thanks! |
#6038 was introduced but we had mongodb tests disabled. Though the new scenario has been disabled temporarily for mongo. I activated it again and updated the mongo test fixtures to represent the same case as the ORM ones do.