From a812ab022e910ba4f81159a68ed3d890848af7ec Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:08:39 +0100 Subject: [PATCH 1/6] [O2B-1512] Refactor environment filtering tests into a single case Consolidated multiple granular filtering tests into one comprehensive test that applies all filters in sequence. This streamlines the test suite. --- test/public/envs/overview.test.js | 164 +++--------------------------- 1 file changed, 12 insertions(+), 152 deletions(-) diff --git a/test/public/envs/overview.test.js b/test/public/envs/overview.test.js index 1b9fa872c6..f4613bbe8f 100644 --- a/test/public/envs/overview.test.js +++ b/test/public/envs/overview.test.js @@ -279,7 +279,7 @@ module.exports = () => { expect(loadCallCount).to.equal(0); }); - it('should successfully open the filtering panel', async () => { + it('should successfully filter environments utilising all filters in the process', async () => { // Get the popover key from the filter button's parent const filterButton = await page.waitForSelector('#openFilterToggle'); const popoverKey = await filterButton.evaluate((button) => { @@ -287,170 +287,30 @@ module.exports = () => { }); const filterPanelSelector = `.popover[data-popover-key="${popoverKey}"]`; - // Initially the filtering panel should be closed await page.waitForSelector(filterPanelSelector, { hidden: true }); - // Open the filtering panel await openFilteringPanel(page); await page.waitForSelector(filterPanelSelector, { visible: true }); - }); - - it('should successfully filter environments by their related run numbers', async () => { - /** - * This is the sequence to test filtering the environments based on their related run numbers. - * - * @param {string} selector the filter input selector - * @param {string} inputValue the value to type in the filter input - * @param {string[]} expectedIds the list of expected environment IDs after filtering - * @return {void} - */ - const filterOnRunNumbers = async (selector, inputValue, expectedIds) => { - await fillInput(page, selector, inputValue, ['change']); - await waitForTableLength(page, expectedIds.length); - expect(await page.$$eval('tbody tr', (rows) => rows.map((row) => row.id))).to.eql(expectedIds.map(id => `row${id}`)); - } + await expectAttributeValue(page, '.id-filter input', 'placeholder', 'e.g. CmCvjNbg, TDI59So3d...'); await expectAttributeValue(page, '.runs-filter input', 'placeholder', 'e.g. 553203, 553221, ...'); - - await filterOnRunNumbers('.runs-filter input', '10', ['TDI59So3d', 'Dxi029djX']); - await resetFilters(page); - - await filterOnRunNumbers('.runs-filter input', '103', ['TDI59So3d']); - await resetFilters(page); - - await filterOnRunNumbers('.runs-filter input', '86, 91', ['KGIS12DS', 'VODdsO12d']); - await resetFilters(page); - }); - - it('should successfully filter environments by their status history', async () => { - /** - * This is the sequence to test filtering the environments on their status history. - * - * @param {string} selector the filter input selector - * @param {string} inputValue the value to type in the filter input - * @param {string[]} expectedIds the list of expected environment IDs after filtering - * @return {void} - */ - const filterOnStatusHistory = async (selector, inputValue, expectedIds) => { - await fillInput(page, selector, inputValue, ['change']); - await waitForTableLength(page, expectedIds.length); - expect(await page.$$eval('tbody tr', (rows) => rows.map((row) => row.id))).to.eql(expectedIds.map(id => `row${id}`)); - }; - await expectAttributeValue(page, '.historyItems-filter input', 'placeholder', 'e.g. D-R-X'); - await filterOnStatusHistory('.historyItems-filter input', 'C-R-D-X', ['TDI59So3d']); - await resetFilters(page); - - await filterOnStatusHistory('.historyItems-filter input', 'S-E', ['EIDO13i3D', '8E4aZTjY']); - await resetFilters(page); - - await filterOnStatusHistory('.historyItems-filter input', 'D-E', ['KGIS12DS']); - await resetFilters(page); - }); - - it('should successfully filter environments by their current status', async () => { - /** - * Checks that all the rows of the given table have a valid current status - * - * @param {string[]} authorizedCurrentStatuses the list of valid current statuses - * @return {void} - */ - const checkTableCurrentStatuses = async (authorizedCurrentStatuses) => { - const rows = await page.$$('tbody tr'); - for (const row of rows) { - expect(await row.evaluate((rowItem) => { - const rowId = rowItem.id; - return document.querySelector(`#${rowId}-status-text`).innerText; - })).to.be.oneOf(authorizedCurrentStatuses); - } - }; - - const currentStatusSelectorPrefix = '.status-filter #checkboxes-checkbox-'; - const getCurrentStatusCheckboxSelector = (statusName) => `${currentStatusSelectorPrefix}${statusName}`; + 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']); - await page.$eval(getCurrentStatusCheckboxSelector("RUNNING"), (element) => element.click()); - await waitForTableLength(page, 2); - await checkTableCurrentStatuses(["RUNNING"]); - - await page.$eval(getCurrentStatusCheckboxSelector("DEPLOYED"), (element) => element.click()); - await waitForTableLength(page, 3); - await checkTableCurrentStatuses(["RUNNING", "DEPLOYED"]); - }); - - it('should successfully filter environments by their IDs', async () => { - /** - * This is the sequence to test filtering the environments on IDs. - * - * @param {string} selector the filter input selector - * @param {string} inputValue the value to type in the filter input - * @param {string[]} expectedIds the list of expected environment IDs after filtering - * @return {void} - */ - const filterOnID = async (selector, inputValue, expectedIds) => { - await fillInput(page, selector, inputValue, ['change']); - await waitForTableLength(page, expectedIds.length); - expect(await page.$$eval('tbody tr', (rows) => rows.map((row) => row.id))).to.eql(expectedIds.map(id => `row${id}`)); - }; - - await expectAttributeValue(page, '.id-filter input', 'placeholder', 'e.g. CmCvjNbg, TDI59So3d...'); - - await filterOnID('.id-filter input', 'CmCvjNbg', ['CmCvjNbg']); - await resetFilters(page); - - await filterOnID('.id-filter input', 'CmCvjNbg, TDI59So3d', ['CmCvjNbg', 'TDI59So3d']); - await resetFilters(page); - - await filterOnID('.id-filter input', 'j', ['CmCvjNbg', 'GIDO1jdkD', '8E4aZTjY', 'Dxi029djX']); - await resetFilters(page); - }); - - it('should successfully filter environments by their createdAt date', async () => { - /** - * This is the sequence to test filtering the environments based on their createdAt date - * - * @param {string} selector the filter input selector - * @param {string} fromDate the from date string - * @param {string} fromTime the from time string - * @param {string} toDate the to date string - * @param {string} toTime the to time string - * @param {string[]} expectedIds the list of expected environment IDs after filtering - * @return {void} - */ - const filterOnCreatedAt = async (selector, fromDate, fromTime, toDate, toTime, expectedIds) => { - await fillInput(page, selector.fromTimeSelector, fromTime, ['change']); - await fillInput(page, selector.toTimeSelector, toTime, ['change']); - - await fillInput(page, selector.fromDateSelector, fromDate, ['change']); - await fillInput(page, selector.toDateSelector, toDate, ['change']); - - await waitForTableLength(page, expectedIds.length); - expect(await page.$$eval('tbody tr', (rows) => rows.map((row) => row.id))).to.eql(expectedIds.map(id => `row${id}`)); - }; - - await openFilteringPanel(page); - const createdAtPopoverSelector = await getPopoverSelector(await page.$('.createdAt-filter .popover-trigger')); const periodInputsSelectors = getPeriodInputsSelectors(createdAtPopoverSelector); + await fillInput(page, periodInputsSelectors.fromDateSelector, '2019-08-09', ['change']); + await fillInput(page, periodInputsSelectors.toDateSelector, '2019-08-10', ['change']); + await fillInput(page, periodInputsSelectors.fromTimeSelector, '00:00', ['change']); + await fillInput(page, periodInputsSelectors.toTimeSelector, '23:59', ['change']); - await filterOnCreatedAt( - periodInputsSelectors, - '2019-05-08', - '00:00', - '2019-05-10', - '00:00', - ['eZF99lH6'], - ); - await resetFilters(page); + await waitForTableLength(page, 1); + expect(await page.$$eval('tbody tr', (rows) => rows.map((row) => row.id))).to.eql(['rowTDI59So3d']); - await filterOnCreatedAt( - periodInputsSelectors, - '2019-08-09', - '00:00', - '2019-08-09', - '14:00', - ['GIDO1jdkD', '8E4aZTjY', 'Dxi029djX'], - ); await resetFilters(page); }); }; From 4a9810279543dfbe0c9f8e35271feb6d6552a5c6 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Fri, 16 Jan 2026 17:15:49 +0100 Subject: [PATCH 2/6] [O2B-1520] Enhance runNumbers filter with range support Updated GetAllEnvironmentsDto and GetAllEnvironmentsUseCase to support runNumbers filtering using ranges. Added range to large filter test case. --- lib/domain/dtos/GetAllEnvironmentsDto.js | 5 ++++- .../environment/GetAllEnvironmentsUseCase.js | 14 +++++++++----- test/public/envs/overview.test.js | 8 +++++++- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/domain/dtos/GetAllEnvironmentsDto.js b/lib/domain/dtos/GetAllEnvironmentsDto.js index 8eb1cfba85..2c5e96bffa 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 } = require('../../utilities/rangeUtils.js'); /** * Separate filter DTO for get all environments, because EnvironmentsFilterDto @@ -23,7 +24,9 @@ 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({ + 'any.invalid': '{{#message}}', + }).optional(), created: FromToFilterDto, }); 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/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')); From 5d612c0c485e52857990497f93ad3eb3304c1a7f Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Wed, 21 Jan 2026 16:22:05 +0100 Subject: [PATCH 3/6] [O2B-1520] Improve range validation and error handling in DTOs Refactored range validation logic to use a custom error code (RANGE_INVALID) and improved error messages for invalid input formats in DTOs. Enhanced rangeUtils to handle empty values, multiple dashes, and non-numeric input more robustly. Updated related tests and API validation to reflect new error handling and messaging. --- lib/domain/dtos/GetAllEnvironmentsDto.js | 5 ++- lib/domain/dtos/filters/LhcFillsFilterDto.js | 5 ++- lib/domain/dtos/filters/RunFilterDto.js | 5 ++- lib/utilities/rangeUtils.js | 39 +++++++++++++------- test/api/environments.test.js | 11 ++++++ test/lib/utilities/rangeUtils.test.js | 4 +- 6 files changed, 48 insertions(+), 21 deletions(-) diff --git a/lib/domain/dtos/GetAllEnvironmentsDto.js b/lib/domain/dtos/GetAllEnvironmentsDto.js index 2c5e96bffa..e1ebbd60f2 100644 --- a/lib/domain/dtos/GetAllEnvironmentsDto.js +++ b/lib/domain/dtos/GetAllEnvironmentsDto.js @@ -14,7 +14,7 @@ const Joi = require('joi'); const PaginationDto = require('./PaginationDto'); const { FromToFilterDto } = require('./filters/FromToFilterDto'); -const { validateRange } = require('../../utilities/rangeUtils.js'); +const { validateRange, RANGE_INVALID } = require('../../utilities/rangeUtils.js'); /** * Separate filter DTO for get all environments, because EnvironmentsFilterDto @@ -25,7 +25,8 @@ const FilterDto = Joi.object({ currentStatus: Joi.string().trim().optional(), statusHistory: Joi.string().trim().optional(), 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)', }).optional(), created: FromToFilterDto, }); diff --git a/lib/domain/dtos/filters/LhcFillsFilterDto.js b/lib/domain/dtos/filters/LhcFillsFilterDto.js index d1d2af5929..3f36fe0820 100644 --- a/lib/domain/dtos/filters/LhcFillsFilterDto.js +++ b/lib/domain/dtos/filters/LhcFillsFilterDto.js @@ -11,11 +11,12 @@ * or submit itself to any jurisdiction. */ const Joi = require('joi'); -const { validateRange } = require('../../../utilities/rangeUtils'); +const { validateRange, RANGE_INVALID } = require('../../../utilities/rangeUtils'); 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)', }), }); 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/utilities/rangeUtils.js b/lib/utilities/rangeUtils.js index 4cc9a385de..dea1155878 100644 --- a/lib/utilities/rangeUtils.js +++ b/lib/utilities/rangeUtils.js @@ -19,30 +19,43 @@ * @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; const numbers = value.split(',').map((number) => number.trim()); for (const number of numbers) { + // Check for empty strings (e.g., from trailing commas or double commas) + if (number === '') { + return helpers.error(RANGE_INVALID, { message: 'Empty value found' }); + } + 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}` }); + return helpers.error(RANGE_INVALID, { message: `Invalid range: ${number}` }); + } + const parts = number.split('-'); + // Ensure exactly 2 parts and both are non-empty + if (parts.length !== 2 || parts[0].trim() === '' || parts[1].trim() === '') { + return helpers.error(RANGE_INVALID, { message: `Invalid range: ${number}` }); } - const [start, end] = number.split('-').map((n) => Number(n)); + const [start, end] = parts.map((n) => Number(n)); 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}` }); + return helpers.error(RANGE_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}` }); + // Single number - prevent non-numeric input using Number.isNaN for consistency + const num = Number(number); + if (Number.isNaN(num)) { + return helpers.error(RANGE_INVALID, { message: `Invalid number: ${number}` }); } } } @@ -58,20 +71,20 @@ 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) => { if (number.includes(rangeSplitter)) { - const [start, end] = number.split(rangeSplitter).map((n) => parseInt(n, 10)); - if (!Number.isNaN(start) && !Number.isNaN(end)) { + const [start, end] = number.split(rangeSplitter).map((n) => Number(n)); + if (!Number.isNaN(start) && !Number.isNaN(end) && start <= 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..201c31a94e 100644 --- a/test/api/environments.test.js +++ b/test/api/environments.test.js @@ -225,6 +225,17 @@ 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'); + }); }); describe('POST /api/environments', () => { it('should return 201 if valid data is provided', async () => { diff --git a/test/lib/utilities/rangeUtils.test.js b/test/lib/utilities/rangeUtils.test.js index 509db85a4f..a7802888ca 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 = () => { @@ -65,7 +65,7 @@ module.exports = () => { 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[0]).to.equal(RANGE_INVALID); expect(helpers.error.firstCall.args[1]).to.deep.equal({ message: 'Invalid number: a' }); }); From c34056c6f1d6ca06bf5178ec6a2e8eead78c7132 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Mon, 26 Jan 2026 16:15:48 +0100 Subject: [PATCH 4/6] [O2B-1520] Improve run number range validation and add tests Removed strict rejection of empty entries in run number filters, allowing filters like '103,,' to be processed. Added comprehensive tests for valid and invalid run number ranges, including edge cases with empty entries and invalid formats, to ensure correct API and use case behaviour. --- lib/utilities/rangeUtils.js | 5 --- test/api/environments.test.js | 36 +++++++++++++++++++ .../GetAllEnvironmentsUseCase.test.js | 27 ++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/lib/utilities/rangeUtils.js b/lib/utilities/rangeUtils.js index dea1155878..9e0c9b9523 100644 --- a/lib/utilities/rangeUtils.js +++ b/lib/utilities/rangeUtils.js @@ -27,11 +27,6 @@ export const validateRange = (value, helpers) => { const numbers = value.split(',').map((number) => number.trim()); for (const number of numbers) { - // Check for empty strings (e.g., from trailing commas or double commas) - if (number === '') { - return helpers.error(RANGE_INVALID, { message: 'Empty value found' }); - } - if (number.includes('-')) { // Check if '-' occurs more than once in this part of the range if (number.lastIndexOf('-') !== number.indexOf('-')) { diff --git a/test/api/environments.test.js b/test/api/environments.test.js index 201c31a94e..009cf0df17 100644 --- a/test/api/environments.test.js +++ b/test/api/environments.test.js @@ -236,6 +236,42 @@ module.exports = () => { 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; From 96d6166a7f1f3a17ce8eeb8c33e5965d75b93549 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Mon, 26 Jan 2026 16:22:26 +0100 Subject: [PATCH 5/6] [O2B-1520] Skip invalid numbers in validateRange utility Updated validateRange to skip non-numeric inputs instead of returning an error. --- lib/utilities/rangeUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utilities/rangeUtils.js b/lib/utilities/rangeUtils.js index 9e0c9b9523..4c24bc9147 100644 --- a/lib/utilities/rangeUtils.js +++ b/lib/utilities/rangeUtils.js @@ -50,7 +50,7 @@ export const validateRange = (value, helpers) => { // Single number - prevent non-numeric input using Number.isNaN for consistency const num = Number(number); if (Number.isNaN(num)) { - return helpers.error(RANGE_INVALID, { message: `Invalid number: ${number}` }); + continue; } } } From 2ff9774134a7c1c2a5cb73747eb2e04f50530716 Mon Sep 17 00:00:00 2001 From: Isaac Hill <71404865+isaachilly@users.noreply.github.com> Date: Mon, 26 Jan 2026 16:49:12 +0100 Subject: [PATCH 6/6] [O2B-1520] Improve range validation and update test case Update validateRange to return an error on invalid numbers instead of continuing. Adjust test to remove non-numeric run number from input, reflecting stricter validation. --- lib/utilities/rangeUtils.js | 4 ++-- test/lib/usecases/run/GetAllRunsUseCase.test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/utilities/rangeUtils.js b/lib/utilities/rangeUtils.js index 4c24bc9147..7f6c6256f4 100644 --- a/lib/utilities/rangeUtils.js +++ b/lib/utilities/rangeUtils.js @@ -47,10 +47,10 @@ export const validateRange = (value, helpers) => { return helpers.error(RANGE_INVALID, { message: `Given range exceeds max size of ${MAX_RANGE_SIZE} range: ${number}` }); } } else { - // Single number - prevent non-numeric input using Number.isNaN for consistency + // Single number validation const num = Number(number); if (Number.isNaN(num)) { - continue; + return helpers.error(RANGE_INVALID, { message: `Invalid number: ${number}` }); } } } diff --git a/test/lib/usecases/run/GetAllRunsUseCase.test.js b/test/lib/usecases/run/GetAllRunsUseCase.test.js index 55e5551485..6dd84e852d 100644 --- a/test/lib/usecases/run/GetAllRunsUseCase.test.js +++ b/test/lib/usecases/run/GetAllRunsUseCase.test.js @@ -90,8 +90,8 @@ module.exports = () => { expect(runs[1].runNumber).to.equal(95); }); - it('should return an array, only containing found runs from passed list (run numbers can be missing or non-numbers)', async () => { - getAllRunsDto.query = { filter: { runNumbers: '-2,17, ,400,18' } }; + it('should return an array, only containing found runs from passed list (run numbers can be missing)', async () => { + getAllRunsDto.query = { filter: { runNumbers: '17, ,400,18' } }; const { runs } = await new GetAllRunsUseCase().execute(getAllRunsDto); expect(runs).to.be.an('array');