Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[Fix] embed relations serialization #3410

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

Open
florianJacques wants to merge 8 commits into mongodb:5.x
base: 5.x
Choose a base branch
Loading
from florianJacques:fix-embed-relation-serialization

Conversation

Copy link
Contributor

@florianJacques florianJacques commented Jun 17, 2025 •
edited
Loading

Fixed the serialization of embed relations.
These relationships were not necessarily loaded, and could not use their attribute mutators.

Checklist

  • Add tests and ensure they pass

Copy link
Contributor Author

Hello,
Why Coding Standards / phpcs (pull_request)
Fail ?

Copy link
Member

GromNaN commented Jun 17, 2025

In order to review the PR properly, could you add some tests that are fixed by the change?

Why Coding Standards / phpcs (pull_request) Fail?

GitHub Action fails to commit the fixes that are found by phpcs.

----------------------------------------------------------------------
FILE FIXED REMAINING
----------------------------------------------------------------------
src/Eloquent/EmbedsRelations.php 2 0
src/Eloquent/DocumentModel.php 1 0
----------------------------------------------------------------------
A TOTAL OF 3 ERRORS WERE FIXED IN 2 FILES
----------------------------------------------------------------------

Copy link
Contributor Author

Yes of course, I will write the tests in the next few days
For CI errors, can I do something?

Copy link
Member

GromNaN commented Jun 17, 2025

For CI errors, can I do something?

You can run vendor/bin/phpcbf locally to fix the CS issues.

Copy link
Contributor Author

Thanks, Done! I'll write the tests when I have some time...

Copy link
Contributor Author

Adding test ok

Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this is more of a documentation issue that something we should fix in the code.

As for any relationship, if you want to load the data from an embedded relationship you have to use the Model::$with property or call Builder::with().

paulinevos reacted with thumbs up emoji
$user->addresses()->saveMany([new Address(['city' => 'London'])]);

//Reload document
$user = UserWithEmbeds::where('name', 'John Doe')->first();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to get the address property correctly casted, you must call with('address'):

Suggested change
$user = UserWithEmbeds::where('name', 'John Doe')->first();
$user = UserWithEmbeds::where('name', 'John Doe')->with('addresses')->first();


class UserWithEmbeds extends User
{
protected $withEmbeds = ['addresses'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this new configuration to "fix" the bugged behavior of the embed relationship seems wrong.

The default $with property can be leveraged:

Suggested change
protected $withEmbeds = ['addresses'];
protected $with = ['addresses'];

*
* @return bool
*/
public function isEmbeddedRelation($key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new method is not used and not tested. It doesn't seems to be part of the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GromNaN GromNaN GromNaN requested changes

@paulinevos paulinevos Awaiting requested review from paulinevos paulinevos is a code owner automatically assigned from mongodb/dbx-php

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /