Add Feedback domain module: public submission flow, admin CRUD, reporting #7

2026-08-06T21:01:53Z

Paddy commented

2026-08-06 21:01:53 +00:00

No description provided.

Paddy added 3 commits 2026-08-06 21:01:54 +00:00

Add Feedback domain module: public submission flow, admin CRUD, reporting 17ca6399e0

New /feedback API domain backed by its own FEEDBACK_DB, mirroring the
Calendar domain's router -> service -> DB pool layering:

- Public endpoints (no auth): eligible-events listing, event config,
  submission with honeypot + rate limiting (in-memory + DB backstop).
- Admin endpoints (session-header auth, reusing Calendar's users/sessions
  via a swappable feedback.auth.ts boundary): events/songs/questions CRUD,
  bulk reorder/assignment, aggregated reporting, CSV export.
- Schema in sql/feedback/001_init.sql (8 tables), applied and verified
  against the real FEEDBACK_DB.
- 64 Jest tests covering validation, auth, rate limiting, CSV escaping,
  and report aggregation (pure functions, no DB needed).

Includes fixes from a security review: path traversal defense doesn't
apply here (that's the frontend proxy, separate repo), but the
rate-limiter cluster does - recordSubmission now counts every processed
request (not just successful ones), the in-memory Map evicts empty
entries instead of growing unbounded, FEEDBACK_IP_SALT is required at
boot instead of silently degrading to unsalted hashing, and submission
answer/rating arrays are capped and de-duplicated to bound insert
amplification.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Add example .env 7488ac673f

Allow dev CORS from LAN IPs; add submission deletion 56074d4441

The dev-only CORS bypass in app.ts only ever matched
http://localhost:<port>, never the LAN IP a phone actually connects
through over WiFi - so testing the feedback form from a real device
against a local dev API had its submissions silently rejected by CORS.
Extended the bypass to also allow private LAN ranges (192.168.x.x,
10.x.x.x, 172.16-31.x.x), dev-only as before.

Also adds DELETE /feedback/admin/submissions/:submissionId (cascades
to the submission's answers, guest book entry, and newsletter signup
in explicit dependency order, single-path by submission_id) so an
admin can remove an individual abusive/inappropriate entry - decided
in IMPLEMENTATION_PLAN.md §7 item 8. getGuestBookEntries now also
returns submissionId so the admin UI can target the delete call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Paddy added 3 commits 2026-08-14 17:04:42 +00:00

Remove hardcoded USE statement from feedback schema migration 07d757e9af

001_init.sql opened with USE `nachklang-feedback`, contradicting its own
documented apply instructions (mysql ... <FEEDBACK_DB> < 001_init.sql,
which already selects the database via the command line). Following the
file's own usage note literally would fail unless a database happened to
be named exactly nachklang-feedback rather than whatever FEEDBACK_DB is
configured to in .env.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Add search to the admin guest book endpoint 2988a70d8f

At scale (many submissions after a concert), paging through the guest book 20 entries at a time with no way to find a specific person is impractical. Add an optional ?search= query param that filters entries whose name or message contains the term (case-insensitive), with LIKE wildcards escaped so a literal % or _ in a search term can't be misinterpreted as a pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Paginate and add search to the newsletter signups endpoint 01a914f31e

Unlike guest book and free-text, newsletter signups had no LIMIT at all and the frontend rendered every row in one plain table. Newsletter opt-in is a single checkbox rather than typed text, so it's plausibly the largest per-event list - fix it the same way as guest book: paginated (page/pageSize, capped at 200/page) plus an optional ?search= over first name, last name, and email.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Paddy added 1 commit 2026-08-14 17:56:22 +00:00

Wire up the Salesforce newsletter sync integration 1a51b37097

Implements integrations/salesforce.service.ts per the plan's §5.6 seam
(syncNewsletterSignup(signupId)), against the real contract now that the
Salesforce side exists (see the nk-salesforce repo's
feature/newsletter-signup-integration branch): OAuth2 client-credentials
auth, POST to /services/apexrest/newsletter/signup with
{firstName, lastName, email, eventName}, response gives back which object
(Lead or Person Account) and its id. Token is cached in memory with a
conservative TTL and refreshed on a 401 rather than trusting expires_in,
which Salesforce's client-credentials token response doesn't reliably
return.

submissions.service.ts now captures the newsletter_signups insert's id
and fires syncNewsletterSignup after commit, fire-and-forget - the one
piece that was previously entirely missing, so flipping
SALESFORCE_ENABLED=true would have left every signup stuck at PENDING
forever with nothing to process it (found during an earlier review pass).

Replaced the placeholder SALESFORCE_API_TOKEN env var with
SALESFORCE_CLIENT_ID/SALESFORCE_CLIENT_SECRET in .env.example and
CLAUDE.md, matching the real auth mechanism instead of the static-token
guess from before the contract was known. Also fixed CLAUDE.md's stale
"still scaffolding-only" note about the Feedback domain.

Not yet covered by tests - the Salesforce-side contract was validated
end-to-end against a real sandbox, but this file has no unit tests yet.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Paddy added 2 commits 2026-08-15 16:01:46 +00:00

Add unit tests for the Salesforce integration 080b987914

Covers integrations/salesforce.service.ts: disabled-mode logging-only
path, signup-not-found, success (token fetch + POST + mark SENT), token
reuse across calls, retry-once-on-401, failure marks FAILED with the
error message, and a missing-client-credentials configuration error.
100% statement coverage on the file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Fix silent newsletter validation drops, surface skipped-sync visibility, consolidate duplicated helpers c2ddb11c4c

Three fixes from the earlier review, plus cleanup:

- submissions.service.ts: a newsletter opt-in present but failing
  validation (e.g. malformed email) was silently dropped with no signal
  to the client - the rest of the submission saved, but the visitor had
  no way to know their newsletter signup didn't go through. Added
  newsletterDropped to the submit response so the frontend can tell them.

- reports.admin.service.ts: the newsletter summary tracked
  total/sent/pending/failed but silently omitted SKIPPED (stub-mode)
  signups from any bucket - every current signup showed total>0 with
  every bucket reading 0, indistinguishable from "we don't know what
  happened". Added a skipped count.

- Consolidated two things duplicated across the module: sendServerError
  (reimplemented ~11 times, three of those as identical local copies of
  the same function) into feedback.errors.ts, and formatDatetime/
  toMysqlDatetime (the same local-time formatting logic under two names,
  in csv.service.ts and events.admin.service.ts respectively) into
  feedback.dates.ts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feature/feedback-module:feature/feedback-module

git checkout feature/feedback-module

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Nachklang/API#7