Skip to content

Commit f3001be

Browse files
isaachillyisaachilly
authored
[O2B-1520] Add filtering by run intervals to envs (#2062)
- add filtering by run number ranges to environments page - update GetAllEnvironmentsDto.js and GetAllEnvironmentsUseCase.js to support using ranges - add UI, usecase and API tests for valid and invalid run number ranges. - revert range validation logic (as well as its tests) to state it was in before PR #2033 - improve error messages for invalid input formats for validateRange and apply to DTOs that use it and any tests
1 parent 7f6dbfd commit f3001be

File tree

9 files changed

+113
-50
lines changed

9 files changed

+113
-50
lines changed

‎lib/domain/dtos/GetAllEnvironmentsDto.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
const Joi = require('joi');
1515
const PaginationDto = require('./PaginationDto');
1616
const { FromToFilterDto } = require('./filters/FromToFilterDto');
17+
const { validateRange, RANGE_INVALID } = require('../../utilities/rangeUtils.js');
1718

1819
/**
1920
* Separate filter DTO for get all environments, because EnvironmentsFilterDto
@@ -23,7 +24,10 @@ const FilterDto = Joi.object({
2324
ids: Joi.string().trim().optional(),
2425
currentStatus: Joi.string().trim().optional(),
2526
statusHistory: Joi.string().trim().optional(),
26-
runNumbers: Joi.string().trim().optional(),
27+
runNumbers: Joi.string().trim().custom(validateRange).messages({
28+
[RANGE_INVALID]: '{{#message}}',
29+
'string.base': 'Run numbers must be comma-separated numbers or ranges (e.g. 12,15-18)',
30+
}).optional(),
2731
created: FromToFilterDto,
2832
});
2933

‎lib/domain/dtos/filters/LhcFillsFilterDto.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@
1111
* or submit itself to any jurisdiction.
1212
*/
1313
const Joi = require('joi');
14-
const { validateRange } = require('../../../utilities/rangeUtils');
14+
const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils');
1515
const { validateTimeDuration } = require('../../../utilities/validateTime');
1616

1717
exports.LhcFillsFilterDto = Joi.object({
1818
hasStableBeams: Joi.boolean(),
1919
fillNumbers: Joi.string().trim().custom(validateRange).messages({
20-
'any.invalid': '{{#message}}',
20+
[RANGE_INVALID]: '{{#message}}',
21+
'string.base': 'Fill numbers must be comma-separated numbers or ranges (e.g. 12,15-18)',
2122
}),
2223
runDuration: validateTimeDuration,
2324
beamDuration: validateTimeDuration,

‎lib/domain/dtos/filters/RunFilterDto.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const { IntegerComparisonDto, FloatComparisonDto } = require('./NumericalCompari
1818
const { RUN_CALIBRATION_STATUS } = require('../../enums/RunCalibrationStatus.js');
1919
const { RUN_DEFINITIONS } = require('../../enums/RunDefinition.js');
2020
const { singleRunsCollectionCustomCheck } = require('../utils.js');
21-
const { validateRange } = require('../../../utilities/rangeUtils.js');
21+
const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils.js');
2222

2323
const DetectorsFilterDto = Joi.object({
2424
operator: Joi.string().valid('or', 'and', 'none').required(),
@@ -33,7 +33,8 @@ const EorReasonFilterDto = Joi.object({
3333

3434
exports.RunFilterDto = Joi.object({
3535
runNumbers: Joi.string().trim().custom(validateRange).messages({
36-
'any.invalid': '{{#message}}',
36+
[RANGE_INVALID]: '{{#message}}',
37+
'string.base': 'Run numbers must be comma-separated numbers or ranges (e.g. 12,15-18)',
3738
}),
3839
calibrationStatuses: Joi.array().items(...RUN_CALIBRATION_STATUS),
3940
definitions: CustomJoi.stringArray().items(Joi.string().uppercase().trim().valid(...RUN_DEFINITIONS)),

‎lib/usecases/environment/GetAllEnvironmentsUseCase.js‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ const { ApiConfig } = require('../../config/index.js');
2121
const { Op } = require('sequelize');
2222
const { dataSource } = require('../../database/DataSource.js');
2323
const { statusAcronyms } = require('../../domain/enums/StatusAcronyms.js');
24+
const { unpackNumberRange } = require('../../utilities/rangeUtils.js');
25+
const { splitStringToStringsTrimmed } = require('../../utilities/stringUtils.js');
2426

2527
/**
2628
* Subquery to select the latest history item for each environment.
@@ -182,19 +184,21 @@ class GetAllEnvironmentsUseCase {
182184
}
183185

184186
if (runNumbersExpression) {
185-
// Convert the string of run numbers to an array of numbers
186-
const filters = runNumbersExpression.split(',').map((filter) => Number(filter.trim()));
187+
const runNumberCriteria = splitStringToStringsTrimmed(runNumbersExpression, ',');
187188

188-
if (filters.length) {
189+
const finalRunNumberList = Array.from(unpackNumberRange(runNumberCriteria));
190+
191+
// Check that the final run numbers list contains at least one valid run number
192+
if (finalRunNumberList.length > 0) {
189193
filterQueryBuilder.include({
190194
association: 'runs',
191195
where: {
192196
// Filter should be like with only one filter and exact with more than one filter
193-
runNumber: { [filters.length === 1 ? Op.substring : Op.in]: filters },
197+
runNumber: { [finalRunNumberList.length === 1 ? Op.substring : Op.in]: finalRunNumberList },
194198
},
195199
});
196200
}
197-
}
201+
};
198202

199203
const filteredEnvironmentsIds = (await EnvironmentRepository.findAll(filterQueryBuilder)).map(({ id }) => id);
200204
// If no environments match the filter, return an empty result

‎lib/utilities/rangeUtils.js‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,25 @@
1919
* @param {*} helpers The helpers object
2020
* @returns {Object} The value if validation passes
2121
*/
22+
export const RANGE_INVALID = 'range.invalid';
23+
2224
export const validateRange = (value, helpers) => {
2325
const MAX_RANGE_SIZE = 100;
2426

2527
const numbers = value.split(',').map((number) => number.trim());
2628

2729
for (const number of numbers) {
2830
if (number.includes('-')) {
29-
// Check if '-' occurs more than once in this part of the range
30-
if (number.lastIndexOf('-') !== number.indexOf('-')) {
31-
return helpers.error('any.invalid', { message: `Invalid range: ${number}` });
32-
}
33-
const [start, end] = number.split('-').map((n) => Number(n));
31+
const parts = number.split('-');
32+
const [start, end] = parts.map((n) => parseInt(n, 10));
33+
3434
if (Number.isNaN(start) || Number.isNaN(end) || start > end) {
35-
return helpers.error('any.invalid', { message: `Invalid range: ${number}` });
35+
return helpers.error(RANGE_INVALID, { message: `Invalid range: ${number}` });
3636
}
3737
const rangeSize = end - start + 1;
3838

3939
if (rangeSize > MAX_RANGE_SIZE) {
40-
return helpers.error('any.invalid', { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` });
41-
}
42-
} else {
43-
// Prevent non-numeric input.
44-
if (isNaN(number)) {
45-
return helpers.error('any.invalid', { message: `Invalid number: ${number}` });
40+
return helpers.error(RANGE_INVALID, { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` });
4641
}
4742
}
4843
}
@@ -58,20 +53,20 @@ export const validateRange = (value, helpers) => {
5853
* @returns {Set<Number>} set containing the unpacked range.
5954
*/
6055
export function unpackNumberRange(numbersRanges, rangeSplitter = '-') {
61-
// Set to prevent duplicate values.
6256
const resultNumbers = new Set();
6357

6458
numbersRanges.forEach((number) => {
6559
if (number.includes(rangeSplitter)) {
6660
const [start, end] = number.split(rangeSplitter).map((n) => parseInt(n, 10));
6761
if (!Number.isNaN(start) && !Number.isNaN(end)) {
6862
for (let i = start; i <= end; i++) {
69-
resultNumbers.add(Number(i));
63+
resultNumbers.add(i);
7064
}
7165
}
7266
} else {
73-
if (!isNaN(number)) {
74-
resultNumbers.add(Number(number));
67+
const num = Number(number);
68+
if (!Number.isNaN(num)) {
69+
resultNumbers.add(num);
7570
}
7671
}
7772
});

‎test/api/environments.test.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,53 @@ module.exports = () => {
225225
expect(environments[0].id).to.be.equal('TDI59So3d');
226226
expect(environments[1].id).to.be.equal('Dxi029djX');
227227
});
228+
229+
it('should successfully filter environments with query on run number range', async () => {
230+
const response = await request(server).get('/api/environments?filter[runNumbers]=100-105');
231+
232+
expect(response.status).to.equal(200);
233+
const environments = response.body.data;
234+
expect(environments.length).to.be.equal(2);
235+
// Should include all environments with run numbers between 100 and 105
236+
expect(environments[0].id).to.be.equal('TDI59So3d');
237+
expect(environments[1].id).to.be.equal('Dxi029djX');
238+
});
239+
240+
it('should successfully filter environments when run number filter contains empty entries', async () => {
241+
const response = await request(server).get('/api/environments?filter[runNumbers]=103,,');
242+
243+
expect(response.status).to.equal(200);
244+
const environments = response.body.data;
245+
expect(environments).to.have.lengthOf(1);
246+
expect(environments[0].id).to.be.equal('TDI59So3d');
247+
});
248+
249+
it('should return 400 for an invalid run number range, first number greater than second', async () => {
250+
const response = await request(server).get('/api/environments?filter[runNumbers]=105-100');
251+
252+
expect(response.status).to.equal(400);
253+
const { errors } = response.body;
254+
const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers');
255+
expect(rangeError.detail).to.equal('Invalid range: 105-100');
256+
});
257+
258+
it('should return 400 for an invalid run number range, negative start number', async () => {
259+
const response = await request(server).get('/api/environments?filter[runNumbers]=-100-105');
260+
261+
expect(response.status).to.equal(400);
262+
const { errors } = response.body;
263+
const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers');
264+
expect(rangeError.detail).to.equal('Invalid range: -100-105');
265+
});
266+
267+
it('should return 400 for an invalid run number range, no start number', async () => {
268+
const response = await request(server).get('/api/environments?filter[runNumbers]=-105');
269+
270+
expect(response.status).to.equal(400);
271+
const { errors } = response.body;
272+
const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers');
273+
expect(rangeError.detail).to.equal('Invalid range: -105');
274+
});
228275
});
229276
describe('POST /api/environments', () => {
230277
it('should return 201 if valid data is provided', async () => {

‎test/lib/usecases/environment/GetAllEnvironmentsUseCase.test.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,33 @@ module.exports = () => {
162162
expect(environments.map(({ id }) => id)).to.have.members(['TDI59So3d', 'Dxi029djX']);
163163
});
164164

165+
it('should successfully filter environments on a run number range spanning multiple environments', async () => {
166+
getAllEnvsDto.query = { filter: { runNumbers: '100-103' } };
167+
const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto);
168+
169+
expect(environments).to.be.an('array');
170+
expect(environments.length).to.be.equal(2);
171+
expect(environments.map(({ id }) => id)).to.have.members(['Dxi029djX', 'TDI59So3d']);
172+
});
173+
174+
it('should successfully filter environments on a single-value run number range', async () => {
175+
getAllEnvsDto.query = { filter: { runNumbers: '105-105' } };
176+
const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto);
177+
178+
expect(environments).to.be.an('array');
179+
expect(environments.length).to.be.equal(1);
180+
expect(environments[0].id).to.be.equal('TDI59So3d');
181+
});
182+
183+
it('should successfully filter environments when run number filter includes empty entries', async () => {
184+
getAllEnvsDto.query = { filter: { runNumbers: '103, ' } };
185+
const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto);
186+
187+
expect(environments).to.be.an('array');
188+
expect(environments.length).to.be.equal(1);
189+
expect(environments[0].id).to.be.equal('TDI59So3d');
190+
});
191+
165192
it('should successfully filter environments on created from and to', async () => {
166193
const from = Date.now() - 24 * 60 * 60 * 1000; // environment from 24h ago which was created by CreateEnvironmentUseCase.test.js
167194
const to = Date.now() - 10;

‎test/lib/utilities/rangeUtils.test.js‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
*/
1313

1414
const Sinon = require('sinon');
15-
const { validateRange, unpackNumberRange } = require('../../../lib/utilities/rangeUtils.js');
15+
const { validateRange, unpackNumberRange, RANGE_INVALID } = require('../../../lib/utilities/rangeUtils.js');
1616
const { expect } = require('chai');
1717

1818
module.exports = () => {
@@ -61,14 +61,6 @@ module.exports = () => {
6161
expect(result).to.equal(input);
6262
});
6363

64-
it('rejects non-numeric input', () => {
65-
const input = '5,a,7';
66-
validateRange(input, helpers);
67-
expect(helpers.error.calledOnce).to.be.true;
68-
expect(helpers.error.firstCall.args[0]).to.equal('any.invalid');
69-
expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid number: a' });
70-
});
71-
7264
it('rejects range with non-numeric input', () => {
7365
const input = '3-a';
7466
validateRange(input, helpers);
@@ -90,20 +82,6 @@ module.exports = () => {
9082
expect(result).to.equal(input);
9183
});
9284

93-
it('rejects range containing more than one `-`', () => {
94-
const input = '1-2-3';
95-
validateRange(input, helpers);
96-
expect(helpers.error.calledOnce).to.be.true;
97-
expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid range: 1-2-3' });
98-
});
99-
100-
it('rejects range containing more than one `-`, at end', () => {
101-
const input = '1-2-';
102-
validateRange(input, helpers);
103-
expect(helpers.error.calledOnce).to.be.true;
104-
expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid range: 1-2-' });
105-
});
106-
10785
// MAX_RANGE_SIZE = 100, should this change, also change this test...
10886
it('rejects a range that exceeds MAX_RANGE_SIZE', () => {
10987
const input = '1-101';

‎test/public/envs/overview.test.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,15 @@ module.exports = () => {
296296
await expectAttributeValue(page, '.runs-filter input', 'placeholder', 'e.g. 553203, 553221, ...');
297297
await expectAttributeValue(page, '.historyItems-filter input', 'placeholder', 'e.g. D-R-X');
298298

299+
// range of runNumbers
300+
await fillInput(page, '.runs-filter input', '103-104', ['change']);
301+
await waitForTableLength(page, 1);
302+
// substring of a runNumber
303+
await fillInput(page, '.runs-filter input', '10', ['change']);
304+
299305
await fillInput(page, '.id-filter input', 'Dxi029djX, TDI59So3d', ['change']);
300306
await page.$eval('.status-filter #checkboxes-checkbox-DESTROYED', (element) => element.click());
301-
await fillInput(page, '.runs-filter input', '10', ['change']);
307+
302308
await fillInput(page, '.historyItems-filter input', 'C-R-D-X', ['change']);
303309

304310
const createdAtPopoverSelector = await getPopoverSelector(await page.$('.createdAt-filter .popover-trigger'));

0 commit comments

Comments
 (0)