fix(archives): run search, filter and sort server-side

ArchivesPage fetched one 20-row page and then filtered and sorted only that
array in memory, while "Showing X of 802" / "Page 1 of 41" kept reporting the
full unfiltered count. Searching for an archive that exists but is not on the
current page returned a false "0 results" with no hint the search was
page-scoped.

The backend did not support the params (it read only page/limit and hardcoded
orderBy archived_at desc), so all three are new. Follows adminEvents/crud.js
for the shape and customerAccountsService for the case-insensitive predicate:
whereRaw with a bound parameter, never interpolated, and sortBy whitelisted to
date/name/size before it reaches orderBy. The same applyFilters() closure runs
against both the count query and the row query, so the total cannot drift from
the rows again.

Frontend mirrors EventsListPage: 300ms debounce, reset to page 1 on any query
change, placeholderData so keystrokes don't flash the spinner.

Two interpretation calls:
- sortBy=size orders by summed photo bytes, not the zip's on-disk size. The
  Size column comes from a per-row fs.stat done after pagination and there is
  no archive_size column, so a global sort on the real zip size would stat all
  802 files per request. Ordering is near-identical except for rows whose zip
  is missing. Adding events.archive_size would be a migration, out of scope.
- No LIKE-metacharacter escaping. escapeLikePattern() does .replace(/'/g,"''"),
  which corrupts a bound value ("Sarah's Birthday"), and its backslash escaping
  is a no-op on SQLite without an ESCAPE clause. Matched customerAccountsService
  instead. A literal % typed by an admin acts as a wildcard in a read-only
  search; no injection risk.

Pre-existing and untouched: the four stat cards still aggregate the current
page only.

Refs testplan REPORT.md #9 (Part 3, I.01).
This commit is contained in:
Paul Nothaft
2026-09-01 16:31:09 +02:00
parent 3f6c81a846
commit fc7cb226f4
5 changed files with 424 additions and 50 deletions
@@ -0,0 +1,209 @@
/**
* The archives list must resolve search / type filter / sort in SQL.
*
* Before this, GET /admin/archives ignored every query param except page and
* limit: the UI fetched one 20-row page and filtered it in JavaScript while
* the pagination footer kept reporting the unfiltered server-side total. An
* archive that matched the search but lived on another page came back as a
* false "0 results". These tests pin the params the route now honours, and
* — the part that actually made the bug visible — that `pagination.total`
* describes the *filtered* set.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-archquery-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'archquery-test-secret';
const request = require('supertest');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal, buildRouteApp } = require('../integration/helpers/crmDb');
describe('GET /admin/archives query params (#I.01)', () => {
let db; let cleanup; let app; let token;
// name, type, archived_at, photo sizes
const fixtures = [
['Alpha Wedding', 'wedding', '2026-01-05T10:00:00.000Z', [300]],
['Bravo Birthday', 'birthday', '2026-02-05T10:00:00.000Z', [100]],
['Charlie Wedding', 'wedding', '2026-03-05T10:00:00.000Z', [500, 400]],
['Delta Corporate', 'corporate', '2026-04-05T10:00:00.000Z', [200]],
['Echo WEDDING Gala', 'wedding', '2026-05-05T10:00:00.000Z', [50]],
];
const list = async (query) => {
const res = await request(app)
.get('/admin/archives')
.query(query)
.set('Authorization', `Bearer ${token}`);
expect(res.status).toBe(200);
return res.body;
};
const names = (body) => body.archives.map((a) => a.eventName);
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const role = await db('roles').where({ name: 'super_admin' }).first();
const inserted = await db('admin_users').insert({
username: 'arch-admin',
email: 'arch-admin@example.com',
password_hash: await bcrypt.hash('Passw0rd!', 4),
role_id: role.id,
is_active: 1,
created_at: new Date().toISOString(),
updated_at: new Date().toISOString(),
}).returning('id');
const adminId = inserted[0]?.id ?? inserted[0];
token = jwt.sign(
{ id: adminId, username: 'arch-admin', type: 'admin', role: 'super_admin', loginTime: Date.now() },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' },
);
let i = 0;
for (const [eventName, eventType, archivedAt, sizes] of fixtures) {
const slug = `arch-${i++}`;
const ev = await db('events').insert({
slug,
event_type: eventType,
event_name: eventName,
event_date: '2026-08-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_token: `tok-${slug}`,
share_link: `/gallery/${slug}/tok-${slug}`,
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 0,
is_archived: 1,
is_draft: 0,
archived_at: archivedAt,
created_at: new Date().toISOString(),
}).returning('id');
const eventId = ev[0]?.id ?? ev[0];
let p = 0;
for (const size of sizes) {
await db('photos').insert({
event_id: eventId,
filename: `${slug}-${p++}.jpg`,
path: `events/archived/${slug}.jpg`,
type: 'individual',
size_bytes: size,
uploaded_at: new Date().toISOString(),
});
}
}
// A live event that must never surface in the archives list.
await db('events').insert({
slug: 'not-archived',
event_type: 'wedding',
event_name: 'Alpha Live Wedding',
event_date: '2026-08-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_token: 'tok-not-archived',
share_link: '/gallery/not-archived/tok-not-archived',
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
created_at: new Date().toISOString(),
});
app = buildRouteApp('/admin/archives', require('../../src/routes/adminArchives'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
test('no params: every archive, newest first, unfiltered total', async () => {
const body = await list({});
expect(names(body)).toEqual([
'Echo WEDDING Gala', 'Delta Corporate', 'Charlie Wedding', 'Bravo Birthday', 'Alpha Wedding',
]);
expect(body.pagination.total).toBe(5);
});
test('search filters in SQL and the total describes the filtered set', async () => {
const body = await list({ search: 'wedding' });
// Case-insensitive, matches "Echo WEDDING Gala" too, and never the
// non-archived "Alpha Live Wedding".
expect(names(body).sort()).toEqual(['Alpha Wedding', 'Charlie Wedding', 'Echo WEDDING Gala']);
expect(body.pagination.total).toBe(3);
expect(body.pagination.totalPages).toBe(1);
});
test('search reaches rows that are not on page 1 — the actual bug', async () => {
// limit=2 puts "Alpha Wedding" (oldest) on page 3 of the unfiltered list.
// Client-side filtering of page 1 returned nothing for this query.
const body = await list({ search: 'alpha', limit: 2, page: 1 });
expect(names(body)).toEqual(['Alpha Wedding']);
expect(body.pagination.total).toBe(1);
});
test('search with no match returns an empty page and a zero total', async () => {
const body = await list({ search: 'zzz-nothing' });
expect(body.archives).toEqual([]);
expect(body.pagination.total).toBe(0);
expect(body.pagination.totalPages).toBe(0);
});
test('type filter narrows the rows and the total; "all" is a no-op', async () => {
const filtered = await list({ type: 'wedding' });
expect(names(filtered).sort()).toEqual(['Alpha Wedding', 'Charlie Wedding', 'Echo WEDDING Gala']);
expect(filtered.pagination.total).toBe(3);
const all = await list({ type: 'all' });
expect(all.pagination.total).toBe(5);
});
test('search and type filter combine', async () => {
const body = await list({ search: 'wedding', type: 'birthday' });
expect(body.archives).toEqual([]);
expect(body.pagination.total).toBe(0);
});
test('sortBy=name orders across the whole set, not just the page', async () => {
const page1 = await list({ sortBy: 'name', limit: 2, page: 1 });
expect(names(page1)).toEqual(['Alpha Wedding', 'Bravo Birthday']);
const page3 = await list({ sortBy: 'name', limit: 2, page: 3 });
expect(names(page3)).toEqual(['Echo WEDDING Gala']);
});
test('sortBy=size orders by archived content size, largest first', async () => {
const body = await list({ sortBy: 'size' });
expect(names(body)).toEqual([
'Charlie Wedding', // 900
'Alpha Wedding', // 300
'Delta Corporate', // 200
'Bravo Birthday', // 100
'Echo WEDDING Gala' // 50
]);
});
test('an unknown sortBy falls back to the date ordering', async () => {
const body = await list({ sortBy: 'events.id; drop table events' });
expect(names(body)[0]).toBe('Echo WEDDING Gala');
expect(body.pagination.total).toBe(5);
});
test('quotes in the search are bound as a value, not injected as SQL', async () => {
const body = await list({ search: '\'; DROP TABLE events; --' });
expect(body.archives).toEqual([]);
// The table is still there.
expect((await list({})).pagination.total).toBe(5);
});
});
+43 -16
View File
@@ -19,26 +19,53 @@ const router = express.Router();
router.get('/', adminAuth, requirePermission('archives.view'), async (req, res) => {
try {
const { page, limit, offset } = getPagination(req);
const search = typeof req.query.search === 'string' ? req.query.search.trim() : '';
const type = typeof req.query.type === 'string' ? req.query.type.trim() : '';
const sortBy = ['date', 'name', 'size'].includes(req.query.sortBy) ? req.query.sortBy : 'date';
// Get total count
const totalCount = await db('events')
.where('is_archived', formatBoolean(true))
.count('id as count')
// Search and type filtering run in SQL so both the returned rows and
// the total count cover the whole archive table, not just the page the
// client happens to be on. Values are bound, never interpolated.
const applyFilters = (query) => {
if (search) {
query.whereRaw('LOWER(events.event_name) LIKE ?', [`%${search.toLowerCase()}%`]);
}
if (type && type !== 'all') {
query.where('events.event_type', type);
}
return query;
};
// Get total count (of the filtered set, so pagination stays truthful)
const totalCount = await applyFilters(
db('events').where('events.is_archived', formatBoolean(true))
)
.count('events.id as count')
.first();
// Get archived events
const archives = await db('events')
.select(
'events.*',
db.raw('COUNT(DISTINCT photos.id) as photo_count'),
db.raw('SUM(photos.size_bytes) as total_size')
)
.leftJoin('photos', 'events.id', 'photos.event_id')
.where('events.is_archived', formatBoolean(true))
.groupBy('events.id')
.orderBy('events.archived_at', 'desc')
.limit(limit)
.offset(offset);
const archivesQuery = applyFilters(
db('events')
.select(
'events.*',
db.raw('COUNT(DISTINCT photos.id) as photo_count'),
db.raw('SUM(photos.size_bytes) as total_size')
)
.leftJoin('photos', 'events.id', 'photos.event_id')
.where('events.is_archived', formatBoolean(true))
).groupBy('events.id');
if (sortBy === 'name') {
archivesQuery.orderBy('events.event_name', 'asc');
} else if (sortBy === 'size') {
// The zip's on-disk size is only known after the per-row fs.stat below,
// so a global size sort has to use the archived content size instead.
archivesQuery.orderByRaw('COALESCE(SUM(photos.size_bytes), 0) desc');
} else {
archivesQuery.orderBy('events.archived_at', 'desc');
}
const archives = await archivesQuery.limit(limit).offset(offset);
// Check if archive files exist and get their sizes
const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');
+24 -31
View File
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import {
Archive,
Download,
@@ -18,7 +18,7 @@ import { toast } from 'react-toastify';
import { Button, Input, Card, Loading } from '../../components/common';
import { PermissionGate } from '../../components/admin/PermissionGate';
import { useQuery } from '@tanstack/react-query';
import { archiveService } from '../../services/archive.service';
import { archiveService, type ArchiveSortBy } from '../../services/archive.service';
import { useTranslation } from 'react-i18next';
import { useLocalizedDate } from '../../hooks/useLocalizedDate';
import { useMutationWithToast } from '../../hooks';
@@ -28,11 +28,23 @@ export const ArchivesPage: React.FC = () => {
const { t } = useTranslation();
const { formatTime: fmtTime } = useLocalizedDate();
const [searchTerm, setSearchTerm] = useState('');
const [debouncedSearchTerm, setDebouncedSearchTerm] = useState('');
const [filterType, setFilterType] = useState<string>('all');
const [sortBy, setSortBy] = useState<'date' | 'size' | 'name'>('date');
const [sortBy, setSortBy] = useState<ArchiveSortBy>('date');
const [currentPage, setCurrentPage] = useState(1);
// const navigate = useNavigate();
useEffect(() => {
const t = setTimeout(() => setDebouncedSearchTerm(searchTerm.trim()), 300);
return () => clearTimeout(t);
}, [searchTerm]);
// Reset to page 1 whenever the query changes so users don't get stuck on a
// page index that no longer exists in the new result set.
useEffect(() => {
setCurrentPage(1);
}, [debouncedSearchTerm, filterType, sortBy]);
// Helper function to safely format dates
const formatDate = (dateString: string | null | undefined, formatStr: string): string => {
if (!dateString) return '';
@@ -44,37 +56,18 @@ export const ArchivesPage: React.FC = () => {
}
};
// Fetch archives from API
// Fetch archives from API. Search, type filter and sort are all applied
// server-side against the whole archive table — doing them in the client
// silently scoped them to the 20 rows of the current page while the
// pagination footer kept reporting the unfiltered total.
const { data: archivesData, isLoading } = useQuery({
queryKey: ['admin-archives', currentPage],
queryFn: () => archiveService.getArchives(currentPage, 20),
queryKey: ['admin-archives', currentPage, debouncedSearchTerm, filterType, sortBy],
queryFn: () => archiveService.getArchives(currentPage, 20, debouncedSearchTerm || undefined, filterType, sortBy),
placeholderData: (prev) => prev,
});
const archives = archivesData?.archives || [];
const filteredArchives = archives.filter(archive => {
if (filterType !== 'all' && archive.eventType !== filterType) {
return false;
}
if (searchTerm) {
const term = searchTerm.toLowerCase();
return archive.eventName.toLowerCase().includes(term);
}
return true;
}).sort((a, b) => {
switch (sortBy) {
case 'name':
return a.eventName.localeCompare(b.eventName);
case 'size':
return b.archiveSize - a.archiveSize;
case 'date':
default:
const dateA = a.archivedAt ? new Date(a.archivedAt).getTime() : 0;
const dateB = b.archivedAt ? new Date(b.archivedAt).getTime() : 0;
return dateB - dateA;
}
});
const getTotalSize = () => {
return archives.reduce((sum, archive) => sum + archive.archiveSize, 0);
};
@@ -257,14 +250,14 @@ export const ArchivesPage: React.FC = () => {
</tr>
</thead>
<tbody className="bg-white dark:bg-neutral-800 divide-y divide-neutral-200 dark:divide-neutral-700">
{filteredArchives.length === 0 ? (
{archives.length === 0 ? (
<tr>
<td colSpan={6} className="px-6 py-12 text-center text-neutral-500 dark:text-neutral-400">
{t('archives.noArchivesFound')}
</td>
</tr>
) : (
filteredArchives.map((archive) => (
archives.map((archive) => (
<tr key={archive.id} className="hover:bg-neutral-50 dark:hover:bg-neutral-700/50">
<td className="px-6 py-4">
<div>
@@ -0,0 +1,136 @@
/**
* /admin/archives search, type filter and sort were applied client-side to
* whatever 20-row page happened to be loaded (QA I.01), while the
* pagination footer kept reporting the full server-side total. An archive on
* page 7 was invisible to a search, with no hint the search was page-scoped.
*
* These pin the contract that fixes it: every control is a server query param,
* and changing any of them goes back to page 1.
*/
import React from 'react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { MemoryRouter } from 'react-router-dom';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
vi.mock('react-i18next', async () => {
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
return {
...actual,
useTranslation: () => ({
t: (k: string, fb?: unknown) => (typeof fb === 'string' ? fb : k),
i18n: { language: 'en' },
}),
};
});
vi.mock('react-toastify', () => ({ toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() } }));
vi.mock('../../../hooks/usePublicSettings', () => ({
PUBLIC_SETTINGS_QUERY_KEY: ['public-settings'],
usePublicSettings: () => ({ data: {} }),
}));
vi.mock('../../../contexts/PermissionsContext', () => ({
usePermissions: () => ({
hasPermission: () => true,
hasAnyPermission: () => true,
hasAllPermissions: () => true,
isSuperAdmin: true,
isLoading: false,
}),
}));
const getArchives = vi.fn();
vi.mock('../../../services/archive.service', () => ({
archiveService: {
getArchives: (...args: unknown[]) => getArchives(...args),
restoreArchive: vi.fn(),
deleteArchive: vi.fn(),
downloadArchive: vi.fn(),
formatBytes: (b: number) => `${b} B`,
},
}));
import { ArchivesPage } from '../ArchivesPage';
const page = (archives: unknown[], total: number) => ({
archives,
pagination: { page: 1, limit: 20, total, totalPages: Math.ceil(total / 20) },
});
const archive = (id: number, eventName: string, eventType = 'wedding') => ({
id,
slug: `slug-${id}`,
eventName,
eventDate: '2026-08-01',
eventType,
hostEmail: 'h@example.com',
archivedAt: '2026-08-02T10:00:00.000Z',
expiresAt: '2026-09-01T10:00:00.000Z',
photoCount: 3,
originalSize: 100,
archiveSize: 100,
archivePath: 'events/archived/x.zip',
});
function renderPage() {
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
return render(
<QueryClientProvider client={qc}>
<MemoryRouter>
<ArchivesPage />
</MemoryRouter>
</QueryClientProvider>
);
}
describe('ArchivesPage server-side query (QA I.01)', () => {
beforeEach(() => {
getArchives.mockReset();
getArchives.mockResolvedValue(page([archive(1, 'Alpha Wedding')], 802));
});
it('sends the search term to the server instead of filtering the loaded page', async () => {
renderPage();
const input = await screen.findByPlaceholderText('archives.searchPlaceholder');
expect(getArchives).toHaveBeenLastCalledWith(1, 20, undefined, 'all', 'date');
await userEvent.type(input, 'bravo');
// Debounced — one request for the settled term, not one per keystroke.
await waitFor(
() => expect(getArchives).toHaveBeenLastCalledWith(1, 20, 'bravo', 'all', 'date'),
{ timeout: 2000 }
);
});
it('sends the type filter and the sort key to the server', async () => {
renderPage();
await screen.findByDisplayValue('archives.allTypes');
await userEvent.selectOptions(screen.getByDisplayValue('archives.allTypes'), 'birthday');
await waitFor(() => expect(getArchives).toHaveBeenLastCalledWith(1, 20, undefined, 'birthday', 'date'));
await userEvent.selectOptions(screen.getByDisplayValue('archives.sortByDate'), 'size');
await waitFor(() => expect(getArchives).toHaveBeenLastCalledWith(1, 20, undefined, 'birthday', 'size'));
});
it('renders exactly the rows the server returned, unfiltered by the client', async () => {
// A row the old client-side filter would have dropped: the server decided
// it matches, so the page must show it.
getArchives.mockResolvedValue(page([archive(2, 'Bravo Birthday', 'birthday')], 1));
renderPage();
expect(await screen.findByText('Bravo Birthday')).toBeInTheDocument();
});
it('goes back to page 1 when the query changes', async () => {
renderPage();
await userEvent.click(await screen.findByText('common.next'));
await waitFor(() => expect(getArchives).toHaveBeenLastCalledWith(2, 20, undefined, 'all', 'date'));
await userEvent.selectOptions(screen.getByDisplayValue('archives.allTypes'), 'corporate');
await waitFor(() => expect(getArchives).toHaveBeenLastCalledWith(1, 20, undefined, 'corporate', 'date'));
});
});
+12 -3
View File
@@ -33,6 +33,8 @@ export interface ArchiveDetails extends Archive {
};
}
export type ArchiveSortBy = 'date' | 'name' | 'size';
export interface ArchivesResponse {
archives: Archive[];
pagination: {
@@ -44,10 +46,17 @@ export interface ArchivesResponse {
}
export const archiveService = {
// Get all archives with pagination
async getArchives(page: number = 1, limit: number = 20): Promise<ArchivesResponse> {
// Get all archives pagination, search, type filter and sort are all
// resolved server-side so they apply to the whole archive table.
async getArchives(
page: number = 1,
limit: number = 20,
search?: string,
type?: string,
sortBy?: ArchiveSortBy
): Promise<ArchivesResponse> {
const response = await api.get<ArchivesResponse>('/admin/archives', {
params: { page, limit }
params: { page, limit, search: search || undefined, type: type || undefined, sortBy }
});
return response.data;
},