Skip to content
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

Handle field without mappedBy #6219

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

VincentLanglet
Copy link
Member

Subject

I am targeting this branch, because BC.

Closes #6218.

Changelog

### Fixed
- Allow to use AdminType with unidirectional field.

@VincentLanglet VincentLanglet requested a review from a team July 22, 2020 21:15
@phansys phansys added the patch label Jul 22, 2020
@VincentLanglet VincentLanglet requested a review from a team July 22, 2020 22:54
@jordisala1991 jordisala1991 merged commit aa71ab4 into sonata-project:3.x Jul 23, 2020
@jordisala1991
Copy link
Member

Thank you @VincentLanglet

@haivala
Copy link

haivala commented Jul 31, 2020

I think because of this there should be release 3.72.1

@VincentLanglet
Copy link
Member Author

I think because of this there should be release 3.72.1

I'll make a release today. But it will be 3.73

@VincentLanglet VincentLanglet deleted the fixSetObject branch July 31, 2020 07:26
@haivala
Copy link

haivala commented Jul 31, 2020

thank you!

@cjgordon
Copy link

cjgordon commented Aug 11, 2020

Hi @VincentLanglet

I believe this breaks admins using doctrine MongoDB and embedded collections as there is no mappedBy attribute for these mappings.

Example mapping:

     /**
      * @MongoDB\EmbedMany(targetDocument="Comment")
      */
     private $comments;

Break happens on: ObjectManipulator.php line 56 with error message: 'Notice: Undefined index: mappedBy'

I believe you just need to add a check to ensure that if the mappedBy property is not present that instance is returned. I have tested this myself and it has resolved the issue.

        if (!isset($parentFieldDescription->getAssociationMapping()['mappedBy'])) {
            return $instance;
        }
        
        $mappedBy = $parentFieldDescription->getAssociationMapping()['mappedBy'];

Cheers
Chris

@VincentLanglet
Copy link
Member Author

HI @cjgordon.

if (!isset($parentFieldDescription->getAssociationMapping()['mappedBy'])) {
    return $instance;
}

$mappedBy = $parentFieldDescription->getAssociationMapping()['mappedBy'];

Could be a better way to write this, since isset(null) is false.

As a quick fix, do you want to make the PR ?

Also could you dump the $parentFieldDescription->getAssociationMapping() value ? To know if there is something similar to mappedBy in order to provide the same feature to mongoDB than I provided to doctrine orm.

@cjgordon
Copy link

@VincentLanglet I'll do a PR for this on the weekend.

@VincentLanglet
Copy link
Member Author

I done this @cjgordon #6297

@cjgordon
Copy link

Excellent thank you. Apologies for being slack on this.

VincentLanglet added a commit to VincentLanglet/SonataAdminBundle that referenced this pull request Aug 22, 2020
greg0ire pushed a commit to VincentLanglet/SonataAdminBundle that referenced this pull request Aug 22, 2020
That attribute is not guaranteed to exist, that's for example the case
of @mongodb\EmbedMany

See sonata-project#6219 (comment)
jordisala1991 pushed a commit that referenced this pull request Aug 22, 2020
That attribute is not guaranteed to exist, that's for example the case
of @mongodb\EmbedMany

See #6219 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants