Skip to content

Commit

Permalink
Return an empty collection when where(:foo => []) is given
Browse files Browse the repository at this point in the history
The previous behaviour was to return everything! This is now
consistent with ActiveRecord and avoids a needless request.

The new behaviour is not applied to has_one associations as they
currently seem to ignore the where method entirely.
  • Loading branch information
chewi committed Sep 19, 2018
1 parent 8b7c6e9 commit e8ec222
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
10 changes: 7 additions & 3 deletions lib/her/model/associations/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@ def fetch(opts = {})
return @parent.attributes[@name] unless @params.any? || @parent.attributes[@name].blank?
return @opts[:default].try(:dup) if @parent.new?

path = build_association_path -> { "#{@parent.request_path(@params)}#{@opts[:path]}" }
@klass.get(path, @params).tap do |result|
@cached_result = result unless @params.any?
if @params.values.include?([]) && !is_a?(HasOneAssociation)
Her::Collection.new
else
path = build_association_path -> { "#{@parent.request_path(@params)}#{@opts[:path]}" }
@klass.get(path, @params).tap do |result|
@cached_result = result unless @params.any?
end
end
end

Expand Down
12 changes: 8 additions & 4 deletions lib/her/model/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,14 @@ def kind_of?(thing)
# @private
def fetch
@_fetch ||= begin
path = @parent.build_request_path(@parent.collection_path, @params)
method = @parent.method_for(:find)
@parent.request(@params.merge(:_method => method, :_path => path)) do |parsed_data, _|
@parent.new_collection(parsed_data)
if @params.values.include?([])
Her::Collection.new
else
path = @parent.build_request_path(@parent.collection_path, @params)
method = @parent.method_for(:find)
@parent.request(@params.merge(:_method => method, :_path => path)) do |parsed_data, _|
@parent.new_collection(parsed_data)
end
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions spec/model/associations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,13 @@
expect(user.organization.respond_to?(:empty?)).to be_truthy
expect(user.organization).not_to be_empty
end

it "returns an empty collection without fetching when [] is a parameter" do
Foo::Comment.should_not_receive(:request)
comments = user.comments.where(:foo_id => [])
comments.should respond_to(:length)
comments.size.should eql 0
end
end

context "without included parent data" do
Expand Down
7 changes: 7 additions & 0 deletions spec/model/relation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@
expect(Foo::User.create(fullname: "George Michael Bluth").id).to eq(3)
expect(Foo::User.all.size).to eql 3
end

it "returns an empty collection without fetching when [] is given" do
Foo::User.should_not_receive(:request)
users = Foo::User.where(:fullname => [])
users.should respond_to(:length)
users.size.should eql 0
end
end

context "for parent class" do
Expand Down

0 comments on commit e8ec222

Please sign in to comment.