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

support proper merging of nested conditionals #33

Merged
merged 1 commit into from
Nov 4, 2013
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Next Release
* [#24](https://github.com/intridea/grape-entity/pull/24): Return documentation with `as` param considered - [@drakula2k](https://github.com/drakula2k).
* [#27](https://github.com/intridea/grape-entity/pull/27): Properly serializing hashes - [@clintonb](https://github.com/clintonb).
* [#28](https://github.com/intridea/grape-entity/pull/28): Look for method on entity before calling on the object - [@MichaelXavier](https://github.com/MichaelXavier).
* [#33](https://github.com/intridea/grape-entity/pull/33): Support proper merging of nested conditionals - [@wyattisimo](https://github.com/wyattisimo).
* Your contribution here.

0.3.0 (2013-03-29)
Expand Down
62 changes: 49 additions & 13 deletions lib/grape_entity/entity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ def entity(options = {})
# @option options :documentation Define documenation for an exposed
# field, typically the value is a hash with two fields, type and desc.
def self.expose(*args, &block)
options = args.last.is_a?(Hash) ? args.pop : {}
options = (@block_options ||= []).inject({}){|final, step| final.merge!(step)}.merge(options)
options = merge_options(args.last.is_a?(Hash) ? args.pop : {})

if args.size > 1
raise ArgumentError, "You may not use the :as option on multi-attribute exposures." if options[:as]
Expand Down Expand Up @@ -421,22 +420,59 @@ def valid_exposure?(attribute, exposure_options)
end

def conditions_met?(exposure_options, options)
if_condition = exposure_options[:if]
unless_condition = exposure_options[:unless]

case if_condition
when Hash; if_condition.each_pair{|k,v| return false if options[k.to_sym] != v }
when Proc; return false unless if_condition.call(object, options)
when Symbol; return false unless options[if_condition]
if_conditions = (exposure_options[:if_extras] || []).dup
if_conditions << exposure_options[:if] unless exposure_options[:if].nil?

if_conditions.each do |if_condition|
case if_condition
when Hash; if_condition.each_pair{|k,v| return false if options[k.to_sym] != v }
when Proc; return false unless if_condition.call(object, options)
when Symbol; return false unless options[if_condition]
end
end
Copy link
Member

Choose a reason for hiding this comment

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

I know this was in existing code, but I think return is unnecessary here. Remove it if it's the case. Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the way this is written, I think they are necessary in order to stop evaluating conditions and return from the method as soon as anything evaluates to false. If we remove them the false values will be ignored and the execution will just continue looping through conditions and then eventually down to that true at the end of the method.


case unless_condition
when Hash; unless_condition.each_pair{|k,v| return false if options[k.to_sym] == v}
when Proc; return false if unless_condition.call(object, options)
when Symbol; return false if options[unless_condition]
unless_conditions = (exposure_options[:unless_extras] || []).dup
unless_conditions << exposure_options[:unless] unless exposure_options[:unless].nil?

unless_conditions.each do |unless_condition|
case unless_condition
when Hash; unless_condition.each_pair{|k,v| return false if options[k.to_sym] == v}
when Proc; return false if unless_condition.call(object, options)
when Symbol; return false if options[unless_condition]
end
end

true
end

private

# Merges the given options with current block options.
#
# @param options [Hash] Exposure options.
def self.merge_options(options)
opts = {}

merge_logic = proc do |key, existing_val, new_val|
if [:if, :unless].include?(key)
if existing_val.is_a?(Hash) && new_val.is_a?(Hash)
existing_val.merge(new_val)
elsif new_val.is_a?(Hash)
(opts["#{key}_extras".to_sym] ||= []) << existing_val
new_val
else
(opts["#{key}_extras".to_sym] ||= []) << new_val
existing_val
end
else
new_val
end
end

opts.merge (@block_options ||= []).inject({}) { |final, step|
final.merge(step, &merge_logic)
}.merge(options, &merge_logic)
end

end
end
90 changes: 86 additions & 4 deletions spec/grape_entity/entity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,96 @@
subject.exposures[:awesome_thing].should == {:if => {:awesome => true}, :using => 'Something'}
end

it 'should allow for overrides' do
it 'should override nested :as option' do
subject.class_eval do
with_options(:if => {:awesome => true}) do
expose :less_awesome_thing, :if => {:awesome => false}
with_options(:as => :sweet) do
expose :awesome_thing, :as => :extra_smooth
end
end

subject.exposures[:awesome_thing].should == {:as => :extra_smooth}
end

it "should merge nested :if option" do
match_proc = lambda {|obj, opts| true }

subject.class_eval do
# Symbol
with_options(:if => :awesome) do
# Hash
with_options(:if => {:awesome => true}) do
# Proc
with_options(:if => match_proc) do
# Hash (override existing key and merge new key)
with_options(:if => {:awesome => false, :less_awesome => true}) do
expose :awesome_thing
end
end
end
end
end

subject.exposures[:awesome_thing].should == {
:if => {:awesome => false, :less_awesome => true},
:if_extras => [:awesome, match_proc]
}
end

it 'should merge nested :unless option' do
match_proc = lambda {|obj, opts| true }

subject.class_eval do
# Symbol
with_options(:unless => :awesome) do
# Hash
with_options(:unless => {:awesome => true}) do
# Proc
with_options(:unless => match_proc) do
# Hash (override existing key and merge new key)
with_options(:unless => {:awesome => false, :less_awesome => true}) do
expose :awesome_thing
end
end
end
end
end

subject.exposures[:awesome_thing].should == {
:unless => {:awesome => false, :less_awesome => true},
:unless_extras => [:awesome, match_proc]
}
end

it 'should override nested :using option' do
subject.class_eval do
with_options(:using => 'Something') do
expose :awesome_thing, :using => 'SometingElse'
end
end

subject.exposures[:awesome_thing].should == {:using => 'SometingElse'}
end

it 'should override nested :proc option' do
match_proc = lambda {|obj, opts| 'more awesomer' }

subject.class_eval do
with_options(:proc => lambda {|obj, opts| 'awesome' }) do
expose :awesome_thing, :proc => match_proc
end
end

subject.exposures[:awesome_thing].should == {:proc => match_proc}
end

it 'should override nested :documentation option' do
subject.class_eval do
with_options(:documentation => {desc: 'Description.'}) do
expose :awesome_thing, :documentation => {desc: 'Other description.'}
end
end

subject.exposures[:less_awesome_thing].should == {:if => {:awesome => false}}
subject.exposures[:awesome_thing].should == {:documentation => {desc: 'Other description.'}}
end
end

Expand Down