Upgrades moongose and fixes to work with Node 8#151
Upgrades moongose and fixes to work with Node 8#151CGeorges wants to merge 11 commits intoallcount:masterfrom CGeorges:master
Conversation
…is not consistent or desired
Allows API filtering based on reference other property other than ID by passing a NULL id:
http://localhost:9080/api/entity/Keys/?filtering={"filtering":{"market":{"id":null,"name":"se"}}}
chikh
left a comment
chikh
left a comment
There was a problem hiding this comment.
Good ideas, but have a few questions...
| "RAD" | ||
| ], | ||
| "version": "1.24.1", | ||
| "version": "1.25.0", |
There was a problem hiding this comment.
Seems like you are fixing bug (project doesn't work with the Node 8) according to the PR's title. So according to the http://semver.org/ it should be 1.24.2, shoudn't it?
There was a problem hiding this comment.
Having that last update was too long ago, I wanted to avoid any build updates that might break existing projects. It can also be viewed as a feature "Support for Node8), along with the feature that I commited today.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "name": "AllcountJS", | |||
| "name": "allcountjs", | |||
There was a problem hiding this comment.
Why have you decided to change letter's case in the name?
There was a problem hiding this comment.
As per bower warning "bower invalid-meta The "name" is recommended to be lowercase, can contain digits, dots, dashes"
blueimp-load-image
Outdated
| @@ -0,0 +1,8 @@ | |||
| module.exports = require('./load-image') | |||
There was a problem hiding this comment.
Could you please explain why you've added file blueimp-load-image? What is it for?
There was a problem hiding this comment.
File was added by a bower package. I've removed it from repository and added it to .gitignore
package.json
Outdated
| "moment": "^2.6.0", | ||
| "mongoose": "~3.8.21", | ||
| "mongoose-long": "^0.0.2", | ||
| "mongoose": "^4.13.0", |
There was a problem hiding this comment.
Updating dependencies is a great idea, but I think it's better to stick with ~ or even "exact version" (no symbol) since ^ could possibly break something.
There was a problem hiding this comment.
You're right, I've changed to only minor versions ~
package.json
Outdated
| "passport": "^0.2.0", | ||
| "passport-local": "^1.0.0", | ||
| "q": "1.0.1", | ||
| "q": "^1.5.1", |
There was a problem hiding this comment.
You're right, I've changed to only minor versions ~
| service.findRange = function (table, filteringAndSorting, start, count) { | ||
| // Force numeric | ||
| start = Number(start); | ||
| count = Number(count); |
There was a problem hiding this comment.
Well... The idea is OK, but what if someone will pass not a number here? It's better to make full check here. And doesn't driver has it's own protection against wrong arguments?
There was a problem hiding this comment.
Start&Count are pagination related, they will always be integers. With Node8 and updated moongose this crashes allcountjs as it always expects a number. Unfortunately driver doesn't do the conversion, it would just crash with error.
|
I've made some changes as per your comments along with a new feature to allow filtering on other properties rather than ids for references. blueimp-image-load apparently comes from a bower package, i've removed the file from repository and added it to .gitignore. |
2 participants
First release out of many that I'm planning to add.