From 1ce068de11837750d9589bf58e0c25a1828785f8 Mon Sep 17 00:00:00 2001 From: Brian Raymond Date: Mon, 13 Apr 2026 02:10:24 +0000 Subject: [PATCH 01/31] Improved Error Messages in PreferencesComponent --- .../components/admin/PreferencesComponent.tsx | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 406e402cef..88b84d2e7d 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -329,7 +329,20 @@ export default function PreferencesComponent() { invalid={invalidFuncs.warningFileSize()} /> - + {Number(localAdminPref.defaultWarningFileSize) < 0 ? ( + + ) : ( + + )}
@@ -345,7 +358,17 @@ export default function PreferencesComponent() { invalid={invalidFuncs.fileSizeLimit()} /> - + {Number(localAdminPref.defaultFileSizeLimit) < 0 ? ( + + ) : ( + + )}
From a0c9cd8774a17411b67638c8c9003b1021f55c15 Mon Sep 17 00:00:00 2001 From: GoodKimchi <224943594+GoodKimchi@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:57:01 -0700 Subject: [PATCH 02/31] style: fix indentation in PreferencesComponent --- .../components/admin/PreferencesComponent.tsx | 88 +++++++++++-------- 1 file changed, 50 insertions(+), 38 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 88b84d2e7d..2701a53fb0 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -2,44 +2,48 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { cloneDeep, isEqual } from 'lodash'; +import {cloneDeep, isEqual} from 'lodash'; import * as moment from 'moment'; import * as React from 'react'; -import { FormattedMessage } from 'react-intl'; -import { Button, Input, FormFeedback } from 'reactstrap'; -import { UnsavedWarningComponent } from '../UnsavedWarningComponent'; -import { preferencesApi } from '../../redux/api/preferencesApi'; +import {FormattedMessage} from 'react-intl'; +import {Button, Input, FormFeedback} from 'reactstrap'; +import {UnsavedWarningComponent} from '../UnsavedWarningComponent'; +import {preferencesApi} from '../../redux/api/preferencesApi'; import { MIN_DATE, MIN_DATE_MOMENT, MAX_DATE, MAX_DATE_MOMENT, MAX_ERRORS } from '../../redux/selectors/adminSelectors'; -import { PreferenceRequestItem } from '../../types/items'; -import { ChartTypes } from '../../types/redux/graph'; -import { LanguageTypes } from '../../types/redux/i18n'; -import { AreaUnitType } from '../../utils/getAreaUnitConversion'; -import { showErrorNotification, showSuccessNotification } from '../../utils/notifications'; -import { useTranslate } from '../../redux/componentHooks'; +import {PreferenceRequestItem} from '../../types/items'; +import {ChartTypes} from '../../types/redux/graph'; +import {LanguageTypes} from '../../types/redux/i18n'; +import {AreaUnitType} from '../../utils/getAreaUnitConversion'; +import {showErrorNotification, showSuccessNotification} from '../../utils/notifications'; +import {useTranslate} from '../../redux/componentHooks'; import TimeZoneSelect from '../TimeZoneSelect'; -import { defaultAdminState } from '../../redux/slices/adminSlice'; -import { checkboxStyle, labelStyle } from '../../styles/modalStyle'; +import {defaultAdminState} from '../../redux/slices/adminSlice'; +import {checkboxStyle, labelStyle} from '../../styles/modalStyle'; /** * @returns Preferences Component for Administrative use */ export default function PreferencesComponent() { const translate = useTranslate(); - const { data: adminPreferences = defaultAdminState } = preferencesApi.useGetPreferencesQuery(); + const {data: adminPreferences = defaultAdminState} = preferencesApi.useGetPreferencesQuery(); const [localAdminPref, setLocalAdminPref] = React.useState(cloneDeep(adminPreferences)); const [submitPreferences] = preferencesApi.useSubmitPreferencesMutation(); const [hasChanges, setHasChanges] = React.useState(false); // mutation will invalidate preferences tag and will be re-fetched. // On query response, reset local changes to response - React.useEffect(() => { setLocalAdminPref(cloneDeep(adminPreferences)); }, [adminPreferences]); + React.useEffect(() => { + setLocalAdminPref(cloneDeep(adminPreferences)); + }, [adminPreferences]); // Compare the API response against the localState to determine changes - React.useEffect(() => { setHasChanges(!isEqual(adminPreferences, localAdminPref)); }, [localAdminPref, adminPreferences]); + React.useEffect(() => { + setHasChanges(!isEqual(adminPreferences, localAdminPref)); + }, [localAdminPref, adminPreferences]); const makeLocalChanges = (key: keyof PreferenceRequestItem, value: PreferenceRequestItem[keyof PreferenceRequestItem]) => { - setLocalAdminPref({ ...localAdminPref, [key]: value }); + setLocalAdminPref({...localAdminPref, [key]: value}); }; const discardChanges = () => { @@ -63,7 +67,9 @@ export default function PreferencesComponent() { const maxMoment = moment(localAdminPref.defaultMeterMaximumDate); return !maxMoment.isValid() || !maxMoment.isSameOrBefore(MAX_DATE_MOMENT) || !maxMoment.isSameOrAfter(minMoment); }, - readingGap: (): boolean => { return Number(localAdminPref.defaultMeterReadingGap) < 0; }, + readingGap: (): boolean => { + return Number(localAdminPref.defaultMeterReadingGap) < 0; + }, meterErrors: (): boolean => { return Number(localAdminPref.defaultMeterMaximumErrors) < 0 @@ -93,12 +99,12 @@ export default function PreferencesComponent() {

{translate('graph.settings')}

- : + :

{ Object.values(ChartTypes).map(chartType => (
-
+
); } From 6c56579a8aa61f57fa3a09a680a5e0508533d3b8 Mon Sep 17 00:00:00 2001 From: GoodKimchi <224943594+GoodKimchi@users.noreply.github.com> Date: Tue, 7 Jul 2026 14:09:07 -0700 Subject: [PATCH 03/31] Revert "style: fix indentation in PreferencesComponent" This reverts commit a0c9cd8774a17411b67638c8c9003b1021f55c15. --- .../components/admin/PreferencesComponent.tsx | 88 ++++++++----------- 1 file changed, 38 insertions(+), 50 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 2701a53fb0..88b84d2e7d 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -2,48 +2,44 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import {cloneDeep, isEqual} from 'lodash'; +import { cloneDeep, isEqual } from 'lodash'; import * as moment from 'moment'; import * as React from 'react'; -import {FormattedMessage} from 'react-intl'; -import {Button, Input, FormFeedback} from 'reactstrap'; -import {UnsavedWarningComponent} from '../UnsavedWarningComponent'; -import {preferencesApi} from '../../redux/api/preferencesApi'; +import { FormattedMessage } from 'react-intl'; +import { Button, Input, FormFeedback } from 'reactstrap'; +import { UnsavedWarningComponent } from '../UnsavedWarningComponent'; +import { preferencesApi } from '../../redux/api/preferencesApi'; import { MIN_DATE, MIN_DATE_MOMENT, MAX_DATE, MAX_DATE_MOMENT, MAX_ERRORS } from '../../redux/selectors/adminSelectors'; -import {PreferenceRequestItem} from '../../types/items'; -import {ChartTypes} from '../../types/redux/graph'; -import {LanguageTypes} from '../../types/redux/i18n'; -import {AreaUnitType} from '../../utils/getAreaUnitConversion'; -import {showErrorNotification, showSuccessNotification} from '../../utils/notifications'; -import {useTranslate} from '../../redux/componentHooks'; +import { PreferenceRequestItem } from '../../types/items'; +import { ChartTypes } from '../../types/redux/graph'; +import { LanguageTypes } from '../../types/redux/i18n'; +import { AreaUnitType } from '../../utils/getAreaUnitConversion'; +import { showErrorNotification, showSuccessNotification } from '../../utils/notifications'; +import { useTranslate } from '../../redux/componentHooks'; import TimeZoneSelect from '../TimeZoneSelect'; -import {defaultAdminState} from '../../redux/slices/adminSlice'; -import {checkboxStyle, labelStyle} from '../../styles/modalStyle'; +import { defaultAdminState } from '../../redux/slices/adminSlice'; +import { checkboxStyle, labelStyle } from '../../styles/modalStyle'; /** * @returns Preferences Component for Administrative use */ export default function PreferencesComponent() { const translate = useTranslate(); - const {data: adminPreferences = defaultAdminState} = preferencesApi.useGetPreferencesQuery(); + const { data: adminPreferences = defaultAdminState } = preferencesApi.useGetPreferencesQuery(); const [localAdminPref, setLocalAdminPref] = React.useState(cloneDeep(adminPreferences)); const [submitPreferences] = preferencesApi.useSubmitPreferencesMutation(); const [hasChanges, setHasChanges] = React.useState(false); // mutation will invalidate preferences tag and will be re-fetched. // On query response, reset local changes to response - React.useEffect(() => { - setLocalAdminPref(cloneDeep(adminPreferences)); - }, [adminPreferences]); + React.useEffect(() => { setLocalAdminPref(cloneDeep(adminPreferences)); }, [adminPreferences]); // Compare the API response against the localState to determine changes - React.useEffect(() => { - setHasChanges(!isEqual(adminPreferences, localAdminPref)); - }, [localAdminPref, adminPreferences]); + React.useEffect(() => { setHasChanges(!isEqual(adminPreferences, localAdminPref)); }, [localAdminPref, adminPreferences]); const makeLocalChanges = (key: keyof PreferenceRequestItem, value: PreferenceRequestItem[keyof PreferenceRequestItem]) => { - setLocalAdminPref({...localAdminPref, [key]: value}); + setLocalAdminPref({ ...localAdminPref, [key]: value }); }; const discardChanges = () => { @@ -67,9 +63,7 @@ export default function PreferencesComponent() { const maxMoment = moment(localAdminPref.defaultMeterMaximumDate); return !maxMoment.isValid() || !maxMoment.isSameOrBefore(MAX_DATE_MOMENT) || !maxMoment.isSameOrAfter(minMoment); }, - readingGap: (): boolean => { - return Number(localAdminPref.defaultMeterReadingGap) < 0; - }, + readingGap: (): boolean => { return Number(localAdminPref.defaultMeterReadingGap) < 0; }, meterErrors: (): boolean => { return Number(localAdminPref.defaultMeterMaximumErrors) < 0 @@ -99,12 +93,12 @@ export default function PreferencesComponent() {

{translate('graph.settings')}

- : + :

{ Object.values(ChartTypes).map(chartType => (
-
+
); } From 93d67babb6abe6ffa9df262822dd0393ec395796 Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Mon, 13 Jul 2026 16:25:16 -0700 Subject: [PATCH 04/31] updated PreferencesComponent.tsx submitPreferences error message to be specific + added to data.ts + added getInvalidFieldNames() --- .../components/admin/PreferencesComponent.tsx | 28 +++++++++++++++++-- src/client/app/translations/data.ts | 9 ++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 8199d16c45..7d8aa21b0c 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -72,15 +72,29 @@ export default function PreferencesComponent() { warningFileSize: (): boolean => { return Number(localAdminPref.defaultWarningFileSize) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) + || Number(localAdminPref.defaultWarningFileSize) > Number.MAX_SAFE_INTEGER; }, fileSizeLimit: (): boolean => { return Number(localAdminPref.defaultFileSizeLimit) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) + || Number(localAdminPref.defaultFileSizeLimit) > Number.MAX_SAFE_INTEGER; } }; + const getInvalidFieldNames = (): string => { + let invalidFieldNames = ''; + if (invalidFuncs.readingFreq()) invalidFieldNames += translate('default.meter.reading.frequency') + ', '; + if (invalidFuncs.minDate()) invalidFieldNames += translate('default.meter.minimum.date') + ', '; + if (invalidFuncs.maxDate()) invalidFieldNames += translate('default.meter.maximum.date') + ', '; + if (invalidFuncs.readingGap()) invalidFieldNames += translate('default.meter.reading.gap') + ', '; + if (invalidFuncs.meterErrors()) invalidFieldNames += translate('default.meter.maximum.errors') + ', '; + if (invalidFuncs.warningFileSize()) invalidFieldNames += translate('default.warning.file.size') + ', '; + if (invalidFuncs.fileSizeLimit()) invalidFieldNames += translate('default.file.size.limit') + ', '; + return invalidFieldNames.slice(0, -2); + }; + return (
{ + const invalidFieldNames = getInvalidFieldNames(); + if (invalidFieldNames) { + showErrorNotification( + translate('failed.to.submit.changes') + '(' + invalidFieldNames + translate('failed.to.submit.changes.fields') + ); + return; + } + submitPreferences(localAdminPref) .unwrap() .then(() => { @@ -403,6 +425,8 @@ export default function PreferencesComponent() { showErrorNotification(translate('failed.to.submit.changes') + err.data); }); }} + /* TODO DEBUG: removed the invalidFuncs check so that we can test the new error messages that display specific reasons */ + /*disabled={!hasChanges}*/ disabled={!hasChanges || Object.values(invalidFuncs).some(check => check())} color='primary' > diff --git a/src/client/app/translations/data.ts b/src/client/app/translations/data.ts index bf9886cd54..22b5849e31 100644 --- a/src/client/app/translations/data.ts +++ b/src/client/app/translations/data.ts @@ -215,7 +215,8 @@ const LocaleTranslationData = { "failed.to.delete.map": "Failed to delete map", "failed.to.edit.map": "Failed to edit map", "failed.to.link.graph": "Failed to link graph", - "failed.to.submit.changes": "Failed to submit changes ", + "failed.to.submit.changes": "Failed to submit changes: ", + "failed.to.submit.changes.fields": " has invalid values)", "false": "False", "from.1.to.1000": "from 1 to 1000", "gps": "GPS: latitude, longitude", @@ -824,7 +825,8 @@ const LocaleTranslationData = { "failed.to.delete.map": "Échec de la suppression d'une carte", "failed.to.edit.map": "Échec de la modification d'une carte", "failed.to.link.graph": "Échec de lier le graphique", - "failed.to.submit.changes": "Échec de l'envoi des modifications", + "failed.to.submit.changes": "Échec de l'envoi des modifications: ", + "failed.to.submit.changes.fields": " has invalid values)\u{26A1}", "false": "Faux", "from.1.to.1000": "from 1 to 1000\u{26A1}", "gps": "GPS: latitude, longitude\u{26A1}", @@ -1433,7 +1435,8 @@ const LocaleTranslationData = { "failed.to.delete.map": "No se pudo borrar el mapa", "failed.to.edit.map": "No se pudo editar el mapa", "failed.to.link.graph": "No se pudo vincular el gráfico", - "failed.to.submit.changes": "No se pudo entregar los cambios", + "failed.to.submit.changes": "No se pudo entregar los cambios: ", + "failed.to.submit.changes.fields": " has invalid values)\u{26A1}", "false": "Falso", "from.1.to.1000": "from 1 to 1000\u{26A1}", "gps": "GPS: latitud, longitud", From 1f7751069962105edec864427b9b4977525bcad4 Mon Sep 17 00:00:00 2001 From: Oscar Bedolla <224943594+GoodKimchi@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:43:36 -0700 Subject: [PATCH 05/31] style: fix indentation in PreferencesComponent --- .../app/components/admin/PreferencesComponent.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 7d8aa21b0c..739f0d6d76 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -350,11 +350,11 @@ export default function PreferencesComponent() { /> ) : ( )} @@ -375,12 +375,12 @@ export default function PreferencesComponent() { {Number(localAdminPref.defaultFileSizeLimit) < 0 ? ( ) : ( )} From c640e6347d796847f15c8cbc310f730a6bbffef8 Mon Sep 17 00:00:00 2001 From: Oscar Bedolla <224943594+GoodKimchi@users.noreply.github.com> Date: Wed, 15 Jul 2026 12:06:27 -0700 Subject: [PATCH 06/31] fix: restore file size validation behavior --- src/client/app/components/admin/PreferencesComponent.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 739f0d6d76..c790113113 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -72,14 +72,12 @@ export default function PreferencesComponent() { warningFileSize: (): boolean => { return Number(localAdminPref.defaultWarningFileSize) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) - || Number(localAdminPref.defaultWarningFileSize) > Number.MAX_SAFE_INTEGER; + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); }, fileSizeLimit: (): boolean => { return Number(localAdminPref.defaultFileSizeLimit) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) - || Number(localAdminPref.defaultFileSizeLimit) > Number.MAX_SAFE_INTEGER; + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); } }; From 2b06772e4b10a20fc0fc42b7657a2bd68aa20528 Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Wed, 15 Jul 2026 12:21:02 -0700 Subject: [PATCH 07/31] merge conflicts --- src/server/routes/compareReadings.js | 4 +-- src/server/routes/preferences.js | 30 ++++++++--------- src/server/routes/readings.js | 27 +++++++-------- src/server/routes/unitReadings.js | 17 ++++++---- src/server/util/timeValidation.js | 49 ++++++---------------------- 5 files changed, 51 insertions(+), 76 deletions(-) diff --git a/src/server/routes/compareReadings.js b/src/server/routes/compareReadings.js index e59a6c58c1..7a579150d1 100644 --- a/src/server/routes/compareReadings.js +++ b/src/server/routes/compareReadings.js @@ -11,9 +11,7 @@ const { getConnection } = require('../db'); const Reading = require('../models/Reading'); const { STRING_GENERAL_MAX_LENGTH, NUMERIC_ID_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); -const { isValidIsoDuration } = require('../util/timeValidation'); - -const DATE_TIME_WITH_TIME_REGEX = /^\d{4}-\d{2}-\d{2}(?:T| )\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:?\d{2})?$/; +const { isValidIsoDateTime, isValidIsoDuration } = require('../util/timeValidation'); function validateMeterCompareReadingsParams(params) { const validParams = { diff --git a/src/server/routes/preferences.js b/src/server/routes/preferences.js index 3c6ae829a4..01725030bd 100644 --- a/src/server/routes/preferences.js +++ b/src/server/routes/preferences.js @@ -11,6 +11,7 @@ const { getConnection } = require('../db'); const { STRING_GENERAL_MAX_LENGTH, STRING_SHORT_MAX_LENGTH: SHORT_STRING_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidIsoDateTime } = require('../util/timeValidation'); +const { isValidIsoDateTime } = require('../util/timeValidation'); const router = express.Router(); @@ -81,6 +82,7 @@ router.post('/', adminAuthMiddleware('edit site preferences'), async (req, res) maxLength: SHORT_STRING_MAX_LENGTH }, // PostgreSQL interval string; does not use moment so only length-limited here + // PostgreSQL interval string; does not use moment so only length-limited here defaultMeterReadingFrequency: { type: 'string', maxLength: SHORT_STRING_MAX_LENGTH @@ -111,26 +113,24 @@ router.post('/', adminAuthMiddleware('edit site preferences'), async (req, res) } } }; + const prefs = req.body.preferences || {}; if (!validate(req.body, validParams).valid) { - return res.sendStatus(HTTP_CODES.BAD_REQUEST); - } - - const prefs = req.body.preferences; - if ( + res.sendStatus(HTTP_CODES.BAD_REQUEST); + } else if ( // preferences.js does not use moment; validate date strings directly (prefs.defaultMeterMinimumDate && !isValidIsoDateTime(prefs.defaultMeterMinimumDate)) || (prefs.defaultMeterMaximumDate && !isValidIsoDateTime(prefs.defaultMeterMaximumDate)) ) { - return res.sendStatus(HTTP_CODES.BAD_REQUEST); - } - - const conn = getConnection(); - try { - const rows = await Preferences.update(prefs, conn); - return res.json(rows); - } catch (err) { - log.error(`Error while performing POST update preferences: ${err}`, err); - return res.sendStatus(HTTP_CODES.INTERNAL_SERVER_ERROR); + res.sendStatus(HTTP_CODES.BAD_REQUEST); + } else { + const conn = getConnection(); + try { + const rows = await Preferences.update(req.body.preferences, conn); + res.json(rows); + } catch (err) { + log.error(`Error while performing POST update preferences: ${err}`, err); + res.sendStatus(HTTP_CODES.INTERNAL_SERVER_ERROR); + } } }); diff --git a/src/server/routes/readings.js b/src/server/routes/readings.js index 4d1e9924f3..2b6206b960 100644 --- a/src/server/routes/readings.js +++ b/src/server/routes/readings.js @@ -12,6 +12,7 @@ const { getConnection } = require('../db'); const { STRING_GENERAL_MAX_LENGTH: GENERAL_STRING_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidTimeInterval } = require('../util/timeValidation'); +const { isValidTimeInterval } = require('../util/timeValidation'); const router = express.Router(); @@ -41,15 +42,15 @@ router.get('/line/count/meters/:meter_ids', optionalAuthMiddleware, async (req, } } }; - if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid || !isValidTimeInterval(req.query.timeInterval, true)) { + if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid) { + res.sendStatus(HTTP_CODES.BAD_REQUEST); + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { - let meterIDs; - let timeInterval; + const conn = getConnection(); + const meterIDs = req.params.meter_ids.split(',').map(s => parseInt(s)); + const timeInterval = TimeInterval.fromString(req.query.timeInterval); try { - const conn = getConnection(); - meterIDs = req.params.meter_ids.split(',').map(s => parseInt(s)); - timeInterval = TimeInterval.fromString(req.query.timeInterval); let count = 0; for (var i = 0; i < meterIDs.length; i++) { const curr = await Reading.getCountByMeterIDAndDateRange(meterIDs[i], timeInterval.startTimestamp, timeInterval.endTimestamp, conn); @@ -95,16 +96,16 @@ router.get('/line/raw/meter/:meter_id', optionalAuthMiddleware, async (req, res) } } }; - if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid || !isValidTimeInterval(req.query.timeInterval, true)) { + if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid) { + res.sendStatus(HTTP_CODES.BAD_REQUEST); + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { - let meterID; - let timeInterval; + const conn = getConnection(); + // Get the routed meter id and time for the desired readings. + const meterID = req.params.meter_id; + const timeInterval = TimeInterval.fromString(req.query.timeInterval); try { - const conn = getConnection(); - // Get the routed meter id and time for the desired readings. - meterID = req.params.meter_id; - timeInterval = TimeInterval.fromString(req.query.timeInterval); // Get the raw readings for this meter over time range desired. // Note this returns unusual identifiers to save space and does not return the meter id. const rawReadings = await Reading.getReadingsByMeterIDAndDateRange(meterID, timeInterval.startTimestamp, timeInterval.endTimestamp, conn); diff --git a/src/server/routes/unitReadings.js b/src/server/routes/unitReadings.js index f40fbf9b87..0101eaef35 100644 --- a/src/server/routes/unitReadings.js +++ b/src/server/routes/unitReadings.js @@ -15,6 +15,7 @@ const moment = require('moment'); const { STRING_GENERAL_MAX_LENGTH, NUMERIC_ID_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidTimeInterval } = require('../util/timeValidation'); +const { isValidTimeInterval } = require('../util/timeValidation'); function validateMeterLineReadingsParams(params) { const validParams = { @@ -407,7 +408,7 @@ function createRouter() { router.get('/line/meters/:meter_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateMeterLineReadingsParams(req.params) && validateLineReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval, true)) { + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const meterIDs = req.params.meter_ids.split(',').map(idStr => Number(idStr)); @@ -422,7 +423,7 @@ function createRouter() { router.get('/line/groups/:group_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateGroupLineReadingsParams(req.params) && validateLineReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval, true)) { + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const groupIDs = req.params.group_ids.split(',').map(idStr => Number(idStr)); @@ -437,7 +438,7 @@ function createRouter() { router.get('/bar/meters/:meter_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateMeterBarReadingsParams(req.params) && validateBarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval, true)) { + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const meterIDs = req.params.meter_ids.split(',').map(idStr => Number(idStr)); @@ -453,7 +454,7 @@ function createRouter() { router.get('/bar/groups/:group_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateGroupBarReadingsParams(req.params) && validateBarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval, true)) { + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const groupIDs = req.params.group_ids.split(',').map(idStr => Number(idStr)); @@ -469,7 +470,7 @@ function createRouter() { router.get('/radar/meters/:meter_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateMeterRadarReadingsParams(req.params) && validateRadarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval, true)) { + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const meterIDs = req.params.meter_ids.split(',').map(idStr => Number(idStr)); @@ -484,7 +485,7 @@ function createRouter() { router.get('/radar/groups/:group_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateGroupRadarReadingsParams(req.params) && validateRadarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval, true)) { + } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const groupIDs = req.params.group_ids.split(',').map(idStr => Number(idStr)); @@ -501,6 +502,8 @@ function createRouter() { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); + } else if (!isValidTimeInterval(req.query.timeInterval)) { + res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { // Get time range to validate 1 year or less. const timeInterval = TimeInterval.fromString(req.query.timeInterval); @@ -531,6 +534,8 @@ function createRouter() { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); + } else if (!isValidTimeInterval(req.query.timeInterval)) { + res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { // Get time range to validate 1 year or less. const timeInterval = TimeInterval.fromString(req.query.timeInterval); diff --git a/src/server/util/timeValidation.js b/src/server/util/timeValidation.js index 00be29101b..8b6e7eac6c 100644 --- a/src/server/util/timeValidation.js +++ b/src/server/util/timeValidation.js @@ -5,7 +5,6 @@ const moment = require('moment'); const ISO_DURATION_REGEX = /^P(?!$)(\d+Y)?(\d+M)?(\d+D)?(T(\d+H)?(\d+M)?(\d+(\.\d+)?S)?)?$/; -const ISO_DATETIME_WITH_TIMEZONE_REGEX = /^\d{4}-\d{2}-\d{2}T.+(?:Z|[+-]\d{2}:?\d{2})$/; /** * Returns true if value is a strictly valid ISO 8601 datetime string (with timezone). @@ -13,10 +12,7 @@ const ISO_DATETIME_WITH_TIMEZONE_REGEX = /^\d{4}-\d{2}-\d{2}T.+(?:Z|[+-]\d{2}:?\ * @returns {boolean} */ function isValidIsoDateTime(value) { - if (typeof value !== 'string') { - return false; - } - return ISO_DATETIME_WITH_TIMEZONE_REGEX.test(value) && moment.parseZone(value, moment.ISO_8601, true).isValid(); + return moment.parseZone(value, moment.ISO_8601, true).isValid(); } /** @@ -25,51 +21,26 @@ function isValidIsoDateTime(value) { * @returns {boolean} */ function isValidIsoDuration(value) { - if (typeof value !== 'string') { - return false; - } - const duration = moment.duration(value); - return ISO_DURATION_REGEX.test(value) && moment.isDuration(duration) && duration.isValid() && duration.asMilliseconds() > 0; + return ISO_DURATION_REGEX.test(value); } /** * Returns true if value is a valid timeInterval string as used by OED's TimeInterval class. - * Accepted forms: 'all', 'ISO_ISO', and optionally 'ISO_' (right unbounded) or '_ISO' (left unbounded). + * Accepted forms: 'all', 'ISO_ISO', 'ISO_' (right unbounded), '_ISO' (left unbounded). * Each non-empty timestamp component must be a valid ISO 8601 datetime. * @param {string} value - * @param {boolean} allowOneSided true if 'ISO_' and '_ISO' should be accepted * @returns {boolean} */ -function isValidTimeInterval(value, allowOneSided = false) { - if (typeof value !== 'string') { - return false; - } - // 'all' means an unbounded interval covering all available data. - if (value === 'all') { - return true; - } - // A time interval needs an underscore between the start and end times. +function isValidTimeInterval(value) { + if (value === 'all') return true; const underscoreIndex = value.indexOf('_'); - if (underscoreIndex === -1) { - return false; - } + if (underscoreIndex === -1) return false; const start = value.substring(0, underscoreIndex); const end = value.substring(underscoreIndex + 1); - // Empty start or end times are allowed only when the route supports them. - if ((!start || !end) && !allowOneSided) { - return false; - } - // Reject '_' because it has no start or end time. - if (!start && !end) { - return false; - } - // Check the start and end times only if they were provided. - if (start && !isValidIsoDateTime(start)) { - return false; - } - if (end && !isValidIsoDateTime(end)) { - return false; - } + // At least one side must be present, and any present side must be a valid ISO datetime. + if (!start && !end) return false; + if (start && !isValidIsoDateTime(start)) return false; + if (end && !isValidIsoDateTime(end)) return false; return true; } From e4ba5feb509fa6220274b255e9edacae80830ef0 Mon Sep 17 00:00:00 2001 From: Audrey Dang Date: Tue, 28 Apr 2026 23:07:53 -0500 Subject: [PATCH 08/31] refactor and add tests for time and duration validation --- .../test/routes/compareReadingsParamsTest.js | 8 --- src/server/test/util/timeValidationTests.js | 57 ++----------------- 2 files changed, 5 insertions(+), 60 deletions(-) diff --git a/src/server/test/routes/compareReadingsParamsTest.js b/src/server/test/routes/compareReadingsParamsTest.js index 53eda6f119..7f36b55b4b 100644 --- a/src/server/test/routes/compareReadingsParamsTest.js +++ b/src/server/test/routes/compareReadingsParamsTest.js @@ -112,14 +112,6 @@ mocha.describe('Compare Readings Parameter Validation', () => { } }); - mocha.it('should accept legacy date-time format with a space separator', async () => { - const res = await chai.request(app) - .get(`${BASE_METER_ENDPOINT}/1`) - .query({ ...validQuery, curr_start: '2023-01-01 00:00:00', curr_end: '2023-01-02 00:00:00' }); - - expect(res.status).to.equal(HTTP_CODES.OK); - }); - mocha.it('should reject invalid curr_end format', async () => { const invalidDates = [ 'not-a-date', diff --git a/src/server/test/util/timeValidationTests.js b/src/server/test/util/timeValidationTests.js index 7b616da2bc..f15d3ab46f 100644 --- a/src/server/test/util/timeValidationTests.js +++ b/src/server/test/util/timeValidationTests.js @@ -5,7 +5,7 @@ */ const { expect } = require('chai'); -const mocha = require('mocha'); +const { mocha } = require('../common'); const { isValidIsoDateTime, isValidIsoDuration, isValidTimeInterval } = require('../../util/timeValidation'); mocha.describe('timeValidation utility', () => { @@ -52,19 +52,6 @@ mocha.describe('timeValidation utility', () => { expect(isValidIsoDateTime(v), v).to.equal(false); } }); - - mocha.it('should reject non-string values', () => { - const invalid = [ - ['2023-01-01T00:00:00.000Z'], - null, - undefined, - {}, - 123 - ]; - for (const v of invalid) { - expect(isValidIsoDateTime(v), String(v)).to.equal(false); - } - }); }); mocha.describe('isValidIsoDuration', () => { @@ -85,7 +72,6 @@ mocha.describe('timeValidation utility', () => { const invalid = [ 'P', // empty duration 'P1X', // invalid designator - 'PT0S', // zero duration '1D', // missing leading P 'not-a-duration', '2023-01-01T00:00:00Z', // datetime, not duration @@ -95,19 +81,6 @@ mocha.describe('timeValidation utility', () => { expect(isValidIsoDuration(v), v).to.equal(false); } }); - - mocha.it('should reject non-string values', () => { - const invalid = [ - ['P1D'], - null, - undefined, - {}, - 123 - ]; - for (const v of invalid) { - expect(isValidIsoDuration(v), String(v)).to.equal(false); - } - }); }); mocha.describe('isValidTimeInterval', () => { @@ -120,19 +93,14 @@ mocha.describe('timeValidation utility', () => { expect(isValidTimeInterval(v)).to.equal(true); }); - mocha.it('should accept left-unbounded _ISO format when one-sided intervals are allowed', () => { + mocha.it('should accept left-unbounded _ISO format', () => { const v = '_2023-12-31T23:59:59.999Z'; - expect(isValidTimeInterval(v, true)).to.equal(true); + expect(isValidTimeInterval(v)).to.equal(true); }); - mocha.it('should accept right-unbounded ISO_ format when one-sided intervals are allowed', () => { + mocha.it('should accept right-unbounded ISO_ format', () => { const v = '2023-01-01T00:00:00.000Z_'; - expect(isValidTimeInterval(v, true)).to.equal(true); - }); - - mocha.it('should reject one-sided intervals by default', () => { - expect(isValidTimeInterval('_2023-12-31T23:59:59.999Z')).to.equal(false); - expect(isValidTimeInterval('2023-01-01T00:00:00.000Z_')).to.equal(false); + expect(isValidTimeInterval(v)).to.equal(true); }); mocha.it('should reject strings with no underscore', () => { @@ -155,20 +123,5 @@ mocha.describe('timeValidation utility', () => { mocha.it('should reject empty underscore with no timestamps', () => { expect(isValidTimeInterval('_')).to.equal(false); }); - - mocha.it('should reject non-string values', () => { - const validInterval = '2023-01-01T00:00:00.000Z_2023-12-31T23:59:59.999Z'; - const invalid = [ - [validInterval], - ['all'], - null, - undefined, - {}, - 123 - ]; - for (const v of invalid) { - expect(isValidTimeInterval(v), String(v)).to.equal(false); - } - }); }); }); From 0b22fac7cf06de9e27b8b3391697350df5275802 Mon Sep 17 00:00:00 2001 From: Audrey Dang Date: Tue, 28 Apr 2026 23:12:05 -0500 Subject: [PATCH 09/31] add maxLength and comments for non-moment time fields in csv pipeline --- src/server/services/csvPipeline/ValidationSchemas.js | 9 ++++++--- .../services/csvPipeline/validateCsvUploadParams.js | 11 +++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/server/services/csvPipeline/ValidationSchemas.js b/src/server/services/csvPipeline/ValidationSchemas.js index 39c1c46693..a893db9f75 100644 --- a/src/server/services/csvPipeline/ValidationSchemas.js +++ b/src/server/services/csvPipeline/ValidationSchemas.js @@ -40,15 +40,18 @@ class BooleanParam extends EnumParam { class StringParam extends Param { /** - * * @param {string} paramName - The name of the parameter. - * @param {string} pattern - Regular expression pattern to be used in validation. This can be undefined to avoid checking. + * @param {string} pattern - Regular expression pattern for validation. Can be undefined to skip pattern check. * @param {string} description - The description of what the parameter needs to be. + * @param {number} [maxLength] - Optional maximum string length to guard against oversized inputs. */ - constructor(paramName, pattern, description) { + constructor(paramName, pattern, description, maxLength) { super(paramName, description); this.pattern = pattern; this.type = 'string'; + if (maxLength !== undefined) { + this.maxLength = maxLength; + } } } diff --git a/src/server/services/csvPipeline/validateCsvUploadParams.js b/src/server/services/csvPipeline/validateCsvUploadParams.js index b3b3f92fca..6198e5961e 100644 --- a/src/server/services/csvPipeline/validateCsvUploadParams.js +++ b/src/server/services/csvPipeline/validateCsvUploadParams.js @@ -7,6 +7,7 @@ const { CSVPipelineError } = require('./CustomErrors'); const { Param, EnumParam, BooleanParam, StringParam } = require('./ValidationSchemas'); const failure = require('./failure'); const validate = require('jsonschema').validate; +const { STRING_GENERAL_MAX_LENGTH, STRING_SHORT_MAX_LENGTH } = require('../../util/validationConstants'); // This is only used for meter page inputs but put here so next one above that related to. /** @@ -117,13 +118,15 @@ const VALIDATION = { ...COMMON_PROPERTIES, cumulative: new EnumParam('cumulative', BooleanCheckArray), cumulativeReset: new EnumParam('cumulativeReset', BooleanCheckArray), - cumulativeResetStart: new StringParam('cumulativeResetStart', undefined, undefined), - cumulativeResetEnd: new StringParam('cumulativeResetEnd', undefined, undefined), + // Time-of-day strings (HH:MM:SS); do not use moment so only length-limited here + cumulativeResetStart: new StringParam('cumulativeResetStart', undefined, undefined, STRING_SHORT_MAX_LENGTH), + cumulativeResetEnd: new StringParam('cumulativeResetEnd', undefined, undefined, STRING_SHORT_MAX_LENGTH), duplications: new StringParam('duplications', '^\\d+$|^(?![\s\S])', 'duplications must be an integer or empty.'), endOnly: new EnumParam('endOnly', BooleanCheckArray), honorDst: new EnumParam('honorDst', BooleanCheckArray), - lengthGap: new StringParam('lengthGap', undefined, undefined), - lengthVariation: new StringParam('lengthVariation', undefined, undefined), + // Numeric duration values passed to the pipeline; do not use moment so only length-limited here + lengthGap: new StringParam('lengthGap', undefined, undefined, STRING_GENERAL_MAX_LENGTH), + lengthVariation: new StringParam('lengthVariation', undefined, undefined, STRING_GENERAL_MAX_LENGTH), refreshReadings: new EnumParam('refreshReadings', BooleanCheckArray), relaxedParsing: new EnumParam('relaxedParsing', BooleanCheckArray), timeSort: new EnumParam('timeSort', [MeterTimeSortTypesJS.increasing, MeterTimeSortTypesJS.decreasing]), From 35a46f079fbaabeac1ebcfad1e2630e1f910a9d6 Mon Sep 17 00:00:00 2001 From: GoodKimchi <224943594+GoodKimchi@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:52:00 -0700 Subject: [PATCH 10/31] chore: remove csv pipeline changes (out of scope) --- src/server/services/csvPipeline/ValidationSchemas.js | 9 +++------ .../services/csvPipeline/validateCsvUploadParams.js | 11 ++++------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/server/services/csvPipeline/ValidationSchemas.js b/src/server/services/csvPipeline/ValidationSchemas.js index a893db9f75..39c1c46693 100644 --- a/src/server/services/csvPipeline/ValidationSchemas.js +++ b/src/server/services/csvPipeline/ValidationSchemas.js @@ -40,18 +40,15 @@ class BooleanParam extends EnumParam { class StringParam extends Param { /** + * * @param {string} paramName - The name of the parameter. - * @param {string} pattern - Regular expression pattern for validation. Can be undefined to skip pattern check. + * @param {string} pattern - Regular expression pattern to be used in validation. This can be undefined to avoid checking. * @param {string} description - The description of what the parameter needs to be. - * @param {number} [maxLength] - Optional maximum string length to guard against oversized inputs. */ - constructor(paramName, pattern, description, maxLength) { + constructor(paramName, pattern, description) { super(paramName, description); this.pattern = pattern; this.type = 'string'; - if (maxLength !== undefined) { - this.maxLength = maxLength; - } } } diff --git a/src/server/services/csvPipeline/validateCsvUploadParams.js b/src/server/services/csvPipeline/validateCsvUploadParams.js index 6198e5961e..b3b3f92fca 100644 --- a/src/server/services/csvPipeline/validateCsvUploadParams.js +++ b/src/server/services/csvPipeline/validateCsvUploadParams.js @@ -7,7 +7,6 @@ const { CSVPipelineError } = require('./CustomErrors'); const { Param, EnumParam, BooleanParam, StringParam } = require('./ValidationSchemas'); const failure = require('./failure'); const validate = require('jsonschema').validate; -const { STRING_GENERAL_MAX_LENGTH, STRING_SHORT_MAX_LENGTH } = require('../../util/validationConstants'); // This is only used for meter page inputs but put here so next one above that related to. /** @@ -118,15 +117,13 @@ const VALIDATION = { ...COMMON_PROPERTIES, cumulative: new EnumParam('cumulative', BooleanCheckArray), cumulativeReset: new EnumParam('cumulativeReset', BooleanCheckArray), - // Time-of-day strings (HH:MM:SS); do not use moment so only length-limited here - cumulativeResetStart: new StringParam('cumulativeResetStart', undefined, undefined, STRING_SHORT_MAX_LENGTH), - cumulativeResetEnd: new StringParam('cumulativeResetEnd', undefined, undefined, STRING_SHORT_MAX_LENGTH), + cumulativeResetStart: new StringParam('cumulativeResetStart', undefined, undefined), + cumulativeResetEnd: new StringParam('cumulativeResetEnd', undefined, undefined), duplications: new StringParam('duplications', '^\\d+$|^(?![\s\S])', 'duplications must be an integer or empty.'), endOnly: new EnumParam('endOnly', BooleanCheckArray), honorDst: new EnumParam('honorDst', BooleanCheckArray), - // Numeric duration values passed to the pipeline; do not use moment so only length-limited here - lengthGap: new StringParam('lengthGap', undefined, undefined, STRING_GENERAL_MAX_LENGTH), - lengthVariation: new StringParam('lengthVariation', undefined, undefined, STRING_GENERAL_MAX_LENGTH), + lengthGap: new StringParam('lengthGap', undefined, undefined), + lengthVariation: new StringParam('lengthVariation', undefined, undefined), refreshReadings: new EnumParam('refreshReadings', BooleanCheckArray), relaxedParsing: new EnumParam('relaxedParsing', BooleanCheckArray), timeSort: new EnumParam('timeSort', [MeterTimeSortTypesJS.increasing, MeterTimeSortTypesJS.decreasing]), From 260e5243a10c084884b501a6bd650089fbf444aa Mon Sep 17 00:00:00 2001 From: Thao Dang Date: Sun, 12 Jul 2026 17:53:21 -0500 Subject: [PATCH 11/31] improve time validation for route parameters --- src/server/routes/compareReadings.js | 9 ++--- src/server/routes/preferences.js | 28 ++++++++------- src/server/routes/readings.js | 26 +++++++------- src/server/routes/unitReadings.js | 12 +++---- .../test/routes/compareReadingsParamsTest.js | 8 +++++ src/server/test/util/timeValidationTests.js | 14 +++++--- src/server/util/timeValidation.js | 36 +++++++++++++------ 7 files changed, 83 insertions(+), 50 deletions(-) diff --git a/src/server/routes/compareReadings.js b/src/server/routes/compareReadings.js index 7a579150d1..ba97c8558d 100644 --- a/src/server/routes/compareReadings.js +++ b/src/server/routes/compareReadings.js @@ -11,7 +11,9 @@ const { getConnection } = require('../db'); const Reading = require('../models/Reading'); const { STRING_GENERAL_MAX_LENGTH, NUMERIC_ID_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); -const { isValidIsoDateTime, isValidIsoDuration } = require('../util/timeValidation'); +const { isValidIsoDuration } = require('../util/timeValidation'); + +const DATE_TIME_WITH_TIME_REGEX = /^\d{4}-\d{2}-\d{2}(?:T| )\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:?\d{2})?$/; function validateMeterCompareReadingsParams(params) { const validParams = { @@ -77,9 +79,6 @@ function validateQueryParams(queryParams) { } function isValidCompareDateTime(value) { - if (typeof value !== 'string') { - return false; - } return DATE_TIME_WITH_TIME_REGEX.test(value) && moment.parseZone(value, [moment.ISO_8601, 'YYYY-MM-DD HH:mm:ss'], true).isValid(); } @@ -125,6 +124,7 @@ function createRouter() { const currEndRaw = req.query.curr_end; const shiftRaw = req.query.shift; + if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); return; @@ -148,6 +148,7 @@ function createRouter() { const currEndRaw = req.query.curr_end; const shiftRaw = req.query.shift; + if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); return; diff --git a/src/server/routes/preferences.js b/src/server/routes/preferences.js index 01725030bd..b35aef9dac 100644 --- a/src/server/routes/preferences.js +++ b/src/server/routes/preferences.js @@ -113,24 +113,26 @@ router.post('/', adminAuthMiddleware('edit site preferences'), async (req, res) } } }; - const prefs = req.body.preferences || {}; if (!validate(req.body, validParams).valid) { - res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if ( + return res.sendStatus(HTTP_CODES.BAD_REQUEST); + } + + const prefs = req.body.preferences; + if ( // preferences.js does not use moment; validate date strings directly (prefs.defaultMeterMinimumDate && !isValidIsoDateTime(prefs.defaultMeterMinimumDate)) || (prefs.defaultMeterMaximumDate && !isValidIsoDateTime(prefs.defaultMeterMaximumDate)) ) { - res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else { - const conn = getConnection(); - try { - const rows = await Preferences.update(req.body.preferences, conn); - res.json(rows); - } catch (err) { - log.error(`Error while performing POST update preferences: ${err}`, err); - res.sendStatus(HTTP_CODES.INTERNAL_SERVER_ERROR); - } + return res.sendStatus(HTTP_CODES.BAD_REQUEST); + } + + const conn = getConnection(); + try { + const rows = await Preferences.update(prefs, conn); + return res.json(rows); + } catch (err) { + log.error(`Error while performing POST update preferences: ${err}`, err); + return res.sendStatus(HTTP_CODES.INTERNAL_SERVER_ERROR); } }); diff --git a/src/server/routes/readings.js b/src/server/routes/readings.js index 2b6206b960..62faabdeb7 100644 --- a/src/server/routes/readings.js +++ b/src/server/routes/readings.js @@ -42,15 +42,15 @@ router.get('/line/count/meters/:meter_ids', optionalAuthMiddleware, async (req, } } }; - if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid) { - res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid || !isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { - const conn = getConnection(); - const meterIDs = req.params.meter_ids.split(',').map(s => parseInt(s)); - const timeInterval = TimeInterval.fromString(req.query.timeInterval); + let meterIDs; + let timeInterval; try { + const conn = getConnection(); + meterIDs = req.params.meter_ids.split(',').map(s => parseInt(s)); + timeInterval = TimeInterval.fromString(req.query.timeInterval); let count = 0; for (var i = 0; i < meterIDs.length; i++) { const curr = await Reading.getCountByMeterIDAndDateRange(meterIDs[i], timeInterval.startTimestamp, timeInterval.endTimestamp, conn); @@ -96,16 +96,16 @@ router.get('/line/raw/meter/:meter_id', optionalAuthMiddleware, async (req, res) } } }; - if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid) { - res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + if (!validate(req.params, validParams).valid || !validate(req.query, validQueries).valid || !isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { - const conn = getConnection(); - // Get the routed meter id and time for the desired readings. - const meterID = req.params.meter_id; - const timeInterval = TimeInterval.fromString(req.query.timeInterval); + let meterID; + let timeInterval; try { + const conn = getConnection(); + // Get the routed meter id and time for the desired readings. + meterID = req.params.meter_id; + timeInterval = TimeInterval.fromString(req.query.timeInterval); // Get the raw readings for this meter over time range desired. // Note this returns unusual identifiers to save space and does not return the meter id. const rawReadings = await Reading.getReadingsByMeterIDAndDateRange(meterID, timeInterval.startTimestamp, timeInterval.endTimestamp, conn); diff --git a/src/server/routes/unitReadings.js b/src/server/routes/unitReadings.js index 0101eaef35..8e1a0fad2d 100644 --- a/src/server/routes/unitReadings.js +++ b/src/server/routes/unitReadings.js @@ -408,7 +408,7 @@ function createRouter() { router.get('/line/meters/:meter_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateMeterLineReadingsParams(req.params) && validateLineReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + } else if (!isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const meterIDs = req.params.meter_ids.split(',').map(idStr => Number(idStr)); @@ -423,7 +423,7 @@ function createRouter() { router.get('/line/groups/:group_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateGroupLineReadingsParams(req.params) && validateLineReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + } else if (!isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const groupIDs = req.params.group_ids.split(',').map(idStr => Number(idStr)); @@ -438,7 +438,7 @@ function createRouter() { router.get('/bar/meters/:meter_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateMeterBarReadingsParams(req.params) && validateBarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + } else if (!isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const meterIDs = req.params.meter_ids.split(',').map(idStr => Number(idStr)); @@ -454,7 +454,7 @@ function createRouter() { router.get('/bar/groups/:group_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateGroupBarReadingsParams(req.params) && validateBarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + } else if (!isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const groupIDs = req.params.group_ids.split(',').map(idStr => Number(idStr)); @@ -470,7 +470,7 @@ function createRouter() { router.get('/radar/meters/:meter_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateMeterRadarReadingsParams(req.params) && validateRadarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + } else if (!isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const meterIDs = req.params.meter_ids.split(',').map(idStr => Number(idStr)); @@ -485,7 +485,7 @@ function createRouter() { router.get('/radar/groups/:group_ids', optionalAuthMiddleware, async (req, res) => { if (!(validateGroupRadarReadingsParams(req.params) && validateRadarReadingsQueryParams(req.query))) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { + } else if (!isValidTimeInterval(req.query.timeInterval, true)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { const groupIDs = req.params.group_ids.split(',').map(idStr => Number(idStr)); diff --git a/src/server/test/routes/compareReadingsParamsTest.js b/src/server/test/routes/compareReadingsParamsTest.js index 7f36b55b4b..53eda6f119 100644 --- a/src/server/test/routes/compareReadingsParamsTest.js +++ b/src/server/test/routes/compareReadingsParamsTest.js @@ -112,6 +112,14 @@ mocha.describe('Compare Readings Parameter Validation', () => { } }); + mocha.it('should accept legacy date-time format with a space separator', async () => { + const res = await chai.request(app) + .get(`${BASE_METER_ENDPOINT}/1`) + .query({ ...validQuery, curr_start: '2023-01-01 00:00:00', curr_end: '2023-01-02 00:00:00' }); + + expect(res.status).to.equal(HTTP_CODES.OK); + }); + mocha.it('should reject invalid curr_end format', async () => { const invalidDates = [ 'not-a-date', diff --git a/src/server/test/util/timeValidationTests.js b/src/server/test/util/timeValidationTests.js index f15d3ab46f..1749137750 100644 --- a/src/server/test/util/timeValidationTests.js +++ b/src/server/test/util/timeValidationTests.js @@ -72,6 +72,7 @@ mocha.describe('timeValidation utility', () => { const invalid = [ 'P', // empty duration 'P1X', // invalid designator + 'PT0S', // zero duration '1D', // missing leading P 'not-a-duration', '2023-01-01T00:00:00Z', // datetime, not duration @@ -93,14 +94,19 @@ mocha.describe('timeValidation utility', () => { expect(isValidTimeInterval(v)).to.equal(true); }); - mocha.it('should accept left-unbounded _ISO format', () => { + mocha.it('should accept left-unbounded _ISO format when one-sided intervals are allowed', () => { const v = '_2023-12-31T23:59:59.999Z'; - expect(isValidTimeInterval(v)).to.equal(true); + expect(isValidTimeInterval(v, true)).to.equal(true); }); - mocha.it('should accept right-unbounded ISO_ format', () => { + mocha.it('should accept right-unbounded ISO_ format when one-sided intervals are allowed', () => { const v = '2023-01-01T00:00:00.000Z_'; - expect(isValidTimeInterval(v)).to.equal(true); + expect(isValidTimeInterval(v, true)).to.equal(true); + }); + + mocha.it('should reject one-sided intervals by default', () => { + expect(isValidTimeInterval('_2023-12-31T23:59:59.999Z')).to.equal(false); + expect(isValidTimeInterval('2023-01-01T00:00:00.000Z_')).to.equal(false); }); mocha.it('should reject strings with no underscore', () => { diff --git a/src/server/util/timeValidation.js b/src/server/util/timeValidation.js index 8b6e7eac6c..a703dba4c2 100644 --- a/src/server/util/timeValidation.js +++ b/src/server/util/timeValidation.js @@ -5,6 +5,7 @@ const moment = require('moment'); const ISO_DURATION_REGEX = /^P(?!$)(\d+Y)?(\d+M)?(\d+D)?(T(\d+H)?(\d+M)?(\d+(\.\d+)?S)?)?$/; +const ISO_DATETIME_WITH_TIMEZONE_REGEX = /^\d{4}-\d{2}-\d{2}T.+(?:Z|[+-]\d{2}:?\d{2})$/; /** * Returns true if value is a strictly valid ISO 8601 datetime string (with timezone). @@ -12,7 +13,7 @@ const ISO_DURATION_REGEX = /^P(?!$)(\d+Y)?(\d+M)?(\d+D)?(T(\d+H)?(\d+M)?(\d+(\.\ * @returns {boolean} */ function isValidIsoDateTime(value) { - return moment.parseZone(value, moment.ISO_8601, true).isValid(); + return ISO_DATETIME_WITH_TIMEZONE_REGEX.test(value) && moment.parseZone(value, moment.ISO_8601, true).isValid(); } /** @@ -21,26 +22,41 @@ function isValidIsoDateTime(value) { * @returns {boolean} */ function isValidIsoDuration(value) { - return ISO_DURATION_REGEX.test(value); + const duration = moment.duration(value); + return ISO_DURATION_REGEX.test(value) && moment.isDuration(duration) && duration.isValid() && duration.asMilliseconds() > 0; } /** * Returns true if value is a valid timeInterval string as used by OED's TimeInterval class. - * Accepted forms: 'all', 'ISO_ISO', 'ISO_' (right unbounded), '_ISO' (left unbounded). + * Accepted forms: 'all', 'ISO_ISO', and optionally 'ISO_' (right unbounded) or '_ISO' (left unbounded). * Each non-empty timestamp component must be a valid ISO 8601 datetime. * @param {string} value + * @param {boolean} allowOneSided true if 'ISO_' and '_ISO' should be accepted * @returns {boolean} */ -function isValidTimeInterval(value) { - if (value === 'all') return true; +function isValidTimeInterval(value, allowOneSided = false) { + if (value === 'all') { + return true; + } const underscoreIndex = value.indexOf('_'); - if (underscoreIndex === -1) return false; + if (underscoreIndex === -1) { + return false; + } const start = value.substring(0, underscoreIndex); const end = value.substring(underscoreIndex + 1); - // At least one side must be present, and any present side must be a valid ISO datetime. - if (!start && !end) return false; - if (start && !isValidIsoDateTime(start)) return false; - if (end && !isValidIsoDateTime(end)) return false; + // One-sided intervals have an empty start or end around the underscore. + if ((!start || !end) && !allowOneSided) { + return false; + } + if (!start && !end) { + return false; + } + if (start && !isValidIsoDateTime(start)) { + return false; + } + if (end && !isValidIsoDateTime(end)) { + return false; + } return true; } From e45123f4db09615277ae56015f4d6e04fcc52a27 Mon Sep 17 00:00:00 2001 From: Thao Dang Date: Mon, 13 Jul 2026 15:29:44 -0500 Subject: [PATCH 12/31] add comments for time interval validation --- src/server/util/timeValidation.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/server/util/timeValidation.js b/src/server/util/timeValidation.js index a703dba4c2..0077772e72 100644 --- a/src/server/util/timeValidation.js +++ b/src/server/util/timeValidation.js @@ -35,22 +35,26 @@ function isValidIsoDuration(value) { * @returns {boolean} */ function isValidTimeInterval(value, allowOneSided = false) { + // 'all' means an unbounded interval covering all available data. if (value === 'all') { return true; } + // A time interval needs an underscore between the start and end times. const underscoreIndex = value.indexOf('_'); if (underscoreIndex === -1) { return false; } const start = value.substring(0, underscoreIndex); const end = value.substring(underscoreIndex + 1); - // One-sided intervals have an empty start or end around the underscore. + // Empty start or end times are allowed only when the route supports them. if ((!start || !end) && !allowOneSided) { return false; } + // Reject '_' because it has no start or end time. if (!start && !end) { return false; } + // Check the start and end times only if they were provided. if (start && !isValidIsoDateTime(start)) { return false; } From 9d8288217e776087915ac2f767860639daba90d1 Mon Sep 17 00:00:00 2001 From: Thao Dang Date: Tue, 14 Jul 2026 13:39:10 -0500 Subject: [PATCH 13/31] check non-string route input in time validation --- src/server/routes/compareReadings.js | 3 ++ src/server/test/util/timeValidationTests.js | 43 ++++++++++++++++++++- src/server/util/timeValidation.js | 9 +++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/server/routes/compareReadings.js b/src/server/routes/compareReadings.js index ba97c8558d..55951a2b4d 100644 --- a/src/server/routes/compareReadings.js +++ b/src/server/routes/compareReadings.js @@ -79,6 +79,9 @@ function validateQueryParams(queryParams) { } function isValidCompareDateTime(value) { + if (typeof value !== 'string') { + return false; + } return DATE_TIME_WITH_TIME_REGEX.test(value) && moment.parseZone(value, [moment.ISO_8601, 'YYYY-MM-DD HH:mm:ss'], true).isValid(); } diff --git a/src/server/test/util/timeValidationTests.js b/src/server/test/util/timeValidationTests.js index 1749137750..7b616da2bc 100644 --- a/src/server/test/util/timeValidationTests.js +++ b/src/server/test/util/timeValidationTests.js @@ -5,7 +5,7 @@ */ const { expect } = require('chai'); -const { mocha } = require('../common'); +const mocha = require('mocha'); const { isValidIsoDateTime, isValidIsoDuration, isValidTimeInterval } = require('../../util/timeValidation'); mocha.describe('timeValidation utility', () => { @@ -52,6 +52,19 @@ mocha.describe('timeValidation utility', () => { expect(isValidIsoDateTime(v), v).to.equal(false); } }); + + mocha.it('should reject non-string values', () => { + const invalid = [ + ['2023-01-01T00:00:00.000Z'], + null, + undefined, + {}, + 123 + ]; + for (const v of invalid) { + expect(isValidIsoDateTime(v), String(v)).to.equal(false); + } + }); }); mocha.describe('isValidIsoDuration', () => { @@ -82,6 +95,19 @@ mocha.describe('timeValidation utility', () => { expect(isValidIsoDuration(v), v).to.equal(false); } }); + + mocha.it('should reject non-string values', () => { + const invalid = [ + ['P1D'], + null, + undefined, + {}, + 123 + ]; + for (const v of invalid) { + expect(isValidIsoDuration(v), String(v)).to.equal(false); + } + }); }); mocha.describe('isValidTimeInterval', () => { @@ -129,5 +155,20 @@ mocha.describe('timeValidation utility', () => { mocha.it('should reject empty underscore with no timestamps', () => { expect(isValidTimeInterval('_')).to.equal(false); }); + + mocha.it('should reject non-string values', () => { + const validInterval = '2023-01-01T00:00:00.000Z_2023-12-31T23:59:59.999Z'; + const invalid = [ + [validInterval], + ['all'], + null, + undefined, + {}, + 123 + ]; + for (const v of invalid) { + expect(isValidTimeInterval(v), String(v)).to.equal(false); + } + }); }); }); diff --git a/src/server/util/timeValidation.js b/src/server/util/timeValidation.js index 0077772e72..00be29101b 100644 --- a/src/server/util/timeValidation.js +++ b/src/server/util/timeValidation.js @@ -13,6 +13,9 @@ const ISO_DATETIME_WITH_TIMEZONE_REGEX = /^\d{4}-\d{2}-\d{2}T.+(?:Z|[+-]\d{2}:?\ * @returns {boolean} */ function isValidIsoDateTime(value) { + if (typeof value !== 'string') { + return false; + } return ISO_DATETIME_WITH_TIMEZONE_REGEX.test(value) && moment.parseZone(value, moment.ISO_8601, true).isValid(); } @@ -22,6 +25,9 @@ function isValidIsoDateTime(value) { * @returns {boolean} */ function isValidIsoDuration(value) { + if (typeof value !== 'string') { + return false; + } const duration = moment.duration(value); return ISO_DURATION_REGEX.test(value) && moment.isDuration(duration) && duration.isValid() && duration.asMilliseconds() > 0; } @@ -35,6 +41,9 @@ function isValidIsoDuration(value) { * @returns {boolean} */ function isValidTimeInterval(value, allowOneSided = false) { + if (typeof value !== 'string') { + return false; + } // 'all' means an unbounded interval covering all available data. if (value === 'all') { return true; From a8129c0b8992f0abcb03f64c7eaba8d5f4f83fe6 Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Wed, 15 Jul 2026 12:35:32 -0700 Subject: [PATCH 14/31] resolved error that occurred from merge conflict --- src/server/routes/compareReadings.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/server/routes/compareReadings.js b/src/server/routes/compareReadings.js index 55951a2b4d..e59a6c58c1 100644 --- a/src/server/routes/compareReadings.js +++ b/src/server/routes/compareReadings.js @@ -127,7 +127,6 @@ function createRouter() { const currEndRaw = req.query.curr_end; const shiftRaw = req.query.shift; - if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); return; @@ -151,7 +150,6 @@ function createRouter() { const currEndRaw = req.query.curr_end; const shiftRaw = req.query.shift; - if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); return; From e4e2f7a1cf2785461c28e8e1c9f7a427c7041d4d Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Wed, 15 Jul 2026 13:30:11 -0700 Subject: [PATCH 15/31] resolved errors from merge conflict (duplicated code) + restored changes to PreferencesComponent.tsx --- src/client/app/components/admin/PreferencesComponent.tsx | 6 ++++-- src/server/routes/preferences.js | 1 - src/server/routes/readings.js | 1 - src/server/routes/unitReadings.js | 5 ----- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index c790113113..739f0d6d76 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -72,12 +72,14 @@ export default function PreferencesComponent() { warningFileSize: (): boolean => { return Number(localAdminPref.defaultWarningFileSize) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) + || Number(localAdminPref.defaultWarningFileSize) > Number.MAX_SAFE_INTEGER; }, fileSizeLimit: (): boolean => { return Number(localAdminPref.defaultFileSizeLimit) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) + || Number(localAdminPref.defaultFileSizeLimit) > Number.MAX_SAFE_INTEGER; } }; diff --git a/src/server/routes/preferences.js b/src/server/routes/preferences.js index b35aef9dac..a773e2a253 100644 --- a/src/server/routes/preferences.js +++ b/src/server/routes/preferences.js @@ -11,7 +11,6 @@ const { getConnection } = require('../db'); const { STRING_GENERAL_MAX_LENGTH, STRING_SHORT_MAX_LENGTH: SHORT_STRING_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidIsoDateTime } = require('../util/timeValidation'); -const { isValidIsoDateTime } = require('../util/timeValidation'); const router = express.Router(); diff --git a/src/server/routes/readings.js b/src/server/routes/readings.js index 62faabdeb7..4d1e9924f3 100644 --- a/src/server/routes/readings.js +++ b/src/server/routes/readings.js @@ -12,7 +12,6 @@ const { getConnection } = require('../db'); const { STRING_GENERAL_MAX_LENGTH: GENERAL_STRING_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidTimeInterval } = require('../util/timeValidation'); -const { isValidTimeInterval } = require('../util/timeValidation'); const router = express.Router(); diff --git a/src/server/routes/unitReadings.js b/src/server/routes/unitReadings.js index 8e1a0fad2d..f40fbf9b87 100644 --- a/src/server/routes/unitReadings.js +++ b/src/server/routes/unitReadings.js @@ -15,7 +15,6 @@ const moment = require('moment'); const { STRING_GENERAL_MAX_LENGTH, NUMERIC_ID_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidTimeInterval } = require('../util/timeValidation'); -const { isValidTimeInterval } = require('../util/timeValidation'); function validateMeterLineReadingsParams(params) { const validParams = { @@ -502,8 +501,6 @@ function createRouter() { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { - res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { // Get time range to validate 1 year or less. const timeInterval = TimeInterval.fromString(req.query.timeInterval); @@ -534,8 +531,6 @@ function createRouter() { res.sendStatus(HTTP_CODES.BAD_REQUEST); } else if (!isValidTimeInterval(req.query.timeInterval)) { res.sendStatus(HTTP_CODES.BAD_REQUEST); - } else if (!isValidTimeInterval(req.query.timeInterval)) { - res.sendStatus(HTTP_CODES.BAD_REQUEST); } else { // Get time range to validate 1 year or less. const timeInterval = TimeInterval.fromString(req.query.timeInterval); From 2f5d8e569c6388cb41c84b12aab148278ff248e6 Mon Sep 17 00:00:00 2001 From: Oscar Bedolla <224943594+GoodKimchi@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:16:16 -0700 Subject: [PATCH 16/31] fix: restore infinity bound for file size limit --- .../components/admin/PreferencesComponent.tsx | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 739f0d6d76..eac0755a47 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -372,17 +372,13 @@ export default function PreferencesComponent() { invalid={invalidFuncs.fileSizeLimit()} /> - {Number(localAdminPref.defaultFileSizeLimit) < 0 ? ( - - ) : ( - - )} +
From fa217a46a071623b49a5d89146ce7e821dddfabd Mon Sep 17 00:00:00 2001 From: GoodKimchi <224943594+GoodKimchi@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:07:20 -0700 Subject: [PATCH 17/31] fix: remove duplicate line after merge conflict --- src/server/routes/preferences.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server/routes/preferences.js b/src/server/routes/preferences.js index a773e2a253..3c6ae829a4 100644 --- a/src/server/routes/preferences.js +++ b/src/server/routes/preferences.js @@ -81,7 +81,6 @@ router.post('/', adminAuthMiddleware('edit site preferences'), async (req, res) maxLength: SHORT_STRING_MAX_LENGTH }, // PostgreSQL interval string; does not use moment so only length-limited here - // PostgreSQL interval string; does not use moment so only length-limited here defaultMeterReadingFrequency: { type: 'string', maxLength: SHORT_STRING_MAX_LENGTH From 11cbbf7d3eeee66538fc8493092457d374f15ba2 Mon Sep 17 00:00:00 2001 From: Andrew-Bonner Date: Tue, 31 Mar 2026 19:16:05 -0400 Subject: [PATCH 18/31] updated the login route to make sure the bcrypt.copare function is called on a null users to eliminate time difference --- src/server/routes/login.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/routes/login.js b/src/server/routes/login.js index ef17d53888..df76ac93d3 100644 --- a/src/server/routes/login.js +++ b/src/server/routes/login.js @@ -49,6 +49,7 @@ router.post('/', credentialsRequestValidationMiddleware, async (req, res) => { let isValid; if (user === null) { // User did not exist so return false. + isValid = await bcrypt.compare(req.body.password, user.passwordHash); isValid = false; } else { isValid = await bcrypt.compare(req.body.password, user.passwordHash); From 717e55747595a950a1573086eb5fbcc8b8167902 Mon Sep 17 00:00:00 2001 From: Andrew-Bonner Date: Tue, 21 Apr 2026 10:28:05 -0400 Subject: [PATCH 19/31] call the bcrypt function without assigning it to the value and just setting the isValid value to false. That way, this function is not being assigned to a value and is just running. --- src/server/routes/login.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/routes/login.js b/src/server/routes/login.js index df76ac93d3..ff4a38c001 100644 --- a/src/server/routes/login.js +++ b/src/server/routes/login.js @@ -48,8 +48,8 @@ router.post('/', credentialsRequestValidationMiddleware, async (req, res) => { const user = await User.getByUsername(req.body.username, conn); let isValid; if (user === null) { - // User did not exist so return false. - isValid = await bcrypt.compare(req.body.password, user.passwordHash); + // call the bcrypt.compare() without assigning it valid user to eliminate time differation + await bcrypt.compare(req.body.password, user.passwordHash); isValid = false; } else { isValid = await bcrypt.compare(req.body.password, user.passwordHash); From c9ccbdbd6f4b747799baf620a5d1e8b8677e4e49 Mon Sep 17 00:00:00 2001 From: Alangdi Date: Tue, 14 Jul 2026 12:07:33 -0700 Subject: [PATCH 20/31] Fix login timing for missing usernames --- src/server/routes/login.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/server/routes/login.js b/src/server/routes/login.js index ff4a38c001..3a9f03ba05 100644 --- a/src/server/routes/login.js +++ b/src/server/routes/login.js @@ -15,6 +15,7 @@ const { PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, USERNAME_MIN_LENGTH, USERNAME_ const { HTTP_CODES } = require('../util/httpCodes'); const router = express.Router(); +const DUMMY_PASSWORD_HASH = '$2a$10$7EqJtq98hPqEX7fNZaFWoOHIoQStbSNRaCbkWa3vgKwK3/q5YLhKa'; /** * Authenticate users and return a JSON Web Token with their user ID. @@ -46,14 +47,15 @@ router.post('/', credentialsRequestValidationMiddleware, async (req, res) => { const conn = getConnection(); try { const user = await User.getByUsername(req.body.username, conn); - let isValid; - if (user === null) { - // call the bcrypt.compare() without assigning it valid user to eliminate time differation - await bcrypt.compare(req.body.password, user.passwordHash); - isValid = false; - } else { - isValid = await bcrypt.compare(req.body.password, user.passwordHash); - } + +// User did not exist so return false. +// +// Use a fixed bcrypt hash when the user does not exist. This keeps the +// password comparison path similar for existing and non-existing users, +// reducing the timing difference that could reveal valid usernames. +const passwordHash = user === null ? DUMMY_PASSWORD_HASH : user.passwordHash; +const passwordMatches = await bcrypt.compare(req.body.password, passwordHash); +const isValid = user !== null && passwordMatches; if (isValid) { const token = jwt.sign({ data: user.id }, secretToken, { expiresIn: 86400 }); res.json({ token: token, username: user.username, role: user.role }); From 6f42579b2eac068fe4fe588464ed12282cff32bc Mon Sep 17 00:00:00 2001 From: Alnagdi Mohsen Date: Wed, 15 Jul 2026 19:02:36 +0000 Subject: [PATCH 21/31] Address login timing review comments --- src/server/routes/login.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/server/routes/login.js b/src/server/routes/login.js index 3a9f03ba05..03cac6917d 100644 --- a/src/server/routes/login.js +++ b/src/server/routes/login.js @@ -15,7 +15,7 @@ const { PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, USERNAME_MIN_LENGTH, USERNAME_ const { HTTP_CODES } = require('../util/httpCodes'); const router = express.Router(); -const DUMMY_PASSWORD_HASH = '$2a$10$7EqJtq98hPqEX7fNZaFWoOHIoQStbSNRaCbkWa3vgKwK3/q5YLhKa'; +const DUMMY_PASSWORD_HASH = '$2a$10$N6cWKczGlZaT2ReVzJ48pu8t87bpatdCnpI50fXQ7SnHO23LL7Nfe'; /** * Authenticate users and return a JSON Web Token with their user ID. @@ -47,13 +47,13 @@ router.post('/', credentialsRequestValidationMiddleware, async (req, res) => { const conn = getConnection(); try { const user = await User.getByUsername(req.body.username, conn); +const user = await User.getByUsername(req.body.username, conn); -// User did not exist so return false. -// -// Use a fixed bcrypt hash when the user does not exist. This keeps the -// password comparison path similar for existing and non-existing users, -// reducing the timing difference that could reveal valid usernames. -const passwordHash = user === null ? DUMMY_PASSWORD_HASH : user.passwordHash; +// This hash is used only when the username does not exist. It keeps +// the bcrypt comparison path similar for existing and non-existing +// users without allowing a missing user to log in. +const dummyPasswordHash = '$2a$10$N6cWKczGlZaT2ReVzJ48pu8t87bpatdCnpI50fXQ7SnHO23LL7Nfe'; +const passwordHash = user === null ? dummyPasswordHash : user.passwordHash; const passwordMatches = await bcrypt.compare(req.body.password, passwordHash); const isValid = user !== null && passwordMatches; if (isValid) { From b68d77633d91269158fab5d6879ab6b284486d9b Mon Sep 17 00:00:00 2001 From: Alnagdi Mohsen Date: Wed, 15 Jul 2026 19:12:47 +0000 Subject: [PATCH 22/31] Fix duplicate user declaration --- src/server/routes/login.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/server/routes/login.js b/src/server/routes/login.js index 03cac6917d..fde7add1dc 100644 --- a/src/server/routes/login.js +++ b/src/server/routes/login.js @@ -47,15 +47,14 @@ router.post('/', credentialsRequestValidationMiddleware, async (req, res) => { const conn = getConnection(); try { const user = await User.getByUsername(req.body.username, conn); -const user = await User.getByUsername(req.body.username, conn); -// This hash is used only when the username does not exist. It keeps -// the bcrypt comparison path similar for existing and non-existing -// users without allowing a missing user to log in. -const dummyPasswordHash = '$2a$10$N6cWKczGlZaT2ReVzJ48pu8t87bpatdCnpI50fXQ7SnHO23LL7Nfe'; -const passwordHash = user === null ? dummyPasswordHash : user.passwordHash; -const passwordMatches = await bcrypt.compare(req.body.password, passwordHash); -const isValid = user !== null && passwordMatches; + // This hash is used only when the username does not exist. It keeps + // the bcrypt comparison path similar for existing and non-existing + // users without allowing a missing user to log in. + const dummyPasswordHash = '$2a$10$N6cWKczGlZaT2ReVzJ48pu8t87bpatdCnpI50fXQ7SnHO23LL7Nfe'; + const passwordHash = user === null ? dummyPasswordHash : user.passwordHash; + const passwordMatches = await bcrypt.compare(req.body.password, passwordHash); + const isValid = user !== null && passwordMatches; if (isValid) { const token = jwt.sign({ data: user.id }, secretToken, { expiresIn: 86400 }); res.json({ token: token, username: user.username, role: user.role }); From efd834ee92d96b9cab99966772f00a98ee4c8184 Mon Sep 17 00:00:00 2001 From: Alnagdi Mohsen Date: Thu, 16 Jul 2026 17:27:39 +0000 Subject: [PATCH 23/31] Remove unused dummy hash constant --- src/server/routes/login.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/server/routes/login.js b/src/server/routes/login.js index fde7add1dc..e9af660ab1 100644 --- a/src/server/routes/login.js +++ b/src/server/routes/login.js @@ -15,8 +15,6 @@ const { PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, USERNAME_MIN_LENGTH, USERNAME_ const { HTTP_CODES } = require('../util/httpCodes'); const router = express.Router(); -const DUMMY_PASSWORD_HASH = '$2a$10$N6cWKczGlZaT2ReVzJ48pu8t87bpatdCnpI50fXQ7SnHO23LL7Nfe'; - /** * Authenticate users and return a JSON Web Token with their user ID. * @param {String} username From d4fef5a8d434f3f6a7fa54fcc777dbb950f51532 Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Wed, 15 Jul 2026 22:34:40 -0700 Subject: [PATCH 24/31] moved omit() calls from client to API layer for relevant files + added buildConversionSubmitState() helper function in CreateConversionModalComponent.tsx + updated outdated comments / added new comments --- .../admin/users/CreateUserModalComponent.tsx | 4 ++++ .../CreateConversionModalComponent.tsx | 21 +++++++++++-------- .../meters/CreateMeterModalComponent.tsx | 4 ++-- .../unit/CreateUnitModalComponent.tsx | 10 ++++----- src/client/app/redux/api/metersApi.ts | 3 ++- src/client/app/redux/api/unitsApi.ts | 4 +++- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/client/app/components/admin/users/CreateUserModalComponent.tsx b/src/client/app/components/admin/users/CreateUserModalComponent.tsx index b94eed1be0..1a905e86f4 100644 --- a/src/client/app/components/admin/users/CreateUserModalComponent.tsx +++ b/src/client/app/components/admin/users/CreateUserModalComponent.tsx @@ -109,6 +109,10 @@ export default function CreateUserModal() { // End Modal show/close const handleSubmit = async () => { + // id is not used when creating a newUser. omit() could be used to exclude id for userDetails to be consistent with the implementations + // on other client files, but it is not necessary at the moment. In addition, introducing omit() in the client files for User could cause + // unexpected issues. We are acknowledging that the codebase currently has a different implementation for handleSubmit() compared to other + // client files. const newUser: User = { username: userDetails.username, role: userDetails.role, password: userDetails.password, note: userDetails.note }; createUser(newUser) .unwrap() diff --git a/src/client/app/components/conversion/CreateConversionModalComponent.tsx b/src/client/app/components/conversion/CreateConversionModalComponent.tsx index 6d0d27bce3..0a627ba4aa 100644 --- a/src/client/app/components/conversion/CreateConversionModalComponent.tsx +++ b/src/client/app/components/conversion/CreateConversionModalComponent.tsx @@ -161,11 +161,18 @@ export default function CreateConversionModalComponent() { }; /* End Warning Modal */ + // The helper function will handle the omit() which separates it from the addConversionMutation. + // The addConversionMutation will call this helper function, so that omit() is not directly called. + // This helper function will also computes bidirectional based on the current source/destination selections + const buildConversionSubmitState = (state: typeof conversionState) => ({ + ...omit(state, 'sourceOptions', 'destinationOptions'), + bidirectional: (isMeterSource() || isSuffixUsed()) ? false : state.bidirectional + }); + // Submit const handleSubmit = () => { // Used for the ShowErrorNotification - const pending = {...omit(conversionState, 'sourceOptions', 'destinationOptions'), - bidirectional: (isMeterSource() || isSuffixUsed()) ? false : conversionState.bidirectional}; + const pending = buildConversionSubmitState(conversionState); setPendingConversion(pending); // Show warning modal if slope and intercept are both 0 @@ -180,8 +187,7 @@ export default function CreateConversionModalComponent() { // Omit the source options , do not need to send in request so remove here. // If source is a meter, make bidirectional false // If source or destination is a suffix unit, make bidirectional false - addConversionMutation({...omit(conversionState, 'sourceOptions', 'destinationOptions'), - bidirectional: (isMeterSource() || isSuffixUsed()) ? false : conversionState.bidirectional}) + addConversionMutation(buildConversionSubmitState(conversionState)) .unwrap() .then(() => { // Show source/destination identifiers (not numeric IDs) @@ -248,17 +254,14 @@ export default function CreateConversionModalComponent() { setShowUnsavedWarning(false); setHasUnsavedChanges(false); if (conversionState.slope === 0 && conversionState.intercept === 0) { - setPendingConversion({...omit(conversionState, 'sourceOptions', 'destinationOptions'), - bidirectional: (isMeterSource() || isSuffixUsed()) ? false : conversionState.bidirectional}); + setPendingConversion(buildConversionSubmitState(conversionState)); setWarningMessage(translate('conversion.slope.intercept.zero')); setShowWarningModal(true); } else if (validConversion) { setShowModal(false); - addConversionMutation({...omit(conversionState, 'sourceOptions', 'destinationOptions'), - bidirectional: (isMeterSource() || isSuffixUsed()) ? false : conversionState.bidirectional - }) + addConversionMutation(buildConversionSubmitState(conversionState)) .unwrap() .then(() => { // Show source/destination identifiers (not numeric IDs) diff --git a/src/client/app/components/meters/CreateMeterModalComponent.tsx b/src/client/app/components/meters/CreateMeterModalComponent.tsx index bfd2103e5e..fc139359a5 100644 --- a/src/client/app/components/meters/CreateMeterModalComponent.tsx +++ b/src/client/app/components/meters/CreateMeterModalComponent.tsx @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { range, omit } from 'lodash'; +import { range } from 'lodash'; import * as moment from 'moment'; import * as React from 'react'; import { useEffect, useState } from 'react'; @@ -179,7 +179,7 @@ export default function CreateMeterModalComponent(props: CreateMeterModalProps): if (inputOk) { // The input passed validation. const submitState = { - ...omit(meterDetails, 'id'), + ...meterDetails, // GPS may have been updated so create updated state to submit. gps: gps, // Set default identifier as name if left blank diff --git a/src/client/app/components/unit/CreateUnitModalComponent.tsx b/src/client/app/components/unit/CreateUnitModalComponent.tsx index d2567f9d97..f70eb63ea1 100644 --- a/src/client/app/components/unit/CreateUnitModalComponent.tsx +++ b/src/client/app/components/unit/CreateUnitModalComponent.tsx @@ -19,7 +19,6 @@ import { MIN_VAL, MAX_VAL } from '../../utils/input'; import { LineGraphRates } from '../../types/redux/graph'; import { customRateValid, isCustomRate } from '../../utils/unitInput'; import { SimpleUnsavedWarningComponent } from '../SimpleUnsavedWarningComponent'; -import { omit } from 'lodash'; /** * Defines the create unit modal form @@ -57,10 +56,10 @@ export default function CreateUnitModalComponent() { secInRate: LineGraphRates.hour * 3600, suffix: '', note: '', - // These two values are necessary but are not used. - // The client code makes the id for the selected unit and default graphic unit be -99 + // The id property is necessary but not used. + // The client code makes the id for the selected unit // so it can tell it is not yet assigned and do the correct logic for that case. - // The units API expects these values to be undefined on call so that the database can assign their values. + // The units API expects this value to be undefined on call so that the database can assign their values. id: -99, minVal: MIN_VAL, maxVal: MAX_VAL, @@ -207,8 +206,7 @@ export default function CreateUnitModalComponent() { // Close modal first to avoid repeat clicks setShowModal(false); const submitState = { - // id is not part of create. - ...omit(state, 'id'), + ...state, // Set default identifier as name if left blank identifier: !state.identifier || state.identifier.length === 0 ? state.name : state.identifier, // set displayable to none if unit is meter diff --git a/src/client/app/redux/api/metersApi.ts b/src/client/app/redux/api/metersApi.ts index 5e1a555194..70e47aaca6 100644 --- a/src/client/app/redux/api/metersApi.ts +++ b/src/client/app/redux/api/metersApi.ts @@ -11,6 +11,7 @@ import { MeterData } from '../../types/redux/meters'; import { durationFormat } from '../../utils/durationFormat'; import { baseApi } from './baseApi'; import { conversionsApi } from './conversionsApi'; +import { omit } from 'lodash'; export const meterAdapter = createEntityAdapter({ sortComparer: (meterA, meterB) => meterA.identifier?.localeCompare(meterB.identifier, undefined, { sensitivity: 'accent' }) @@ -54,7 +55,7 @@ export const metersApi = baseApi.injectEndpoints({ query: meter => ({ url: 'api/meters/addMeter', method: 'POST', - body: { ...meter } + body: omit(meter, ['id']) }), transformResponse: (data: MeterData) => ({ ...data, readingFrequency: durationFormat(data.readingFrequency) }), onQueryStarted: (_arg, { dispatch, queryFulfilled }) => { diff --git a/src/client/app/redux/api/unitsApi.ts b/src/client/app/redux/api/unitsApi.ts index eae58d7a49..bb8982056e 100644 --- a/src/client/app/redux/api/unitsApi.ts +++ b/src/client/app/redux/api/unitsApi.ts @@ -7,12 +7,14 @@ import { RootState } from 'store'; import { UnitData, UnitDataById } from '../../types/redux/units'; import { baseApi } from './baseApi'; import { conversionsApi } from './conversionsApi'; +import { omit } from 'lodash'; export const unitsAdapter = createEntityAdapter({ sortComparer: (unitA, unitB) => unitA.identifier?.localeCompare(unitB.identifier, undefined, { sensitivity: 'accent' }) }); export const unitsInitialState = unitsAdapter.getInitialState(); export type UnitDataState = EntityState; + export const unitsApi = baseApi.injectEndpoints({ endpoints: builder => ({ getUnitsDetails: builder.query({ @@ -26,7 +28,7 @@ export const unitsApi = baseApi.injectEndpoints({ query: unitDataArgs => ({ url: 'api/units/addUnit', method: 'POST', - body: { ...unitDataArgs } + body: omit(unitDataArgs, ['id']) }), onQueryStarted: (_arg, api) => { api.queryFulfilled From dfa83208bb6ab48400cfee9ae23ec5ccdbd5bd70 Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Thu, 16 Jul 2026 13:56:50 -0700 Subject: [PATCH 25/31] updated comments + removed extra whitespace --- .../components/admin/users/CreateUserModalComponent.tsx | 9 +++++---- .../conversion/CreateConversionModalComponent.tsx | 7 ++++--- src/client/app/redux/api/unitsApi.ts | 1 - 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/client/app/components/admin/users/CreateUserModalComponent.tsx b/src/client/app/components/admin/users/CreateUserModalComponent.tsx index 1a905e86f4..c2b7b4a54e 100644 --- a/src/client/app/components/admin/users/CreateUserModalComponent.tsx +++ b/src/client/app/components/admin/users/CreateUserModalComponent.tsx @@ -109,10 +109,11 @@ export default function CreateUserModal() { // End Modal show/close const handleSubmit = async () => { - // id is not used when creating a newUser. omit() could be used to exclude id for userDetails to be consistent with the implementations - // on other client files, but it is not necessary at the moment. In addition, introducing omit() in the client files for User could cause - // unexpected issues. We are acknowledging that the codebase currently has a different implementation for handleSubmit() compared to other - // client files. + // id is not used when creating a newUser, and is one of several userDetails fields that are only used internally to this component + // (e.g. to track status like passwordMatch) rather than being sent to the createUser route. omit() could be used to exclude these unused + // fields from userDetails to be consistent with the implementations on other client files, but it is not necessary at the moment. In addition, + // introducing omit() in the client files for User could cause unexpected issues. We are acknowledging that the codebase currently has a different + // implementation for handleSubmit() compared to other client files. const newUser: User = { username: userDetails.username, role: userDetails.role, password: userDetails.password, note: userDetails.note }; createUser(newUser) .unwrap() diff --git a/src/client/app/components/conversion/CreateConversionModalComponent.tsx b/src/client/app/components/conversion/CreateConversionModalComponent.tsx index 0a627ba4aa..d2181733c3 100644 --- a/src/client/app/components/conversion/CreateConversionModalComponent.tsx +++ b/src/client/app/components/conversion/CreateConversionModalComponent.tsx @@ -161,9 +161,10 @@ export default function CreateConversionModalComponent() { }; /* End Warning Modal */ - // The helper function will handle the omit() which separates it from the addConversionMutation. - // The addConversionMutation will call this helper function, so that omit() is not directly called. - // This helper function will also computes bidirectional based on the current source/destination selections + // This helper function will fix up the argument that will be used in addConversionMutation(). + // The helper function will handle the omit() which separates it from the addConversionMutation(). + // This helper function will also computes bidirectional based on the current source/destination selections. + // This helper function is introduced to allow the CreateConversion to be similar to the Create requests on other client files. const buildConversionSubmitState = (state: typeof conversionState) => ({ ...omit(state, 'sourceOptions', 'destinationOptions'), bidirectional: (isMeterSource() || isSuffixUsed()) ? false : state.bidirectional diff --git a/src/client/app/redux/api/unitsApi.ts b/src/client/app/redux/api/unitsApi.ts index bb8982056e..0395eb7f97 100644 --- a/src/client/app/redux/api/unitsApi.ts +++ b/src/client/app/redux/api/unitsApi.ts @@ -14,7 +14,6 @@ export const unitsAdapter = createEntityAdapter({ export const unitsInitialState = unitsAdapter.getInitialState(); export type UnitDataState = EntityState; - export const unitsApi = baseApi.injectEndpoints({ endpoints: builder => ({ getUnitsDetails: builder.query({ From 4f8a49069e353a2107cb2f7b88d5904a3e3a0bf8 Mon Sep 17 00:00:00 2001 From: Steven Huss-Lederman Date: Thu, 16 Jul 2026 16:40:28 -0500 Subject: [PATCH 26/31] small comment wording update --- .../app/components/admin/users/CreateUserModalComponent.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/app/components/admin/users/CreateUserModalComponent.tsx b/src/client/app/components/admin/users/CreateUserModalComponent.tsx index c2b7b4a54e..263eead62e 100644 --- a/src/client/app/components/admin/users/CreateUserModalComponent.tsx +++ b/src/client/app/components/admin/users/CreateUserModalComponent.tsx @@ -109,8 +109,8 @@ export default function CreateUserModal() { // End Modal show/close const handleSubmit = async () => { - // id is not used when creating a newUser, and is one of several userDetails fields that are only used internally to this component - // (e.g. to track status like passwordMatch) rather than being sent to the createUser route. omit() could be used to exclude these unused + // id is not used when creating a newUser. Several userDetails fields that are only used internally to this component + // (e.g. to track status like passwordMatch) should not be sent to the createUser route. omit() could be used to exclude these unused // fields from userDetails to be consistent with the implementations on other client files, but it is not necessary at the moment. In addition, // introducing omit() in the client files for User could cause unexpected issues. We are acknowledging that the codebase currently has a different // implementation for handleSubmit() compared to other client files. From b6ddf632500f8be41c768326f37170ebac5e6b1f Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Fri, 17 Jul 2026 12:26:08 -0700 Subject: [PATCH 27/31] updated failed.to.submit.changes + updated error notifications + removed TODO DEBUG --- src/client/app/components/admin/PreferencesComponent.tsx | 6 ++---- src/client/app/translations/data.ts | 9 ++++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index eac0755a47..3718edf5aa 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -407,7 +407,7 @@ export default function PreferencesComponent() { const invalidFieldNames = getInvalidFieldNames(); if (invalidFieldNames) { showErrorNotification( - translate('failed.to.submit.changes') + '(' + invalidFieldNames + translate('failed.to.submit.changes.fields') + translate('failed.to.submit.changes.saved') + '(' + invalidFieldNames + translate('failed.to.submit.changes.fields') ); return; } @@ -418,11 +418,9 @@ export default function PreferencesComponent() { showSuccessNotification(translate('updated.preferences')); }) .catch(err => { - showErrorNotification(translate('failed.to.submit.changes') + err.data); + showErrorNotification(translate('failed.to.submit.changes.saved') + err.data); }); }} - /* TODO DEBUG: removed the invalidFuncs check so that we can test the new error messages that display specific reasons */ - /*disabled={!hasChanges}*/ disabled={!hasChanges || Object.values(invalidFuncs).some(check => check())} color='primary' > diff --git a/src/client/app/translations/data.ts b/src/client/app/translations/data.ts index 22b5849e31..de8f1debcb 100644 --- a/src/client/app/translations/data.ts +++ b/src/client/app/translations/data.ts @@ -215,7 +215,8 @@ const LocaleTranslationData = { "failed.to.delete.map": "Failed to delete map", "failed.to.edit.map": "Failed to edit map", "failed.to.link.graph": "Failed to link graph", - "failed.to.submit.changes": "Failed to submit changes: ", + "failed.to.submit.changes": "Failed to submit changes ", + "failed.to.submit.changes.saved": "Failed to submit changes: ", "failed.to.submit.changes.fields": " has invalid values)", "false": "False", "from.1.to.1000": "from 1 to 1000", @@ -825,7 +826,8 @@ const LocaleTranslationData = { "failed.to.delete.map": "Échec de la suppression d'une carte", "failed.to.edit.map": "Échec de la modification d'une carte", "failed.to.link.graph": "Échec de lier le graphique", - "failed.to.submit.changes": "Échec de l'envoi des modifications: ", + "failed.to.submit.changes": "Échec de l'envoi des modifications ", + "failed.to.submit.changes.saved": "Failed to submit changes: \u{26A1}", "failed.to.submit.changes.fields": " has invalid values)\u{26A1}", "false": "Faux", "from.1.to.1000": "from 1 to 1000\u{26A1}", @@ -1435,7 +1437,8 @@ const LocaleTranslationData = { "failed.to.delete.map": "No se pudo borrar el mapa", "failed.to.edit.map": "No se pudo editar el mapa", "failed.to.link.graph": "No se pudo vincular el gráfico", - "failed.to.submit.changes": "No se pudo entregar los cambios: ", + "failed.to.submit.changes": "No se pudo entregar los cambios ", + "failed.to.submit.changes.saved": "Failed to submit changes: \u{26A1}", "failed.to.submit.changes.fields": " has invalid values)\u{26A1}", "false": "Falso", "from.1.to.1000": "from 1 to 1000\u{26A1}", From 34d82d1cb0c0d1bcc288a9f1bb26c0a9cbb39009 Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Thu, 23 Jul 2026 13:19:43 -0700 Subject: [PATCH 28/31] reverted specific error messages / second recheck of invalidFuncs + reverted changes in data.ts --- .../components/admin/PreferencesComponent.tsx | 22 +------------------ src/client/app/translations/data.ts | 6 ----- 2 files changed, 1 insertion(+), 27 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index d5dca2f210..cbf778177d 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -83,18 +83,6 @@ export default function PreferencesComponent() { } }; - const getInvalidFieldNames = (): string => { - let invalidFieldNames = ''; - if (invalidFuncs.readingFreq()) invalidFieldNames += translate('default.meter.reading.frequency') + ', '; - if (invalidFuncs.minDate()) invalidFieldNames += translate('default.meter.minimum.date') + ', '; - if (invalidFuncs.maxDate()) invalidFieldNames += translate('default.meter.maximum.date') + ', '; - if (invalidFuncs.readingGap()) invalidFieldNames += translate('default.meter.reading.gap') + ', '; - if (invalidFuncs.meterErrors()) invalidFieldNames += translate('default.meter.maximum.errors') + ', '; - if (invalidFuncs.warningFileSize()) invalidFieldNames += translate('default.warning.file.size') + ', '; - if (invalidFuncs.fileSizeLimit()) invalidFieldNames += translate('default.file.size.limit') + ', '; - return invalidFieldNames.slice(0, -2); - }; - return (
{ - const invalidFieldNames = getInvalidFieldNames(); - if (invalidFieldNames) { - showErrorNotification( - translate('failed.to.submit.changes.saved') + '(' + invalidFieldNames + translate('failed.to.submit.changes.fields') - ); - return; - } - submitPreferences(localAdminPref) .unwrap() .then(() => { showSuccessNotification(translate('updated.preferences')); }) .catch(err => { - showErrorNotification(translate('failed.to.submit.changes.saved') + err.data); + showErrorNotification(translate('failed.to.submit.changes') + err.data); }); }} disabled={!hasChanges || Object.values(invalidFuncs).some(check => check())} diff --git a/src/client/app/translations/data.ts b/src/client/app/translations/data.ts index de8f1debcb..4a5f433b37 100644 --- a/src/client/app/translations/data.ts +++ b/src/client/app/translations/data.ts @@ -216,8 +216,6 @@ const LocaleTranslationData = { "failed.to.edit.map": "Failed to edit map", "failed.to.link.graph": "Failed to link graph", "failed.to.submit.changes": "Failed to submit changes ", - "failed.to.submit.changes.saved": "Failed to submit changes: ", - "failed.to.submit.changes.fields": " has invalid values)", "false": "False", "from.1.to.1000": "from 1 to 1000", "gps": "GPS: latitude, longitude", @@ -827,8 +825,6 @@ const LocaleTranslationData = { "failed.to.edit.map": "Échec de la modification d'une carte", "failed.to.link.graph": "Échec de lier le graphique", "failed.to.submit.changes": "Échec de l'envoi des modifications ", - "failed.to.submit.changes.saved": "Failed to submit changes: \u{26A1}", - "failed.to.submit.changes.fields": " has invalid values)\u{26A1}", "false": "Faux", "from.1.to.1000": "from 1 to 1000\u{26A1}", "gps": "GPS: latitude, longitude\u{26A1}", @@ -1438,8 +1434,6 @@ const LocaleTranslationData = { "failed.to.edit.map": "No se pudo editar el mapa", "failed.to.link.graph": "No se pudo vincular el gráfico", "failed.to.submit.changes": "No se pudo entregar los cambios ", - "failed.to.submit.changes.saved": "Failed to submit changes: \u{26A1}", - "failed.to.submit.changes.fields": " has invalid values)\u{26A1}", "false": "Falso", "from.1.to.1000": "from 1 to 1000\u{26A1}", "gps": "GPS: latitud, longitud", From 7f1f1e8e02a70656afb9059b416078f120a830ff Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Thu, 23 Jul 2026 13:23:33 -0700 Subject: [PATCH 29/31] removed TODO DEBUG code --- src/client/app/components/admin/PreferencesComponent.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index cbf778177d..caacd0c6ca 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -72,14 +72,12 @@ export default function PreferencesComponent() { warningFileSize: (): boolean => { return Number(localAdminPref.defaultWarningFileSize) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) - || Number(localAdminPref.defaultWarningFileSize) > Number.MAX_SAFE_INTEGER; + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); }, fileSizeLimit: (): boolean => { return Number(localAdminPref.defaultFileSizeLimit) < 0 - || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit) - || Number(localAdminPref.defaultFileSizeLimit) > Number.MAX_SAFE_INTEGER; + || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); } }; From be43d7eb167b27e2059bc6008a95f6bbbe3f6d20 Mon Sep 17 00:00:00 2001 From: GoodKimchi <224943594+GoodKimchi@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:21:00 -0700 Subject: [PATCH 30/31] fix: share preferences file size validation limit --- .../app/components/admin/PreferencesComponent.tsx | 11 +++++++++-- src/common/preferencesValidationConstants.d.ts | 7 +++++++ src/common/preferencesValidationConstants.js | 13 +++++++++++++ src/server/routes/preferences.js | 5 +++-- 4 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 src/common/preferencesValidationConstants.d.ts create mode 100644 src/common/preferencesValidationConstants.js diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index caacd0c6ca..408cc687cb 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -21,6 +21,7 @@ import { useTranslate } from '../../redux/componentHooks'; import TimeZoneSelect from '../TimeZoneSelect'; import { defaultAdminState } from '../../redux/slices/adminSlice'; import { checkboxStyle, labelStyle } from '../../styles/modalStyle'; +import { MAX_FILE_SIZE_LIMIT } from '../../../../common/preferencesValidationConstants'; /** * @returns Preferences Component for Administrative use @@ -72,11 +73,13 @@ export default function PreferencesComponent() { warningFileSize: (): boolean => { return Number(localAdminPref.defaultWarningFileSize) < 0 + || Number(localAdminPref.defaultWarningFileSize) > MAX_FILE_SIZE_LIMIT || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); }, fileSizeLimit: (): boolean => { return Number(localAdminPref.defaultFileSizeLimit) < 0 + || Number(localAdminPref.defaultFileSizeLimit) > MAX_FILE_SIZE_LIMIT || Number(localAdminPref.defaultWarningFileSize) > Number(localAdminPref.defaultFileSizeLimit); } }; @@ -324,7 +327,10 @@ export default function PreferencesComponent() { value={localAdminPref.defaultWarningFileSize} onChange={e => makeLocalChanges('defaultWarningFileSize', Number(e.target.value))} min='0' - max={Number(localAdminPref.defaultFileSizeLimit)} + max={Math.min( + Number(localAdminPref.defaultFileSizeLimit), + MAX_FILE_SIZE_LIMIT + )} maxLength={50} invalid={invalidFuncs.warningFileSize()} /> @@ -354,6 +360,7 @@ export default function PreferencesComponent() { value={localAdminPref.defaultFileSizeLimit} onChange={e => makeLocalChanges('defaultFileSizeLimit', Number(e.target.value))} min={Number(localAdminPref.defaultWarningFileSize)} + max={MAX_FILE_SIZE_LIMIT} maxLength={50} invalid={invalidFuncs.fileSizeLimit()} /> @@ -362,7 +369,7 @@ export default function PreferencesComponent() { id="error.bounds" values={{ min: Number(localAdminPref.defaultWarningFileSize), - max: Infinity + max: MAX_FILE_SIZE_LIMIT }} /> diff --git a/src/common/preferencesValidationConstants.d.ts b/src/common/preferencesValidationConstants.d.ts new file mode 100644 index 0000000000..2d50893487 --- /dev/null +++ b/src/common/preferencesValidationConstants.d.ts @@ -0,0 +1,7 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +export const MAX_FILE_SIZE_LIMIT: number; diff --git a/src/common/preferencesValidationConstants.js b/src/common/preferencesValidationConstants.js new file mode 100644 index 0000000000..01be19e597 --- /dev/null +++ b/src/common/preferencesValidationConstants.js @@ -0,0 +1,13 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** + * Shared validation limits for Admin Preferences. + * Used by both frontend and backend to keep validation consistent. + */ +const PREFERENCES_VALIDATION_CONSTANTS = { + MAX_FILE_SIZE_LIMIT: 1000000000 +}; + +module.exports = PREFERENCES_VALIDATION_CONSTANTS; diff --git a/src/server/routes/preferences.js b/src/server/routes/preferences.js index 3c6ae829a4..c77d8685c0 100644 --- a/src/server/routes/preferences.js +++ b/src/server/routes/preferences.js @@ -11,6 +11,7 @@ const { getConnection } = require('../db'); const { STRING_GENERAL_MAX_LENGTH, STRING_SHORT_MAX_LENGTH: SHORT_STRING_MAX_LENGTH } = require('../util/validationConstants'); const { HTTP_CODES } = require('../util/httpCodes'); const { isValidIsoDateTime } = require('../util/timeValidation'); +const { MAX_FILE_SIZE_LIMIT } = require('../../common/preferencesValidationConstants'); const router = express.Router(); @@ -66,12 +67,12 @@ router.post('/', adminAuthMiddleware('edit site preferences'), async (req, res) defaultWarningFileSize: { type: 'number', minimum: 0, - maximum: 1000000000 + maximum: MAX_FILE_SIZE_LIMIT }, defaultFileSizeLimit: { type: 'number', minimum: 0, - maximum: 1000000000 + maximum: MAX_FILE_SIZE_LIMIT }, defaultAreaNormalization: { type: 'boolean' From 61b53de6c43d098a483b8248897c96b2af08c6cd Mon Sep 17 00:00:00 2001 From: Andrei Solomon Duque Date: Mon, 3 Aug 2026 12:28:10 -0700 Subject: [PATCH 31/31] removed redundant min + conditional --- .../components/admin/PreferencesComponent.tsx | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/client/app/components/admin/PreferencesComponent.tsx b/src/client/app/components/admin/PreferencesComponent.tsx index 408cc687cb..767859556f 100644 --- a/src/client/app/components/admin/PreferencesComponent.tsx +++ b/src/client/app/components/admin/PreferencesComponent.tsx @@ -326,7 +326,6 @@ export default function PreferencesComponent() { type='number' value={localAdminPref.defaultWarningFileSize} onChange={e => makeLocalChanges('defaultWarningFileSize', Number(e.target.value))} - min='0' max={Math.min( Number(localAdminPref.defaultFileSizeLimit), MAX_FILE_SIZE_LIMIT @@ -335,20 +334,13 @@ export default function PreferencesComponent() { invalid={invalidFuncs.warningFileSize()} /> - {Number(localAdminPref.defaultWarningFileSize) < 0 ? ( - - ) : ( - - )} +