acquia / reservoir

A back end for your front end: a content repository. Powered by Drupal 8, JSON API and OAuth2.
244 stars 30 forks source link

Need to provide ID when updating (PATCH) even though the URL contains the ID. #26

Open damontgomery opened 7 years ago

damontgomery commented 7 years ago

As described in the wiki guide, https://github.com/acquia/reservoir/wiki/Quick-Start-Guide#update-an-article

You need to make a request like,

PATCH to /jsonapi/node/article/141962d7-2180-4419-bb8e-a9c1d337aeb2

with body

{
  "data": {
    "id": "141962d7-2180-4419-bb8e-a9c1d337aeb2",
    "attributes": {
      "title": "Mr Patchy"
    }
  }
}

Which seems a bit unnecessary since the URL already contains the ID and I don't think you can pass multiple pieces of content at the same time.

wimleers commented 7 years ago

Thanks for reporting this! This is definitely a bug in https://www.drupal.org/project/jsonapi.

wimleers commented 7 years ago

Actually… turns out that the JSON API spec requires this: http://jsonapi.org/format/#document-resource-objects.

damontgomery commented 7 years ago

This makes sense at some level, but the spec seems to say that you need both type and id and that's not the case. You don't need to provide type and it still works. Maybe Drupal is parsing the URL and filling that in. In that case, I don't see why it can't do the same for id. I mean, it's not a hard thing to do either way, so it's definitely a could have.

wimleers commented 7 years ago

You're right! Issue created: https://www.drupal.org/node/2888889.

chris-hamper commented 7 years ago

Client applications should be written to conform to the JSON API spec, as that (hopefully) ensures that they will be compatible with all implementations of the JSON API spec. That's the reason for the spec, after all.

I think it's bad practice to expend extra effort only to enable poorly implemented clients to be created.

wimleers commented 7 years ago

@chris-hamper It's late here, so maybe that's why, but … I'm not sure I see what the point is that you're making?

damontgomery commented 7 years ago

I guess the idea is that everyone should provide id and type even if they don't need to since that's the specification.

It seems like Drupal doesn't require both right now and let's through requests that have enough for it to know what to do.

So, either Drupal JSON API should require both (and probably break sites) or require neither.

wimleers commented 7 years ago

@chris-hamper also commented at https://www.drupal.org/node/2888889#comment-12141288 — given that, I think his comment was really just supporting @damontgomery's remark/argumentation :)