-
Notifications
You must be signed in to change notification settings - Fork 216
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
ES6 6.0 / Node v4.2.4+ Peer Review for v6.0.0 #73
base: master
Are you sure you want to change the base?
Changes from 14 commits
779dd55
cda8344
ff90322
0c7c89c
9020404
67dc751
1fa8a35
c81fc3e
8236882
beb90fa
9c0fd24
fadddcf
0a139e9
323fe54
5a881ae
7a8a11c
7867da7
d8e8299
8581975
3366694
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
language: node_js | ||
|
||
node_js: | ||
- "4.2.4" | ||
- "4.2" | ||
- "stable" | ||
|
||
services: | ||
- mongodb | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
'use strict'; | ||
|
||
const mongoose = require('mongoose'); | ||
|
||
/** | ||
* @package mongoose-paginate | ||
* @param {Object} [query={}] | ||
|
@@ -18,13 +20,13 @@ | |
|
||
function paginate(query, options, callback) { | ||
query = query || {}; | ||
options = Object.assign({}, paginate.options, options); | ||
let select = options.select; | ||
options = options || {}; | ||
let select = options.select || null; | ||
let sort = options.sort; | ||
let populate = options.populate; | ||
let lean = options.lean || false; | ||
let leanWithId = options.leanWithId ? options.leanWithId : true; | ||
let limit = options.limit ? options.limit : 10; | ||
let leanWithId = options.leanWithId || false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most people don't need an "id" specifically, they just need { "lean" : true }, and to access _id, whether it is id or _id. Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By default mongoose add |
||
let limit = options.limit || 10; | ||
let page, offset, skip, promises; | ||
if (options.offset) { | ||
offset = options.offset; | ||
|
@@ -37,58 +39,66 @@ function paginate(query, options, callback) { | |
offset = 0; | ||
skip = offset; | ||
} | ||
if (limit) { | ||
let docsQuery = this.find(query) | ||
.select(select) | ||
.sort(sort) | ||
.skip(skip) | ||
.limit(limit) | ||
.lean(lean); | ||
if (populate) { | ||
[].concat(populate).forEach((item) => { | ||
docsQuery.populate(item); | ||
}); | ||
} | ||
promises = { | ||
docs: docsQuery.exec(), | ||
count: this.count(query).exec() | ||
}; | ||
if (lean && leanWithId) { | ||
promises.docs = promises.docs.then((docs) => { | ||
docs.forEach((doc) => { | ||
doc.id = String(doc._id); | ||
}); | ||
return docs; | ||
}); | ||
} | ||
let docsQuery = this.find(query) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if limit is zero? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Limit is specified below, want to push a test to show me what you mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
.select(select) | ||
.sort(sort) | ||
.skip(skip) | ||
.limit(limit) | ||
.lean(lean); | ||
if (populate) { | ||
[].concat(populate).forEach((item) => { | ||
docsQuery.populate(item); | ||
}); | ||
} | ||
let countQuery = this.count(query); | ||
promises = { | ||
docs: docsQuery.exec(), | ||
count: countQuery.exec() | ||
}; | ||
promises = Object.keys(promises).map((x) => promises[x]); | ||
return Promise.all(promises).then((data) => { | ||
let result = { | ||
docs: data.docs, | ||
total: data.count, | ||
limit: limit | ||
}; | ||
if (offset !== undefined) { | ||
result.offset = offset; | ||
} | ||
if (page !== undefined) { | ||
result.page = page; | ||
result.pages = Math.ceil(data.count / limit) || 1; | ||
} | ||
if (typeof callback === 'function') { | ||
return callback(null, result); | ||
} | ||
let promise = new Promise(); | ||
promise.resolve(result); | ||
return promise; | ||
let promise = new Promise((resolve, reject) => { | ||
Promise.all(promises).then((data) => { | ||
let docs = data[0]; | ||
let count = data[1]; | ||
let result = { | ||
docs: docs, | ||
count: count, | ||
limit: limit | ||
}; | ||
if (lean && leanWithId) { | ||
result.docs = result.docs.map((doc) => { | ||
doc.id = String(doc._id); | ||
return doc; | ||
}); | ||
} | ||
if (offset !== undefined) { | ||
result.offset = offset; | ||
} | ||
if (page !== undefined) { | ||
result.page = page; | ||
result.pages = Math.ceil(result.count / limit) || 1; | ||
} | ||
if (typeof callback === 'function') { | ||
return callback(null, result); | ||
} | ||
resolve(result); | ||
}, (error) => { | ||
if (typeof callback === 'function') { | ||
return callback(error, null); | ||
} | ||
reject(error); | ||
}); | ||
}); | ||
return promise; | ||
} | ||
|
||
/** | ||
* @param {Schema} schema | ||
* use native ES6 promises verus mpromise | ||
*/ | ||
|
||
mongoose.Promise = global.Promise; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be wary about this line. Some people like to set |
||
|
||
module.exports = function(schema) { | ||
schema.statics.paginate = paginate; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove
paginate.options
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jokero There was nothing ever sent into paginate.options and that which was wasn't being tested. It could be added if we had it in the docs and tests for it ... feel free to push :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paginate.options, limit=0, leanWithId=true were added in 5.0.0, tested and documented
https://github.com/edwardhotchkiss/mongoose-paginate/tree/v5.0.0#set-custom-default-options-for-all-queries
https://github.com/edwardhotchkiss/mongoose-paginate/tree/v5.0.0#zero-limit