From 766351b588bb9d3b270acb1b8c64bd6e242366c2 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Fri, 3 Jul 2026 09:01:01 +0200 Subject: [PATCH] fix: mirror #734 onto decomposed files (PG NaN slideshow seed, SQLite bool renders) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same two pre-existing-on-main bugs, at their post-decomposition locations: clampIntOrUndefined in adminEvents/crud.js slideshow seed; !! coercion in EventDetailsHeader, EventInformationCard, ClientAccessCard. Keeps this branch correct in either merge order with #734 — when merging main afterwards, resolve the adminEvents.js modify/delete conflict by keeping the deletion. --- .../utils/numericHelpers.clampInt.test.js | 48 +++++++++++++++++++ backend/src/routes/adminEvents/crud.js | 9 +++- backend/src/utils/numericHelpers.js | 18 ++++++- .../admin/event-details/ClientAccessCard.tsx | 3 +- .../event-details/EventDetailsHeader.tsx | 3 +- .../event-details/EventInformationCard.tsx | 7 +-- 6 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 backend/__tests__/utils/numericHelpers.clampInt.test.js diff --git a/backend/__tests__/utils/numericHelpers.clampInt.test.js b/backend/__tests__/utils/numericHelpers.clampInt.test.js new file mode 100644 index 00000000..905c598c --- /dev/null +++ b/backend/__tests__/utils/numericHelpers.clampInt.test.js @@ -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); + }); +}); diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index c7f3264b..67e985a0 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -24,6 +24,7 @@ const { normaliseEventTimeTriple } = require('../../services/eventService'); const { hasColumnCached } = require('../../utils/schemaCache'); const { requireEventOwnership } = require('../../middleware/ownership'); const { getAppSetting } = require('../../utils/appSettings'); +const { clampIntOrUndefined } = require('../../utils/numericHelpers'); const { getFrontendBaseUrl } = require('../../utils/frontendUrl'); const downloadZipService = require('../../services/downloadZipService'); const { validateHeroImageAnchor, getEventFieldRequirements, readBooleanSetting, getDownloadProtectionDefaults, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers'); @@ -369,7 +370,13 @@ module.exports = (router) => { let slideshowSeed = {}; if (await hasColumnCached('events', 'show_interval_ms')) { 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 i = intP(await getAppSetting('slideshow_interval_ms', undefined), 1000, 120000); const tr = oneOf(await getAppSetting('slideshow_transition', undefined), SLIDESHOW_TRANSITIONS); diff --git a/backend/src/utils/numericHelpers.js b/backend/src/utils/numericHelpers.js index b0ce39ff..6099d73a 100644 --- a/backend/src/utils/numericHelpers.js +++ b/backend/src/utils/numericHelpers.js @@ -31,4 +31,20 @@ function ensureNumber(value, fallback = 0) { 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 }; diff --git a/frontend/src/pages/admin/event-details/ClientAccessCard.tsx b/frontend/src/pages/admin/event-details/ClientAccessCard.tsx index 069c9805..2496b42c 100644 --- a/frontend/src/pages/admin/event-details/ClientAccessCard.tsx +++ b/frontend/src/pages/admin/event-details/ClientAccessCard.tsx @@ -49,7 +49,8 @@ export const ClientAccessCard: React.FC = ({ event, refet - {event?.client_access_enabled && ( + {/* !! — SQLite integer boolean; bare 0 renders as literal "0" */} + {!!event?.client_access_enabled && ( <> {/* Set/Change PIN */}
diff --git a/frontend/src/pages/admin/event-details/EventDetailsHeader.tsx b/frontend/src/pages/admin/event-details/EventDetailsHeader.tsx index af886b57..02c33725 100644 --- a/frontend/src/pages/admin/event-details/EventDetailsHeader.tsx +++ b/frontend/src/pages/admin/event-details/EventDetailsHeader.tsx @@ -207,7 +207,8 @@ export const EventDetailsHeader: React.FC = ({
{/* Draft Banner */} - {event.is_draft && !event.is_archived && ( + {/* !! — SQLite returns integer booleans; a bare 0 would render as literal "0" */} + {!!event.is_draft && !event.is_archived && (
diff --git a/frontend/src/pages/admin/event-details/EventInformationCard.tsx b/frontend/src/pages/admin/event-details/EventInformationCard.tsx index a24b4650..51b25c00 100644 --- a/frontend/src/pages/admin/event-details/EventInformationCard.tsx +++ b/frontend/src/pages/admin/event-details/EventInformationCard.tsx @@ -835,13 +835,14 @@ export const EventInformationCard: React.FC = ({ }`}> {event.protection_level || 'standard'} - {event.disable_right_click && ( + {/* !! on the next three — SQLite integer booleans render literal "0" when falsy */} + {!!event.disable_right_click && ( {t('events.rightClickBlocked', 'Right-click blocked')} )} - {event.enable_devtools_protection && ( + {!!event.enable_devtools_protection && ( {t('events.devtoolsDetection', 'DevTools detection')} @@ -853,7 +854,7 @@ export const EventInformationCard: React.FC = ({ {t('events.downloadsDisabled', 'Downloads disabled')} )} - {event.watermark_downloads && ( + {!!event.watermark_downloads && ( {t('events.watermarked', 'Watermarked')}