fix(events): NaN from slideshow seed breaks event creation on PostgreSQL
The create route seeds show_interval_ms/show_transition_ms from app_settings through an inline guard that pre-checked Number.isFinite(+v) but then used parseInt(v). The two disagree for null/''/true — +null is 0 (finite) while parseInt(null) is NaN — so when the slideshow settings rows are absent (getAppSetting returns its null default), NaN flowed through Math.min/Math.max into the INSERT. PostgreSQL rejects NaN for integer columns; SQLite silently stores NULL, which is why every SQLite-based test passed while POST /api/admin/events 500'd on the PG dev stack and broke the e2e smoke suite. Fix: parse first, then check — clampIntOrUndefined in utils/numericHelpers (unit-tested against every failure-mode input). Verified end-to-end: the previously-failing minimal create now succeeds against the PG dev stack.
This commit is contained in:
@@ -0,0 +1,48 @@
|
|||||||
|
/**
|
||||||
|
* Regression tests for clampIntOrUndefined — the slideshow-seed NaN bug.
|
||||||
|
*
|
||||||
|
* The event-create route seeds show_interval_ms/show_transition_ms from
|
||||||
|
* app_settings via an int-parse-and-clamp. The old inline guard
|
||||||
|
* (`Number.isFinite(+v) ? parseInt(v) : undefined`) disagreed with itself
|
||||||
|
* for null/''/true: `+null` is 0 (finite) but `parseInt(null)` is NaN, so
|
||||||
|
* NaN flowed through Math.min/Math.max into the INSERT. PostgreSQL
|
||||||
|
* rejects NaN for integer columns ("invalid input syntax for type
|
||||||
|
* integer: NaN") while SQLite silently stores NULL — so POST
|
||||||
|
* /api/admin/events 500'd on PG whenever the slideshow settings rows
|
||||||
|
* were absent (getAppSetting returns its null default).
|
||||||
|
*/
|
||||||
|
|
||||||
|
const { clampIntOrUndefined } = require('../../src/utils/numericHelpers');
|
||||||
|
|
||||||
|
describe('clampIntOrUndefined', () => {
|
||||||
|
it('returns undefined for null (the getAppSetting missing-row default)', () => {
|
||||||
|
expect(clampIntOrUndefined(null, 1000, 120000)).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns undefined for undefined, empty string, and booleans', () => {
|
||||||
|
expect(clampIntOrUndefined(undefined, 1000, 120000)).toBeUndefined();
|
||||||
|
expect(clampIntOrUndefined('', 1000, 120000)).toBeUndefined();
|
||||||
|
expect(clampIntOrUndefined(true, 1000, 120000)).toBeUndefined();
|
||||||
|
expect(clampIntOrUndefined(false, 1000, 120000)).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns undefined for non-numeric garbage', () => {
|
||||||
|
expect(clampIntOrUndefined('fast', 1000, 120000)).toBeUndefined();
|
||||||
|
expect(clampIntOrUndefined({}, 1000, 120000)).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never returns NaN for any of the failure-mode inputs', () => {
|
||||||
|
for (const v of [null, undefined, '', true, false, 'x', {}, []]) {
|
||||||
|
const out = clampIntOrUndefined(v, 100, 5000);
|
||||||
|
expect(Number.isNaN(out)).toBe(false);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parses and clamps valid values', () => {
|
||||||
|
expect(clampIntOrUndefined('2500', 1000, 120000)).toBe(2500);
|
||||||
|
expect(clampIntOrUndefined(2500, 1000, 120000)).toBe(2500);
|
||||||
|
expect(clampIntOrUndefined('500', 1000, 120000)).toBe(1000);
|
||||||
|
expect(clampIntOrUndefined(999999, 1000, 120000)).toBe(120000);
|
||||||
|
expect(clampIntOrUndefined('2500.9', 1000, 120000)).toBe(2500);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -27,6 +27,7 @@ const { validateFileType } = require('../utils/fileSecurityUtils');
|
|||||||
const { requireEventOwnership } = require('../middleware/ownership');
|
const { requireEventOwnership } = require('../middleware/ownership');
|
||||||
const { requireFeatureFlag } = require('../middleware/requireFeatureFlag');
|
const { requireFeatureFlag } = require('../middleware/requireFeatureFlag');
|
||||||
const { getAppSetting } = require('../utils/appSettings');
|
const { getAppSetting } = require('../utils/appSettings');
|
||||||
|
const { clampIntOrUndefined } = require('../utils/numericHelpers');
|
||||||
const { getFrontendBaseUrl } = require('../utils/frontendUrl');
|
const { getFrontendBaseUrl } = require('../utils/frontendUrl');
|
||||||
const downloadZipService = require('../services/downloadZipService');
|
const downloadZipService = require('../services/downloadZipService');
|
||||||
|
|
||||||
@@ -682,7 +683,13 @@ router.post('/', adminAuth, requirePermission('events.create'), [
|
|||||||
let slideshowSeed = {};
|
let slideshowSeed = {};
|
||||||
if (await hasColumnCached('events', 'show_interval_ms')) {
|
if (await hasColumnCached('events', 'show_interval_ms')) {
|
||||||
try {
|
try {
|
||||||
const intP = (v, min, max) => (Number.isFinite(+v) ? Math.min(max, Math.max(min, parseInt(v, 10))) : undefined);
|
// parseInt-first: the previous `Number.isFinite(+v)` pre-check let
|
||||||
|
// NaN through for null/''/true (+null is 0, parseInt(null) is NaN),
|
||||||
|
// producing show_interval_ms=NaN in the INSERT — PG rejects that
|
||||||
|
// with "invalid input syntax for type integer" while SQLite
|
||||||
|
// silently stores NULL, so event creation 500'd on PG whenever the
|
||||||
|
// slideshow app_settings rows were absent.
|
||||||
|
const intP = (v, min, max) => clampIntOrUndefined(v, min, max);
|
||||||
const oneOf = (v, allowed) => (allowed.includes(v) ? v : undefined);
|
const oneOf = (v, allowed) => (allowed.includes(v) ? v : undefined);
|
||||||
const i = intP(await getAppSetting('slideshow_interval_ms', undefined), 1000, 120000);
|
const i = intP(await getAppSetting('slideshow_interval_ms', undefined), 1000, 120000);
|
||||||
const tr = oneOf(await getAppSetting('slideshow_transition', undefined), SLIDESHOW_TRANSITIONS);
|
const tr = oneOf(await getAppSetting('slideshow_transition', undefined), SLIDESHOW_TRANSITIONS);
|
||||||
|
|||||||
@@ -31,4 +31,20 @@ function ensureNumber(value, fallback = 0) {
|
|||||||
return Number.isFinite(n) ? n : fallback;
|
return Number.isFinite(n) ? n : fallback;
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports = { ensureInt, ensureNumber };
|
/**
|
||||||
|
* Parse a value as an integer clamped to [min, max]; `undefined` on
|
||||||
|
* anything that doesn't parse (null, undefined, '', booleans, garbage).
|
||||||
|
*
|
||||||
|
* Exists because the inline guard `Number.isFinite(+v) ? parseInt(v)`
|
||||||
|
* disagrees with itself for null/''/true (`+null` is 0 but
|
||||||
|
* `parseInt(null)` is NaN), which let NaN through Math.min/Math.max
|
||||||
|
* and into an INSERT — PostgreSQL rejects NaN for integer columns
|
||||||
|
* while SQLite silently stores NULL, so it only failed on PG.
|
||||||
|
*/
|
||||||
|
function clampIntOrUndefined(value, min, max) {
|
||||||
|
const n = parseInt(value, 10);
|
||||||
|
if (!Number.isFinite(n)) return undefined;
|
||||||
|
return Math.min(max, Math.max(min, n));
|
||||||
|
}
|
||||||
|
|
||||||
|
module.exports = { ensureInt, ensureNumber, clampIntOrUndefined };
|
||||||
|
|||||||
Reference in New Issue
Block a user