diff --git a/src/app/api/campaigns/[id]/send/route.ts b/src/app/api/campaigns/[id]/send/route.ts index 5aac37f..d4a9c81 100644 --- a/src/app/api/campaigns/[id]/send/route.ts +++ b/src/app/api/campaigns/[id]/send/route.ts @@ -1,7 +1,6 @@ import { NextRequest, NextResponse } from 'next/server' -import { getCampaign, updateCampaignStatus } from '../../../../../server/db/campaigns' +import { getCampaign, updateCampaignStatus, getCampaignRecipients } from '../../../../../server/db/campaigns' import { emailSendQueue } from '../../../../../queues/email-send.queue' -import { withTenant } from '../../../../../server/db/tenant' import { hashEmail } from '../../../../../lib/crypto' import { checkSuppression } from '../../../../../server/suppression/check' import { getTenantId } from '../../../../../lib/tenant-header' @@ -25,22 +24,13 @@ export async function POST(req: NextRequest, { params }: { params: { id: string return NextResponse.json({ error: statusResult.error.message }, { status: 500 }) } - let recipients: { email: string }[] - try { - recipients = await withTenant(tenantId, (client) => - client.query<{ email: string }>( - `SELECT s.email FROM subscribers s - JOIN campaign_recipients cr ON cr.campaign_id = $1 - WHERE (cr.list_id IS NULL OR s.list_id = cr.list_id) - AND s.status = 'active'`, - [params.id] - ) - ) - } catch (e) { + const recipientsResult = await getCampaignRecipients(tenantId, params.id) + if (!recipientsResult.ok) { // Rollback auf draft — verhindert verwaisten 'sending'-Status await updateCampaignStatus(tenantId, params.id, 'draft') return NextResponse.json({ error: 'Empfänger konnten nicht geladen werden' }, { status: 500 }) } + const recipients = recipientsResult.data // Suppression-Check PFLICHT — kein Opt-out-Empfänger darf in die Queue const unsuppressedRecipients = await Promise.all( diff --git a/src/app/api/campaigns/campaigns-api.test.ts b/src/app/api/campaigns/campaigns-api.test.ts index b6c18ee..cb951e8 100644 --- a/src/app/api/campaigns/campaigns-api.test.ts +++ b/src/app/api/campaigns/campaigns-api.test.ts @@ -1,9 +1,26 @@ -import { describe, it, expect } from 'vitest' +import { describe, it, expect, vi } from 'vitest' -// Dieser Test-Block ist übersprungen, weil next/server in Vitest nicht -// vollständig verfügbar ist. Die Haupttests sind die bestehenden Unit-Tests -// in src/queues/, src/server/ und src/lib/. -describe.skip('API Route Exports', () => { +vi.mock('../../../server/db/campaigns', () => ({ + createCampaign: vi.fn(), + listCampaigns: vi.fn(), + getCampaign: vi.fn(), + updateCampaign: vi.fn(), + updateCampaignStatus: vi.fn(), + getCampaignRecipients: vi.fn(), +})) +vi.mock('../../../queues/email-send.queue', () => ({ + emailSendQueue: { addBulk: vi.fn() }, +})) +vi.mock('../../../server/suppression/check', () => ({ + checkSuppression: vi.fn(), +})) +vi.mock('../../../lib/validation', () => ({ + CreateCampaignSchema: { safeParse: vi.fn().mockReturnValue({ success: false, error: { issues: [] } }) }, + UpdateCampaignSchema: { safeParse: vi.fn().mockReturnValue({ success: false, error: { issues: [] } }) }, + ScheduleCampaignSchema: { safeParse: vi.fn().mockReturnValue({ success: false, error: { issues: [] } }) }, +})) + +describe('API Route Exports', () => { it('route.ts exportiert GET und POST', async () => { const mod = await import('./route') expect(typeof mod.GET).toBe('function') diff --git a/src/server/db/campaigns.test.ts b/src/server/db/campaigns.test.ts index f25d59e..69ddcac 100644 --- a/src/server/db/campaigns.test.ts +++ b/src/server/db/campaigns.test.ts @@ -15,6 +15,7 @@ import { updateCampaign, listCampaigns, updateCampaignStatus, + getCampaignRecipients, } from './campaigns' const mockRow = { @@ -31,7 +32,7 @@ const mockRow = { } describe('createCampaign', () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => mockClient.query.mockReset()) it('gibt erstellte Kampagne zurück', async () => { mockClient.query.mockResolvedValueOnce([mockRow]) @@ -55,7 +56,7 @@ describe('createCampaign', () => { }) describe('getCampaign', () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => mockClient.query.mockReset()) it('gibt Kampagne zurück wenn gefunden', async () => { mockClient.query.mockResolvedValueOnce([mockRow]) @@ -72,7 +73,7 @@ describe('getCampaign', () => { }) describe('listCampaigns', () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => mockClient.query.mockReset()) it('gibt leeres Array zurück wenn keine Kampagnen', async () => { mockClient.query.mockResolvedValueOnce([]) @@ -89,10 +90,12 @@ describe('listCampaigns', () => { }) describe('updateCampaign', () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => mockClient.query.mockReset()) it('aktualisiert nur übergebene Felder', async () => { - mockClient.query.mockResolvedValueOnce([{ ...mockRow, name: 'Neu' }]) + mockClient.query + .mockResolvedValueOnce([{ id: 'uuid-1', status: 'draft' }]) // SELECT Existenz-Check + .mockResolvedValueOnce([{ ...mockRow, name: 'Neu' }]) // UPDATE RETURNING const result = await updateCampaign('tenant1', 'uuid-1', { name: 'Neu' }) expect(result.ok).toBe(true) if (result.ok) expect(result.data.name).toBe('Neu') @@ -103,16 +106,54 @@ describe('updateCampaign', () => { expect(result.ok).toBe(false) }) - it('gibt err zurück wenn Kampagne nicht im Draft-Status', async () => { - mockClient.query.mockResolvedValueOnce([]) + it('gibt err mit "nicht gefunden" zurück wenn Kampagne nicht existiert', async () => { + // Bug 2: muss "nicht gefunden" melden, nicht "Draft-Status" + mockClient.query.mockResolvedValueOnce([]) // SELECT: nicht gefunden + const result = await updateCampaign('tenant1', 'uuid-nicht-existent', { name: 'Neu' }) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error.message).toContain('nicht gefunden') + }) + + it('gibt err mit "Draft-Status" zurück wenn Kampagne nicht im Draft ist', async () => { + mockClient.query.mockResolvedValueOnce([{ id: 'uuid-1', status: 'sent' }]) // SELECT: gefunden, aber sent const result = await updateCampaign('tenant1', 'uuid-1', { name: 'Neu' }) expect(result.ok).toBe(false) if (!result.ok) expect(result.error.message).toContain('Draft-Status') }) }) +describe('getCampaignRecipients', () => { + beforeEach(() => mockClient.query.mockReset()) + + it('nutzt EXISTS-Subquery statt Cross-Join', async () => { + mockClient.query.mockResolvedValueOnce([{ email: 'a@example.com' }]) + await getCampaignRecipients('tenant1', 'campaign-1') + const [sql] = mockClient.query.mock.calls[0] as [string, unknown[]] + // Bug 1: muss EXISTS statt JOIN verwenden um Duplikate zu vermeiden + expect(sql).toContain('EXISTS') + expect(sql).not.toMatch(/JOIN.*campaign_recipients.*ON.*campaign_id\s*=\s*\$1/i) + }) + + it('gibt deduplizierte E-Mail-Adressen zurück', async () => { + mockClient.query.mockResolvedValueOnce([ + { email: 'a@example.com' }, + { email: 'b@example.com' }, + ]) + const result = await getCampaignRecipients('tenant1', 'campaign-1') + expect(result.ok).toBe(true) + if (result.ok) expect(result.data).toHaveLength(2) + }) + + it('gibt leeres Array zurück wenn keine Empfänger', async () => { + mockClient.query.mockResolvedValueOnce([]) + const result = await getCampaignRecipients('tenant1', 'campaign-1') + expect(result.ok).toBe(true) + if (result.ok) expect(result.data).toEqual([]) + }) +}) + describe('updateCampaignStatus', () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => mockClient.query.mockReset()) it('setzt Status auf scheduled', async () => { mockClient.query.mockResolvedValueOnce([{ ...mockRow, status: 'scheduled' }]) diff --git a/src/server/db/campaigns.ts b/src/server/db/campaigns.ts index 1dd5e10..582e633 100644 --- a/src/server/db/campaigns.ts +++ b/src/server/db/campaigns.ts @@ -79,15 +79,48 @@ export async function updateCampaign( if (fields.length === 0) return err(new Error('Keine Felder zum Aktualisieren')) try { - values.push(id) - const rows = await withTenant(tenantId, (client) => - client.query( - `UPDATE campaigns SET ${fields.join(', ')} WHERE id = $${idx} AND status = 'draft' RETURNING *`, + return await withTenant(tenantId, async (client) => { + // Bug 2: Existenz und Status getrennt prüfen um korrekten Fehler zu melden + const existing = await client.query<{ id: string; status: string }>( + 'SELECT id, status FROM campaigns WHERE id = $1', + [id] + ) + if (existing.length === 0) return err(new Error('Kampagne nicht gefunden')) + if (existing[0].status !== 'draft') return err(new Error('Kampagne nicht im Draft-Status')) + + values.push(id) + const rows = await client.query( + `UPDATE campaigns SET ${fields.join(', ')} WHERE id = $${idx} RETURNING *`, values ) + if (rows.length === 0) return err(new Error('Kampagne nicht gefunden')) + return ok(rowToCampaign(rows[0])) + }) + } catch (e) { + return err(e instanceof Error ? e : new Error(String(e))) + } +} + +export async function getCampaignRecipients( + tenantId: string, + campaignId: string +): Promise> { + try { + // Bug 1: EXISTS-Subquery verhindert Duplikate durch kartesisches Produkt + const rows = await withTenant(tenantId, (client) => + client.query<{ email: string }>( + `SELECT s.email + FROM subscribers s + WHERE s.status = 'active' + AND EXISTS ( + SELECT 1 FROM campaign_recipients cr + WHERE cr.campaign_id = $1 + AND (cr.list_id IS NULL OR cr.list_id = s.list_id) + )`, + [campaignId] + ) ) - if (rows.length === 0) return err(new Error('Kampagne nicht im Draft-Status')) - return ok(rowToCampaign(rows[0])) + return ok(rows) } catch (e) { return err(e instanceof Error ? e : new Error(String(e))) }