From 94a9551743c81c7c2fa7003bccae4cf0a791f010 Mon Sep 17 00:00:00 2001 From: papb Date: Wed, 7 Aug 2019 22:20:57 -0300 Subject: [PATCH 1/2] refactor(tests): refactor model.count tests --- test/integration/model/count.test.js | 218 +++++++++++++-------------- 1 file changed, 101 insertions(+), 117 deletions(-) diff --git a/test/integration/model/count.test.js b/test/integration/model/count.test.js index 65cee884647e..5658ecf11e11 100644 --- a/test/integration/model/count.test.js +++ b/test/integration/model/count.test.js @@ -6,119 +6,97 @@ const chai = require('chai'), DataTypes = require('../../../lib/data-types'); describe(Support.getTestDialectTeaser('Model'), () => { - beforeEach(function() { - this.User = this.sequelize.define('User', { - username: DataTypes.STRING, - age: DataTypes.INTEGER - }); - this.Project = this.sequelize.define('Project', { - name: DataTypes.STRING - }); - - this.User.hasMany(this.Project); - this.Project.belongsTo(this.User); - - return this.sequelize.sync({ force: true }); - }); - describe('count', () => { beforeEach(function() { - return this.User.bulkCreate([ - { username: 'boo' }, - { username: 'boo2' } - ]).then(() => { - return this.User.findOne(); - }).then(user => { - return user.createProject({ - name: 'project1' - }); + this.User = this.sequelize.define('User', { + username: DataTypes.STRING, + age: DataTypes.INTEGER }); + this.Project = this.sequelize.define('Project', { + name: DataTypes.STRING + }); + + this.User.hasMany(this.Project); + this.Project.belongsTo(this.User); + + return this.sequelize.sync({ force: true }); }); it('should count rows', function() { - return expect(this.User.count()).to.eventually.equal(2); + return this.User.bulkCreate([ + { username: 'foo' }, + { username: 'bar' } + ]).then(() => { + return expect(this.User.count()).to.eventually.equal(2); + }); }); it('should support include', function() { - return expect(this.User.count({ - include: [{ - model: this.Project, - where: { - name: 'project1' - } - }] - })).to.eventually.equal(1); + return this.User.bulkCreate([ + { username: 'foo' }, + { username: 'bar' } + ]).then(() => this.User.findOne()) + .then(user => user.createProject({ name: 'project1' })) + .then(() => { + return expect(this.User.count({ + include: [{ + model: this.Project, + where: { name: 'project1' } + }] + })).to.eventually.equal(1); + }); }); - it('should return attributes', function() { - return this.User.create({ - username: 'valak', - createdAt: (new Date()).setFullYear(2015) - }) - .then(() => - this.User.count({ - attributes: ['createdAt'], - group: ['createdAt'] - }) - ) - .then(users => { - expect(users.length).to.be.eql(2); - - // have attributes - expect(users[0].createdAt).to.exist; - expect(users[1].createdAt).to.exist; - }); + it('should count groups correctly and return attributes', function() { + return this.User.bulkCreate([ + { username: 'foo' }, + { username: 'bar' }, + { + username: 'valak', + createdAt: (new Date()).setFullYear(2015) + } + ]).then(() => this.User.count({ + attributes: ['createdAt'], + group: ['createdAt'] + })).then(users => { + expect(users.length).to.be.eql(2); + expect(users[0].createdAt).to.exist; + expect(users[1].createdAt).to.exist; + }); }); it('should not return NaN', function() { - return this.sequelize.sync({ force: true }) - .then(() => - this.User.bulkCreate([ - { username: 'valak', age: 10 }, - { username: 'conjuring', age: 20 }, - { username: 'scary', age: 10 } - ]) - ) - .then(() => - this.User.count({ - where: { age: 10 }, - group: ['age'], - order: ['age'] - }) - ) - .then(result => { - // TODO: `parseInt` should not be needed, see #10533 - expect(parseInt(result[0].count, 10)).to.be.eql(2); - return this.User.count({ - where: { username: 'fire' } - }); - }) - .then(count => { - expect(count).to.be.eql(0); - return this.User.count({ - where: { username: 'fire' }, - group: 'age' - }); - }) - .then(count => { - expect(count).to.be.eql([]); + return this.User.bulkCreate([ + { username: 'valak', age: 10 }, + { username: 'conjuring', age: 20 }, + { username: 'scary', age: 10 } + ]).then(() => this.User.count({ + where: { age: 10 }, + group: ['age'], + order: ['age'] + })).then(result => { + // TODO: `parseInt` should not be needed, see #10533 + expect(parseInt(result[0].count, 10)).to.be.eql(2); + return this.User.count({ + where: { username: 'fire' } + }); + }).then(count => { + expect(count).to.be.eql(0); + return this.User.count({ + where: { username: 'fire' }, + group: 'age' }); + }).then(count => { + expect(count).to.be.eql([]); + }); }); it('should be able to specify column for COUNT()', function() { - return this.sequelize.sync({ force: true }) - .then(() => - this.User.bulkCreate([ - { username: 'ember', age: 10 }, - { username: 'angular', age: 20 }, - { username: 'mithril', age: 10 } - ]) - ) - .then(() => - this.User.count({ - col: 'username' - }) - ) + return this.User.bulkCreate([ + { username: 'ember', age: 10 }, + { username: 'angular', age: 20 }, + { username: 'mithril', age: 10 } + ]).then(() => this.User.count({ col: 'username' })) .then(count => { expect(count).to.be.eql(3); return this.User.count({ @@ -132,43 +110,49 @@ describe(Support.getTestDialectTeaser('Model'), () => { }); it('should be able to use where clause on included models', function() { - const queryObject = { + const countOptions = { col: 'username', include: [this.Project], where: { '$Projects.name$': 'project1' } }; - return this.User.count(queryObject).then(count => { - expect(count).to.be.eql(1); - queryObject.where['$Projects.name$'] = 'project2'; - return this.User.count(queryObject); - }).then(count => { - expect(count).to.be.eql(0); - }); + return this.User.bulkCreate([ + { username: 'foo' }, + { username: 'bar' } + ]).then(() => this.User.findOne()) + .then(user => user.createProject({ name: 'project1' })) + .then(() => { + return this.User.count(countOptions).then(count => { + expect(count).to.be.eql(1); + countOptions.where['$Projects.name$'] = 'project2'; + return this.User.count(countOptions); + }); + }) + .then(count => { + expect(count).to.be.eql(0); + }); }); it('should be able to specify column for COUNT() with includes', function() { - return this.sequelize.sync({ force: true }).then(() => - this.User.bulkCreate([ - { username: 'ember', age: 10 }, - { username: 'angular', age: 20 }, - { username: 'mithril', age: 10 } - ]) - ).then(() => - this.User.count({ - col: 'username', - distinct: true, - include: [this.Project] - }) - ).then(count => { + return this.User.bulkCreate([ + { username: 'ember', age: 10 }, + { username: 'angular', age: 20 }, + { username: 'mithril', age: 10 } + ]).then(() => this.User.count({ + col: 'username', + distinct: true, + include: [this.Project] + })).then(count => { expect(count).to.be.eql(3); return this.User.count({ col: 'age', distinct: true, include: [this.Project] }); - }).then(count => expect(count).to.be.eql(2)); + }).then(count => { + expect(count).to.be.eql(2); + }); }); }); From 76bf06874663da815eb6cdfb62966f5cf7319988 Mon Sep 17 00:00:00 2001 From: papb Date: Wed, 7 Aug 2019 22:32:51 -0300 Subject: [PATCH 2/2] fix(count): fix null count with includes --- lib/model.js | 4 +++- test/integration/model/count.test.js | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/model.js b/lib/model.js index 035cb5bfc520..95171aad3c69 100644 --- a/lib/model.js +++ b/lib/model.js @@ -2017,7 +2017,9 @@ class Model { */ static count(options) { return Promise.try(() => { - options = _.defaults(Utils.cloneDeep(options), { hooks: true }); + options = Utils.cloneDeep(options); + options = _.defaults(options, { hooks: true }); + options.raw = true; if (options.hooks) { return this.runHooks('beforeCount', options); } diff --git a/test/integration/model/count.test.js b/test/integration/model/count.test.js index 5658ecf11e11..0d7a6d0f6387 100644 --- a/test/integration/model/count.test.js +++ b/test/integration/model/count.test.js @@ -155,5 +155,26 @@ describe(Support.getTestDialectTeaser('Model'), () => { }); }); + it('should work correctly with include and whichever raw option', function() { + const Post = this.sequelize.define('Post', {}); + this.User.hasMany(Post); + return Post.sync({ force: true }) + .then(() => Promise.all([this.User.create({}), Post.create({})])) + .then(([user, post]) => user.addPost(post)) + .then(() => Promise.all([ + this.User.count(), + this.User.count({ raw: undefined }), + this.User.count({ raw: false }), + this.User.count({ raw: true }), + this.User.count({ include: Post }), + this.User.count({ include: Post, raw: undefined }), + this.User.count({ include: Post, raw: false }), + this.User.count({ include: Post, raw: true }) + ])) + .then(counts => { + expect(counts).to.deep.equal([1, 1, 1, 1, 1, 1, 1, 1]); + }); + }); + }); });