-
-
Notifications
You must be signed in to change notification settings - Fork 907
Rework to improve and simplify identifiers management #3825
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
Rework to improve and simplify identifiers management #3825
Conversation
} | ||
|
||
return $id; | ||
$identifiersKeys = $subresourceIdentifiersKeys; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will change with the rework of the subresources
583f086
to
66caad1
Compare
793f94d
to
d4af145
Compare
@soyuka this broke custom identifiers, /**
* @ApiResource(
* itemOperations={
* "get": {
* "method": "GET",
* "path": "/customers/{id}"
* }
* }
* )
*/
Class Customer
{
/**
* @ApiProperty(identifier=false)
*
* @ORM\Id
* @ORM\Column(type="uuid")
* @ORM\GeneratedValue(strategy="NONE")
*/
protected UuidInterface $id;
/**
* @ApiProperty(identifier=true)
*
* @ORM\Column(type="string")
*/
protected string $externalId;
}
EDIT: changing the operation path from |
@bendaviesthe change is intentional I've taken great care to try and make the change as soft as possible. I may have forgotten something here. What you can do is: /**
* @ApiResource(
* itemOperations={
* "get": {
* "method": "GET",
* "path": "/customers/{id}",
* "identifiers": {"id": {Customer::class, "externalId"}}
* }
* }
* )
*/
Class Customer
{
/**
* @ApiProperty(identifier=false)
*
* @ORM\Id
* @ORM\Column(type="uuid")
* @ORM\GeneratedValue(strategy="NONE")
*/
protected UuidInterface $id;
/**
* @ApiProperty(identifier=true)
*
* @ORM\Column(type="string")
*/
protected string $externalId;
} Did it at least throw an error helping you fix it? |
@soyuka yes, this error was thrown:
thanks for the code sample using |
hm what if i remove my |
Yes removing the path should work! Indeed identifiers are not documented yet I need to find some time, in the meantime you can use https://github.com/api-platform/core/blob/master/docs/adr/0001-resource-identifiers.md as reference. |
works fine. But what if I need both id and slug as identifiers?
|
Move logic of identifiers extractor upwards, use only identifier converter downwards
TODO:
When removing deprecations on 3.0 we should:
The diff at https://github.com/api-platform/core/pull/3664/files can be used as a reference.