Skip to content

Generate iri for child related resources #2905

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 19, 2019
Merged

Generate iri for child related resources #2905

merged 1 commit into from
Jul 19, 2019

Conversation

@bastnic
Copy link
Contributor

@bastnic bastnic commented Jul 4, 2019 •
edited by teohhanhui
Loading

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets api-platform/api-platform#1159
License MIT
Doc PR N/A

For child relations, iri is not set and it breaks varnish cache invalidation. See api-platform/api-platform#1159 for tests samples

@bastnic bastnic changed the base branch from master to 2.4 July 4, 2019 15:28
@bastnic
Copy link
Contributor Author

bastnic commented Jul 4, 2019

I do not have the issue, but I suppose we should also unset it for the iterator case https://github.com/api-platform/core/pull/2905/files#diff-3e45e8e6ce08f8a7cbbbda1601d5ff29R535

@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 4, 2019

Could you add a regression test?

@teohhanhui teohhanhui self-assigned this Jul 4, 2019
@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 4, 2019

I suppose we should also unset it for the iterator case

Yes please. 😄

@bastnic bastnic force-pushed the feature/fix-child-iri branch from bf24bcb to 12470f2 Compare July 4, 2019 20:19
@bastnic
Copy link
Contributor Author

bastnic commented Jul 4, 2019

@teohhanhui something like that?

bastnic
bastnic commented Jul 4, 2019
View reviewed changes
teohhanhui
teohhanhui suggested changes Jul 5, 2019
View reviewed changes
@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 5, 2019

But hmm... I think we should have a Behat functional test for this.

@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 19, 2019 •
edited
Loading

@bastnic Do you mind allowing access to your branch? https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork

Never mind, I'll take your commit and open another PR. 😄

@bastnic
Copy link
Contributor Author

bastnic commented Jul 19, 2019

not at all, and it seems that I already allowed maintainers.

@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 19, 2019

@bastnic Okay, maybe it's a bug with the code suggestions feature of GitHub. 😄

@teohhanhui teohhanhui force-pushed the feature/fix-child-iri branch from 12470f2 to fb86294 Compare July 19, 2019 12:02
@teohhanhui teohhanhui requested review from dunglas and soyuka July 19, 2019 12:40
@soyuka soyuka merged commit 269b052 into api-platform:2.4 Jul 19, 2019
@soyuka
Copy link
Member

soyuka commented Jul 19, 2019

thanks @bastnic @teohhanhui !

@bastnic
Copy link
Contributor Author

bastnic commented Jul 19, 2019

Thanks @teohhanhui and @soyuka 👍

@bastnic bastnic mentioned this pull request Oct 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soyuka soyuka soyuka approved these changes

@dunglas dunglas Awaiting requested review from dunglas

+1 more reviewer

@teohhanhui teohhanhui teohhanhui approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@teohhanhui teohhanhui

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bastnic @teohhanhui @soyuka