Skip to content

added an example spec for hash delegates and an example implemantation #85

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

Conversation

dspaeth-faber
Copy link
Contributor

Specs for a hash object as delegate requested with #84

@dblock
Copy link
Member

dblock commented Jul 17, 2014

I haven't tried it, but maybe we need a proper presenter for the Hash type in grape-entity. I mean, what's different between a real model class and a Hash anyway?

@dspaeth-faber
Copy link
Contributor Author

I haven't tried it, but maybe we need a proper presenter for the Hash type in grape-entity.

👍

But, it will be a breaking change (regarding to the specs).

Also what need's a change in my point of view is the representation of Collections.
I want to be able to use my collection as items in my presenter. Example:

  present  collection_items,
                with: Entities::Collection

With this example every item will be wrapped in an Entities::Collection and there
is no easy way to wrap the hole collection in an Entities::Collection and present every
item in this collection in an Entities::Item

I think the entity should have a flag or option to show that it can handle a collection
and don't wrapp every item.

Did you have a look at https://github.com/objects-on-rails/display-case? I like it's abroach to
check if the real object should be decorated with the presenter.

@dblock
Copy link
Member

dblock commented Jul 17, 2014

We can make breaking changes as long as they are meaningfully better.

Personally the display-case indirection is one level too pure for me :)

@dspaeth-faber
Copy link
Contributor Author

Personally the display-case indirection is one level too pure for me :)

Can you please explain what you are missing? I don't want to head into the wrong direction.

@dblock
Copy link
Member

dblock commented Jul 18, 2014

I was just commenting on display-case. That's really beyond the scope of this workitem.

@dspaeth-faber
Copy link
Contributor Author

@dblock I did refactore my proposal maybe it will fit.

@dblock
Copy link
Member

dblock commented Aug 1, 2014

I really like the implementation and I also do like present_collection very much. Could you please update README, add to the CHANGELOG and squash the commits? I'll merge. Thx.

@dblock
Copy link
Member

dblock commented Aug 1, 2014

Should present_collection be represent_collection as it's parallel to represent?

@dspaeth-faber
Copy link
Contributor Author

present_collection is much a like root it only says, hey this Entity is used for presenting a collection, don't do any magic with object to present. So it isn't the same like represent. Another approach would be to create a subclass of Entity like CollectionEntity and override represent. But I have no clue how to make it simply work with include Grape::Entity::DSL in a nother class.

@dspaeth-faber
Copy link
Contributor Author

PS: With a subclass you have to fix

base.const_set(:Entity, Class.new(ancestor_entity_class || Grape::Entity)) unless const_defined?(:Entity)

in the DSL module some how to allow Entity be a kind of CollectionEntity

@dblock
Copy link
Member

dblock commented Aug 1, 2014

I figured it out re: present_collection, deleted my comment but not fast enough :)

@dspaeth-faber
Copy link
Contributor Author

Ah, ok. Different timezone's have there drawback's, so I have to be fast ;)

Hashes can now be presented
Added an option to present an collection of objects with a single entity
@dspaeth-faber
Copy link
Contributor Author

@dblock changed & squashed

@@ -1,6 +1,7 @@
Next
====

* [#85](https://github.com/intridea/grape-entity/pull/85): Added `present_collection` to indicate that an `Entity` presents an entire Collection. Hashes can now be passed as object to be presented. The `Hash` keys can be referenced by expose. - [@dspaeth-faber](https://github.com/dspaeth-faber).
Copy link
Member

Choose a reason for hiding this comment

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

Extra period before the -! :)

@dblock
Copy link
Member

dblock commented Aug 1, 2014

Merged via b54bf68. Thank you.

@dblock dblock closed this Aug 1, 2014
@dspaeth-faber dspaeth-faber mentioned this pull request Sep 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants