- 
                Notifications
    You must be signed in to change notification settings 
- Fork 75
Add paginator helper #40
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
| Almost done. General feedback would be really appreciated | 
| http://sequel.jeremyevans.net/rdoc-plugins/classes/Sequel/Dataset/Pagination.html How to start pagination at a certain uuid? | 
        
          
                test/helpers/paginator_test.rb
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*convenience
| Wow, this is awesome. Thanks a lot @tmaier! It's always super interesting to see how someone else approaches an implementation. So first of all, mind adding a set of tests for non-UUIDs, just so we can double-check that everything works when a range besides  On that same vein, we should also allow the "acceptable ranges" value to be passed into the paginator, probably during construction (or is the idea that  I'm not sure I precisely understood the usage examples. If I get the  Don't you want to account for your current page as well to know whether there will be another? Lastly, would you mind adding it into the resource scaffold so we could see what a front to end usage of the paginator might look like? I really like how explicit setting the next range is. Really awesome stuff. /cc @pedro For a third set of eyes on this one. | 
        
          
                lib/pliny/helpers/paginator.rb
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespace.
| Thank you @brandur for your first comments and suggestions. I will add and fix the issues mentioned within the next few days. paginator(User.count, accepted_ranges: [:id, :foo, :bar])and paginator(User.count) do |paginator|
  paginator[:accepted_ranges] = [:id, :foo, :bar]
end
 def will_paginate?
  count > res[:args][:max].to_i
endThe pagination will always happen if the number of items to show ( I have 200 chicken and want to see 20 at once: I need to paginate 
 Todo for pliny-template
 | 
| Note the in 421c814 updated regex does not match IP addresses. the following test will fail:       describe 'ip addresses' do
        let(:sort_by) { 'ipv4' }
        let(:first) { '192.0.2.235' }
        let(:last) { '198.18.0.0/15' }
        it 'returns Hash' do
          result =
            {
              sort_by: sort_by,
              first: first,
              last: last,
              args: { max: '1000', order: 'desc' }
            }
          assert_equal result, subject.request_options
        end
      endI came up with this regex to match also IPv4 addresses VALUE = /(?:[^\.\s;\/]+|(?:\d{1,3}\.){3}\d{1,3}(\/\d+)?)/It's complex and an edge case. I left it out for now. If you know a regexp which matches all characters, but no blank, no semicolon and not two dots ( | 
| @tmaier The paginator implementation definitely works best for values without dots in them, but I wouldn't worry too much about that for now though. We can improve the matching regex at a later time if issues start coming up. | 
| Introduced  get do
  MultiJson.encode uuid_paginator(Article, args: { max: 10 }).map { |x| serialize(x) }
end | 
| @brandur I would say I'm done for now. As you have access to two implementations, could you compare them and maybe even do a benchmark test to see what and how to improve the paginator | 
| There are now three paginators. 
 | 
| I would like to move this forward. Could anyone review it, please. I'm happy about comments and willing to change the implementation if necessary. | 
| @tmaier Oh man, this is serious fail on my part, and I apologize for it. On Fri, Aug 01, 2014 at 08:52:42AM -0700, Tobias L. Maier wrote: 
 | 
a2a3102    to
    2ba1464      
    Compare
  
    bd42937    to
    b336f87      
    Compare
  
    Original pull request: interagent/pliny-template#120
| @brandur ... you promised... | 
| I'm a horrible person :/ @uhoh-itsmaciek Do you need one of these or were you just looking at open pulls? | 
| The latter. I was browsing PRs after opening #146 and thought this seemed neat. But we're all horrible people; I'm just giving you a hard time. It would be cool to see this in though. Let me know if I can help. | 
| +1 | 
| Closing this in favor for #234. Hope that pagination support will land soon :) | 
| @tmaier I've been waiting for any kind of pagination to land. Still waiting for some of the details to settle on the other PR before going ahead. | 
Unfinished and untested. Just to get the ball rolling.
Comments and suggestions are highly welcome.
options[:end] > countHow to use
In your Endpoint:
If you want to return uuids in the Range header
Use
uuid_paginatorhelperIf numbering the items is enough
Heroku documentation on ranges: https://devcenter.heroku.com/articles/platform-api-reference#ranges