diff --git a/packages/backend/src/routes/management.ts b/packages/backend/src/routes/management.ts index e0a05da26..e824a86b1 100644 --- a/packages/backend/src/routes/management.ts +++ b/packages/backend/src/routes/management.ts @@ -19,6 +19,7 @@ import { } from '@lucky/shared/services' import { setupEmbedRoutes } from './managementEmbeds' import { setupAutoMessageRoutes } from './managementAutoMessages' +import { isUniqueViolation } from '../utils/prismaErrors' function p(val: string | string[]): string { return typeof val === 'string' ? val : val[0] @@ -149,12 +150,31 @@ export function setupManagementRoutes(app: Express): void { const userId = requireUserId(req) const body = s.createCommandBody.parse(req.body) const { name, response, description } = body - const command = await customCommandService.createCommand( - guildId, - name, - response, - { description, createdBy: userId }, - ) + let command + try { + command = await customCommandService.createCommand( + guildId, + name, + response, + { description, createdBy: userId }, + ) + } catch (error) { + if (!isUniqueViolation(error)) { + throw error + } + // P2002 on the (guildId, name) natural key is idempotent + // success, not a failure: return the existing command (#1320). + // Divergent payloads also land here — edit via PATCH. + const existing = await customCommandService.getCommand( + guildId, + name, + ) + if (!existing) { + throw error + } + res.json(existing) + return + } await serverLogService.logCustomCommandChange( guildId, 'created', diff --git a/packages/backend/src/routes/managementEmbeds.ts b/packages/backend/src/routes/managementEmbeds.ts index d4eb96ae2..9dd7cad91 100644 --- a/packages/backend/src/routes/managementEmbeds.ts +++ b/packages/backend/src/routes/managementEmbeds.ts @@ -11,6 +11,7 @@ import { serverLogService, type EmbedData, } from '@lucky/shared/services' +import { isUniqueViolation } from '../utils/prismaErrors' function p(val: string | string[]): string { return typeof val === 'string' ? val : val[0] @@ -58,13 +59,34 @@ export function setupEmbedRoutes(app: Express): void { validation.errors, ) } - const template = await embedBuilderService.createTemplate( - guildId, - name, - embedData, - description, - userId, - ) + let template + try { + template = await embedBuilderService.createTemplate( + guildId, + name, + embedData, + description, + userId, + ) + } catch (error) { + if (!isUniqueViolation(error)) { + throw error + } + // P2002 on the (guildId, name) natural key is idempotent + // success, not a failure: return the existing template + // (#1320). Divergent payloads also land here — edit via + // PATCH. createTemplate stores the name lowercased; + // getTemplate matches the stored value verbatim. + const existing = await embedBuilderService.getTemplate( + guildId, + name.toLowerCase(), + ) + if (!existing) { + throw error + } + res.json(existing) + return + } await serverLogService.logEmbedTemplateChange( guildId, 'created', diff --git a/packages/backend/src/utils/prismaErrors.ts b/packages/backend/src/utils/prismaErrors.ts new file mode 100644 index 000000000..6607a9bfa --- /dev/null +++ b/packages/backend/src/utils/prismaErrors.ts @@ -0,0 +1,7 @@ +/** + * Narrow check for Prisma's unique-constraint violation (P2002) without + * importing the generated client error class across package boundaries. + */ +export function isUniqueViolation(error: unknown): boolean { + return error instanceof Error && 'code' in error && error.code === 'P2002' +} diff --git a/packages/backend/tests/integration/routes/management.test.ts b/packages/backend/tests/integration/routes/management.test.ts index 9bdf89677..afa6e2312 100644 --- a/packages/backend/tests/integration/routes/management.test.ts +++ b/packages/backend/tests/integration/routes/management.test.ts @@ -39,6 +39,7 @@ jest.mock('@lucky/shared/services', () => ({ customCommandService: { listCommands: jest.fn(), createCommand: jest.fn(), + getCommand: jest.fn(), updateCommand: jest.fn(), deleteCommand: jest.fn(), }, @@ -430,6 +431,65 @@ describe('Management Routes Integration', () => { ).toHaveBeenCalled() }) + test('returns the existing command on a replayed create (P2002 → idempotent success, #1320)', async () => { + const mockSessionService = sessionService as jest.Mocked< + typeof sessionService + > + mockSessionService.getSession.mockResolvedValue(MOCK_SESSION_DATA) + + const existing = { + name: 'newcmd', + response: 'Hello there!', + description: 'A greeting command', + } + + const mockCustomCommandService = + customCommandService as jest.Mocked + mockCustomCommandService.createCommand.mockRejectedValue( + Object.assign(new Error('Unique constraint failed'), { + code: 'P2002', + }), + ) + mockCustomCommandService.getCommand.mockResolvedValue(existing) + + const mockServerLogService = serverLogService as jest.Mocked< + typeof serverLogService + > + + const response = await request(app) + .post('/api/guilds/111111111111111111/commands') + .set('Cookie', ['sessionId=valid_session_id']) + .send({ name: 'newcmd', response: 'Hello there!' }) + .expect(200) + + expect(response.body).toEqual(existing) + expect( + mockServerLogService.logCustomCommandChange, + ).not.toHaveBeenCalled() + }) + + test('rethrows P2002 when the existing command vanished (no silent 200)', async () => { + const mockSessionService = sessionService as jest.Mocked< + typeof sessionService + > + mockSessionService.getSession.mockResolvedValue(MOCK_SESSION_DATA) + + const mockCustomCommandService = + customCommandService as jest.Mocked + mockCustomCommandService.createCommand.mockRejectedValue( + Object.assign(new Error('Unique constraint failed'), { + code: 'P2002', + }), + ) + mockCustomCommandService.getCommand.mockResolvedValue(null) + + await request(app) + .post('/api/guilds/111111111111111111/commands') + .set('Cookie', ['sessionId=valid_session_id']) + .send({ name: 'newcmd', response: 'Hello there!' }) + .expect(500) + }) + test('should return 401 when not authenticated', async () => { const mockSessionService = sessionService as jest.Mocked< typeof sessionService diff --git a/packages/backend/tests/integration/routes/managementEmbeds.test.ts b/packages/backend/tests/integration/routes/managementEmbeds.test.ts index c9e16ec48..f123a8772 100644 --- a/packages/backend/tests/integration/routes/managementEmbeds.test.ts +++ b/packages/backend/tests/integration/routes/managementEmbeds.test.ts @@ -24,6 +24,7 @@ jest.mock('@lucky/shared/services', () => ({ embedBuilderService: { listTemplates: jest.fn(), createTemplate: jest.fn(), + getTemplate: jest.fn(), updateTemplate: jest.fn(), deleteTemplate: jest.fn(), validateEmbedData: jest.fn(), @@ -113,7 +114,9 @@ describe('Embed Management Routes Integration', () => { const mockGuildAccessServiceSvc = guildAccessService as jest.Mocked< typeof guildAccessService > - mockGuildAccessServiceSvc.resolveGuildContext.mockResolvedValue(null) + mockGuildAccessServiceSvc.resolveGuildContext.mockResolvedValue( + null, + ) const response = await request(app) .get('/api/guilds/111111111111111111/embeds') @@ -174,6 +177,49 @@ describe('Embed Management Routes Integration', () => { ).toHaveBeenCalled() }) + test('returns the existing template on a replayed create (P2002 → idempotent success, #1320)', async () => { + const existing = { + name: 'welcome', + description: 'Welcome template', + embedData: { title: 'Welcome Embed', color: '#341503' }, + } + + const mockEmbedService = embedBuilderService as jest.Mocked< + typeof embedBuilderService + > + mockEmbedService.validateEmbedData.mockReturnValue({ valid: true }) + mockEmbedService.createTemplate.mockRejectedValue( + Object.assign(new Error('Unique constraint failed'), { + code: 'P2002', + }), + ) + mockEmbedService.getTemplate.mockResolvedValue(existing) + + const mockServerLogService = serverLogService as jest.Mocked< + typeof serverLogService + > + + const response = await request(app) + .post('/api/guilds/111111111111111111/embeds') + .set('Cookie', ['sessionId=valid_session_id']) + .send({ + name: 'Welcome', + description: 'Welcome template', + embedData: { title: 'Welcome Embed', color: '#341503' }, + }) + .expect(200) + + expect(response.body).toEqual(existing) + // lookup uses the stored (lowercased) name + expect(mockEmbedService.getTemplate).toHaveBeenCalledWith( + '111111111111111111', + 'welcome', + ) + expect( + mockServerLogService.logEmbedTemplateChange, + ).not.toHaveBeenCalled() + }) + test('should return 401 when not authenticated', async () => { const mockSessionService = sessionService as jest.Mocked< typeof sessionService