diff --git a/backend/__tests__/routes/adminArchivesQuery.test.js b/backend/__tests__/routes/adminArchivesQuery.test.js new file mode 100644 index 00000000..1904f53f --- /dev/null +++ b/backend/__tests__/routes/adminArchivesQuery.test.js @@ -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); + }); +}); diff --git a/backend/src/routes/adminArchives.js b/backend/src/routes/adminArchives.js index 4a25213a..5a3ffdd1 100644 --- a/backend/src/routes/adminArchives.js +++ b/backend/src/routes/adminArchives.js @@ -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'); diff --git a/frontend/src/pages/admin/ArchivesPage.tsx b/frontend/src/pages/admin/ArchivesPage.tsx index 67ff5038..31352b97 100644 --- a/frontend/src/pages/admin/ArchivesPage.tsx +++ b/frontend/src/pages/admin/ArchivesPage.tsx @@ -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('all'); - const [sortBy, setSortBy] = useState<'date' | 'size' | 'name'>('date'); + const [sortBy, setSortBy] = useState('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 = () => { - {filteredArchives.length === 0 ? ( + {archives.length === 0 ? ( {t('archives.noArchivesFound')} ) : ( - filteredArchives.map((archive) => ( + archives.map((archive) => (
diff --git a/frontend/src/pages/admin/__tests__/archivesServerSideQuery.test.tsx b/frontend/src/pages/admin/__tests__/archivesServerSideQuery.test.tsx new file mode 100644 index 00000000..afd66dd5 --- /dev/null +++ b/frontend/src/pages/admin/__tests__/archivesServerSideQuery.test.tsx @@ -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('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( + + + + + + ); +} + +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')); + }); +}); diff --git a/frontend/src/services/archive.service.ts b/frontend/src/services/archive.service.ts index 3e6c97bd..6cd58bce 100644 --- a/frontend/src/services/archive.service.ts +++ b/frontend/src/services/archive.service.ts @@ -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 { + // 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 { const response = await api.get('/admin/archives', { - params: { page, limit } + params: { page, limit, search: search || undefined, type: type || undefined, sortBy } }); return response.data; },