Skip to content

Conversation

@Tjeerd
Copy link
Contributor

@Tjeerd Tjeerd commented Apr 5, 2019

Not using the persistResult could cause issues in generating the IRI for
new resources because the ID was not set on the controllerResult but
only on the persistResult for example.

Q A
Bug fix? yes
New feature? no
BC breaks? not sure?
Deprecations? no
Tests pass? yes
Fixed tickets #2692
License MIT
Doc PR not relevant?

Copy link

@mikelerch mikelerch left a comment

Choose a reason for hiding this comment

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

Looks good.

@soyuka soyuka requested a review from teohhanhui April 5, 2019 09:52
@soyuka
Copy link
Member

soyuka commented Apr 5, 2019

May you add a regression test with your example?

@Tjeerd
Copy link
Contributor Author

Tjeerd commented Apr 5, 2019

May you add a regression test with your example?

I feel this already is, sort of, covered by the test I updated, but I will try to add a new test.

@Tjeerd
Copy link
Contributor Author

Tjeerd commented Apr 8, 2019

Ok, I've updated the test to have an assertion to test for any regression issues of the previous behaviour.

Not using the persistResult could cause issues in generating the IRI for
new resources because the ID was not set on the controllerResult but
only on the persistResult for example.
@teohhanhui teohhanhui merged commit 39900b9 into api-platform:2.4 Apr 9, 2019
@teohhanhui
Copy link
Contributor

Thanks @Tjeerd! 🎉

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants