Skip to content

Commit

Permalink
fix: regression when fetching has-many and not selecting any fields o…
Browse files Browse the repository at this point in the history
…n a hasone/belongsto relation (#720)
  • Loading branch information
romain-gilliotte authored May 12, 2021
1 parent c98479d commit 74ed623
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/services/has-many-getter.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { pick } from 'lodash';
import Operators from '../utils/operators';
import QueryUtils from '../utils/query';
import PrimaryKeysManager from './primary-keys-manager';
Expand Down Expand Up @@ -35,8 +36,7 @@ class HasManyGetter extends ResourcesGetter {
as: associationName,
scope: false,
required: !!buildOptions.forCount, // Why?
where: options.where,
include: options.include,
...pick(options, ['where', 'include']),
}],
});

Expand Down
19 changes: 12 additions & 7 deletions src/services/line-stat-getter.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,22 @@ ${groupByDateFieldFormated}), 'yyyy-MM-dd 00:00:00')`);
const queryOptions = new QueryOptions(model, { includeRelations: true });
await queryOptions.filterByConditionTree(filters, timezone);

const { include, where } = queryOptions.sequelizeOptions;
const records = await model.unscoped().findAll({
include: include
? include.map((includeProperties) => ({ ...includeProperties, attributes: [] }))
: undefined,
where,
const sequelizeOptions = {
...queryOptions.sequelizeOptions,
attributes: [getGroupByDateInterval(), getAggregate()],
group: getGroupBy(),
order: getOrder(),
raw: true,
});
};

// do not load related properties
if (sequelizeOptions.include) {
sequelizeOptions.include = sequelizeOptions.include.map(
(includeProperties) => ({ ...includeProperties, attributes: [] }),
);
}

const records = await model.unscoped().findAll(sequelizeOptions);

return {
value: fillEmptyDateInterval(
Expand Down
18 changes: 11 additions & 7 deletions src/services/pie-stat-getter.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,8 @@ function PieStatGetter(model, params, options) {
const queryOptions = new QueryOptions(model, { includeRelations: true });
await queryOptions.filterByConditionTree(filters, timezone);

const { include, where } = queryOptions.sequelizeOptions;
const records = await model.unscoped().findAll({
include: include
? include.map((includeProperties) => ({ ...includeProperties, attributes: [] }))
: undefined,
where,
const sequelizeOptions = {
...queryOptions.sequelizeOptions,
attributes: [
[options.Sequelize.col(groupByField), ALIAS_GROUP_BY],
[
Expand All @@ -101,7 +97,15 @@ function PieStatGetter(model, params, options) {
group: getGroupBy(),
order: [[options.Sequelize.literal(ALIAS_AGGREGATE), 'DESC']],
raw: true,
});
};

if (sequelizeOptions.include) {
sequelizeOptions.include = sequelizeOptions.include.map(
(includeProperties) => ({ ...includeProperties, attributes: [] }),
);
}

const records = await model.unscoped().findAll(sequelizeOptions);

return { value: formatResults(records) };
};
Expand Down
28 changes: 28 additions & 0 deletions test/databases.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2990,6 +2990,34 @@ const HasManyDissociator = require('../src/services/has-many-dissociator');
});
});

describe('request on the has-many getter without relations', () => {
it('should generate a valid SQL query', async () => {
expect.assertions(1);
const { models, sequelizeOptions } = initializeSequelize();
const params = {
recordId: 100,
associationName: 'addresses',
fields: {
address: 'line,zipCode,city,country',
},
page: { number: '1', size: '20' },
timezone: 'Europe/Paris',
};
try {
await new HasManyGetter(
models.user,
models.address,
sequelizeOptions,
params,
)
.perform();
expect(true).toBeTrue();
} finally {
connectionManager.closeConnection();
}
});
});

describe('request on the has-many-getter with a sort on an attribute', () => {
it('should generate a valid SQL query', async () => {
expect.assertions(1);
Expand Down

0 comments on commit 74ed623

Please sign in to comment.