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
Schema should allow link objects as pagination links #1379
Schema should allow link objects as pagination links #1379
Conversation
schema
Outdated
@@ -292,28 +292,28 @@ | |||
"first": { | |||
"description": "The first page of data", | |||
"oneOf": [ | |||
{ "type": "string", "format": "uri-reference" }, | |||
{ "$ref": "#/definitions/linkage" }, |
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.
s/linkage/link/
I suspect this is a typo caused by IDE autocompletion :)
This appears to be essentially the same problem as #1159 encountered and #1160 fixed. The spec talks about pagination links as being a set of 4 keys that MUST be used for pagination links, and it also talks about how each link MUST be either a string or a link object. The schema currently requires it to be a string, and that's what this PR fixes (at least once the typo I mentioned above is fixed). |
Probably caused by your IDE's autocompletion :) (Fixing this per json-api#1379 (review).)
Created PR to fix my comment above (#1379 (review)) |
Probably caused by your IDE's autocompletion :) (Fixing this per json-api#1379 (review).)
Thanks Wim! |
LGTM! 🚢 EDIT: as they say, the proof is in the pudding: https://www.drupal.org/project/jsonapi/issues/3037852#comment-13004970 :) |
As a rule, our JSON:API implementation uses link objects for every link. This ensures clients written against our implementation are maximally forward compatible (so they don't break if and when a link has metadata added to it).
We've even applied this rule to pagination links. We've recently discovered that our responses are failing validation for this reason.
It seems like this is a small oversight in the schema and is not an intentional validation rule.
I need to validate that this schema change is correct tomorrow. However, I wanted to throw the PR out there for any initial reactions in case it actually is intentional and I've missed something. Therefore, I'm leaving this as a draft (which, BTW, is a nifty new GH feature!).