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

Modify the paginationCollection to accept jugglingdb option #1

Open
lforge opened this issue Apr 26, 2012 · 3 comments
Open

Modify the paginationCollection to accept jugglingdb option #1

lforge opened this issue Apr 26, 2012 · 3 comments

Comments

@lforge
Copy link
Contributor

lforge commented Apr 26, 2012

FYI, when I use your pagination routine, I realize that it is not processing the other jugglingdb option such as order, where. So I have customized your code a bit by at least accepting the order option:

In the index.js file, I have made the following change:

// orm method
function paginateCollection(opts, callback) {
    var limit = opts.limit || 10;
    var page = opts.page || 1;
    var order = opts.order||'1';  // added this line
    var Model = this;

    Model.count(function (err, totalRecords) {

    // modified the following line to accept order option

        Model.all({limit: limit, offset: (page - 1) * limit, order: order }, function (err, records) { 
            if (err) return callback(err);
            records.totalRecords = totalRecords;
            records.currentPage = page;
            records.totalPages = Math.ceil(totalRecords / limit);
            callback(null, records);
        });
    });
}

Do you think that you can add the support of where option and make this as part of your main code then? I am not too familiar with github just yet. Perhaps I can fork this and add this change for you? thanks.

Thanks.

@1602
Copy link
Owner

1602 commented Apr 26, 2012

I'm not sure line "opts.order||'1';" is correct, because order param
requires fieldname and direction. For example 'date DESC', or 'id', or 'id
ASC'.
Sure feel free to fork and submit your pull request, if it works for you.

On Thu, Apr 26, 2012 at 4:52 PM, lforge <
[email protected]

wrote:

FYI, when I use your pagination routine, I realize that it is not
processing the other jugglingdb option such as order, where. So I have
customized your code a bit by at least accepting the order option:

In the index.js file, I have made the following change:

// orm method
function paginateCollection(opts, callback) {
var limit = opts.limit || 10;
var page = opts.page || 1;
var order = opts.order||'1'; // added this line
var Model = this;

Model.count(function (err, totalRecords) {

// modified the following line to accept order option

   Model.all({limit: limit, offset: (page - 1) * limit, order: order

}, function (err, records) {
if (err) return callback(err);
records.totalRecords = totalRecords;
records.currentPage = page;
records.totalPages = Math.ceil(totalRecords / limit);
callback(null, records);
});
});
}

Do you think that you can add the support of where option and make this as
part of your main code then? I am not too familiar with github just yet.
Perhaps I can fork this and add this change for you? thanks.

Thanks.


Reply to this email directly or view it on GitHub:
#1

Thanks,
Anatoliy Chakkaev

@lforge
Copy link
Contributor Author

lforge commented Apr 26, 2012

In this case, the order field data are passed into the routine. So in the controller, you will do this:

action(function index() {
    this.title = v_form_title_p;  // Updated to use new controller level variable.
    var page = req.param('page') || 1;

    Player_v.paginate({order: 'first_name, last_name', limit: 10, page: page}, function (err, players) {
       if(!err) {
        render({
            players: players
        });
       }
    });
});

So the '1' default should generate the SQL to be "order by 1" by default. I think that it should be ok. But I may be missing something here though.

I will give it a shot on forking it. Thanks.

@lforge
Copy link
Contributor Author

lforge commented May 22, 2012

I have submitted a pull request for my change. If this works, we can close this issue. Thanks.

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

2 participants