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

fix: search on nested sub-entity that doesn't use "id" as its ORM id #5623

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

mrossard
Copy link
Contributor

Q A
Branch? 3.1
Tickets #5605

This should fix that use case when ids are not composite.
Composite IDs would still need to be addressed with a custom filter, but that's even more of a fringe case...and would require quite of lot of work as far as i can see. I still left todo comments in case someone wants to tackle that.

@mrossard
Copy link
Contributor Author

Just noticed #5618 - not much work to merge as far as i see, but both PRs share a goal to be able to use identifiers that are not always "int $id".

/**
* @var DummyWithSubEntity $entity
*/
$entity = $this->itemProvider->provide($operation, $uriVariables, $context);
Copy link
Member

Choose a reason for hiding this comment

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

I still want to check why this is needed.

Copy link
Contributor Author

@mrossard mrossard Jun 15, 2023

Choose a reason for hiding this comment

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

I think a simple explanation is that entityClass gives us the link from the ApiResource to the entity, but nothing really tells us how to convert an entity to a resource, so when the framework gets a subentity it doesn't have a way to know which resource it's supposed to convert it to if you don't do it explicitly in a custom provider.

@soyuka soyuka merged commit b8cbdb1 into api-platform:3.1 Jul 5, 2023
@soyuka
Copy link
Member

soyuka commented Jul 5, 2023

Thanks @mrossard !

soyuka added a commit to soyuka/core that referenced this pull request Aug 11, 2023
soyuka added a commit that referenced this pull request Aug 11, 2023
Romaixn pushed a commit to Romaixn/core that referenced this pull request Oct 3, 2023
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