Skip to content

fix(search): keep SQL placeholders aligned when QSL status + DXCC filters combine - #236

Merged
patrickrb merged 1 commit into
mainfrom
optio/task-3b3bfcd8-7792-4dde-9b08-32f5ba65151f
Jul 24, 2026
Merged

fix(search): keep SQL placeholders aligned when QSL status + DXCC filters combine#236
patrickrb merged 1 commit into
mainfrom
optio/task-3b3bfcd8-7792-4dde-9b08-32f5ba65151f

Conversation

@patrickrb

Copy link
Copy Markdown
Owner

Problem

The advanced contact search (GET /api/contacts/search, backing the /search page) breaks whenever an operator combines a QSL status filter with a DXCC entity filter.

The route built its parameterized WHERE clause with a single paramCount counter that was bumped once per active filter:

paramCount++;
switch (key) { ... }

But the qslStatus branch adds a SQL predicate (confirmed = true / (confirmed = false OR confirmed IS NULL)) without pushing a bound value. That desyncs paramCount from the queryParams array. Because dxcc is applied after qslStatus, selecting e.g. "Confirmed" and a DXCC entity produced a $N placeholder with no matching parameter:

  • Count query (query(countSql, queryParams)) referenced $3 while only 2 params were supplied → Postgres there is no parameter $3 → the whole request 500s with "Internal server error".
  • Paginated query collided the dxcc placeholder with the LIMIT placeholder, silently filtering dxcc = <limit value> (e.g. dxcc = 20) and returning wrong rows.

The search UI sends both fields together, so any DX-focused search scoped by confirmation status was affected.

Solution

Extracted the WHERE-clause construction into a pure, server-import-free module src/lib/contact-search.ts (same pattern as @/lib/grid / @/lib/bands, so it's unit-testable without a DB). Placeholders are now numbered off the length of the params array as each value is pushed — a predicate-only filter simply doesn't push, so it can never shift a later filter's placeholder.

The route now delegates to buildContactSearchQuery(userId, filters), dropping ~55 lines of inline query-building. Behavior is otherwise identical (also added a small guard so a non-numeric dxcc is ignored rather than bound as NaN).

Testing

  • New unit tests (tests/contact-search.spec.ts, 10 cases) covering each filter, the all/blank sentinels, the predicate-only QSL-status branch, and specifically the QSL-status + DXCC alignment regression (asserts every $N in the clause has a matching param).
  • npm run typecheck — clean
  • npm run lint — clean
  • npm run build — succeeds
  • Full pure-function suite (grid/bands/adif/frequency/contact-search) — 58 passed

Backwards compatibility

No API surface change — same query params, same response shape. Purely a correctness fix plus test coverage.

Future follow-up

  • Per CLAUDE.md, this route's query params (gridLocator, startDate, endDate, qslStatus) are still camelCase — a known drifting surface deferred to a later snake_case sweep. Left untouched here to keep the change focused.

🤖 Generated with Claude Code

…ters combine

The contact-search route incremented a shared placeholder counter for every
active filter, but the `qslStatus` branch adds a predicate (`confirmed = true`)
without binding a value. That desynced the counter from the params array, so
selecting a QSL status *and* a DXCC entity emitted a `$N` with no matching
parameter — the COUNT query 500'd and the paginated query compared `dxcc`
against the LIMIT value.

Extract the WHERE-clause construction into a pure, server-import-free
`@/lib/contact-search` builder that numbers placeholders off the params array
as values are pushed, so predicate-only filters can never shift later ones.
Adds unit tests covering the alignment regression and the full filter set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nodelog Ready Ready Preview, Comment Jul 24, 2026 2:10am

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant