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

JsonApi::Deserialization YARD docs are misleading #2384

Open
wasabigeek opened this issue Jun 26, 2020 · 9 comments
Open

JsonApi::Deserialization YARD docs are misleading #2384

wasabigeek opened this issue Jun 26, 2020 · 9 comments

Comments

@wasabigeek
Copy link

wasabigeek commented Jun 26, 2020

Expected behavior vs actual behavior

In the YARD docs the example suggests that using a symbol-keyed hash will work:

# document = {
# data: {
# id: 1,
# type: 'post',
# attributes: {
# title: 'Title 1',
# date: '2015-12-20'
# },
# associations: {
# author: {
# data: {
# type: 'user',
# id: 2
# }
# },
# second_author: {
# data: nil
# },
# comments: {
# data: [{
# type: 'comment',
# id: 3
# },{
# type: 'comment',
# id: 4
# }]
# }
# }
# }
# }

(Edited)

But in the code, it will actually check via string-keys:
https://github.com/rails-api/active_model_serializers/blob/v0.10.10/lib/active_model_serializers/adapter/json_api/deserialization.rb#L118

@wasifhossain
Copy link
Member

In the YARD docs it suggests that using a symbol-keyed hash will work:

# keys: Hash of translated keys (e.g. :author => :user).

it says about the keys option; not about the data found inside the JSON API document.

you may take a look at an example how it can be used:

# parse(document, only: [:title, :date, :author],
# keys: { date: :published_at },
# polymorphic: [:author]) #=>

@wasifhossain
Copy link
Member

wasifhossain commented Jun 27, 2020

But in the code, it will actually check via string-keys:

because payload is expected to be a valid JSON API document, either string-keys or ActionController::Parameters, as mentioned in:

# @param [Hash|ActionController::Parameters] document

and in the latter case, its converted to a hash with string-keys, as can be seen in:

document = document.dup.permit!.to_h if document.is_a?(ActionController::Parameters)

@wasifhossain
Copy link
Member

wasifhossain commented Jun 27, 2020

to be honest, I have found one gotcha though: it should be relationships according to the JSON API spec in place of associations on L35, as found in:

# document = {
# data: {
# id: 1,
# type: 'post',
# attributes: {
# title: 'Title 1',
# date: '2015-12-20'
# },
# associations: {
# author: {
# data: {
# type: 'user',
# id: 2
# }
# },
# second_author: {
# data: nil
# },
# comments: {
# data: [{
# type: 'comment',
# id: 3
# },{
# type: 'comment',
# id: 4
# }]
# }
# }
# }
# }

may be because the spec was like that during the time when this example was written :)

@wasabigeek
Copy link
Author

Hey @wasifhossain thanks for pointing that out to me >_< I guess I should have pointed to this example instead - this would be a symbol-keyed hash, no?

to be honest, I have found one gotcha though: it should be relationships according to the JSON API spec in place of associations on L35, as found in:

# document = {
# data: {
# id: 1,
# type: 'post',
# attributes: {
# title: 'Title 1',
# date: '2015-12-20'
# },
# associations: {
# author: {
# data: {
# type: 'user',
# id: 2
# }
# },
# second_author: {
# data: nil
# },
# comments: {
# data: [{
# type: 'comment',
# id: 3
# },{
# type: 'comment',
# id: 4
# }]
# }
# }
# }
# }

may be because the spec was like that during the time when this example was written :)

@wasifhossain
Copy link
Member

according to the spec, a valid JSON API document keys should be stringified, as shown in this typical example

https://jsonapi.org/format/#document-compound-documents

{
  "data": [{
    "type": "articles",
    "id": "1",
    "attributes": {
      "title": "JSON:API paints my bikeshed!"
    },
    "links": {
      "self": "http://example.com/articles/1"
    },
    "relationships": {
      "author": {
        "links": {
          "self": "http://example.com/articles/1/relationships/author",
          "related": "http://example.com/articles/1/author"
        },
        "data": { "type": "people", "id": "9" }
      },
      "comments": {
        "links": {
          "self": "http://example.com/articles/1/relationships/comments",
          "related": "http://example.com/articles/1/comments"
        },
        "data": [
          { "type": "comments", "id": "5" },
          { "type": "comments", "id": "12" }
        ]
      }
    }
  }],
  "included": [{
    "type": "people",
    "id": "9",
    "attributes": {
      "first-name": "Dan",
      "last-name": "Gebhardt",
      "twitter": "dgeb"
    },
    "links": {
      "self": "http://example.com/people/9"
    }
  }, {
    "type": "comments",
    "id": "5",
    "attributes": {
      "body": "First!"
    },
    "relationships": {
      "author": {
        "data": { "type": "people", "id": "2" }
      }
    },
    "links": {
      "self": "http://example.com/comments/5"
    }
  }, {
    "type": "comments",
    "id": "12",
    "attributes": {
      "body": "I like XML better"
    },
    "relationships": {
      "author": {
        "data": { "type": "people", "id": "9" }
      }
    },
    "links": {
      "self": "http://example.com/comments/12"
    }
  }]
}

@wasabigeek
Copy link
Author

@wasifhossain I get that - since we're documenting in Ruby though, should we not expect that users will read this as Ruby?

Happy to close this either way...

@wasifhossain
Copy link
Member

I will push a PR with a working example for this api (#2384 (comment))

thanks for pointing it out 👍

@wasabigeek
Copy link
Author

I'd be happy to make a PR if you don't have time, just wanted to be sure that I had the correct interpretation!

@wasifhossain
Copy link
Member

sure thing. please go ahead with this. I will have a look later. thanks!

wasabigeek added a commit to wasabigeek/active_model_serializers that referenced this issue Jun 29, 2020
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

No branches or pull requests

2 participants