Skip to content

Commit

Permalink
Fixed present to not overwrite the previously added contents of the r…
Browse files Browse the repository at this point in the history
…esponse body whebn called more than once.
  • Loading branch information
mfunaro authored and dblock committed Nov 10, 2014
1 parent 58c17e9 commit 9cf4c7e
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* [#757](https://github.com/intridea/grape/pull/757): Changed `desc` can now be used with a block syntax - [@dspaeth-faber](https://github.com/dspaeth-faber).
* [#779](https://github.com/intridea/grape/pull/779): Fixed using `values` with a `default` proc - [@ShPakvel](https://github.com/ShPakvel).
* [#799](https://github.com/intridea/grape/pull/799): Fixed custom validators with required `Hash`, `Array` types - [@bwalex](https://github.com/bwalex).
* [#784](https://github.com/intridea/grape/pull/784): Fixed `present` to not overwrite the previously added contents of the response body whebn called more than once - [@mfunaro](https://github.com/mfunaro).
* Your contribution here.

0.9.0 (8/27/2014)
Expand Down
7 changes: 6 additions & 1 deletion lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,12 @@ def present(*args)
end

representation = { root => representation } if root
representation = (@body || {}).merge(key => representation) if key
if key
representation = (@body || {}).merge(key => representation)
elsif entity_class.present? && representation.respond_to?('merge')
representation = (@body || {}).merge(representation)
end

body representation
end

Expand Down
29 changes: 29 additions & 0 deletions spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,35 @@ def initialize
end
end
end

describe 'with' do
describe 'multiple entities' do
let(:entity_mock1) do
entity_mock1 = Object.new
allow(entity_mock1).to receive(:represent).and_return(dummy1: 'dummy1')
entity_mock1
end

let(:entity_mock2) do
entity_mock2 = Object.new
allow(entity_mock2).to receive(:represent).and_return(dummy2: 'dummy2')
entity_mock2
end

describe 'instance' do
before do
subject.present 'dummy1', with: entity_mock1
subject.present 'dummy2', with: entity_mock2
end

it 'presents both dummy objects' do
expect(subject.body[:dummy1]).to eq 'dummy1'
expect(subject.body[:dummy2]).to eq 'dummy2'
end
end

end
end
end

describe '#declared' do
Expand Down

3 comments on commit 9cf4c7e

@eguneys
Copy link

Choose a reason for hiding this comment

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

I am using grape-roar, and this change breaks my code:

module ProductRepresenter
  include Roar::JSON
  include Roar::Hypermedia
  include Grape::Roar::Representer

  link :self do |opts|
    request = Grape::Request.new(opts[:env])
    "#{request.url}"
  end
end

opts parameter, supposed to include the environment, is empty {wrap: false}. Any idea why?

@dblock
Copy link
Member

@dblock dblock commented on 9cf4c7e Jan 12, 2015

Choose a reason for hiding this comment

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

@eguneys Try to build a repro, I don't think this is this code. Also https://github.com/dblock/grape-with-roar/blob/master/api/presenters/spline_presenter.rb uses this version just file and gets env.

@dblock
Copy link
Member

@dblock dblock commented on 9cf4c7e Jun 2, 2015

Choose a reason for hiding this comment

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

@eguneys I tracked this down and opened #1023 which is likely the same problem you are having, I am probably going to undo the respond_to 'merge' part.

Please sign in to comment.