-
Notifications
You must be signed in to change notification settings - Fork 109
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
Add support to render with a root key and meta attributes #135
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
188839b
add support for root elements
ritikesh 63244e6
make root optional - prepare is used from association_extractor as well
ritikesh f2825d7
bump version and changelog
ritikesh 2b7a4ec
add support for meta attributes
ritikesh 30fdf2f
update comments
ritikesh 2cac094
fix changelog, hoist data variable
ritikesh 4f60b74
cleanup prepare method and separate concerns
ritikesh 92d3258
refactoring
ritikesh e27a82f
Merge branch 'master' into fork_master
ritikesh b75b0a2
raise error for meta+root
ritikesh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,33 @@ And the output would look like: | |
} | ||
``` | ||
|
||
### Collections | ||
|
||
You can also pass a collection object or an array to the render method. | ||
|
||
```ruby | ||
puts UserBlueprint.render(User.all) | ||
``` | ||
|
||
This will result in JSON that looks something like this: | ||
|
||
```json | ||
[ | ||
{ | ||
"uuid": "733f0758-8f21-4719-875f-262c3ec743af", | ||
"email": "[email protected]", | ||
"first_name": "John", | ||
"last_name": "Doe" | ||
}, | ||
{ | ||
"uuid": "733f0758-8f21-4719-875f-743af262c3ec", | ||
"email": "[email protected]", | ||
"first_name": "John", | ||
"last_name": "Doe 2" | ||
} | ||
] | ||
``` | ||
|
||
### Renaming | ||
|
||
You can rename the resulting JSON keys in both fields and associations by using the `name` option. | ||
|
@@ -101,6 +128,77 @@ Output: | |
} | ||
``` | ||
|
||
### Root | ||
You can also optionally pass in a root key to wrap your resulting json in: | ||
```ruby | ||
class UserBlueprint < Blueprinter::Base | ||
identifier :uuid | ||
field :email, name: :login | ||
|
||
view :normal do | ||
fields :first_name, :last_name | ||
end | ||
end | ||
``` | ||
|
||
Usage: | ||
```ruby | ||
puts UserBlueprint.render(user, view: :normal, root: :user) | ||
``` | ||
|
||
Output: | ||
```json | ||
{ | ||
"user": { | ||
"uuid": "733f0758-8f21-4719-875f-262c3ec743af", | ||
"first_name": "John", | ||
"last_name": "Doe", | ||
"login": "[email protected]" | ||
} | ||
} | ||
``` | ||
|
||
### Meta attributes | ||
You can additionally add meta-data to the json as well: | ||
```ruby | ||
class UserBlueprint < Blueprinter::Base | ||
identifier :uuid | ||
field :email, name: :login | ||
|
||
view :normal do | ||
fields :first_name, :last_name | ||
end | ||
end | ||
``` | ||
|
||
Usage: | ||
```ruby | ||
json = UserBlueprint.render(user, view: :normal, root: :user, meta: {links: [ | ||
'https://app.mydomain.com', | ||
'https://alternate.mydomain.com' | ||
]}) | ||
puts json | ||
``` | ||
|
||
Output: | ||
```json | ||
{ | ||
"user": { | ||
"uuid": "733f0758-8f21-4719-875f-262c3ec743af", | ||
"first_name": "John", | ||
"last_name": "Doe", | ||
"login": "[email protected]" | ||
}, | ||
"meta": { | ||
"links": [ | ||
"https://app.mydomain.com", | ||
"https://alternate.mydomain.com" | ||
] | ||
} | ||
} | ||
``` | ||
Note: For meta attributes, a [root](#root) is mandatory. | ||
|
||
### Exclude fields | ||
You can specifically choose to exclude certain fields for specific views | ||
```ruby | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
module Blueprinter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 this is a great step in organization! |
||
module BaseHelpers | ||
def self.included(base) | ||
base.extend(SingletonMethods) | ||
end | ||
|
||
module SingletonMethods | ||
private | ||
def active_record_relation?(object) | ||
!!(defined?(ActiveRecord::Relation) && | ||
object.is_a?(ActiveRecord::Relation)) | ||
end | ||
|
||
def prepare_for_render(object, options) | ||
view_name = options.delete(:view) || :default | ||
root = options.delete(:root) | ||
meta = options.delete(:meta) | ||
validate_root_and_meta(root, meta) | ||
prepare(object, view_name: view_name, local_options: options, root: root, meta: meta) | ||
end | ||
|
||
def prepare_data(object, view_name, local_options) | ||
prepared_object = include_associations(object, view_name: view_name) | ||
if array_like?(object) | ||
prepared_object.map do |obj| | ||
object_to_hash(obj, | ||
view_name: view_name, | ||
local_options: local_options) | ||
end | ||
else | ||
object_to_hash(prepared_object, | ||
view_name: view_name, | ||
local_options: local_options) | ||
end | ||
end | ||
|
||
def prepend_root_and_meta(data, root, meta) | ||
return data unless root | ||
ret = { root => data } | ||
meta ? ret.merge!(meta: meta) : ret | ||
end | ||
|
||
def inherited(subclass) | ||
subclass.send(:view_collection).inherit(view_collection) | ||
end | ||
|
||
def object_to_hash(object, view_name:, local_options:) | ||
view_collection.fields_for(view_name).each_with_object({}) do |field, hash| | ||
next if field.skip?(object, local_options) | ||
hash[field.name] = field.extract(object, local_options) | ||
end | ||
end | ||
|
||
def validate_root_and_meta(root, meta) | ||
case root | ||
when String, Symbol | ||
# no-op | ||
when NilClass | ||
raise BlueprinterError, "meta requires a root to be passed" if meta | ||
else | ||
raise BlueprinterError, "root should be one of String, Symbol, NilClass" | ||
end | ||
end | ||
|
||
def include_associations(object, view_name:) | ||
unless defined?(ActiveRecord::Base) && | ||
object.is_a?(ActiveRecord::Base) && | ||
object.respond_to?(:klass) | ||
return object | ||
end | ||
# TODO: Do we need to support more than `eager_load` ? | ||
fields_to_include = associations(view).select { |a| | ||
a.options[:include] != false | ||
}.map(&:method) | ||
if !fields_to_include.empty? | ||
object.eager_load(*fields_to_include) | ||
else | ||
object | ||
end | ||
end | ||
|
||
def jsonify(blob) | ||
Blueprinter.configuration.jsonify(blob) | ||
end | ||
|
||
def current_view | ||
@current_view ||= view_collection[:default] | ||
end | ||
|
||
def view_collection | ||
@view_collection ||= ViewCollection.new | ||
end | ||
|
||
def array_like?(object) | ||
object.is_a?(Array) || active_record_relation?(object) | ||
end | ||
|
||
def associations(view_name = :default) | ||
view_collection.fields_for(view_name).select { |f| f.options[:association] } | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
module Blueprinter | ||
VERSION = '0.12.1' | ||
VERSION = '0.13.0' | ||
end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not enforced, currently
[Any] :root
is more accurate.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't
[Any]
refer to any object whereas, we would only allow for a symbol / string as a hash key? Documentation should reflect on what is the expected input, I wouldn't want to add ais_a
check in the method for that sake.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather deliberately decide to be open to different input or not. A type check is completely reasonable if we want to restrict to symbol/string. Is there a reason you don't want to enforce typing here?
A type coercion does happens regardless in
.render
and.render_as_json
, but not in.render_as_hash
. This is because the underlying.as_json
and.jsonify
turn{ 1 => "hello" }
into{ "1" => "hello"}
. I think it's confusing and dishonest to allow that to happen while.render_as_hash
's underlying.prepare_for_render
leaves us with the original{ 1 => "hello" }
.I'm torn here because I don't think it would be wise for anyone to pass in an array, hash, or any random object, but technically now they can and it will not blow up. In those cases I would prefer to either document and fully support
any
input or restrict the input and treat it as an error and handle it gracefully by returning an instructive error message. One of my biggest pains migrating to AMS v0.10 was the lack of helpful error messages and confusion about the breaking changes surroundingroot
behavior specifically...So let's look at some of the behavior this introduces:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I understand where you are coming from, but I would disagree here, because the method definitions/documentations both suggest a
json
orjson like
output - which is pretty much self-explanatory to have strings as keys in the output.Adding extra
.is_a?(Symbol) || .is_a?(String)
for every render call does seem like an unnecessary overhead for something that should be the implementers' job. I'm however, okay with changing the type toAny
in the comments.However, I still feel strongly about being indicative and not strict in this case - that you need to pass a symbol/string - This would cover most use-cases anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the contrary, I agree with you, we need to have proper documentation and exceptions being raised when those are not followed. Have made the changes along with some refactoring. Please let me know if you have any other concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AllPurposeName I think its ok to have it as
[Symbol|String]
because that is what we want to support. We don't want to support[Any]
even though it is technically possible in this implementation.Users can of course pass in Array or Hash or some other crazy type, and they would do so at their own risk. If we don't document passing in those types, we will not be responsible for breaking it in future updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AllPurposeName, @philipqnguyen, I've retained the
Symbol | String
documentation and have added a check to raise exception if anything else is passed for root along with a test case for the same.I agree with @AllPurposeName that it'd be counter intuitive not raising an exception when calling out that limited types are supported.
Along with that, I've done some other refactoring as well. The base class was becoming heavier with lot of helper methods and active record helper has not grown as it was anticipated. Hence, I have combined them into one and have moved all private helper methods there. You can refer to my most recent commit.
Let me know your thoughts.