diff --git a/lib/domain/dtos/GetAllEnvironmentsDto.js b/lib/domain/dtos/GetAllEnvironmentsDto.js index 8eb1cfba85..e1ebbd60f2 100644 --- a/lib/domain/dtos/GetAllEnvironmentsDto.js +++ b/lib/domain/dtos/GetAllEnvironmentsDto.js @@ -14,6 +14,7 @@ const Joi = require('joi'); const PaginationDto = require('./PaginationDto'); const { FromToFilterDto } = require('./filters/FromToFilterDto'); +const { validateRange, RANGE_INVALID } = require('../../utilities/rangeUtils.js'); /** * Separate filter DTO for get all environments, because EnvironmentsFilterDto @@ -23,7 +24,10 @@ const FilterDto = Joi.object({ ids: Joi.string().trim().optional(), currentStatus: Joi.string().trim().optional(), statusHistory: Joi.string().trim().optional(), - runNumbers: Joi.string().trim().optional(), + runNumbers: Joi.string().trim().custom(validateRange).messages({ + [RANGE_INVALID]: '{{#message}}', + 'string.base': 'Run numbers must be comma-separated numbers or ranges (e.g. 12,15-18)', + }).optional(), created: FromToFilterDto, }); diff --git a/lib/domain/dtos/filters/LhcFillsFilterDto.js b/lib/domain/dtos/filters/LhcFillsFilterDto.js index f2664d0cc7..5c71b5bd4a 100644 --- a/lib/domain/dtos/filters/LhcFillsFilterDto.js +++ b/lib/domain/dtos/filters/LhcFillsFilterDto.js @@ -11,13 +11,14 @@ * or submit itself to any jurisdiction. */ const Joi = require('joi'); -const { validateRange } = require('../../../utilities/rangeUtils'); +const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils'); const { validateTimeDuration } = require('../../../utilities/validateTime'); exports.LhcFillsFilterDto = Joi.object({ hasStableBeams: Joi.boolean(), fillNumbers: Joi.string().trim().custom(validateRange).messages({ - 'any.invalid': '{{#message}}', + [RANGE_INVALID]: '{{#message}}', + 'string.base': 'Fill numbers must be comma-separated numbers or ranges (e.g. 12,15-18)', }), runDuration: validateTimeDuration, beamDuration: validateTimeDuration, diff --git a/lib/domain/dtos/filters/RunFilterDto.js b/lib/domain/dtos/filters/RunFilterDto.js index 0feda0ddbc..2dc9ce98ad 100644 --- a/lib/domain/dtos/filters/RunFilterDto.js +++ b/lib/domain/dtos/filters/RunFilterDto.js @@ -18,7 +18,7 @@ const { IntegerComparisonDto, FloatComparisonDto } = require('./NumericalCompari const { RUN_CALIBRATION_STATUS } = require('../../enums/RunCalibrationStatus.js'); const { RUN_DEFINITIONS } = require('../../enums/RunDefinition.js'); const { singleRunsCollectionCustomCheck } = require('../utils.js'); -const { validateRange } = require('../../../utilities/rangeUtils.js'); +const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils.js'); const DetectorsFilterDto = Joi.object({ operator: Joi.string().valid('or', 'and', 'none').required(), @@ -33,7 +33,8 @@ const EorReasonFilterDto = Joi.object({ exports.RunFilterDto = Joi.object({ runNumbers: Joi.string().trim().custom(validateRange).messages({ - 'any.invalid': '{{#message}}', + [RANGE_INVALID]: '{{#message}}', + 'string.base': 'Run numbers must be comma-separated numbers or ranges (e.g. 12,15-18)', }), calibrationStatuses: Joi.array().items(...RUN_CALIBRATION_STATUS), definitions: CustomJoi.stringArray().items(Joi.string().uppercase().trim().valid(...RUN_DEFINITIONS)), diff --git a/lib/usecases/environment/GetAllEnvironmentsUseCase.js b/lib/usecases/environment/GetAllEnvironmentsUseCase.js index fd01f05813..c742c53b62 100644 --- a/lib/usecases/environment/GetAllEnvironmentsUseCase.js +++ b/lib/usecases/environment/GetAllEnvironmentsUseCase.js @@ -21,6 +21,8 @@ const { ApiConfig } = require('../../config/index.js'); const { Op } = require('sequelize'); const { dataSource } = require('../../database/DataSource.js'); const { statusAcronyms } = require('../../domain/enums/StatusAcronyms.js'); +const { unpackNumberRange } = require('../../utilities/rangeUtils.js'); +const { splitStringToStringsTrimmed } = require('../../utilities/stringUtils.js'); /** * Subquery to select the latest history item for each environment. @@ -182,19 +184,21 @@ class GetAllEnvironmentsUseCase { } if (runNumbersExpression) { - // Convert the string of run numbers to an array of numbers - const filters = runNumbersExpression.split(',').map((filter) => Number(filter.trim())); + const runNumberCriteria = splitStringToStringsTrimmed(runNumbersExpression, ','); - if (filters.length) { + const finalRunNumberList = Array.from(unpackNumberRange(runNumberCriteria)); + + // Check that the final run numbers list contains at least one valid run number + if (finalRunNumberList.length > 0) { filterQueryBuilder.include({ association: 'runs', where: { // Filter should be like with only one filter and exact with more than one filter - runNumber: { [filters.length === 1 ? Op.substring : Op.in]: filters }, + runNumber: { [finalRunNumberList.length === 1 ? Op.substring : Op.in]: finalRunNumberList }, }, }); } - } + }; const filteredEnvironmentsIds = (await EnvironmentRepository.findAll(filterQueryBuilder)).map(({ id }) => id); // If no environments match the filter, return an empty result diff --git a/lib/utilities/rangeUtils.js b/lib/utilities/rangeUtils.js index 4cc9a385de..8ff95d293d 100644 --- a/lib/utilities/rangeUtils.js +++ b/lib/utilities/rangeUtils.js @@ -19,6 +19,8 @@ * @param {*} helpers The helpers object * @returns {Object} The value if validation passes */ +export const RANGE_INVALID = 'range.invalid'; + export const validateRange = (value, helpers) => { const MAX_RANGE_SIZE = 100; @@ -26,23 +28,16 @@ export const validateRange = (value, helpers) => { for (const number of numbers) { if (number.includes('-')) { - // Check if '-' occurs more than once in this part of the range - if (number.lastIndexOf('-') !== number.indexOf('-')) { - return helpers.error('any.invalid', { message: `Invalid range: ${number}` }); - } - const [start, end] = number.split('-').map((n) => Number(n)); + const parts = number.split('-'); + const [start, end] = parts.map((n) => parseInt(n, 10)); + if (Number.isNaN(start) || Number.isNaN(end) || start > end) { - return helpers.error('any.invalid', { message: `Invalid range: ${number}` }); + return helpers.error(RANGE_INVALID, { message: `Invalid range: ${number}` }); } const rangeSize = end - start + 1; if (rangeSize > MAX_RANGE_SIZE) { - return helpers.error('any.invalid', { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` }); - } - } else { - // Prevent non-numeric input. - if (isNaN(number)) { - return helpers.error('any.invalid', { message: `Invalid number: ${number}` }); + return helpers.error(RANGE_INVALID, { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` }); } } } @@ -58,7 +53,6 @@ export const validateRange = (value, helpers) => { * @returns {Set} set containing the unpacked range. */ export function unpackNumberRange(numbersRanges, rangeSplitter = '-') { - // Set to prevent duplicate values. const resultNumbers = new Set(); numbersRanges.forEach((number) => { @@ -66,12 +60,13 @@ export function unpackNumberRange(numbersRanges, rangeSplitter = '-') { const [start, end] = number.split(rangeSplitter).map((n) => parseInt(n, 10)); if (!Number.isNaN(start) && !Number.isNaN(end)) { for (let i = start; i <= end; i++) { - resultNumbers.add(Number(i)); + resultNumbers.add(i); } } } else { - if (!isNaN(number)) { - resultNumbers.add(Number(number)); + const num = Number(number); + if (!Number.isNaN(num)) { + resultNumbers.add(num); } } }); diff --git a/test/api/environments.test.js b/test/api/environments.test.js index 770df255a9..009cf0df17 100644 --- a/test/api/environments.test.js +++ b/test/api/environments.test.js @@ -225,6 +225,53 @@ module.exports = () => { expect(environments[0].id).to.be.equal('TDI59So3d'); expect(environments[1].id).to.be.equal('Dxi029djX'); }); + + it('should successfully filter environments with query on run number range', async () => { + const response = await request(server).get('/api/environments?filter[runNumbers]=100-105'); + + expect(response.status).to.equal(200); + const environments = response.body.data; + expect(environments.length).to.be.equal(2); + // Should include all environments with run numbers between 100 and 105 + expect(environments[0].id).to.be.equal('TDI59So3d'); + expect(environments[1].id).to.be.equal('Dxi029djX'); + }); + + it('should successfully filter environments when run number filter contains empty entries', async () => { + const response = await request(server).get('/api/environments?filter[runNumbers]=103,,'); + + expect(response.status).to.equal(200); + const environments = response.body.data; + expect(environments).to.have.lengthOf(1); + expect(environments[0].id).to.be.equal('TDI59So3d'); + }); + + it('should return 400 for an invalid run number range, first number greater than second', async () => { + const response = await request(server).get('/api/environments?filter[runNumbers]=105-100'); + + expect(response.status).to.equal(400); + const { errors } = response.body; + const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers'); + expect(rangeError.detail).to.equal('Invalid range: 105-100'); + }); + + it('should return 400 for an invalid run number range, negative start number', async () => { + const response = await request(server).get('/api/environments?filter[runNumbers]=-100-105'); + + expect(response.status).to.equal(400); + const { errors } = response.body; + const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers'); + expect(rangeError.detail).to.equal('Invalid range: -100-105'); + }); + + it('should return 400 for an invalid run number range, no start number', async () => { + const response = await request(server).get('/api/environments?filter[runNumbers]=-105'); + + expect(response.status).to.equal(400); + const { errors } = response.body; + const rangeError = errors.find((err) => err.source.pointer === '/data/attributes/query/filter/runNumbers'); + expect(rangeError.detail).to.equal('Invalid range: -105'); + }); }); describe('POST /api/environments', () => { it('should return 201 if valid data is provided', async () => { diff --git a/test/lib/usecases/environment/GetAllEnvironmentsUseCase.test.js b/test/lib/usecases/environment/GetAllEnvironmentsUseCase.test.js index 1100027612..96b4ee1c11 100644 --- a/test/lib/usecases/environment/GetAllEnvironmentsUseCase.test.js +++ b/test/lib/usecases/environment/GetAllEnvironmentsUseCase.test.js @@ -162,6 +162,33 @@ module.exports = () => { expect(environments.map(({ id }) => id)).to.have.members(['TDI59So3d', 'Dxi029djX']); }); + it('should successfully filter environments on a run number range spanning multiple environments', async () => { + getAllEnvsDto.query = { filter: { runNumbers: '100-103' } }; + const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto); + + expect(environments).to.be.an('array'); + expect(environments.length).to.be.equal(2); + expect(environments.map(({ id }) => id)).to.have.members(['Dxi029djX', 'TDI59So3d']); + }); + + it('should successfully filter environments on a single-value run number range', async () => { + getAllEnvsDto.query = { filter: { runNumbers: '105-105' } }; + const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto); + + expect(environments).to.be.an('array'); + expect(environments.length).to.be.equal(1); + expect(environments[0].id).to.be.equal('TDI59So3d'); + }); + + it('should successfully filter environments when run number filter includes empty entries', async () => { + getAllEnvsDto.query = { filter: { runNumbers: '103, ' } }; + const { environments } = await new GetAllEnvironmentsUseCase().execute(getAllEnvsDto); + + expect(environments).to.be.an('array'); + expect(environments.length).to.be.equal(1); + expect(environments[0].id).to.be.equal('TDI59So3d'); + }); + it('should successfully filter environments on created from and to', async () => { const from = Date.now() - 24 * 60 * 60 * 1000; // environment from 24h ago which was created by CreateEnvironmentUseCase.test.js const to = Date.now() - 10; diff --git a/test/lib/utilities/rangeUtils.test.js b/test/lib/utilities/rangeUtils.test.js index 509db85a4f..2188852fbf 100644 --- a/test/lib/utilities/rangeUtils.test.js +++ b/test/lib/utilities/rangeUtils.test.js @@ -12,7 +12,7 @@ */ const Sinon = require('sinon'); -const { validateRange, unpackNumberRange } = require('../../../lib/utilities/rangeUtils.js'); +const { validateRange, unpackNumberRange, RANGE_INVALID } = require('../../../lib/utilities/rangeUtils.js'); const { expect } = require('chai'); module.exports = () => { @@ -61,14 +61,6 @@ module.exports = () => { expect(result).to.equal(input); }); - it('rejects non-numeric input', () => { - const input = '5,a,7'; - validateRange(input, helpers); - expect(helpers.error.calledOnce).to.be.true; - expect(helpers.error.firstCall.args[0]).to.equal('any.invalid'); - expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid number: a' }); - }); - it('rejects range with non-numeric input', () => { const input = '3-a'; validateRange(input, helpers); @@ -90,20 +82,6 @@ module.exports = () => { expect(result).to.equal(input); }); - it('rejects range containing more than one `-`', () => { - const input = '1-2-3'; - validateRange(input, helpers); - expect(helpers.error.calledOnce).to.be.true; - expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid range: 1-2-3' }); - }); - - it('rejects range containing more than one `-`, at end', () => { - const input = '1-2-'; - validateRange(input, helpers); - expect(helpers.error.calledOnce).to.be.true; - expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid range: 1-2-' }); - }); - // MAX_RANGE_SIZE = 100, should this change, also change this test... it('rejects a range that exceeds MAX_RANGE_SIZE', () => { const input = '1-101'; diff --git a/test/public/envs/overview.test.js b/test/public/envs/overview.test.js index f4613bbe8f..73eecd9a4f 100644 --- a/test/public/envs/overview.test.js +++ b/test/public/envs/overview.test.js @@ -296,9 +296,15 @@ module.exports = () => { await expectAttributeValue(page, '.runs-filter input', 'placeholder', 'e.g. 553203, 553221, ...'); await expectAttributeValue(page, '.historyItems-filter input', 'placeholder', 'e.g. D-R-X'); + // range of runNumbers + await fillInput(page, '.runs-filter input', '103-104', ['change']); + await waitForTableLength(page, 1); + // substring of a runNumber + await fillInput(page, '.runs-filter input', '10', ['change']); + await fillInput(page, '.id-filter input', 'Dxi029djX, TDI59So3d', ['change']); await page.$eval('.status-filter #checkboxes-checkbox-DESTROYED', (element) => element.click()); - await fillInput(page, '.runs-filter input', '10', ['change']); + await fillInput(page, '.historyItems-filter input', 'C-R-D-X', ['change']); const createdAtPopoverSelector = await getPopoverSelector(await page.$('.createdAt-filter .popover-trigger'));