trailblazer / roar

Parse and render REST API documents using representers.
http://www.trailblazer.to/gems/roar
MIT License
1.85k stars 138 forks source link

:skip_render option still renders an empty _embedded key with an empty array in HAL #161

Open doughsay opened 9 years ago

doughsay commented 9 years ago

Hi there, sorry to keep bothering you about these HAL rendering issues! On the latest 1.0.3 I'm still having the following issue:

Roar::VERSION
=> "1.0.3"

class PersonRepresenter < Roar::Decorator
  include Roar::JSON::HAL

  property :id, :skip_render => true
  property :name

  collection :favorite_foods,
             :decorator    => FavoriteFoodRepresenter,
             :class        => OpenStruct,
             :as           => :favoriteFoods,
             :embedded     => true,
             :skip_render  => true,
             :render_empty => false
end

class FavoriteFoodRepresenter < Roar::Decorator
  include Roar::JSON::HAL

  property :id, :skip_render => true
  property :name
end

json_string1 = "{\"id\": 123, \"name\": \"Bob\", \"_embedded\": {\"favoriteFoods\": [{\"id\": 321, \"name\": \"Pizza\"}]}}"
json_string2 = "{\"id\": 124, \"name\": \"Joe\", \"_embedded\": {\"favoriteFoods\": []}}"

person1 = PersonRepresenter.new(OpenStruct.new).from_json(json_string1)
=> #<OpenStruct id=123, name="Bob", favorite_foods=[#<OpenStruct id=321, name="Pizza">]>

person2 = PersonRepresenter.new(OpenStruct.new).from_json(json_string2)
=> #<OpenStruct id=124, name="Joe", favorite_foods=[]>

PersonRepresenter.new(person1).to_json
=> "{\"name\":\"Bob\",\"_embedded\":{\"favoriteFoods\":[]}}"

PersonRepresenter.new(person2).to_json
=> "{\"name\":\"Joe\"}"

As you can see from the above contrived example, the skip_render option only empties out the related collection, but still renders an empty array in its place. In addition to that, the render_empty option won't help in this scenario either, although it does work when the collection is legitimately empty.

Hope this helps!

apotonick commented 9 years ago

Haha, I had to look up my own API and see how :skip_render is interpreted! :blush:

The :skip_render option is run "per item" and I think that is the problem: while the items are left out, the original collection "wrapper" is still rendered. Can you verify that without HAL, please? (In a test, ideally?)?

apotonick commented 9 years ago

Awesome news: In awesome Representable 2.4, which is gonna be awesome, we have a pipelined architecture per property. That means, you can set up a pipeline for, say, a collection and say "run :skip_render, for every item, but also after that for the entire property". It's so awesome.

collection :songs, render_pipeline: [Get, SkipNil, Iterate[SkipRender], SkipRender, Write]
doughsay commented 9 years ago

That DOES sound awesome. Can't wait! :+1: