5.3 UnitTests #930
5.3 UnitTests #930
Conversation
7a330d7 to
1a6f337
Compare
3238367 to
e82ad50
Compare
|
@jenssegers Finally all tests fully passed :) Would you please Review and Approve this PR ? |
| $relationCount = array_count_values(array_map(function ($id) { | ||
| return (string) $id; // Convert Back ObjectIds to Strings | ||
| }, $query->pluck($relation->getHasCompareKey()))); | ||
| }, is_array($relations) ? $relations : $relations->toArray())); |
There was a problem hiding this comment.
This tests fail without this check:
- RelationsTest.php:384
- RelationsTest.php:433
Error: array_map(): Argument #2 should be an array
Laravel has returns arrays instead of Collections!
|
Looking good! |
| @@ -233,11 +234,11 @@ public function getAttribute($key) | |||
|
|
|||
| // This attribute matches an embedsOne or embedsMany relation so we need | |||
| // to return the relation results instead of the interal attributes. | |||
| if ($relations instanceof EmbedsOneOrMany) { | |||
| if ($relations instanceof EmbedsOneOrMany or $relations instanceof MorphTo) { | |||
There was a problem hiding this comment.
This seems a bit odd, this is very specific to embedded relations, why was MorpTo added here?
There was a problem hiding this comment.
RelationsTest.php:367 : Failed asserting that null is an instance of class "User".
As a workaround to working MorphTo Relations will be directly returned and resolved.
Is it necessary to separate this logical part?
There was a problem hiding this comment.
Might be better to split the logic indeed, I'll have to check what exactly is going wrong because this is weird.
|
Let me know when your changes are done! |
|
@jenssegers last commit was due to accidental fast-forward :) if you think something is missing just let me know :) |
|
Ill release this as an RC and go over everything soon. |
5.3 UnitTests
I'm working on Unit Tests to be fully passed.
Update: All tests passed!