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

Expose Pagination to client consumers #76

Closed
kadamwhite opened this issue Aug 6, 2014 · 12 comments
Closed

Expose Pagination to client consumers #76

kadamwhite opened this issue Aug 6, 2014 · 12 comments
Assignees
Milestone

Comments

@kadamwhite
Copy link
Collaborator

Many responses from the API will be paginated, either explicitly or by default. For example, if post collections return 10 items by default, and you have 100 items in your DB, both of these will come back with a header link with rel="next":

wp.posts()...
wp.posts().filter('posts_per_page', 2)...

The general approach I have taken with this library is to let the user specify a request, and return what they asked for. At the moment the way this manifests is that if you request wp.posts()..., you get the posts collection (that is, the response body) back in the request callback: the headers are not included. I favored this over returning the full object, because this felt like a very clunky thing to have to do every time:

wp.posts().then(function(resp) {
  return resp.body;
}).then(function(posts) {
  // do something with the posts collection
});

when in many cases it's only the response body that you care about. The one exception is paginated collections, as it is the response header that includes the pagination information (via the link header and two custom header properties).

I want to provide an intuitive way to expose collection paging to users, preferably without making it substantially more verbose to request a specific resource. I am not sure how to proceed.

Option 1: Return the raw request object

This is what is outlined above, where the request object is passed back in full and you can manually grab the body or check the link header as you need to. This feels like a bad solution because it makes the default usage (get a specific resource) more difficult by requiring a pipe through a then, and it doesn't give you any additional functionality (you need to manually parse headers to handle paging).

Option 2: Augment the response data with paging properties

The idea is to augment the response object with properties or methods relating to pagination in cases when the resulting data is determined to be paginated. There were various ways it could be implemented:

Example 1, just expose the links and pagination headers:

wp.posts().then(function(posts) {
  // How many posts did I get back?
  console.log( posts.length ); // normal array

  // posts also has a .paging property, on which is set custom
  // pagination-related information

  // How many posts, total...?
  console.log( posts.paging.total );
  // Next collection URL?
  console.log( posts.paging.next );
  // Previous?
  console.log( posts.paging.prev );
  // total number of pages?
  console.log( posts.paging.totalPages );
});

Example 2, give the returned collection methods that would function as fully-fledged WPRequest objects

wp.posts().then(function(posts) {
  // Make a request for the next page of posts
  posts.next().then(function(nextPagePosts) {
    // and so on
    posts.next().then(function(thirdPagePosts) {
      // and (halcyon and) on and on
    })
  });
});

Things I like about this approach:

  • Doesn't expose the headers unless you asked for them directly (with wp.posts.head()), which keeps the data returned from the service directly accessible to clients without transformation
  • Fits the existing paradigm we've been discussing in Implement lo-dash/underscore support for return values #5, where collections would get augmented with Lodash operators (like Backbone collections/LayoutManager child view collections)

That said, when I pitched this to a collaborator he described it as "yucky" and I couldn't think of any other examples of systems that work this way. I want to present it for review anyway, though, because it was my first thought.

Option 2A: Let users define custom data augmentation methods

This is an extension of one way the above could be implemented. If you imagine a prototype method on CollectionRequest called transform, which could be used to register a methods that would be run on the request response before being returned (which would replace the existing returnBody and returnHeaders methods), you could let users opt-in to things like lodash collection methods, pagination handling, or their own custom transformations:

wp.posts().transform(wp.transforms.paginate).then(function(posts) {
  // posts has some default pagination methods/props
});
wp.posts().transform(wp.transforms.collection).then(function(posts) {
  // Now it's got Underscore methods, and this would work:
  posts.pluck('titles')...
});
wp.posts().transform(myPluckTitles).then(function(posts) {
  // posts is now an array of titles
});
wp.posts().transform(myGetPagingLinks).then(function(links) {
  // now it's a bare object of links -- or whatever you want it to be
});
// Could also be exposed directly, through a custom chaining method
wp.posts().paginated().get(/* ... */);

Upshots:

  • Much more flexible
  • Provides a consistent API for handling pagination, and for other types of transformation like augmenting collections with lodash collection methods

Downsides:

  • Would probably require making some sort of wp.transforms or wp.utils namespace to hold the default transformation methods
  • More chaining methods is not/should not be the answer to everything
  • Introduces unintuitive behavior vs having a consistent default

Option 3: Return a custom object

This is in some ways an extension of 1, just with some of the things I noted addressed. Basically, all requests come back as an object with four properties:

  • headers: the raw, unprocessed HTTP headers
  • body: the raw, unprocessed response body
  • data: the data returned by the request (which is the body, but the important part is that data feels more intuitive and accurate vs what the information actually is: I shouldn't care that it's the body, I care that it's the data returned from the server.)
  • paging/links: custom properties designating previous/next collections, etc

The advantage of this over the first option is that this, to me, feels more intuitive than returning resp.body:

wp.posts().then(function(resp) {
  // collection:
  console.log( resp.data );
  // pagination URL:
  console.log( resp.links.next );
});

The disadvantage is that I'm right back where I started in terms of having to pipe responses through yet another then in order to boil them down to the actual response body, if that's what I wanted originally.


I am probably overlooking additional options, here. These are the actual pagination-related header properties exposed by the API endpoint, for an example collection of two posts:

link: '</wp-json/posts?filter%5Bposts_per_page%5D=2&page=1>; rel="prev",
    </wp-json/posts?filter%5Bposts_per_page%5D=2&page=3>; rel="next",
    <http://my.site.com/wp-json/posts/214>; rel="item"; title="Article Title",
    <http://my.site.com/wp-json/posts/211>; rel="item"; title="Article Title"',
'x-wp-total': '17',
'x-wp-totalpages': '9',

It's possible the WordPress API itself could be extended to provide more headers or more links such as a first or last page, as it's a work-in-progress, but for the time being I'm trying to figure out if there's any other approach than the ones outlined above to make answering these questions (and others like them) easy:

Questions a solution should answer

  1. Are there more pages in this collection?
  2. What page am I on?
  3. How do I get to the previous/next pages?

Any recommendations for good API clients that handle this sort of behavior are welcome, as is commentary or dissension around the above options I have outlined.

@kadamwhite kadamwhite added this to the Polish milestone Aug 6, 2014
@kadamwhite kadamwhite self-assigned this Aug 6, 2014
@kadamwhite
Copy link
Collaborator Author

Tagging @carldanley @tkellen @bmac @tbranyen @rmccue @iros for your thoughts, since I've either talked to you about this project, or heard you mentioned by people I've talked to about it

@kadamwhite
Copy link
Collaborator Author

Also @tlovett1 @tollmanz

@carldanley
Copy link
Contributor

@kadamwhite Excellent writeup. All of your points are great. While I was reading through it, I really felt like the following would be a decent solution:

  • provide a .head() method that allows you to retrieve the header
  • provide a .raw() method that returns the raw request object, if needed (includes option 1)
  • Instead of automatically augmenting data based on assumptions, expose an augmentation layer for the developer to either use built in transformations (pagination being 1 of those) OR write their own augmentation to transform the data based on what they need.

This would solve most of what you need and allow the developer to make decisions about pagination as needed.

I need more time to digest this idea but these are my initial thoughts, at the very least. If you expose an augmentation layer, you can introduce new stuff to enhance the library as needed. Meanwhile, you're allowing the developer to write their own methods; which I think results in a very powerful API.

@kadamwhite
Copy link
Collaborator Author

Thanks for the quick response, @carldanley. Some thoughts:

  • .head() exists, as it is one of the request methods available on WPRequest. As such, getting the headers independently is still possible.
  • .raw() is interesting, and worth considering regardless of the decision here.
  • I believe some sort of default behavior is needed, because the link header is a raw string: You have to use a link parser library like parse-link-header or parse-links to deconstruct that into something useful; I think the value that's returned should be exposed in some way to client consumers, because being able to work intuitively with pagination is going to be critical because requests will be limited to max 100 items at once.

@carldanley
Copy link
Contributor

Playing devil's advocate here:

Couldn't you provide the augmentation layer and a built-in transformation for handling pagination? Then have links automatically consume this transformation to deliver the results you're looking for?

@tlovett1
Copy link
Member

tlovett1 commented Aug 7, 2014

I'm a fan of option 2. It seems the most intuitive to me rather than having to deal with checking headers. Pagination needs to be added to collections in the WP API backbone client so I'll definitely be following this closely.

@iros
Copy link

iros commented Aug 7, 2014

This is a great writeup @kadamwhite.

here's my 2 cents:

Considering you're making a network request, using then is not that unreasonable. If anything, it should be clear to the user that this is an async operation and might take time. As such, this approach feels the most natural to me.

wp.posts().then(function(resp) {
  // collection:
  console.log( resp.data );
  // pagination URL:
  console.log( resp.links.next );
});

I do like the properties you established in Option 1, example 2 in terms of expressiveness.
Not sure this helps much, but I think you're on the right track.

@kadamwhite
Copy link
Collaborator Author

@iros, there weren't any examples under Option 1: did you mean Option 2, example 2?

The library uses then, or a node-style callback syntax, to handle the asynchronicity no matter what; these are equivalent in the current build:

// Do a GET
wp.posts().get(function(err, data) {});
wp.posts().then(function(data) {}, function(err) {});

// Do a POST
wp.posts().data({}).post(function(err, data) {});
wp.posts().data({}).post().then(function(err, data) {});

As such, it's not the presence of then that bothers me: it's that in the above, the value with which the promise is resolved is the request payload, and if we converted that into the full request (option 1) then every handler would have to explicitly pluck the body off. Not the end of the world, but that's what I was trying to articulate that I was hoping to avoid with Option 2.

@tbranyen
Copy link
Collaborator

tbranyen commented Aug 8, 2014

@kadamwhite I think augmenting the collection makes the most sense. Simply exposing headers or transforms is useful, but not particularly great for pagination discovery. Also introducing a new method next() is rather confusing since you already have a way to fetch posts.

I think that Option 2 offers all the flexibility that's necessary assuming you also change how wp.posts() works so that passing in a page offset will fetch it:

// Make the first request for the first page with the default limit.
wp.posts().then(function(page1) {
  // Resolve with both the first and second page.
  return Promise.all([
    page1,
    // Also fetch the second page for example purposes.
    wp.posts({ page: page1.paging.next })
  ]);
}).then(pages) {});

This would make it very clear on how you could fetch all pages. Building up that Promise.all array and then chaining with a .then.

/2c

@kadamwhite
Copy link
Collaborator Author

Alright; this is the proposed structure for collection responses:

  • collection (array)
    • paging (object)
      • total (int; total # of records)
      • totalPages (int; total # of pages)
      • links (object)
        • next (string; URL to next collection)
        • prev (string; URL to previous collection)

I'm also planning to pre-bind paging.next and paging.prev properties, which would be WPRequest objects bound directly to the URLs exposed through paging.links.(prev|next). The reasoning for this is to make it easy to immediately trigger a request for the next or previous collection without changing the behavior of the base wp.* methods (i.e., the parameter suggestion @tbranyen brought up), and to make a clear distinction between exposing the raw URL in paging.links.(next|prev) (which is all we have for links right now) vs a user-facing object in paging.(next|prev).

kadamwhite added a commit that referenced this issue Sep 11, 2014
This implementation follows the structure outlined in the discussion of #76:
parse the header information, and use it to augment the response collection.
The presence or absence of `._paging` can therefore be used to quickly
assess whether the response collection is complete, or just one page's worth.

I ended up not taking the approach of implementing this as a drop-in method
to mutate the response data, as I needed pagination handling now and we
have more discussion to do before we go forward with the response collection
augmentation interface. There was also a question of whether that interface
would be exposed pre-returnBody, or after it; if we went with the latter,
there would be no way to access header information.

We've opened a conversation with @rmccue about how Pagination may evolve
within the ReST API plugin; this solution will evolve along with the plugin
itself, at least up until the API hits core.
@kadamwhite
Copy link
Collaborator Author

Initial implementation largely handled in #77, #79 and #80.

@kadamwhite
Copy link
Collaborator Author

Closing as this is officially released in #82 (0.3.0); we may come in and refactor this to make pagination work more as a chained output transformation, rather than part of the core WPRequest functionality, but the public API for pagination info (at least for what the WP API currently provides) is implemented as documented above.

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

No branches or pull requests

5 participants