From 575863e0fe6bb1f02e53b62ffe014ee401d681e5 Mon Sep 17 00:00:00 2001 From: Uwamari Goddey Date: Mon, 5 Oct 2026 02:59:11 -0400 Subject: [PATCH] fix(teams): answer a duplicate team name with 409 Creating a team with a name that is already taken ended in the controller's generic handler and returned 500. The unique violation on the team name is now answered with 409 and a fixed message. Every other database error keeps the existing 500 response. Co-Authored-By: Claude Opus 5.5 --- backend/src/controllers/teams.controller.ts | 20 ++ .../__tests__/teams-duplicate-name.test.ts | 193 ++++++++++++++++++ 2 files changed, 213 insertions(+) create mode 100644 backend/src/routes/__tests__/teams-duplicate-name.test.ts diff --git a/backend/src/controllers/teams.controller.ts b/backend/src/controllers/teams.controller.ts index 7c9c082..84cffe2 100644 --- a/backend/src/controllers/teams.controller.ts +++ b/backend/src/controllers/teams.controller.ts @@ -4,6 +4,16 @@ import { CreateTeamRequest, ApiResponse } from '../types'; const repository = new TeamsRepository(); +const TEAM_NAME_CONFLICT_MESSAGE = 'A team with this name already exists.'; + +// Only the unique violation on the team name. Every other database error, +// including a unique violation on any other constraint, is left to the +// caller's generic handling. +function isTeamNameConflict(error: unknown): boolean { + const { code, constraint } = (error ?? {}) as { code?: unknown; constraint?: unknown }; + return code === '23505' && constraint === 'teams_name_key'; +} + export class TeamsController { async getAll(req: Request, res: Response): Promise { try { @@ -82,6 +92,16 @@ export class TeamsController { res.status(201).json(response); } catch (error) { + if (isTeamNameConflict(error)) { + const response: ApiResponse = { + success: false, + error: TEAM_NAME_CONFLICT_MESSAGE, + message: TEAM_NAME_CONFLICT_MESSAGE, + }; + res.status(409).json(response); + return; + } + console.error('Error creating team:', error); const response: ApiResponse = { success: false, diff --git a/backend/src/routes/__tests__/teams-duplicate-name.test.ts b/backend/src/routes/__tests__/teams-duplicate-name.test.ts new file mode 100644 index 0000000..988b43d --- /dev/null +++ b/backend/src/routes/__tests__/teams-duplicate-name.test.ts @@ -0,0 +1,193 @@ +/** + * POST /api/teams when the requested name is already taken. + * + * Behaviour under test: + * - The unique violation on the team name is answered with 409 and a fixed + * message. The body carries nothing else: no identifiers, no existing row. + * - A successful create is unchanged (201 with the created team). + * - Every other database error is unchanged (500 with the generic message), + * including a unique violation on any other constraint. + * + * There is no team update route, so a rename has no path to cover. + * + * Real routes over an in-process HTTP server against live Postgres. Only + * authService.verifyToken (to choose the caller) is stubbed, plus the + * repository for the one error the schema cannot produce. + */ +import express from 'express'; +import http from 'http'; +import { Pool } from 'pg'; +import teamsRoutes from '../teams.routes'; +import { errorHandler } from '../../middleware/error-handler'; +import { authService } from '../../services/auth.service'; +import { TeamsRepository } from '../../repositories/teams.repository'; +import { pool as appPool } from '../../config/database'; + +function dbConfig() { + return { + host: process.env.DB_HOST || 'localhost', + port: parseInt(process.env.DB_PORT || '5432'), + database: process.env.DB_NAME || 'platform_portal', + user: process.env.DB_USER || 'postgres', + password: process.env.DB_PASSWORD || 'postgres', + }; +} + +const CONFLICT_MESSAGE = 'A team with this name already exists.'; + +const pool = new Pool(dbConfig()); + +function uniqueSuffix(): string { + return `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; +} + +function teamName(): string { + return `teams-duplicate-name-${uniqueSuffix()}`; +} + +async function teamsNamed(name: string) { + const { rows } = await pool.query('SELECT id, organization_id FROM teams WHERE name = $1', [name]); + return rows; +} + +let server: http.Server; +let baseUrl: string; +let orgId: string; +let userId: string; +let errorSpy: jest.SpyInstance; + +beforeAll(async () => { + const suffix = uniqueSuffix(); + const org = await pool.query( + `INSERT INTO organizations (name, slug, display_name, subscription_tier, subscription_status) + VALUES ($1, $2, $1, 'enterprise', 'active') RETURNING id`, + [`Teams Duplicate Name ${suffix}`, `teams-duplicate-name-${suffix}`] + ); + orgId = org.rows[0].id; + const user = await pool.query( + `INSERT INTO users (email, password_hash, full_name) VALUES ($1, 'x', 'Teams Duplicate Name User') RETURNING id`, + [`teams-duplicate-name-${suffix}@example.com`] + ); + userId = user.rows[0].id; + await pool.query( + `INSERT INTO organization_memberships (organization_id, user_id, role, joined_at, is_active) + VALUES ($1, $2, 'owner', NOW(), true)`, + [orgId, userId] + ); + + const app = express(); + app.use(express.json()); + app.use('/api/teams', teamsRoutes); + app.use(errorHandler); + server = http.createServer(app); + await new Promise((resolve) => server.listen(0, resolve)); + const address = server.address(); + const port = typeof address === 'object' && address ? address.port : 0; + baseUrl = `http://127.0.0.1:${port}/api`; +}); + +beforeEach(() => { + errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(authService, 'verifyToken').mockReturnValue({ + userId, + email: 'teams-duplicate-name-caller@example.com', + organizationId: orgId, + role: 'owner', + type: 'access', + } as unknown as ReturnType); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + await pool.query('DELETE FROM teams WHERE organization_id = $1', [orgId]); + await pool.query('DELETE FROM organization_memberships WHERE organization_id = $1', [orgId]); + await pool.query('DELETE FROM organizations WHERE id = $1', [orgId]); + await pool.query('DELETE FROM users WHERE id = $1', [userId]); + await pool.end(); + await appPool.end(); +}); + +function createTeam(body: Record) { + return fetch(`${baseUrl}/teams`, { + method: 'POST', + headers: { Authorization: 'Bearer test-token', 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); +} + +describe('POST /api/teams', () => { + it('creates a team as before', async () => { + const name = teamName(); + + const res = await createTeam({ name, owner: 'owner@example.com', description: 'First' }); + + expect(res.status).toBe(201); + const body = await res.json(); + expect(body.success).toBe(true); + expect(body.message).toBe('Team created successfully'); + expect(body.data).toMatchObject({ + name, + owner: 'owner@example.com', + description: 'First', + organization_id: orgId, + }); + expect(await teamsNamed(name)).toEqual([{ id: body.data.id, organization_id: orgId }]); + }); + + it('answers 409 with a fixed message when the name is already taken', async () => { + const name = teamName(); + const first = await createTeam({ name, owner: 'owner@example.com' }); + expect(first.status).toBe(201); + const firstId = (await first.json()).data.id; + + const res = await createTeam({ name, owner: 'second-owner@example.com', description: 'Second' }); + + expect(res.status).toBe(409); + // The whole body: nothing about the existing team or any organization. + expect(await res.json()).toEqual({ + success: false, + error: CONFLICT_MESSAGE, + message: CONFLICT_MESSAGE, + }); + // The existing team is untouched and no second row was written. + expect(await teamsNamed(name)).toEqual([{ id: firstId, organization_id: orgId }]); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('leaves an unrelated database error as a 500', async () => { + // Longer than the name column allows: a database error that is not a + // unique violation. + const name = `${teamName()}-${'x'.repeat(260)}`; + + const res = await createTeam({ name, owner: 'owner@example.com' }); + + expect(res.status).toBe(500); + expect(await res.json()).toEqual({ success: false, error: 'Failed to create team' }); + expect(await teamsNamed(name)).toEqual([]); + }); + + it('leaves a unique violation on any other constraint as a 500', async () => { + jest.spyOn(TeamsRepository.prototype, 'create').mockRejectedValueOnce( + Object.assign(new Error('duplicate key value violates unique constraint "some_other_key"'), { + code: '23505', + constraint: 'some_other_key', + }) + ); + + const res = await createTeam({ name: teamName(), owner: 'owner@example.com' }); + + expect(res.status).toBe(500); + expect(await res.json()).toEqual({ success: false, error: 'Failed to create team' }); + }); + + it('still rejects a request with missing required fields with 400', async () => { + const res = await createTeam({ name: teamName() }); + + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ success: false, error: 'Missing required fields: name, owner' }); + }); +});