Skip to content

Commit

Permalink
Fix accidental eager-loading count results (#2397)
Browse files Browse the repository at this point in the history
  • Loading branch information
lehni committed Jul 20, 2023
1 parent e78af21 commit a773b88
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
5 changes: 3 additions & 2 deletions lib/queryBuilder/operations/eager/WhereInEagerOperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const promiseUtils = require('../../../utils/promiseUtils');

const { EagerOperation } = require('./EagerOperation');
const { isMsSql, isOracle, isSqlite } = require('../../../utils/knexUtils');
const { asArray, flatten, chunk } = require('../../../utils/objectUtils');
const { isObject, asArray, flatten, chunk } = require('../../../utils/objectUtils');
const { ValidationErrorType } = require('../../../model/ValidationError');
const { createModifier } = require('../../../utils/createModifier');
const { RelationDoesNotExistError } = require('../../../model/RelationDoesNotExistError');
Expand Down Expand Up @@ -63,7 +63,8 @@ class WhereInEagerOperation extends EagerOperation {

const models = asArray(result);

if (!models.length) {
// Check models to be actual objects, to filter out `count` results (#2397).
if (!models.length || !isObject(models[0])) {
return result;
}

Expand Down
28 changes: 27 additions & 1 deletion tests/integration/patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const expect = require('expect.js');
const Promise = require('bluebird');
const { inheritModel } = require('../../lib/model/inheritModel');
const { expectPartialEqual: expectPartEql } = require('./../../testUtils/testUtils');
const { Model, ValidationError, raw } = require('../../');
const { Model, QueryBuilder, ValidationError, raw } = require('../../');
const { isPostgres, isSqlite } = require('../../lib/utils/knexUtils');
const mockKnexFactory = require('../../testUtils/mockKnex');

Expand Down Expand Up @@ -330,6 +330,32 @@ module.exports = (session) => {
});
});
});

it('should not attempt to eager-load relations on patch `count` results when using overwritten `execute()` to load relations (#2397)', () => {
let runBeforeCalled = 0;

class MyModel1 extends Model1 {}

MyModel1.QueryBuilder = class MyQueryBuilder1 extends QueryBuilder {
execute() {
this.withGraphFetched('model1Relation2');
return super.execute();
}
};

return MyModel1.query()
.context({
runBefore() {
runBeforeCalled++;
},
})
.where({ id: 1 })
.patch({ model1Prop1: 'updated text' })
.then((count) => {
expect(count).to.eql(1);
expect(runBeforeCalled).to.eql(1);
});
});
});

describe('.query().patchAndFetchById()', () => {
Expand Down

0 comments on commit a773b88

Please sign in to comment.