diff --git a/scripts/apply-showcase-review.ts b/scripts/apply-showcase-review.ts index cc24965a2..3c40caec4 100644 --- a/scripts/apply-showcase-review.ts +++ b/scripts/apply-showcase-review.ts @@ -1,5 +1,5 @@ import { readFile } from 'node:fs/promises' -import { parseArgs } from 'node:util' +import { isDeepStrictEqual, parseArgs } from 'node:util' import postgres from 'postgres' import * as v from 'valibot' @@ -47,9 +47,9 @@ try { const [rawActor] = await tx.unsafe( `SELECT EXISTS ( SELECT 1 FROM users WHERE id = $1 AND ( - capabilities && ARRAY['admin', 'moderate-showcases']::capability[] OR + capabilities::text::capability[] && ARRAY['admin', 'moderate-showcases']::capability[] OR EXISTS (SELECT 1 FROM role_assignments a JOIN roles r ON r.id = a.role_id - WHERE a.user_id = users.id AND r.capabilities && ARRAY['admin', 'moderate-showcases']::capability[]) + WHERE a.user_id = users.id AND r.capabilities::text::capability[] && ARRAY['admin', 'moderate-showcases']::capability[]) )) AS allowed`, [actorId], ) @@ -95,12 +95,28 @@ try { : 'denied' : 'approved' if ( + row.name === item.name && + new URL(row.url).href === new URL(item.url).href && row.placement === item.placement && row.status === status && row.review_reason === item.reason ) { - unchanged++ - continue + // A prior apply updates updated_at. Only treat that newer row as an + // idempotent replay when it still matches the persisted audit snapshot. + const [audit] = await tx.unsafe( + `SELECT details->'after' AS after FROM audit_logs + WHERE target_type = 'showcase' AND target_id = $1 + AND details->>'source' = 'showcase-review-migration' + ORDER BY created_at DESC LIMIT 1`, + [item.id], + ) + if ( + row.updated_at <= new Date(item.reviewedAt) || + isDeepStrictEqual(audit?.after, JSON.parse(JSON.stringify(raw))) + ) { + unchanged++ + continue + } } if ( row.name !== item.name || @@ -136,7 +152,7 @@ try { ) await tx.unsafe( `INSERT INTO audit_logs (actor_id, action, target_type, target_id, details) - VALUES ($1, 'showcase.moderate', 'showcase', $2, $3::jsonb)`, + VALUES ($1, 'showcase.moderate', 'showcase', $2, $3::text::jsonb)`, [ actorId, item.id, diff --git a/src/routes/community-projects.tsx b/src/routes/community-projects.tsx index 068237df1..9df8deb83 100644 --- a/src/routes/community-projects.tsx +++ b/src/routes/community-projects.tsx @@ -3,15 +3,20 @@ import * as v from 'valibot' import { seo } from '~/utils/seo' import { ShowcaseGallery } from '~/components/ShowcaseGallery' import { getApprovedShowcasesQueryOptions } from '~/queries/showcases' -import { libraryIdSchema, showcaseUseCaseSchema } from '~/utils/schemas' +import { + libraryIdSchema, + showcaseUseCaseSchema, + pageNumberSchema, + pageSizeSchema, +} from '~/utils/schemas' const searchSchema = v.object({ - page: v.optional(v.number(), 1), - pageSize: v.optional(v.number(), 24), - libraryIds: v.optional(v.array(libraryIdSchema)), - useCases: v.optional(v.array(showcaseUseCaseSchema)), + page: v.optional(pageNumberSchema, 1), + pageSize: v.optional(pageSizeSchema, 24), + libraryIds: v.optional(v.pipe(v.array(libraryIdSchema), v.maxLength(16))), + useCases: v.optional(v.pipe(v.array(showcaseUseCaseSchema), v.maxLength(12))), hasSourceCode: v.optional(v.boolean()), - q: v.optional(v.string()), + q: v.optional(v.pipe(v.string(), v.maxLength(120))), }) export const PAGE_SIZE_OPTIONS = [24, 48, 96, 192] as const diff --git a/src/routes/showcase/index.tsx b/src/routes/showcase/index.tsx index c805deaaa..1bf7c1b61 100644 --- a/src/routes/showcase/index.tsx +++ b/src/routes/showcase/index.tsx @@ -3,15 +3,20 @@ import * as v from 'valibot' import { seo } from '~/utils/seo' import { ShowcaseGallery } from '~/components/ShowcaseGallery' import { getApprovedShowcasesQueryOptions } from '~/queries/showcases' -import { libraryIdSchema, showcaseUseCaseSchema } from '~/utils/schemas' +import { + libraryIdSchema, + showcaseUseCaseSchema, + pageNumberSchema, + pageSizeSchema, +} from '~/utils/schemas' const searchSchema = v.object({ - page: v.optional(v.number(), 1), - pageSize: v.optional(v.number(), 24), - libraryIds: v.optional(v.array(libraryIdSchema)), - useCases: v.optional(v.array(showcaseUseCaseSchema)), + page: v.optional(pageNumberSchema, 1), + pageSize: v.optional(pageSizeSchema, 24), + libraryIds: v.optional(v.pipe(v.array(libraryIdSchema), v.maxLength(16))), + useCases: v.optional(v.pipe(v.array(showcaseUseCaseSchema), v.maxLength(12))), hasSourceCode: v.optional(v.boolean()), - q: v.optional(v.string()), + q: v.optional(v.pipe(v.string(), v.maxLength(120))), }) export const PAGE_SIZE_OPTIONS = [24, 48, 96, 192] as const @@ -45,6 +50,7 @@ export const Route = createFileRoute('/showcase/')({ pageSize: deps.pageSize, }, filters: { + placement: 'showcase', libraryIds: deps.libraryIds, useCases: deps.useCases, hasSourceCode: deps.hasSourceCode, diff --git a/src/utils/showcase.functions.ts b/src/utils/showcase.functions.ts index 5932e6313..1e5cc711e 100644 --- a/src/utils/showcase.functions.ts +++ b/src/utils/showcase.functions.ts @@ -395,21 +395,22 @@ export const moderateShowcase = createServerFn({ method: 'POST' }) .handler(async ({ data }) => { const moderator = await requireModerateShowcases() - // Get existing for audit log - const existing = await db - .select() - .from(showcases) - .where(eq(showcases.id, data.showcaseId)) - .limit(1) + await db.transaction(async (tx) => { + // Lock the current state before updating it or recording the audit. + const existing = await tx + .select() + .from(showcases) + .where(eq(showcases.id, data.showcaseId)) + .limit(1) + .for('update') - if (!existing[0]) { - throw new Error('Showcase not found') - } + if (!existing[0]) { + throw new Error('Showcase not found') + } - const status: ShowcaseStatus = - data.action === 'approve' ? 'approved' : 'denied' + const status: ShowcaseStatus = + data.action === 'approve' ? 'approved' : 'denied' - await db.transaction(async (tx) => { await tx .update(showcases) .set({ @@ -463,26 +464,27 @@ export const setShowcaseFeatured = createServerFn({ method: 'POST' }) .handler(async ({ data }) => { const moderator = await requireModerateShowcases() - // Get existing for audit log - const existing = await db - .select() - .from(showcases) - .where(eq(showcases.id, data.showcaseId)) - .limit(1) + await db.transaction(async (tx) => { + // Lock the current state before updating it or recording the audit. + const existing = await tx + .select() + .from(showcases) + .where(eq(showcases.id, data.showcaseId)) + .limit(1) + .for('update') - if (!existing[0]) { - throw new Error('Showcase not found') - } + if (!existing[0]) { + throw new Error('Showcase not found') + } - if ( - data.isFeatured && - (existing[0].status !== 'approved' || - existing[0].placement !== 'showcase') - ) { - throw new Error('Only approved Showcase projects can be featured') - } + if ( + data.isFeatured && + (existing[0].status !== 'approved' || + existing[0].placement !== 'showcase') + ) { + throw new Error('Only approved Showcase projects can be featured') + } - await db.transaction(async (tx) => { await tx .update(showcases) .set({ @@ -641,6 +643,11 @@ export const adminUpdateShowcase = createServerFn({ method: 'POST' }) ) } + const isFeatured = + data.status === 'approved' && + data.placement === 'showcase' && + data.isFeatured + // Update showcase await db.transaction(async (tx) => { const updated = await tx @@ -658,10 +665,7 @@ export const adminUpdateShowcase = createServerFn({ method: 'POST' }) status: data.status, placement: data.placement, reviewReason: data.reviewReason, - isFeatured: - data.status === 'approved' && - data.placement === 'showcase' && - data.isFeatured, + isFeatured, moderationNote: data.moderationNote ?? null, moderatedBy: moderator.userId, moderatedAt: new Date(), @@ -705,7 +709,7 @@ export const adminUpdateShowcase = createServerFn({ method: 'POST' }) status: data.status, placement: data.placement, reviewReason: data.reviewReason, - isFeatured: data.isFeatured, + isFeatured, voteScore: data.voteScore, }, }, diff --git a/tests/showcase-publication.integration.test.ts b/tests/showcase-publication.integration.test.ts index 107b511f0..9e4065d46 100644 --- a/tests/showcase-publication.integration.test.ts +++ b/tests/showcase-publication.integration.test.ts @@ -1,22 +1,25 @@ import assert from 'node:assert/strict' import { createHmac, randomUUID } from 'node:crypto' import { test } from 'node:test' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import postgres from 'postgres' import { chromium } from 'playwright-core' // Opt in with an isolated local database and preview server. Never use production. const databaseUrl = process.env.SHOWCASE_TEST_DATABASE_URL -const previewUrl = process.env.SHOWCASE_TEST_PREVIEW_URL +const previewUrl = process.env.SHOWCASE_TEST_PREVIEW_URL?.replace(/\/+$/, '') +const browserPath = process.env.SHOWCASE_TEST_BROWSER_PATH const secret = process.env.SHOWCASE_TEST_SESSION_SECRET test( 'showcase placement, privacy, review controls, and resubmission', { - skip: !databaseUrl || !previewUrl || !secret, + skip: !databaseUrl || !previewUrl || !secret || !browserPath, timeout: 180_000, }, async () => { - assert.ok(databaseUrl && previewUrl && secret) + assert.ok(databaseUrl && previewUrl && secret && browserPath) assert.equal(new URL(databaseUrl).hostname, '127.0.0.1') assert.equal(new URL(previewUrl).hostname, '127.0.0.1') const sql = postgres(databaseUrl, { max: 1 }) @@ -27,13 +30,12 @@ test( const hiddenId = randomUUID() const pendingId = randomUUID() const browser = await chromium.launch({ - executablePath: - '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome', + executablePath: browserPath, headless: true, }) const context = await browser.newContext() await context.route('**/*', (route) => - new URL(route.request().url()).origin === previewUrl + new URL(route.request().url()).origin === new URL(previewUrl).origin ? route.continue() : route.abort(), ) @@ -85,7 +87,7 @@ test( ) assert.ok(!(await page.content()).includes('INTERNAL-NOTE-MUST-NOT-LEAK')) await page.screenshot({ - path: '/private/tmp/showcase-curated-preview.png', + path: join(tmpdir(), 'showcase-curated-preview.png'), fullPage: true, }) @@ -108,12 +110,12 @@ test( assert.match((await outbound.getAttribute('rel')) || '', /ugc/) assert.match((await outbound.getAttribute('rel')) || '', /nofollow/) await page.screenshot({ - path: '/private/tmp/showcase-community-preview.png', + path: join(tmpdir(), 'showcase-community-preview.png'), fullPage: true, }) await page.setViewportSize({ width: 390, height: 844 }) await page.screenshot({ - path: '/private/tmp/showcase-community-mobile.png', + path: join(tmpdir(), 'showcase-community-mobile.png'), fullPage: true, }) await page.setViewportSize({ width: 1280, height: 900 }) @@ -166,7 +168,7 @@ test( .getByLabel('Reason for this decision') .fill('The submitted destination needs correction.') await page.screenshot({ - path: '/private/tmp/showcase-review-editor.png', + path: join(tmpdir(), 'showcase-review-editor.png'), fullPage: true, }) await page.getByRole('button', { name: 'Save Changes' }).click() @@ -223,13 +225,25 @@ test( assert.equal(edited.placement, 'private') assert.equal(edited.moderation_note, 'INTERNAL-NOTE-MUST-NOT-LEAK') } catch (error) { - console.error((await page.locator('body').innerText()).slice(-6000)) + console.error( + await page + .locator('body') + .innerText({ timeout: 2000 }) + .then((text) => text.slice(-6000)) + .catch(() => 'Page diagnostics unavailable'), + ) throw error } finally { - await browser.close() - await sql`DELETE FROM audit_logs WHERE actor_id IN (${ownerId}, ${moderatorId})` - await sql`DELETE FROM users WHERE id IN (${ownerId}, ${moderatorId})` - await sql.end() + try { + await browser.close() + } finally { + try { + await sql`DELETE FROM audit_logs WHERE actor_id IN (${ownerId}, ${moderatorId})` + await sql`DELETE FROM users WHERE id IN (${ownerId}, ${moderatorId})` + } finally { + await sql.end() + } + } } }, )