Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions packages/backend/src/routes/management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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',
Expand Down
36 changes: 29 additions & 7 deletions packages/backend/src/routes/managementEmbeds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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',
Expand Down
7 changes: 7 additions & 0 deletions packages/backend/src/utils/prismaErrors.ts
Original file line number Diff line number Diff line change
@@ -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'
}
60 changes: 60 additions & 0 deletions packages/backend/tests/integration/routes/management.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
customCommandService: {
listCommands: jest.fn(),
createCommand: jest.fn(),
getCommand: jest.fn(),
updateCommand: jest.fn(),
deleteCommand: jest.fn(),
},
Expand Down Expand Up @@ -430,6 +431,65 @@
).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<typeof customCommandService>
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 () => {

Check failure on line 471 in packages/backend/tests/integration/routes/management.test.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add at least one assertion to this test case.

See more on https://sonarcloud.io/project/issues?id=LucasSantana-Dev_Lucky&issues=AZ67ycpenP16ENwxEdJV&open=AZ67ycpenP16ENwxEdJV&pullRequest=1326
const mockSessionService = sessionService as jest.Mocked<
typeof sessionService
>
mockSessionService.getSession.mockResolvedValue(MOCK_SESSION_DATA)

const mockCustomCommandService =
customCommandService as jest.Mocked<typeof customCommandService>
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down
Loading