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

Checking the output option when calculating property metadata #3696

Merged

Conversation

weaverryan
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? this could change behavior for people relying on bug
Deprecations? no
Tickets none
License MIT
Doc PR not needed

Hi!

Reproducer with instructions here: https://github.com/SymfonyCasts/api-platform/tree/embedded-output-iri-embedded-bug

It's super tricky and subtle. The tl;dr is that when the readableLink of PropertyMetadata is calculated for an embedded property, it uses the serializer groups from the target resource class when it should use the output= class (because that is what will actually be serialized).

I didn't include a test (yet) - I'd like some validation that this it the correct place/idea for this.

Thanks!

@dunglas dunglas requested a review from soyuka August 30, 2020 19:04
@dunglas
Copy link
Member

dunglas commented Aug 30, 2020

+1 on my side! Thanks for the patch.

@soyuka
Copy link
Member

soyuka commented Aug 31, 2020

👍 about the patch, I think that this could go on 2.5

@dunglas
Copy link
Member

dunglas commented Sep 1, 2020

Could you rebase against 2.5 please? It should fix the tests.

@weaverryan weaverryan force-pushed the check-output-class-for-metadata branch from fca73e0 to 4fbd332 Compare September 4, 2020 18:38
@weaverryan
Copy link
Contributor Author

Test fails are legit - I need to check them!

Base automatically changed from master to main January 23, 2021 21:59
@alanpoulain alanpoulain changed the base branch from main to 2.6 March 2, 2021 09:36
@alanpoulain alanpoulain force-pushed the check-output-class-for-metadata branch 3 times, most recently from 1919e68 to 9aab416 Compare March 2, 2021 10:16
@alanpoulain alanpoulain force-pushed the check-output-class-for-metadata branch from 9aab416 to 9a98530 Compare March 2, 2021 10:24
@alanpoulain alanpoulain merged commit cecd948 into api-platform:2.6 Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants