1602 / railway-pagination

Paginatio plugin for RailwayJS MVC framework
9 stars 6 forks source link

Modify the paginationCollection to accept jugglingdb option #1

Open lforge opened 12 years ago

lforge commented 12 years ago

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 commented 12 years ago

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 < reply@reply.github.com

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: https://github.com/1602/railway-pagination/issues/1

Thanks, Anatoliy Chakkaev

lforge commented 12 years ago

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 commented 12 years ago

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