Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Serializer] Fix serialization of items with groups across entities and discrimination map#27826
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Serializer] Fix serialization of items with groups across entities and discrimination map#27826
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Test failures are un-related. |
voodoo-dn commentedJul 3, 2018
This PR works well and fix my problem, thanks. |
| @@ -142,11 +142,16 @@ protected function getAllowedAttributes($classOrObject, array $context, $attribu | |||
| return false; | |||
| } | |||
| if (null !== $this->classDiscriminatorResolver && null !== $discriminatorMapping = $this->classDiscriminatorResolver->getMappingForMappedObject($classOrObject)) { | |||
| $allowedAttributes[] = $attributesAsString ? $discriminatorMapping->getTypeProperty() : new AttributeMetadata($discriminatorMapping->getTypeProperty()); | |||
| $class = is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject; | |||
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.
is_object should be fully-qualified here, to benefit from the optimized opcode.
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.
btw, shouldn't it go inside theif (null !== $this->classDiscriminatorResolver) { ?
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.
Good points, updated 👍
65e0370 toc648b93CompareThere 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.
(failures unrelated)
(sure we don't need more tests?)
@nicolas-grekas the fixture update covers the use-cases I think about so I'd say no 🤔 |
Thank you@sroze. |
… entities and discrimination map (sroze)This PR was merged into the 4.1 branch.Discussion----------[Serializer] Fix serialization of items with groups across entities and discrimination map| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#27816,#27641| License | MIT| Doc PR | øI believe this approach is better than the one taken in#27816. At least, it's an alternative :)Commits-------c648b93 Fix serialization of abstract items with groups across multiple entities
Uh oh!
There was an error while loading.Please reload this page.
I believe this approach is better than the one taken in#27816. At least, it's an alternative :)