Paginator transformer doesn't work on empty collection

[terion-name]

If returning Paginator with no items it bypasses the transformer and raw paginator is outputed

[jasonlewis]

Yeah that's a tough one. A transformer is bound to the items within a paginator and not the paginator itself. There's no way to know what transformer a paginator should use unless it has items. The same goes for collections.

This does raise an interesting question though: what should be done when a collection is empty?

I still think it should return a 200 OK response and not a 404. But perhaps it should simply return an empty object.

{}

Currently if you return an empty collection you'll get an empty array.

[]

And, like you've experienced, returning an empty paginator will result in just the paginator data.

{
    "total": 0,
    "per_page": 15,
    "current_page": 1,
    "last_page": 0,
    "from": 0,
    "to": 0,
    "data": [ ]
}

I'm open to suggestions on this. I'd probably opt for an empty object.

This is going to have to be the job of the response formatters. I'm looking at them now and they probably need to be tweaked slightly. In fact, transformers and formatters seem to be doing similar tasks so I think this entire area needs to be revisited.

So, what do you think the expected response would be when you return an empty collection?

[jasonlewis]

I'm playing around with this locally and have it, at the moment, returning an empty object ({}) when there are no items in the collection. Again, not entirely sure how useful that is.

[terion-name]

There are several suggestions about this.

One — is to return empty collection. It is quite logical — no items, empty collection (to preserve compatibility with full paginator responses it should be {data:[]}).
BUT
There is one usecase, that sows doubts.
It is when request is just falling beyond the pagination range. E.g. we have 5 pages, but request page=6. In this case it is useful to have pagination data, like in regular paginated response. So, response should be:
{data:[], pagination: *default Fractal paginator here*}

These are two variants that I see.

PS
Can you make somehow to push pagination data to meta field? To make it more compatible with spec?

[jasonlewis]

For some reason I always thought Laravel's paginator prevented you from going past the range by checking if current page is greater than total pages. Just checking and it seems it does not. I honestly don't think there's a way to get the data to still be run through the transformer. There's just no way to know what resource the collection should hold, so no way to find out which transformer it belongs to.

The best that can be done is an empty object I think. Preserving the data key would be difficult as that's sort of "Fractal-only" stuff. So for people not using Fractal that would be an unexpected result.

Will think about this and revisit it again in the morning.

In regards to your last question the way the data is constructed (if you're using Fractal) is completely in the hands of Fractal at the moment. Custom serializers are being implemented and I will be shipping the API with a standard serializer. There's a number of "specs" so yeah, it's a tough market. Can't please everyone. 🍭

[terion-name]

There's a number of "specs" so yeah, it's a tough market. Can't please everyone. 🍭

Yeah, but you aim to be compatible with http://jsonapi.org/ and meta is about it =)

[jasonlewis]

I'm not really aiming to be compatible with that at all. 😉 JSON API can be difficult and ugly. But, there's a Fractal serializer coming for it which does support the basics of it. I don't like it though. Side loading relationships is a mess.

[philsturgeon]

So some rules I generally stick to on this stuff is:

• Dont show the paginator if there is no data. A Paginator is a tool, so if the tool is definitely not going to be used dont give it to somebody
• If you request a collection, you should get a collect. It might be an empty collection.

Thats another benefit of the {data:[]} approach. you can see quite clearly that theres sod all in that collection.

Even if you did want to return the useless paginator, it seems weird having the paginator and no other data. A happy compromise for many is:

{
    data : [],
    paginator : {
        ... stuff ....
    }
}

But not everyone can be convinced of the power of always having a "data" in there. :)

[bweston92]

It is more efficient to check pagination.total rather then data.length? Even if both are 0 in my opinion.—
Sent from Mailbox for iPhone

On Tue, May 13, 2014 at 2:49 PM, Phil Sturgeon notifications@github.com
wrote:

So some rules I generally stick to on this stuff is:

• Dont show the paginator if there is no data. A Paginator is a tool, so if the tool is definitely not going to be used dont give it to somebody
• If you request a collection, you should get a collect. It might be an empty collection.
  Thats another benefit of the {data:[]} approach. you can see quite clearly that theres sod all in that collection.
  Even if you did want to return the useless paginator, it seems weird having the paginator and no other data. A happy compromise for many is:

{
    data : [],
    paginator : {
        ... stuff ....
    }
}

But not everyone can be convinced of the power of always having a "data" in there. :)

Reply to this email directly or view it on GitHub:
#38 (comment)

[philsturgeon]

I don't think either approach would make any sort of relevant or interesting difference to performance.

Certainly count($data) is slow when there is data but doing either $resp['data'] === array() or isset($resp['data'][0]) or isset($resp['paginator']) is all essentially identical.

[bweston92]

Ok it is an API therefore you need to take into account all possible implementations. If I have a JavaScript client I'd rather rely on pagination.total then data.length it just seems more logical and readable.—
Sent from Mailbox for iPhone

On Tue, May 13, 2014 at 6:17 PM, Phil Sturgeon notifications@github.com
wrote:

I don't think either approach would make any sort of relevant or interesting difference to performance.

Certainly count($data) is slow when there is data but doing either $resp['data'] === array() or isset($resp['data'][0]) or isset($resp['paginator']) is all essentially identical.

Reply to this email directly or view it on GitHub:
#38 (comment)

[philsturgeon]

Building an API around what makes the most readable JavaScript is not what "take into account all possible implementations" means and if (typeof data[0] != "undefined") is pretty readable for anyone. :)

But yeah I don't much care wether you show the paginator or not, that's the least important part of what I suggested.

[rolfnl]

My preference would be that by default it should return an empty object, because it's verbose.

And isn't that the nice thing about serializers (if you're using a transformer like Fractal anyway), you can make a custom one that does it the way you like it.

[philsturgeon]

Yup, as I said I really dont care too much if you put it there or dont, but putting it there WITHOUT a "data" key is nuts, so if you ARE gonna put it there then probably make sure a "data" key is always around.

{
    data : [],
    paginator : {
        ... stuff ....
    }
}

[bweston92]

When will this be fixed in terms of the paginator coming back as a key?

[jasonlewis]

The only way I can see it returning a key is to hard code it. That's
probably not desirable as not everyone would be expecting a data key.

When will this be fixed in terms of the paginator coming back as a key?

—
Reply to this email directly or view it on
GitHubhttps://github.com//issues/38#issuecomment-43074967
.