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.

Sign up for GitHub

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

Jump to bottom

fix(count): fix null count with includes #11295

Merged
merged 2 commits into from Aug 11, 2019
Merged

fix(count): fix null count with includes #11295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94a9551
refactor(tests): refactor model.count tests
papb Aug 8, 2019
76bf068
fix(count): fix null count with includes
papb Aug 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/model.js
View file
Open in desktop
Expand Up @@ -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);
}
Expand Down
239 changes: 122 additions & 117 deletions test/integration/model/count.test.js
View file
Open in desktop
Expand Up @@ -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({
Expand All @@ -132,43 +110,70 @@ 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);
});
});

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]);
});
});

});
Expand Down