Refactor JSONAPI adapter #1103
Refactor JSONAPI adapter #1103
Conversation
| object.class.model_name.singular | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
These two methods clearly did not belong there, as they are JSONAPI specific.
There was a problem hiding this comment.
👍 I'm happy we finally did it!
beauby
force-pushed
the
fix-jsonapi-ri
branch
2 times, most recently
from
6fdd377 to
549e52c
Compare
beauby
force-pushed
the
fix-jsonapi-ri
branch
from
549e52c to
c4faafd
Compare
| if serializer.respond_to?(:each) | ||
| serializer.map { |s| resource_identifier(s) } | ||
| else | ||
| if options[:virtual_value] |
There was a problem hiding this comment.
After giving it a thought, why would anyone ever use a virtual value for a relationship with json_api? The only value this can take is nil or a Resource Identifier/array of Resource Identifiers, which means the virtual attribute has to be either of those, which means the use case is something like:
PostSerializer < ActiveModel::Serializer
attributes :id, :name
belongs_to :author
def author
{ type: 'authors', id: '3' }
end
endwhich in turn implies that one can't really include the corresponding resource.
Hence, I vote for removing the virtual values for relationships with json_api adapter.
There was a problem hiding this comment.
Maybe we could do something similar to JsonAdapter, that if found a instance of an model object it will query that object and use it as the relationship. I know that it might be an edge case, but would be nice to cover, taking into account the level o abstraction we are working with when trying to cover everyone using AMS 😁
beauby
force-pushed
the
fix-jsonapi-ri
branch
from
dfa49b5 to
bae4951
Compare
beauby
force-pushed
the
fix-jsonapi-ri
branch
from
5ff8288 to
f8c553a
Compare
|
Rebased. |
| @@ -9,7 +9,13 @@ def initialize(serializer, options = {}) | |||
| super | |||
| @hash = { data: [] } | |||
|
|
|||
| if fields = options.delete(:fields) | |||
| @options[:include] ||= [] | |||
| if @options[:include].is_a?(String) | |||
There was a problem hiding this comment.
Can you explain more about this. It's not related to type and id, and I also don't understood the why of String verification + split
|
LGTM, I'm merging it |
Move `id` and `json_api_type` methods from `Serializer` to `JsonApi`.
|
Oh, could you please @beauby open a PR with a new 'how to' article explaning how to override |
beauby
changed the title
id and json_api_type methods from Serializer to JsonApi.
|
@joaomdmoura With this PR, |
|
The article size don't bother me and indeed it can even be a note, just want to make it easier to be found 😄 but I'll check it on #1122 |
idandjson_api_typemethods fromSerializertoJsonApi