Skip to content

Conversation

@toitzi
Copy link
Contributor

@toitzi toitzi commented Nov 12, 2024

Q A
Branch? 4.0
Tickets api-platform/api-platform#2795, #6791
License MIT
Doc PR -

This is still work in progress and needs feedback

@toitzi
Copy link
Contributor Author

toitzi commented Nov 12, 2024

@soyuka Drafting this for now since there probably is some cleanup to do. Also not sure yet if this is the way this should be implemented (see my comments in api-platform/api-platform#2795).

@soyuka
Copy link
Member

soyuka commented Nov 14, 2024

Thanks so much for this, I should've pointed you to linkClass and linkProperty completely forgot about them...

@toitzi toitzi force-pushed the toitzi/relationships branch 3 times, most recently from 2cd2323 to be467c9 Compare November 14, 2024 17:44
@toitzi
Copy link
Contributor Author

toitzi commented Nov 14, 2024

@soyuka I think it is fine now. PR is done, switched to dependency injection and made a funtion for duplicated code

Should be ready to merge now!

Edit: And no worries about the linkClass i like debugging and digging deeper into the codebase, helps me understand it a bit better, which is great for the future. Thanks for you feedback i really appriciate it!

@toitzi toitzi marked this pull request as ready for review November 14, 2024 17:44
@toitzi toitzi force-pushed the toitzi/relationships branch 2 times, most recently from 0bf48af to 2a12e7b Compare November 14, 2024 17:46
fix loading relationships by only loading those related to the model

Closes: api-platform#6791
Signed-off-by: Tobias Oitzinger <[email protected]>
@toitzi toitzi force-pushed the toitzi/relationships branch from 2a12e7b to 9304b68 Compare November 14, 2024 17:47
@soyuka soyuka merged commit dc8c09b into api-platform:4.0 Nov 15, 2024
58 of 59 checks passed
@soyuka soyuka changed the title [Laravel][GraphQL] Fix Relationship Loading fix(laravel) graphQl Relationship loading Nov 15, 2024
@soyuka
Copy link
Member

soyuka commented Nov 15, 2024

Thanks for the fix!

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.

2 participants