fix: Bug 1+2+3 — SQL Cross-Join, PATCH 404 vs 400, API-Test-Skip
- getCampaignRecipients: EXISTS-Subquery statt Cross-Join verhindert Mehrfachversand - updateCampaign: SELECT vor UPDATE unterscheidet 'nicht gefunden' (404) von 'nicht im Draft' (400) - campaigns-api.test.ts: describe.skip entfernt, Mocks für DB-Abhängigkeiten ergänzt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,6 @@
|
|||||||
import { NextRequest, NextResponse } from 'next/server'
|
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 { emailSendQueue } from '../../../../../queues/email-send.queue'
|
||||||
import { withTenant } from '../../../../../server/db/tenant'
|
|
||||||
import { hashEmail } from '../../../../../lib/crypto'
|
import { hashEmail } from '../../../../../lib/crypto'
|
||||||
import { checkSuppression } from '../../../../../server/suppression/check'
|
import { checkSuppression } from '../../../../../server/suppression/check'
|
||||||
import { getTenantId } from '../../../../../lib/tenant-header'
|
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 })
|
return NextResponse.json({ error: statusResult.error.message }, { status: 500 })
|
||||||
}
|
}
|
||||||
|
|
||||||
let recipients: { email: string }[]
|
const recipientsResult = await getCampaignRecipients(tenantId, params.id)
|
||||||
try {
|
if (!recipientsResult.ok) {
|
||||||
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) {
|
|
||||||
// Rollback auf draft — verhindert verwaisten 'sending'-Status
|
// Rollback auf draft — verhindert verwaisten 'sending'-Status
|
||||||
await updateCampaignStatus(tenantId, params.id, 'draft')
|
await updateCampaignStatus(tenantId, params.id, 'draft')
|
||||||
return NextResponse.json({ error: 'Empfänger konnten nicht geladen werden' }, { status: 500 })
|
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
|
// Suppression-Check PFLICHT — kein Opt-out-Empfänger darf in die Queue
|
||||||
const unsuppressedRecipients = await Promise.all(
|
const unsuppressedRecipients = await Promise.all(
|
||||||
|
|||||||
@@ -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
|
vi.mock('../../../server/db/campaigns', () => ({
|
||||||
// vollständig verfügbar ist. Die Haupttests sind die bestehenden Unit-Tests
|
createCampaign: vi.fn(),
|
||||||
// in src/queues/, src/server/ und src/lib/.
|
listCampaigns: vi.fn(),
|
||||||
describe.skip('API Route Exports', () => {
|
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 () => {
|
it('route.ts exportiert GET und POST', async () => {
|
||||||
const mod = await import('./route')
|
const mod = await import('./route')
|
||||||
expect(typeof mod.GET).toBe('function')
|
expect(typeof mod.GET).toBe('function')
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import {
|
|||||||
updateCampaign,
|
updateCampaign,
|
||||||
listCampaigns,
|
listCampaigns,
|
||||||
updateCampaignStatus,
|
updateCampaignStatus,
|
||||||
|
getCampaignRecipients,
|
||||||
} from './campaigns'
|
} from './campaigns'
|
||||||
|
|
||||||
const mockRow = {
|
const mockRow = {
|
||||||
@@ -31,7 +32,7 @@ const mockRow = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe('createCampaign', () => {
|
describe('createCampaign', () => {
|
||||||
beforeEach(() => vi.clearAllMocks())
|
beforeEach(() => mockClient.query.mockReset())
|
||||||
|
|
||||||
it('gibt erstellte Kampagne zurück', async () => {
|
it('gibt erstellte Kampagne zurück', async () => {
|
||||||
mockClient.query.mockResolvedValueOnce([mockRow])
|
mockClient.query.mockResolvedValueOnce([mockRow])
|
||||||
@@ -55,7 +56,7 @@ describe('createCampaign', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe('getCampaign', () => {
|
describe('getCampaign', () => {
|
||||||
beforeEach(() => vi.clearAllMocks())
|
beforeEach(() => mockClient.query.mockReset())
|
||||||
|
|
||||||
it('gibt Kampagne zurück wenn gefunden', async () => {
|
it('gibt Kampagne zurück wenn gefunden', async () => {
|
||||||
mockClient.query.mockResolvedValueOnce([mockRow])
|
mockClient.query.mockResolvedValueOnce([mockRow])
|
||||||
@@ -72,7 +73,7 @@ describe('getCampaign', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe('listCampaigns', () => {
|
describe('listCampaigns', () => {
|
||||||
beforeEach(() => vi.clearAllMocks())
|
beforeEach(() => mockClient.query.mockReset())
|
||||||
|
|
||||||
it('gibt leeres Array zurück wenn keine Kampagnen', async () => {
|
it('gibt leeres Array zurück wenn keine Kampagnen', async () => {
|
||||||
mockClient.query.mockResolvedValueOnce([])
|
mockClient.query.mockResolvedValueOnce([])
|
||||||
@@ -89,10 +90,12 @@ describe('listCampaigns', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe('updateCampaign', () => {
|
describe('updateCampaign', () => {
|
||||||
beforeEach(() => vi.clearAllMocks())
|
beforeEach(() => mockClient.query.mockReset())
|
||||||
|
|
||||||
it('aktualisiert nur übergebene Felder', async () => {
|
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' })
|
const result = await updateCampaign('tenant1', 'uuid-1', { name: 'Neu' })
|
||||||
expect(result.ok).toBe(true)
|
expect(result.ok).toBe(true)
|
||||||
if (result.ok) expect(result.data.name).toBe('Neu')
|
if (result.ok) expect(result.data.name).toBe('Neu')
|
||||||
@@ -103,16 +106,54 @@ describe('updateCampaign', () => {
|
|||||||
expect(result.ok).toBe(false)
|
expect(result.ok).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('gibt err zurück wenn Kampagne nicht im Draft-Status', async () => {
|
it('gibt err mit "nicht gefunden" zurück wenn Kampagne nicht existiert', async () => {
|
||||||
mockClient.query.mockResolvedValueOnce([])
|
// 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' })
|
const result = await updateCampaign('tenant1', 'uuid-1', { name: 'Neu' })
|
||||||
expect(result.ok).toBe(false)
|
expect(result.ok).toBe(false)
|
||||||
if (!result.ok) expect(result.error.message).toContain('Draft-Status')
|
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', () => {
|
describe('updateCampaignStatus', () => {
|
||||||
beforeEach(() => vi.clearAllMocks())
|
beforeEach(() => mockClient.query.mockReset())
|
||||||
|
|
||||||
it('setzt Status auf scheduled', async () => {
|
it('setzt Status auf scheduled', async () => {
|
||||||
mockClient.query.mockResolvedValueOnce([{ ...mockRow, status: 'scheduled' }])
|
mockClient.query.mockResolvedValueOnce([{ ...mockRow, status: 'scheduled' }])
|
||||||
|
|||||||
@@ -79,15 +79,48 @@ export async function updateCampaign(
|
|||||||
if (fields.length === 0) return err(new Error('Keine Felder zum Aktualisieren'))
|
if (fields.length === 0) return err(new Error('Keine Felder zum Aktualisieren'))
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
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)
|
values.push(id)
|
||||||
const rows = await withTenant(tenantId, (client) =>
|
const rows = await client.query(
|
||||||
client.query(
|
`UPDATE campaigns SET ${fields.join(', ')} WHERE id = $${idx} RETURNING *`,
|
||||||
`UPDATE campaigns SET ${fields.join(', ')} WHERE id = $${idx} AND status = 'draft' RETURNING *`,
|
|
||||||
values
|
values
|
||||||
)
|
)
|
||||||
)
|
if (rows.length === 0) return err(new Error('Kampagne nicht gefunden'))
|
||||||
if (rows.length === 0) return err(new Error('Kampagne nicht im Draft-Status'))
|
|
||||||
return ok(rowToCampaign(rows[0]))
|
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<Result<{ email: string }[]>> {
|
||||||
|
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]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
return ok(rows)
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
return err(e instanceof Error ? e : new Error(String(e)))
|
return err(e instanceof Error ? e : new Error(String(e)))
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user