Skip to content

Commit e97e797

Browse files
patrickrbOptio Agentclaude
authored
fix(search): keep SQL placeholders aligned when QSL status + DXCC filters combine (#236)
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: Optio Agent <optio-agent@noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2175b96 commit e97e797

3 files changed

Lines changed: 226 additions & 76 deletions

File tree

‎src/app/api/contacts/search/route.ts‎

Lines changed: 6 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,7 @@ import { NextRequest, NextResponse } from 'next/server';
22
import { verifyToken } from '@/lib/auth';
33
import { query } from '@/lib/db';
44
import { generateAdif, type AdifExportContact } from '@/lib/adif';
5-
6-
interface SearchFilters {
7-
callsign?: string;
8-
name?: string;
9-
qth?: string;
10-
mode?: string;
11-
band?: string;
12-
gridLocator?: string;
13-
startDate?: string;
14-
endDate?: string;
15-
qslStatus?: string;
16-
dxcc?: string;
17-
}
5+
import { buildContactSearchQuery } from '@/lib/contact-search';
186

197
export async function GET(request: NextRequest) {
208
try {
@@ -31,8 +19,11 @@ export async function GET(request: NextRequest) {
3119

3220
const userId = typeof user.userId === 'string' ? parseInt(user.userId, 10) : user.userId;
3321

34-
// Extract search filters
35-
const filters: SearchFilters = {
22+
// Build the parameterized WHERE clause from the query-string filters. The
23+
// builder numbers placeholders off the params array so a predicate-only
24+
// filter (QSL status) can't shift a later value-bound filter (DXCC) onto a
25+
// nonexistent `$N` — the bug that used to 500 confirmed-status + DXCC searches.
26+
const { whereClause, params: queryParams } = buildContactSearchQuery(userId, {
3627
callsign: searchParams.get('callsign') || undefined,
3728
name: searchParams.get('name') || undefined,
3829
qth: searchParams.get('qth') || undefined,
@@ -43,69 +34,8 @@ export async function GET(request: NextRequest) {
4334
endDate: searchParams.get('endDate') || undefined,
4435
qslStatus: searchParams.get('qslStatus') || undefined,
4536
dxcc: searchParams.get('dxcc') || undefined,
46-
};
47-
48-
// Build the WHERE clause and parameters
49-
const whereConditions: string[] = ['user_id = $1'];
50-
const queryParams: (string | number)[] = [userId];
51-
let paramCount = 1;
52-
53-
// Add search conditions
54-
Object.entries(filters).forEach(([key, value]) => {
55-
if (value && value.trim() !== '' && value !== 'all') {
56-
paramCount++;
57-
switch (key) {
58-
case 'callsign':
59-
whereConditions.push(`UPPER(callsign) LIKE UPPER($${paramCount})`);
60-
queryParams.push(`%${value}%`);
61-
break;
62-
case 'name':
63-
whereConditions.push(`UPPER(name) LIKE UPPER($${paramCount})`);
64-
queryParams.push(`%${value}%`);
65-
break;
66-
case 'qth':
67-
whereConditions.push(`UPPER(qth) LIKE UPPER($${paramCount})`);
68-
queryParams.push(`%${value}%`);
69-
break;
70-
case 'mode':
71-
whereConditions.push(`UPPER(mode) = UPPER($${paramCount})`);
72-
queryParams.push(value);
73-
break;
74-
case 'band':
75-
whereConditions.push(`UPPER(band) = UPPER($${paramCount})`);
76-
queryParams.push(value);
77-
break;
78-
case 'gridLocator':
79-
whereConditions.push(`UPPER(grid_locator) LIKE UPPER($${paramCount})`);
80-
queryParams.push(`%${value}%`);
81-
break;
82-
case 'startDate':
83-
whereConditions.push(`DATE(datetime) >= $${paramCount}`);
84-
queryParams.push(value);
85-
break;
86-
case 'endDate':
87-
whereConditions.push(`DATE(datetime) <= $${paramCount}`);
88-
queryParams.push(value);
89-
break;
90-
case 'qslStatus':
91-
// For now, we'll implement basic QSL status filtering
92-
// This can be expanded when QSL fields are added to the schema
93-
if (value === 'confirmed') {
94-
whereConditions.push(`confirmed = true`);
95-
} else if (value === 'not_confirmed') {
96-
whereConditions.push(`(confirmed = false OR confirmed IS NULL)`);
97-
}
98-
break;
99-
case 'dxcc':
100-
whereConditions.push(`dxcc = $${paramCount}`);
101-
queryParams.push(parseInt(value));
102-
break;
103-
}
104-
}
10537
});
10638

107-
const whereClause = whereConditions.join(' AND ');
108-
10939
if (isExport) {
11040
// Export all matching contacts as ADIF
11141
const exportSql = `

‎src/lib/contact-search.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// WHERE-clause builder for the contact search (GET /api/contacts/search).
2+
//
3+
// Kept free of server-only imports (no `pg`, no db pool) so it can be unit
4+
// tested directly, like @/lib/grid and @/lib/bands. The route feeds the result
5+
// straight into `query(sql, params)`.
6+
//
7+
// Placeholders ($2, $3, …) are numbered off the length of the `params` array as
8+
// each value is pushed, so they can never drift out of step with the values.
9+
// The subtle bug this prevents: a predicate-only filter (`qslStatus` adds
10+
// `confirmed = true` with no bound value) must not advance the placeholder
11+
// counter — otherwise a later value-bound filter (`dxcc`) references a `$N`
12+
// that has no matching parameter, 500-ing the COUNT query and mis-binding the
13+
// paginated one.
14+
15+
export interface ContactSearchFilters {
16+
callsign?: string;
17+
name?: string;
18+
qth?: string;
19+
mode?: string;
20+
band?: string;
21+
gridLocator?: string;
22+
startDate?: string;
23+
endDate?: string;
24+
qslStatus?: string;
25+
dxcc?: string;
26+
}
27+
28+
export interface ContactSearchQuery {
29+
/** The full WHERE clause, always anchored on `user_id = $1`. */
30+
whereClause: string;
31+
/** Bound parameter values, index N-1 corresponding to placeholder `$N`. */
32+
params: (string | number)[];
33+
}
34+
35+
/**
36+
* Build the parameterized WHERE clause + bound values for a contact search.
37+
* Blank, whitespace-only, and the `all` sentinel are treated as "no filter".
38+
*/
39+
export function buildContactSearchQuery(
40+
userId: number,
41+
filters: ContactSearchFilters,
42+
): ContactSearchQuery {
43+
const conditions: string[] = ['user_id = $1'];
44+
const params: (string | number)[] = [userId];
45+
46+
// Push a value and return its 1-based placeholder index, keeping the two in
47+
// lockstep no matter how many predicate-only filters precede it.
48+
const bind = (value: string | number): number => params.push(value);
49+
50+
for (const [key, raw] of Object.entries(filters)) {
51+
const value = raw?.trim();
52+
if (!value || value === 'all') continue;
53+
54+
switch (key) {
55+
case 'callsign':
56+
conditions.push(`UPPER(callsign) LIKE UPPER($${bind(`%${value}%`)})`);
57+
break;
58+
case 'name':
59+
conditions.push(`UPPER(name) LIKE UPPER($${bind(`%${value}%`)})`);
60+
break;
61+
case 'qth':
62+
conditions.push(`UPPER(qth) LIKE UPPER($${bind(`%${value}%`)})`);
63+
break;
64+
case 'mode':
65+
conditions.push(`UPPER(mode) = UPPER($${bind(value)})`);
66+
break;
67+
case 'band':
68+
conditions.push(`UPPER(band) = UPPER($${bind(value)})`);
69+
break;
70+
case 'gridLocator':
71+
conditions.push(`UPPER(grid_locator) LIKE UPPER($${bind(`%${value}%`)})`);
72+
break;
73+
case 'startDate':
74+
conditions.push(`DATE(datetime) >= $${bind(value)}`);
75+
break;
76+
case 'endDate':
77+
conditions.push(`DATE(datetime) <= $${bind(value)}`);
78+
break;
79+
case 'qslStatus':
80+
// Predicate-only: adds SQL but binds no value, so it must NOT call bind().
81+
if (value === 'confirmed') {
82+
conditions.push('confirmed = true');
83+
} else if (value === 'not_confirmed') {
84+
conditions.push('(confirmed = false OR confirmed IS NULL)');
85+
}
86+
break;
87+
case 'dxcc': {
88+
const dxcc = parseInt(value, 10);
89+
if (!Number.isNaN(dxcc)) {
90+
conditions.push(`dxcc = $${bind(dxcc)}`);
91+
}
92+
break;
93+
}
94+
}
95+
}
96+
97+
return { whereClause: conditions.join(' AND '), params };
98+
}

‎tests/contact-search.spec.ts‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { test, expect } from '@playwright/test';
2+
import { buildContactSearchQuery } from '@/lib/contact-search';
3+
4+
// Pure-function tests for the contact-search WHERE-clause builder. This is the
5+
// shared query builder behind GET /api/contacts/search — it turns the filter
6+
// set into a parameterized WHERE clause plus its bound values. No DB needed.
7+
//
8+
// The regression these guard: placeholders ($2, $3, …) must stay in lockstep
9+
// with the params array. A filter like `qslStatus` adds a SQL predicate but no
10+
// bound value; the previous code advanced a shared counter for *every* filter,
11+
// so any predicate-only filter shifted the numbering of every later filter —
12+
// combining "confirmed" with a DXCC entity produced a `$N` with no matching
13+
// value (500 on the COUNT query) or a dxcc compared against the LIMIT value.
14+
15+
test.describe('buildContactSearchQuery', () => {
16+
test('always begins with the mandatory user_id predicate', () => {
17+
const { whereClause, params } = buildContactSearchQuery(7, {});
18+
expect(whereClause).toBe('user_id = $1');
19+
expect(params).toEqual([7]);
20+
});
21+
22+
test('LIKE filters use case-insensitive contains matching', () => {
23+
const { whereClause, params } = buildContactSearchQuery(1, { callsign: 'w1aw' });
24+
expect(whereClause).toBe('user_id = $1 AND UPPER(callsign) LIKE UPPER($2)');
25+
expect(params).toEqual([1, '%w1aw%']);
26+
});
27+
28+
test('mode and band match exactly (case-insensitive)', () => {
29+
const { whereClause, params } = buildContactSearchQuery(1, { mode: 'ft8', band: '20m' });
30+
expect(whereClause).toBe(
31+
'user_id = $1 AND UPPER(mode) = UPPER($2) AND UPPER(band) = UPPER($3)'
32+
);
33+
expect(params).toEqual([1, 'ft8', '20m']);
34+
});
35+
36+
test('date range filters compare on the calendar date', () => {
37+
const { whereClause, params } = buildContactSearchQuery(1, {
38+
startDate: '2024-01-01',
39+
endDate: '2024-12-31',
40+
});
41+
expect(whereClause).toBe(
42+
'user_id = $1 AND DATE(datetime) >= $2 AND DATE(datetime) <= $3'
43+
);
44+
expect(params).toEqual([1, '2024-01-01', '2024-12-31']);
45+
});
46+
47+
test('dxcc is bound as an integer', () => {
48+
const { whereClause, params } = buildContactSearchQuery(1, { dxcc: '291' });
49+
expect(whereClause).toBe('user_id = $1 AND dxcc = $2');
50+
expect(params).toEqual([1, 291]);
51+
});
52+
53+
test('qsl status adds a predicate but no bound parameter', () => {
54+
const confirmed = buildContactSearchQuery(1, { qslStatus: 'confirmed' });
55+
expect(confirmed.whereClause).toBe('user_id = $1 AND confirmed = true');
56+
expect(confirmed.params).toEqual([1]);
57+
58+
const notConfirmed = buildContactSearchQuery(1, { qslStatus: 'not_confirmed' });
59+
expect(notConfirmed.whereClause).toBe(
60+
'user_id = $1 AND (confirmed = false OR confirmed IS NULL)'
61+
);
62+
expect(notConfirmed.params).toEqual([1]);
63+
});
64+
65+
test('an unrecognized qsl status contributes nothing', () => {
66+
const { whereClause, params } = buildContactSearchQuery(1, { qslStatus: 'pending' });
67+
expect(whereClause).toBe('user_id = $1');
68+
expect(params).toEqual([1]);
69+
});
70+
71+
// The core regression: a predicate-only filter (qslStatus) sitting *before*
72+
// a value-bound filter (dxcc) must not shift dxcc's placeholder off its value.
73+
test('qsl status combined with dxcc keeps placeholders aligned with params', () => {
74+
const { whereClause, params } = buildContactSearchQuery(1, {
75+
qslStatus: 'confirmed',
76+
dxcc: '291',
77+
});
78+
expect(whereClause).toBe('user_id = $1 AND confirmed = true AND dxcc = $2');
79+
expect(params).toEqual([1, 291]);
80+
81+
// Every `$N` referenced in the clause must have a corresponding param.
82+
const referenced = [...whereClause.matchAll(/\$(\d+)/g)].map(m => Number(m[1]));
83+
expect(Math.max(...referenced)).toBe(params.length);
84+
});
85+
86+
test('blank, whitespace, and "all" sentinel values are ignored', () => {
87+
const { whereClause, params } = buildContactSearchQuery(1, {
88+
callsign: '',
89+
name: ' ',
90+
mode: 'all',
91+
band: '40m',
92+
});
93+
expect(whereClause).toBe('user_id = $1 AND UPPER(band) = UPPER($2)');
94+
expect(params).toEqual([1, '40m']);
95+
});
96+
97+
test('a full filter set numbers every bound placeholder sequentially', () => {
98+
const { whereClause, params } = buildContactSearchQuery(42, {
99+
callsign: 'dl',
100+
name: 'hans',
101+
qth: 'berlin',
102+
mode: 'cw',
103+
band: '15m',
104+
gridLocator: 'jo',
105+
startDate: '2024-06-01',
106+
endDate: '2024-06-30',
107+
qslStatus: 'confirmed',
108+
dxcc: '230',
109+
});
110+
111+
// 10 filters supplied, but qslStatus binds no value → 9 bound params + userId.
112+
expect(params).toEqual([
113+
42, '%dl%', '%hans%', '%berlin%', 'cw', '15m', '%jo%',
114+
'2024-06-01', '2024-06-30', 230,
115+
]);
116+
117+
const referenced = [...whereClause.matchAll(/\$(\d+)/g)].map(m => Number(m[1]));
118+
// Placeholders must be exactly $1..$params.length with no gaps or overshoot.
119+
expect(referenced).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
120+
expect(Math.max(...referenced)).toBe(params.length);
121+
});
122+
});

0 commit comments

Comments
 (0)