Improv/o2 b 1520/add filtering by run intervals to envs

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,
 });
 

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)',
     }),
     beamDuration: validateTimeDuration,
 });

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)),

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

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;
 
@@ -28,21 +30,27 @@ export const validateRange = (value, helpers) => {
         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 validation
+            const num = Number(number);
+            if (Number.isNaN(num)) {
+                return helpers.error(RANGE_INVALID, { message: `Invalid number: ${number}` });
             }
         }
     }
@@ -58,20 +66,20 @@ export const validateRange = (value, helpers) => {
  * @returns {Set<Number>} 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);
             }
         }
     });

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 () => {

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;

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');

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' });
         });
 

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'));