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

Page representer #93

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

summera
Copy link

@summera summera commented Nov 29, 2014

This is a PageRepresenter based off @apotonick blog post. It can be used with Kaminari or will_paginate to easily include pagination with your Decorator Representers.

class VenuesRepresenter < Roar::Decorator
  include Roar::Representer::JSON
  include Roar::Representer::Feature::Hypermedia
  include Roar::Rails::PageRepresenter

 # The rest of your representer...
end

This can also be used with the CollectionRepresenter (#92) to create a paginated representer

class VenuesRepresenter < Roar::Decorator
  include Roar::Representer::JSON
  include Roar::Representer::Feature::Hypermedia
  include Roar::Rails::CollectionRepresenter
  include Roar::Rails::PageRepresenter
end

I updated the README with an explanation.

@apotonick
Copy link
Member

Cool! Loving the idea + implementation. We have to be careful, though. The pagination I suggest in the blog post doesn't follow any MIME format (like HAL or Collection-JSON), I made it up to demonstrate how Roar works.

Everything we merge into Roar's core will be used by people as they might assume we're following a standard (the only standard we follow is the Roar standard). That's the same with link - I just added this to Roar core to illustrate how hypermedia could work and people picked it up and now we have a semi-defined "Roar format" specification.

We should either discuss a generic format (I am secretly working on Object-HAL with @mikekelly) or we should move this into a gem (either roar-contrib or roar-pagination). If we merge, this will get us into "specification trouble".

Don't get me wrong @summera - I love what you did! We just have to be careful with core additions (lesson learned 😉 ). What you think?

@summera
Copy link
Author

summera commented Nov 30, 2014

@apotonick Yea! Sounds good to me. ;) I'm cool with discussing a generic format and getting a roar-pagination gem out there in the mean time.

Am I wrong in thinking that this pagination implementation would follow the HAL standard by simply including Roar::Representer::JSON::HAL instead of Roar::Representer::JSON like so?

class VenuesRepresenter < Roar::Decorator
  include Roar::Representer::JSON::HAL
  include Roar::Rails::PageRepresenter
end

@apotonick
Copy link
Member

Not sure how pagination is defined in HAL - I don't recall that being specified in the official document at all! That's why I'm cautious about merging this.

I also checked your other PR #92 which could go into your roar-contribs gem, too. Again, I tend not to merge this generic behaviour into Roar core. This will set a half-baked standard for representing collections which I haven't completely thought through, yet. So it's better to extract that now, work out good practises and then define a standard (otherwise we'd have to deprecate and change-manage all that, which is usually quite some work).

Are you ok with that? Thanks for your work and trying to help others by publishing what you found helpful - this is exactly how Open-Source is meant to work! ❤️

@summera
Copy link
Author

summera commented Nov 30, 2014

Yep, no problem. I agree 💯 . I'll get crankin on a roar-contribs gem and keep you updated

@summera
Copy link
Author

summera commented Dec 28, 2014

@apotonick Finally got the time to get started on roar-contrib. Haven't made a release yet. Let me know if you have any comments/suggestions 😀

@apotonick
Copy link
Member

That is incredibly cool, @summera, especially because you set a standard for discussion. This gem will help many people understanding how some things work in Roar.

Please, keep me in the loop and I will promote it. ❤️

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