begriffs / clean_pagination

API pagination the way RFC7233 intended it
MIT License
33 stars 3 forks source link

Enabling responses of empty fields. #2

Closed frankywahl closed 10 years ago

frankywahl commented 10 years ago

Because of paginate https://github.com/begriffs/clean_pagination/blob/master/test/clean_pagination_test.rb#L91, if I write something like this in my controller

def action
   paginate Bla.count, max_per_page do |limit, offset|
      render json: Bla.limit(limit).offset(offset)
    end
end

If Bla.count is 0, I never get a response back as it would expect a template. Instead, I have to type two renders, aka adding at the end of the method

   render json: [] if Blah.count == 0

which causes users having two renders inside the controller. (Not nice).

begriffs commented 10 years ago

Good point, this is annoying.

I'll remove the default views used in the test suite and see which tests break. Then we can determine sensible responses the gem can make.

For instance the zero-length response could be an HTTP 204 with an empty body. I originally thought it might respond with [] rather than an empty body but I'd like this gem to work for any content type, not just json.

frankywahl commented 10 years ago

Yeah, I did see your comment on https://github.com/begriffs/clean_pagination/commit/f543249d25165d6f407211c28fc34252765f6045

frankywahl commented 10 years ago

Here is a test that will fail with current version:

test 'no files in json' do
  @controller.stubs(:total_items).returns 0
  @controller.expects(:action).with(100, 0)
  get :index, format: :json
  assert_equal 204, response.status
  assert_equal [], response.body
end
begriffs commented 10 years ago

Thanks, I'll add this test and make it pass over the weekend.

frankywahl commented 10 years ago

Should we not be returning an empty array when it's an API request in JSON. Aka:

https://github.com/frankywahl/clean_pagination/compare/empty_json

The patch given would just make me have to pre-emtively return if it's empty: aka:

render json: [], status: 204 if Blah.count == 0

So making above branch test pass?

begriffs commented 10 years ago

From rfc2616: "The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields." http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.5

Sending a blank response rather than "[]" allows this gem to be used for API mime types other than JSON, like Google Protocol Buffers (https://code.google.com/p/protobuf/).

frankywahl commented 10 years ago

Ok, I see. It makes sense. Thanks!