implement sparse fieldsets #86
Conversation
Azdaroth
force-pushed
the
sparse-fieldsets
branch
from
a7e6c44
to
f041424
Compare
|
@fotinakis Looks good to merge? The specs fail for Ruby head (2.4) because of the |
|
@fotinakis thoughts? |
| post = create(:post, :with_author, long_comments: long_comments) | ||
|
|
||
| serialized_data = JSONAPI::Serializer.serialize(post, fields: {posts: 'title,author', users: ''}, include: 'author') | ||
| expect(serialized_data).to eq ({ |
There was a problem hiding this comment.
The style of these tests should be updated to match how the rest of the tests look, and they should be much shorter if possible. I think the way to do that would be split them into two different concerns like the rests of the tests: 1) put all attribute handling tests in the top internal-only serialize_primary block. 2) Now that sparse fieldset attributes are guaranteed to be tested above, have only short relationships handling tests here. They should look like the other tests, which rely on serialize_primary for the attrributes and only test the relations. See inherits relations test for an example.
Also, nitpicks, should match the style of the other tests as well -- single quotes, spaces around hash-rocket, etc.
| @@ -260,12 +268,18 @@ def self.serialize(objects, options = {}) | |||
| includes = options[:include] | |||
| includes = (includes.is_a?(String) ? includes.split(',') : includes).uniq if includes | |||
|
|
|||
| fields = options[:fields] || {} | |||
| fields = Hash[fields.map do |type, whitelisted_fields| | |||
There was a problem hiding this comment.
This could be cleaned up and commented a bit, it's very non-obvious what it's doing by looking at it.
|
Sorry for the delay @Azdaroth — two comments above, and yeah I think dropping Ruby 2.4 support right now would be fine. |
|
@fotinakis adressed the comments and all specs are green now ;). |
|
Awesome, this looks great. Two last things:
|
|
Good point about decoupling from HTTP request, will update the PR. |
|
@fotinakis Updated with docs :). |
|
|
||
| ```json | ||
| { | ||
| "data" => { |
There was a problem hiding this comment.
This is not valid JSON. :) Need to turn all the => into :
| } | ||
| ``` | ||
|
|
||
| You could also pass an array of specifi fields for given type instead of comma-separated values: |
| You could also pass an array of specifi fields for given type instead of comma-separated values: | ||
|
|
||
| ``` ruby | ||
| JSONAPI::Serializer.serialize(post, fields: {posts: ['title', 'author'], users: ['name']}, include: 'author') |
There was a problem hiding this comment.
Can you set fields into a variable and pass it in? That would make this line shorter and more readable.
Azdaroth
force-pushed
the
sparse-fieldsets
branch
from
ef4d44b
to
3ca3555
Compare
|
Thanks for contributing this! Now released in v0.16.0. |
No description provided.