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: Missing interface in Handle doctrine links guide #6897

Closed
wants to merge 2 commits into from

Conversation

Crovitche-1623
Copy link

Q A
Branch? 4.1
Tickets Closes api-platform/docs#2121
License MIT
Doc PR

In the documentation it's written:

The handleLinks option takes a service name implementing the LinksHandlerInterface or a callable.

I don't know if it's intended but the Employee entity does not implement this interface. I saw that the interface namespace was not the same depending on if we're using Doctrine ODM, Doctrine ORM or Eloquent but I think it must be written somewhere. Otherwise developers will not be notified if the interface change over time.

@Crovitche-1623
Copy link
Author

Note: the tests seems to fail because the existing handleLinks method is no longer static. Should I set it non-static in this PR ?

@@ -429,6 +429,7 @@ public function register(): void
$this->app->bind(OperationMetadataFactoryInterface::class, OperationMetadataFactory::class);

$this->app->tag([
BooleanFilter::class,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you do that?

@@ -29,7 +31,7 @@
)]
#[Get('/company/{companyId}/employees/{id}')]
#[ORM\Entity]
class Employee
class Employee implements LinksHandlerInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

A callable is already used here using stateOptions: new Options(handleLinks: [Employee::class, 'handleLinks'])

@vinceAmstoutz
Copy link
Contributor

Not need, for more info: api-platform/docs#2121 (comment)

@soyuka soyuka closed this Jan 9, 2025
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.

3 participants