diff --git a/src/models/feedback/Feedback.router.ts b/src/models/feedback/Feedback.router.ts index 217cca4..655f31b 100644 --- a/src/models/feedback/Feedback.router.ts +++ b/src/models/feedback/Feedback.router.ts @@ -2,10 +2,9 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../middleware/logger'; import {publicRouter} from './public/public.router'; import {adminRouter} from './admin/admin.router'; +import {sendServerError} from './feedback.errors'; /** * Router Definition @@ -52,12 +51,6 @@ feedbackRouter.get('/', async (req: Request, res: Response) => { try { res.status(200).send('Nachklang e.V. Feedback API Endpoint'); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }); diff --git a/src/models/feedback/admin/admin.router.ts b/src/models/feedback/admin/admin.router.ts index abf2a08..e84b410 100644 --- a/src/models/feedback/admin/admin.router.ts +++ b/src/models/feedback/admin/admin.router.ts @@ -2,9 +2,8 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../../middleware/logger'; import {requireAdminAuth} from '../feedback.auth'; +import {sendServerError} from '../feedback.errors'; import {eventsAdminRouter} from './events.admin.router'; import {songsAdminRouter} from './songs.admin.router'; import {questionsAdminRouter} from './questions.admin.router'; @@ -81,13 +80,7 @@ adminRouter.delete('/submissions/:submissionId', async (req: Request, res: Respo } res.status(204).send(); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - status: 'PROCESSING_ERROR', - message: 'Internal Server Error. Try again later.', - reference: errorGuid - }); + sendServerError(res, e); } }); diff --git a/src/models/feedback/admin/csv.service.ts b/src/models/feedback/admin/csv.service.ts index 0aa91f1..e7f899b 100644 --- a/src/models/feedback/admin/csv.service.ts +++ b/src/models/feedback/admin/csv.service.ts @@ -1,4 +1,5 @@ import {NachklangFeedbackDB} from '../Feedback.db'; +import {formatDatetime} from '../feedback.dates'; const CSV_SEPARATOR = ';'; const UTF8_BOM = ''; @@ -23,15 +24,6 @@ export const escapeCsvField = (value: string | number | null | undefined): strin return str; }; -/** mariadb returns DATETIME columns as JS Date objects - format explicitly, - * otherwise String(date) falls back to the verbose Date.toString() format. */ -export const formatDatetime = (value: Date | string | null): string => { - if (!value) return ''; - const d = value instanceof Date ? value : new Date(value); - const pad = (n: number) => String(n).padStart(2, '0'); - return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`; -}; - const buildCsv = (headers: string[], rows: (string | number | null | undefined)[][]): string => { const lines = [headers.map(escapeCsvField).join(CSV_SEPARATOR)]; for (const row of rows) { diff --git a/src/models/feedback/admin/events.admin.router.ts b/src/models/feedback/admin/events.admin.router.ts index e6c075e..8bcaced 100644 --- a/src/models/feedback/admin/events.admin.router.ts +++ b/src/models/feedback/admin/events.admin.router.ts @@ -2,26 +2,15 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../../middleware/logger'; import * as EventsAdminService from './events.admin.service'; import * as SongsAdminService from './songs.admin.service'; +import {sendServerError} from '../feedback.errors'; /** * Router Definition */ export const eventsAdminRouter = express.Router(); -const sendServerError = (res: Response, e: any) => { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); -}; - /** * @swagger * /feedback/admin/events: diff --git a/src/models/feedback/admin/events.admin.service.ts b/src/models/feedback/admin/events.admin.service.ts index 03a137d..70075b2 100644 --- a/src/models/feedback/admin/events.admin.service.ts +++ b/src/models/feedback/admin/events.admin.service.ts @@ -1,6 +1,7 @@ import {NachklangFeedbackDB} from '../Feedback.db'; import {Song} from '../feedback.interface'; import {CreateEventInput, EventAdminDetail, EventAdminQuestionAssignment, EventAdminSummary, UpdateEventInput} from './admin.interface'; +import {formatDatetime} from '../feedback.dates'; const UMLAUT_MAP: Record = { 'ä': 'ae', 'ö': 'oe', 'ü': 'ue', 'ß': 'ss', @@ -83,11 +84,6 @@ const generateUniqueSlug = async (conn: any, name: string, eventDate: string): P } }; -const toMysqlDatetime = (d: Date): string => { - const pad = (n: number) => String(n).padStart(2, '0'); - return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`; -}; - export const createEvent = async (input: CreateEventInput, createdByEmail: string): Promise => { let conn = await NachklangFeedbackDB.getConnection(); try { @@ -101,7 +97,7 @@ export const createEvent = async (input: CreateEventInput, createdByEmail: strin INSERT INTO events (slug, name, subtitle, event_date, feedback_deadline, intro_text, created_by_email) VALUES (?,?,?,?,?,?,?) RETURNING event_id`; const res = await conn.query(query, [ - slug, input.name, input.subtitle || null, input.eventDate, toMysqlDatetime(deadline), + slug, input.name, input.subtitle || null, input.eventDate, formatDatetime(deadline), input.introText || null, createdByEmail ]); await conn.commit(); diff --git a/src/models/feedback/admin/questions.admin.router.ts b/src/models/feedback/admin/questions.admin.router.ts index bea0db8..75fefb6 100644 --- a/src/models/feedback/admin/questions.admin.router.ts +++ b/src/models/feedback/admin/questions.admin.router.ts @@ -2,25 +2,14 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../../middleware/logger'; import * as QuestionsAdminService from './questions.admin.service'; +import {sendServerError} from '../feedback.errors'; /** * Router Definition */ export const questionsAdminRouter = express.Router(); -const sendServerError = (res: Response, e: any) => { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); -}; - /** * @swagger * /feedback/admin/questions: diff --git a/src/models/feedback/admin/reports.admin.interface.ts b/src/models/feedback/admin/reports.admin.interface.ts index 91abdfb..a94e6ad 100644 --- a/src/models/feedback/admin/reports.admin.interface.ts +++ b/src/models/feedback/admin/reports.admin.interface.ts @@ -58,6 +58,7 @@ export interface EventReport { sent: number; pending: number; failed: number; + skipped: number; }; } diff --git a/src/models/feedback/admin/reports.admin.router.ts b/src/models/feedback/admin/reports.admin.router.ts index eed0244..6e82ad7 100644 --- a/src/models/feedback/admin/reports.admin.router.ts +++ b/src/models/feedback/admin/reports.admin.router.ts @@ -2,27 +2,16 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../../middleware/logger'; import * as ReportsAdminService from './reports.admin.service'; import * as CsvService from './csv.service'; import * as EventsAdminService from './events.admin.service'; +import {sendServerError} from '../feedback.errors'; /** * Router Definition */ export const reportsAdminRouter = express.Router(); -const sendServerError = (res: Response, e: any) => { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); -}; - /** * @swagger * /feedback/admin/events/{eventId}/report: diff --git a/src/models/feedback/admin/reports.admin.service.ts b/src/models/feedback/admin/reports.admin.service.ts index 5210e21..876fc9f 100644 --- a/src/models/feedback/admin/reports.admin.service.ts +++ b/src/models/feedback/admin/reports.admin.service.ts @@ -16,7 +16,7 @@ export const aggregateReport = ( submissionStats: {totalSubmissions: number; firstSubmissionAt: string | null; lastSubmissionAt: string | null}, answerRows: AnswerRow[], guestBookCount: number, - newsletterCounts: {total: number; sent: number; pending: number; failed: number} + newsletterCounts: {total: number; sent: number; pending: number; failed: number; skipped: number} ): EventReport => { const groupKey = (row: AnswerRow) => `${row.questionId ?? 'null'}::${row.questionLabel}`; @@ -137,13 +137,14 @@ export const getReport = async (eventId: number): Promise => `SELECT sync_status, COUNT(*) as cnt FROM newsletter_signups WHERE event_id = ? GROUP BY sync_status`, [eventId] ); - const newsletterCounts = {total: 0, sent: 0, pending: 0, failed: 0}; + const newsletterCounts = {total: 0, sent: 0, pending: 0, failed: 0, skipped: 0}; for (const row of newsletterRows) { const cnt = Number(row.cnt); newsletterCounts.total += cnt; if (row.sync_status === 'SENT') newsletterCounts.sent = cnt; else if (row.sync_status === 'PENDING') newsletterCounts.pending = cnt; else if (row.sync_status === 'FAILED') newsletterCounts.failed = cnt; + else if (row.sync_status === 'SKIPPED') newsletterCounts.skipped = cnt; } return aggregateReport( diff --git a/src/models/feedback/admin/songs.admin.router.ts b/src/models/feedback/admin/songs.admin.router.ts index ad0faaf..52b9f71 100644 --- a/src/models/feedback/admin/songs.admin.router.ts +++ b/src/models/feedback/admin/songs.admin.router.ts @@ -2,9 +2,8 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../../middleware/logger'; import * as SongsAdminService from './songs.admin.service'; +import {sendServerError} from '../feedback.errors'; /** * Router Definition @@ -80,13 +79,7 @@ songsAdminRouter.put('/:songId', async (req: Request, res: Response) => { } res.status(200).send({status: 'OK'}); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }); @@ -99,12 +92,6 @@ songsAdminRouter.delete('/:songId', async (req: Request, res: Response) => { } res.status(204).send(); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }); diff --git a/src/models/feedback/feedback.auth.ts b/src/models/feedback/feedback.auth.ts index d569ef7..91cb292 100644 --- a/src/models/feedback/feedback.auth.ts +++ b/src/models/feedback/feedback.auth.ts @@ -1,7 +1,6 @@ import express from 'express'; -import {Guid} from 'guid-typescript'; -import logger from '../../middleware/logger'; import * as UserService from '../calendar/users/users.service'; +import {sendServerError} from './feedback.errors'; /** * This file is the ONLY place in the feedback module that knows how admin @@ -74,12 +73,6 @@ export const requireAdminAuth: express.RequestHandler = async (req, res, next) = res.locals.admin = identity; next(); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }; diff --git a/src/models/feedback/feedback.dates.ts b/src/models/feedback/feedback.dates.ts new file mode 100644 index 0000000..d39cb65 --- /dev/null +++ b/src/models/feedback/feedback.dates.ts @@ -0,0 +1,13 @@ +/** + * Formats a Date using its local getters (not toISOString/UTC), so the + * wall-clock time the server is running in is what gets stored/displayed - + * never silently shifted by a UTC conversion. Used both for MySQL DATETIME + * literals (events.admin.service.ts) and CSV export (csv.service.ts): same + * requirement, same format, in either context. + */ +export const formatDatetime = (value: Date | string | null): string => { + if (!value) return ''; + const d = value instanceof Date ? value : new Date(value); + const pad = (n: number) => String(n).padStart(2, '0'); + return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`; +}; diff --git a/src/models/feedback/feedback.errors.ts b/src/models/feedback/feedback.errors.ts new file mode 100644 index 0000000..58b003d --- /dev/null +++ b/src/models/feedback/feedback.errors.ts @@ -0,0 +1,18 @@ +import {Response} from 'express'; +import {Guid} from 'guid-typescript'; +import logger from '../../middleware/logger'; + +/** + * The feedback module's standard catch-block response: log with a + * reference guid, never leak the real error message to the client. Every + * router in this module follows this exact convention (see CLAUDE.md). + */ +export const sendServerError = (res: Response, e: any): void => { + const errorGuid = Guid.create().toString(); + logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); + res.status(500).send({ + status: 'PROCESSING_ERROR', + message: 'Internal Server Error. Try again later.', + reference: errorGuid + }); +}; diff --git a/src/models/feedback/public/public.router.ts b/src/models/feedback/public/public.router.ts index 2960392..e5df0b3 100644 --- a/src/models/feedback/public/public.router.ts +++ b/src/models/feedback/public/public.router.ts @@ -2,11 +2,11 @@ * Required External Modules and Interfaces */ import express, {Request, Response} from 'express'; -import {Guid} from 'guid-typescript'; import logger from '../../../middleware/logger'; import {getEligibleEvents, getEventConfigBySlug} from './events.public.service'; import {submitFeedback} from './submissions.service'; import {hashIp, isRateLimited, recordSubmission} from '../feedback.ratelimit'; +import {sendServerError} from '../feedback.errors'; /** * Router Definition @@ -51,13 +51,7 @@ publicRouter.get('/events', async (req: Request, res: Response) => { const events = await getEligibleEvents(); res.status(200).send(events); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }); @@ -106,13 +100,7 @@ publicRouter.get('/events/:slug', async (req: Request, res: Response) => { } res.status(200).send(result.event); } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }); @@ -166,7 +154,7 @@ publicRouter.post('/events/:slug/submissions', async (req: Request, res: Respons // nothing, stay silent about it having failed. if (isHoneypotTriggered(body)) { logger.info('Feedback honeypot triggered', {slug: req.params.slug}); - res.status(201).send({submissionId: -1}); + res.status(201).send({submissionId: -1, newsletterDropped: false}); return; } @@ -196,16 +184,10 @@ publicRouter.post('/events/:slug/submissions', async (req: Request, res: Respons res.status(400).send({status: 'EMPTY_SUBMISSION'}); return; case 'OK': - res.status(201).send({submissionId: result.submissionId}); + res.status(201).send({submissionId: result.submissionId, newsletterDropped: result.newsletterDropped}); return; } } catch (e: any) { - let errorGuid = Guid.create().toString(); - logger.error('Error handling a request: ' + e.message, {reference: errorGuid}); - res.status(500).send({ - 'status': 'PROCESSING_ERROR', - 'message': 'Internal Server Error. Try again later.', - 'reference': errorGuid - }); + sendServerError(res, e); } }); diff --git a/src/models/feedback/public/submission.interface.ts b/src/models/feedback/public/submission.interface.ts index 58b39a7..933d098 100644 --- a/src/models/feedback/public/submission.interface.ts +++ b/src/models/feedback/public/submission.interface.ts @@ -62,6 +62,9 @@ * submissionId: * type: integer * example: 91 + * newsletterDropped: + * type: boolean + * description: True if the newsletter opt-in was present but failed validation (e.g. a malformed email) - the rest of the submission still saved. */ export interface RatingInput { diff --git a/src/models/feedback/public/submissions.service.ts b/src/models/feedback/public/submissions.service.ts index 9d74c02..8f13e4e 100644 --- a/src/models/feedback/public/submissions.service.ts +++ b/src/models/feedback/public/submissions.service.ts @@ -137,7 +137,7 @@ export const validateNewsletter = (input?: NewsletterInput): ValidatedNewsletter }; export type SubmitResult = - | { status: 'OK'; submissionId: number } + | { status: 'OK'; submissionId: number; newsletterDropped: boolean } | { status: 'NOT_FOUND' } | { status: 'CLOSED' } | { status: 'EMPTY' }; @@ -160,6 +160,12 @@ export const submitFeedback = async (slug: string, body: SubmissionRequestBody, const answerRows = validateAnswers(body.answers || [], questionsById, songTitleById); const guestBook = validateGuestBook(body.guestBook); const newsletter = validateNewsletter(body.newsletter); + // body.newsletter is only sent at all when the visitor had the opt-in + // checkbox on (see FeedbackForm.tsx), so a present-but-invalid block + // (e.g. a mistyped email) is distinguishable from "didn't opt in" - the + // rest of the submission still saves, but the client can tell the + // visitor their newsletter signup specifically didn't go through. + const newsletterDropped = !!body.newsletter && !newsletter; if (answerRows.length === 0 && !guestBook && !newsletter) { return {status: 'EMPTY'}; @@ -213,7 +219,7 @@ export const submitFeedback = async (slug: string, body: SubmissionRequestBody, }); } - return {status: 'OK', submissionId}; + return {status: 'OK', submissionId, newsletterDropped}; } catch (err) { await conn.rollback(); throw err; diff --git a/test/feedback/csv.service.test.ts b/test/feedback/csv.service.test.ts index 950f9dc..df00170 100644 --- a/test/feedback/csv.service.test.ts +++ b/test/feedback/csv.service.test.ts @@ -1,4 +1,5 @@ -import {escapeCsvField, formatDatetime} from '../../src/models/feedback/admin/csv.service'; +import {escapeCsvField} from '../../src/models/feedback/admin/csv.service'; +import {formatDatetime} from '../../src/models/feedback/feedback.dates'; describe('escapeCsvField', () => { it('passes plain text through unchanged', () => { diff --git a/test/feedback/reports.admin.service.test.ts b/test/feedback/reports.admin.service.test.ts index 4af9d7c..630960f 100644 --- a/test/feedback/reports.admin.service.test.ts +++ b/test/feedback/reports.admin.service.test.ts @@ -2,7 +2,7 @@ import {aggregateReport} from '../../src/models/feedback/admin/reports.admin.ser import {AnswerRow} from '../../src/models/feedback/admin/reports.admin.interface'; const eventMeta = {eventId: 1, name: 'Sommerkonzert', eventDate: '2026-08-01', feedbackDeadline: '2026-08-15T23:59:59'}; -const emptyNewsletter = {total: 0, sent: 0, pending: 0, failed: 0}; +const emptyNewsletter = {total: 0, sent: 0, pending: 0, failed: 0, skipped: 0}; const row = (overrides: Partial): AnswerRow => ({ submissionId: 1, @@ -90,13 +90,13 @@ describe('aggregateReport - top-level fields', () => { {totalSubmissions: 42, firstSubmissionAt: '2026-08-02T10:00:00.000Z', lastSubmissionAt: '2026-08-10T18:00:00.000Z'}, [], 7, - {total: 10, sent: 6, pending: 2, failed: 2} + {total: 10, sent: 6, pending: 2, failed: 1, skipped: 1} ); expect(report.totalSubmissions).toBe(42); expect(report.firstSubmissionAt).toBe('2026-08-02T10:00:00.000Z'); expect(report.lastSubmissionAt).toBe('2026-08-10T18:00:00.000Z'); expect(report.guestBookCount).toBe(7); - expect(report.newsletter).toEqual({total: 10, sent: 6, pending: 2, failed: 2}); + expect(report.newsletter).toEqual({total: 10, sent: 6, pending: 2, failed: 1, skipped: 1}); expect(report.event).toEqual(eventMeta); });