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

Add dynamic field support #164

Merged
merged 7 commits into from
Jul 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions lib/blueprinter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
require_relative 'helpers/base_helpers'
require_relative 'view'
require_relative 'view_collection'
require_relative 'transformer'

module Blueprinter
class Base
Expand Down Expand Up @@ -274,6 +275,43 @@ def self.fields(*field_names)
end
end



# Specify one transformer to be included for serialization.
# Takes a class which extends Blueprinter::Transformer
#
# @param class name [Class] which implements the method transform to include for
# serialization.
#
#
# @example Specifying a DynamicFieldTransformer transformer for including dynamic fields to be serialized.
# class User
# def custom_columns
# self.dynamic_fields #which is an array of some columns
# end
# def custom_fields
# custom_columns.each_with_object({}){|col,result| result[col] = self.send(col)}
# end
# end
#
# class UserBlueprint < Blueprinter::Base
# fields :first_name, :last_name
# transform DynamicTransformer
# # other code
# end
#
# class DynamicFieldTransformer < Blueprinter::Transformer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be DynamicTransformer? (or transform DynamicFieldTransformer above)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, example mismatch. would appreciate if you can make a PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, see #174

# def transform(hash, object, options)
# hash.merge!(object.dynamic_fields)
# end
# end
#
# @return [Array<Class>] an array of transformers
def self.transform(transformer)
current_view.add_transformer(transformer)
end


# Specify another view that should be mixed into the current view.
#
# @param view_name [Symbol] the view to mix into the current view.
Expand Down
8 changes: 6 additions & 2 deletions lib/blueprinter/helpers/base_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@ def inherited(subclass)
end

def object_to_hash(object, view_name:, local_options:)
view_collection.fields_for(view_name).each_with_object({}) do |field, hash|
result_hash = view_collection.fields_for(view_name).each_with_object({}) do |field, hash|
next if field.skip?(field.name, object, local_options)
hash[field.name] = field.extract(object, local_options)
hash[field.name] = field.extract(object, local_options)
end
view_collection.transformers(view_name).each do |transformer|
transformer.transform(result_hash,object,local_options)
end
result_hash
end

def validate_root_and_meta(root, meta)
Expand Down
12 changes: 12 additions & 0 deletions lib/blueprinter/transformer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module Blueprinter
# @api private
class Transformer
def transform(result_hash,primary_obj, options={})
fail NotImplementedError, "A Transformer must implement #transform"
end

def self.transform(result_hash,primary_obj, options={})
self.new.transform(result_hash,primary_obj, options)
end
end
end
13 changes: 11 additions & 2 deletions lib/blueprinter/view.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
module Blueprinter
# @api private
class View
attr_reader :excluded_field_names, :fields, :included_view_names, :name
attr_reader :excluded_field_names, :fields, :included_view_names, :name, :transformers

def initialize(name, fields: {}, included_view_names: [], excluded_view_names: [])
def initialize(name, fields: {}, included_view_names: [], excluded_view_names: [],transformers: [])
@name = name
@fields = fields
@included_view_names = included_view_names
@excluded_field_names = excluded_view_names
@transformers = transformers
end

def inherit(view)
Expand All @@ -22,6 +23,10 @@ def inherit(view)
view.excluded_field_names.each do |field_name|
exclude_field(field_name)
end

view.transformers.each do |transformer|
self.add_transformer(transformer)
end
end

def include_view(view_name)
Expand All @@ -38,6 +43,10 @@ def exclude_fields(field_names)
end
end

def add_transformer(custom_transformer)
transformers << custom_transformer
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never seen * used for anything but multiplication. Is this common style?

Otherwise, can we come up with a method name that makes clear that it is appending transformer here?
I'm worried we will run out of symbols to define new methods 🤣

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am usually not into operator overloading, but then I was inspired by the method written for adding fields into the view
def <<(field)
fields[field.name] = field
end
I was under the impression that its a style blueprinter follows for adding setters. I agree to all the reviewers @philipqnguyen and @mcclayton when you feel that we can run out of operators to overload. For now I will change the method name to transform :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename to add_transformer? Otherwise it seems like view.transform is already invoking the transformer, which is not entirely accurate since it's just storing it to be called later.


def <<(field)
fields[field.name] = field
end
Expand Down
4 changes: 4 additions & 0 deletions lib/blueprinter/view_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ def fields_for(view_name)
identifier_fields + sorted_fields
end

def transformers(view_name)
views[view_name].transformers
end

def [](view_name)
@views[view_name] ||= View.new(view_name)
end
Expand Down
4 changes: 4 additions & 0 deletions spec/activerecord_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class Vehicle < ActiveRecord::Base
class User < ActiveRecord::Base
attr_accessor :company, :description, :position, :active
has_many :vehicles

def dynamic_fields
{"full_name" => "#{first_name} #{last_name}"}
end
end

ActiveRecord::Schema.define(version: 20181116094242) do
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/model_factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
FactoryBot.define do
factory :user do
first_name { 'Meg' }
last_name { 'Jones' }
last_name { 'Ryan' }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

position { 'Manager' }
description { 'A person' }
company { 'Procore' }
Expand Down
34 changes: 25 additions & 9 deletions spec/integrations/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
company: 'Procore',
birthday: Date.new(1994, 3, 4),
deleted_at: nil,
active: false
active: false,
dynamic_fields: {"full_name" => "Meg Ryan"}
}
end
let(:object_with_attributes) { OpenStruct.new(obj_hash) }
Expand Down Expand Up @@ -288,20 +289,29 @@ def extract(association_name, object, _local_options, _options={})
end

describe 'Using the ApplicationBlueprint pattern' do
let(:obj) { OpenStruct.new(id: 1, name: 'Meg', age: 32) }
let(:obj) { OpenStruct.new(id: 1, first_name: 'Meg',last_name:'Ryan', age: 32) }
let(:transformer) do
Class.new(Blueprinter::Transformer) do
def transform(result_hash, object, options={})
result_hash.merge!({full_name: "#{object.first_name} #{object.last_name}"})
end
end
end
let(:application_blueprint) do
custom_transformer = transformer
Class.new(Blueprinter::Base) do
identifier :id
field :name
field :first_name
field(:overridable) { |o| o.name }

view :with_age do
field :age
transform custom_transformer
end

view :anonymous_age do
include_view :with_age
exclude :name
exclude :first_name
end
end
end
Expand All @@ -312,38 +322,44 @@ def extract(association_name, object, _local_options, _options={})

view :only_age do
include_view :with_age
exclude :name
exclude :first_name
end

view :with_age do
field :last_name
end
end
end

subject { blueprint.render_as_hash(obj) }

it('inherits identifier') { expect(subject[:id]).to eq(obj.id) }
it('inherits field') { expect(subject[:name]).to eq(obj.name) }
it('inherits field') { expect(subject[:first_name]).to eq(obj.first_name) }
it('overrides field definition') { expect(subject[:overridable]).to eq(obj.age) }

describe 'Inheriting views' do
let(:view) { :with_age }
subject { blueprint.render_as_hash(obj, view: view) }

it('includes identifier') { expect(subject[:id]).to eq(obj.id) }
it('includes base fields') { expect(subject[:name]).to eq(obj.name) }
it('includes base fields') { expect(subject[:first_name]).to eq(obj.first_name) }
it('includes view fields') { expect(subject[:age]).to eq(obj.age) }
it('inherits base fields') { expect(subject[:last_name]).to eq(obj.last_name) }
it('inherits transformer fields') { expect(subject[:full_name]).to eq("#{obj.first_name} #{obj.last_name}") }

describe 'With complex views' do
let(:view) { :anonymous_age }

it('includes identifier') { expect(subject[:id]).to eq(obj.id) }
it('includes include_view fields') { expect(subject[:age]).to eq(obj.age) }
it('excludes excluded fields') { expect(subject).to_not have_key(:name) }
it('excludes excluded fields') { expect(subject).to_not have_key(:first_name) }
end

describe 'Referencing views from parent blueprint' do
let(:view) { :only_age }

it('includes include_view fields') { expect(subject[:age]).to eq(obj.age) }
it('excludes excluded fields') { expect(subject).not_to have_key(:name) }
it('excludes excluded fields') { expect(subject).not_to have_key(:first_name) }
end
end
end
Expand Down
18 changes: 18 additions & 0 deletions spec/integrations/shared/base_render_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -425,4 +425,22 @@ def self.unless_method(_field_name, _object, _options)
end
it('returns json with values derived from options') { should eq(result) }
end

context 'Given blueprint has a transformer' do
subject { blueprint.render(obj) }
let(:result) { '{"id":' + obj_id + ',"full_name":"Meg Ryan"}' }
let(:blueprint) do
DynamicFieldsTransformer = Class.new(Blueprinter::Transformer) do
def transform(result_hash, object, options={})
dynamic_fields = (object.is_a? Hash) ? object[:dynamic_fields] : object.dynamic_fields
result_hash.merge!(dynamic_fields)
end
end
Class.new(Blueprinter::Base) do
identifier :id
transform DynamicFieldsTransformer
end
end
it('returns json with values derived from options') { should eq(result) }
end
Copy link
Contributor

@philipqnguyen philipqnguyen Jul 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you also add a test to ensure that the transformers are also inherited? There's some inheritance specs here that you can add to: https://github.com/procore/blueprinter/blame/4ba12dd14edf340f9a4a95f1668785adc8c21ca6/spec/integrations/base_spec.rb#L326-L348

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case has been added.

end