Add supports for :links and :data options to has_many associations for jsonapi adapter #1028
Conversation
|
Hey @lsylvester, I want to review this one, see cool! Could you rebase it please? 😄 |
lsylvester
force-pushed
the
jsonapi-link-v-data
branch
from
a3ffb42 to
5d5715a
Compare
|
@joaomdmoura Rebased. |
|
@lsylvester, How does this work for namespace url? For example: |
| @@ -33,6 +38,10 @@ def id | |||
| @attributes[:id] || @attributes['id'] || object_id | |||
| end | |||
|
|
|||
| def to_param | |||
| id.to_s | |||
| end | |||
There was a problem hiding this comment.
Hiya, Thanks for this. Would you mind helping me understand these changes?
There was a problem hiding this comment.
Yup, could you help us on understanding these? 😄
There was a problem hiding this comment.
to_param and to_model are used in the for url generation.
to_model is called by polymorphic_url and it is expected to return an ActionModel compliant model. Without it, the polymororphic_urlv method will error.
to_param is used to generate the value for the object in the url. Without it, I get a url like http://test.host/tags/%23%3CTag:0xXXXXXX%3E/posts\
There was a problem hiding this comment.
👍 Nice, could you add some comments on top of those method explaining this to help new contributors when jumping into the code 😄
|
@lsylvester You still working on this? |
lsylvester
force-pushed
the
jsonapi-link-v-data
branch
from
5d5715a to
3cbd327
Compare
|
@kimsuelim The is currently no way to provide a name spaced url. Should this be a config option like ActiveModel::Serializer.config.url_namespace = :apior class SomeSerializer < ActiveModel::Serializer
url_namespace :api
end |
| end | ||
| if opts.fetch(:links, false) | ||
| resource[:relationships][name] ||= {} | ||
| resource[:relationships][name][:links] = { related: url_helper.url_for([serializer.object, name]) } |
There was a problem hiding this comment.
Might this PR want to talk to #1018 (though these are different links)
There was a problem hiding this comment.
#1018 is kind stopped for sometime, I wouldn't worry abou that right now, but indeed:
So the link building code should ideally be reusable in different contexts
This would be nice.
|
This is really nice @lsylvester great work, I'm looking forward to see this merged. I would like to ask to also add some docs about this on our new docs, maybe a new article on Sorry about this again 😁, could you rebase it? |
|
@lsylvester are you still actively working on this? It would be also nice to be able to pass an arbitrary object with links |
|
I rebased this PR in #1282 and added more tests |
Adds option to
has_manyrelationships have links (links: true) and turn of data (data: false) for jsonapi.Replaces #840
Fixes #834