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

Skip x entries instead of requesting pages #50

Closed
crunchtime-ali opened this issue Jun 28, 2015 · 19 comments
Closed

Skip x entries instead of requesting pages #50

crunchtime-ali opened this issue Jun 28, 2015 · 19 comments

Comments

@crunchtime-ali
Copy link
Contributor

My backend, Sails delivers paginated data like in this form /api/posts?skip=5&limit=5.
skip means that 5 entries are skipped (not five pages).

Is there any way to get that behaviour running with ember-infinity? If not I could extend it to have a switch bool: skipInsteadOfPage (false) to include this behaviour.

In my backend the first page with 5 entries would be: /api/posts?skip=0&limit=5
The second page is: /api/posts?skip=5&limit=5

@hhff
Copy link
Collaborator

hhff commented Jun 28, 2015

are u saying the first 5 posts are skipped? What does limit mean?

There's a starting page parameter in ember infinity, I'd recommend you use that if possible

@crunchtime-ali
Copy link
Contributor Author

I can't control the backend that's why I have to deal with what I got.
limit is your perPageParam: "limit" but there's nothing like pageParam. All I have is skip which skips x number of entries instead of pages.

Here's a better example to illustrate the matter:
In my backend the first page with 5 entries would be: /api/posts?skip=0&limit=5
The second page is: /api/posts?skip=5&limit=5

@crunchtime-ali
Copy link
Contributor Author

Would you accept a pull-request adding a new boolean parameter like I suggested?

@hhff
Copy link
Collaborator

hhff commented Jun 29, 2015

If it makes sense I would - but I haven't had a chance to think through this yet, and your use case. I'm pretty sure you can get around this problem by juggling the startPage and the perPageParam, because skip sounds like it will always be a multiple of limit

@crunchtime-ali
Copy link
Contributor Author

When starting with startPage = 0 and perPageParam = 5 I get the correct data on the first retrieve.
The next one however will request startPage = 1 instead of startPage = 5 which I would need (startPage = skip for me).
So how could I get around this by using startPage and perPageParam?

@hhff
Copy link
Collaborator

hhff commented Jun 29, 2015

ahhh gotcha. once #43 is merged ur golden!

@kellyselden
Copy link
Collaborator

In order to implement this, you would have to depend on the private member _currentPage.

@crunchtime-ali
Copy link
Contributor Author

Since #43 is merged this can be closed. Great work guys!

@DanLeininger
Copy link

@dj-hedgehog Can you post the code and/or an example Route of how you solved this? I'm in the same boat w/ Sails. Thanks

@crunchtime-ali
Copy link
Contributor Author

@kellyselden I tried this but it can't access _currentPage. What am I doing wrong?

export default Ember.Route.extend(InfinityRoute, {
  perPageParam: "limit",     
  pageParam: "skip",               
  totalPagesParam: "meta.total",   
  skip_page: function () {
    return this.get("_currentPage") * 2;
  }.property(),

  model() {
    return this.infinityModel("post", { perPage: 2,
                                        startingPage: 1
                                      }, { skip: "skip_page"});
  }
});

@DanLeininger Yes I will once it works

@DanLeininger
Copy link

I was (and still am) having the same issue, couldn't access this.get("_currentPage")

@DanLeininger
Copy link

@dj-hedgehog you can get around this by creating a policy in Sails to translate the query before it hits waterline:

add policies/deincrementSkip.js

/**
 * deincrementSkip
 *
 * @module      :: Policy
 * @description :: TODO: You might write a short summary of how this policy works and what it represents here.
 * @help        :: http://sailsjs.org/#!/documentation/concepts/Policies
 */

module.exports = function(req, res, next) {

    var page;
    var limit;
    var skip;

    if (req.query.page && req.query.limit) {
        page = Number(req.query.page);
        limit = Number(req.query.limit);
        page = page - 1; //deincrement the page num so skip will be correct
        skip = limit * page; //calculate the # of records to skip
        req.query.skip = skip; //add skip to the query
        delete req.query.page; //cleanup: remove page from the query

    }

    return next();

};

Then in config/policies

   WhateverController: {
        'find': 'deincrementSkip'
    },

I'm changing the perPage attribute to limit in my Route, like here


import Ember from 'ember';
import InfinityRoute from "ember-infinity/mixins/route";

export
default Ember.Route.extend(InfinityRoute, {

    perPageParam: "limit", // instead of "per_page"
    totalPagesParam: "meta.total", // instead of "meta.total_pages"

    model: function(params) {
        return this.infinityModel("post", {
            perPage: 3,
            startingPage: 1,
            sort: 'createdAt DESC'
        });
    }

});

@crunchtime-ali
Copy link
Contributor Author

@DanLeininger
Thanks for the workaround but I don't want to tinker with my server. IMO Behaviour like this should be handled by the client.

@DanLeininger
Copy link

@dj-hedgehog yep, agree, it'd still be nice to have that startingPage 0 not default to 1.

@kellyselden
Copy link
Collaborator

Try watching _currentPage in your computed property.

  skip_page: function () {
    return this.get("_currentPage") * 2;
  }.property('_currentPage'),

It may not be recalculating.

@crunchtime-ali
Copy link
Contributor Author

This works quite nice for me, thank you 👍

@kellyselden
Copy link
Collaborator

You may want to open an issue/PR to make _currentPage public. Since it is private, it has the potential to change name or function at any time.

@hhff
Copy link
Collaborator

hhff commented Jul 25, 2015

👍 for making it public

@crunchtime-ali
Copy link
Contributor Author

You didn't say whether you want some level of abstraction or just turn _currentPage into a public property . I did the latter one and opened PR #58

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

4 participants