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
feat: BackedEnum resources #6309
base: main
Are you sure you want to change the base?
feat: BackedEnum resources #6309
Conversation
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.
would love to see json-ld supported, I need to add my commit to this at some point :)
8c96867
to
8acce05
Compare
@@ -59,6 +59,9 @@ public function createLinksFromIdentifiers(Metadata $operation): array | |||
|
|||
$link = (new Link())->withFromClass($resourceClass)->withIdentifiers($identifiers); | |||
$parameterName = $identifiers[0]; | |||
if ('value' === $parameterName && enum_exists($resourceClass)) { | |||
$parameterName = 'id'; | |||
} |
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.
Does this match what you had in mind @soyuka?
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.
Uri variable should be as followed:
uriVariables: ['id' => new Link(parameterName: 'id', identifiers: ['value'])]
in the metadata directly so that we don't need any condition here right?
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.
Apologies, but I'm confused again 😇 … I'm probably missing something obvious, but the Link
is constructed directly above, and the parameterName
is set at return: $link->withParameterName($parameterName)
… So is your suggestion how that return array should be shaped? If so what about non-enums?
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.
Which raises another thing about the createLinksFromIdentifiers
method … If the $identifier
array is empty there is an early return (lines 56-58), and then there is the if (1 < \count($identifiers)) {
(lines 66-69) test that looks like dead code, correct?
Scrap this one, I was reading the comparison backwards in my head … confusion reigns today 😟
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.
I need to check this into the details, will try to find some time!
I would use URIs in GraphQL too, as we do for existing IDs and relations. WDYT? |
@dunglas it must be my week for getting confused. 🌞 I'm very happy to make required changes, but could you be so kind as to give a bit more context, please? |
Please look at how I configure a backed enum as an API resource so that each instance has an IRI just like any other API resource. https://github.com/gnito-org/problem-replicator-api-platform-enum-example This is important for machine discovery of APIs and for consistency. |
Thank you @gnito-org, I can reproduce it here. |
e99f735
to
3df9f0b
Compare
37672db
to
b088d10
Compare
e227f3c
to
a069f68
Compare
Intended for 3.4, so now it is just for review.
Depends on #6288 as I want to refactor
BackedEnumPlainResourceTest
test methods into those classes.BackedEnum
svalue
by default, and implementingfunction getId()
, etc, works tooGet
&GetCollection
unless otherwise configuredGET /statuses
GET /statuses/1