Skip to content
Closed
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
74 changes: 74 additions & 0 deletions apps/backend/src/__tests__/slug.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { describe, it, expect, vi } from 'vitest';

import { createSlug, generateUniqueSlug } from '../utils/slug';

describe('createSlug', () => {
it('lowercases and trims input', () => {
expect(createSlug(' Hello World ')).toBe('hello-world');
});

it('replaces spaces with hyphens', () => {
expect(createSlug('My Team Name')).toBe('my-team-name');
});

it('strips non-alphanumeric characters', () => {
expect(createSlug('DevCard @Core!')).toBe('devcard-core');
});

it('collapses multiple hyphens', () => {
expect(createSlug('a--b---c')).toBe('a-b-c');
});

it('removes leading and trailing hyphens', () => {
expect(createSlug('--team--')).toBe('team');
});
});

describe('generateUniqueSlug', () => {
it('returns base slug when it is available', async () => {
const slugExists = vi.fn().mockResolvedValue(false);
const result = await generateUniqueSlug('My Team', slugExists);
expect(result).toBe('my-team');
expect(slugExists).toHaveBeenCalledOnce();
});

it('returns sequential numeric suffix when base slug is taken', async () => {
const slugExists = vi.fn()
.mockResolvedValueOnce(true) // my-team taken
.mockResolvedValueOnce(false); // my-team-1 free
const result = await generateUniqueSlug('My Team', slugExists);
expect(result).toBe('my-team-1');
});

it('increments suffix deterministically until a free slot is found', async () => {
const slugExists = vi.fn()
.mockResolvedValueOnce(true) // my-team
.mockResolvedValueOnce(true) // my-team-1
.mockResolvedValueOnce(true) // my-team-2
.mockResolvedValueOnce(false); // my-team-3 free
const result = await generateUniqueSlug('My Team', slugExists);
expect(result).toBe('my-team-3');
});

it('throws when all 10 suffix candidates are taken', async () => {
const slugExists = vi.fn().mockResolvedValue(true);
await expect(generateUniqueSlug('My Team', slugExists)).rejects.toThrow(
'Unable to generate unique slug',
);
expect(slugExists).toHaveBeenCalledTimes(11); // base + 10 suffixes
});

it('produces consistent slugs across concurrent calls for different inputs', async () => {
const takenSlugs = new Set<string>();
const slugExists = vi.fn(async (slug: string) => takenSlugs.has(slug));

const [a, b] = await Promise.all([
generateUniqueSlug('Alpha Team', slugExists),
generateUniqueSlug('Beta Team', slugExists),
]);

expect(a).toBe('alpha-team');
expect(b).toBe('beta-team');
expect(a).not.toBe(b);
});
Comment on lines +61 to +73

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Test name is misleading and doesn't verify the core concurrency scenario.

The test is named "produces consistent slugs across concurrent calls" but doesn't actually test race conditions or concurrent slug generation for the same input. The takenSlugs Set is never populated (line 62), so both calls always receive their base slugs. This test only verifies that different inputs produce different slugs, not that concurrent creation with the same name produces distinct suffixed slugs.

Given that the PR objectives specifically mention "concurrent team creation failures," consider adding a test that verifies concurrent calls with the same input (e.g., both creating "My Team") produce distinct results (e.g., my-team and my-team-1).

💡 Suggested additional test case
it('produces distinct slugs when concurrent calls use the same input', async () => {
  const takenSlugs = new Set<string>();
  const slugExists = vi.fn(async (slug: string) => {
    if (takenSlugs.has(slug)) {
      return true;
    }
    takenSlugs.add(slug);
    return false;
  });

  const [a, b] = await Promise.all([
    generateUniqueSlug('My Team', slugExists),
    generateUniqueSlug('My Team', slugExists),
  ]);

  expect(a).toBe('my-team');
  expect(b).toBe('my-team-1');
  expect(a).not.toBe(b);
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/backend/src/__tests__/slug.test.ts` around lines 61 - 73, The test
"produces consistent slugs across concurrent calls for different inputs" is
misleading as it only tests different inputs, not actual concurrency scenarios.
The takenSlugs Set is never populated, so it doesn't verify that concurrent
calls with the same input produce distinct suffixed slugs. Add a new test case
that calls generateUniqueSlug twice concurrently with the same input (e.g., 'My
Team'), implement the slugExists mock to properly track and detect collisions by
adding slugs to the Set after checking, and verify that the two concurrent calls
produce distinct results with one returning the base slug and the other
returning a suffixed variant (e.g., 'my-team' and 'my-team-1').

});
52 changes: 48 additions & 4 deletions apps/backend/src/__tests__/team.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { Prisma, TeamRole } from '@prisma/client';
import Fastify from 'fastify';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import Fastify, { FastifyInstance } from 'fastify';
import { PrismaClient, TeamRole } from '@prisma/client';

import { teamRoutes } from '../routes/team';

import type { PrismaClient } from '@prisma/client';
import type { FastifyInstance } from 'fastify';

// ─── Shared mock data ─────────────────────────────────────────────────────────

const MOCK_OWNER_ID = 'user-uuid-001';
Expand Down Expand Up @@ -92,7 +96,7 @@ const prismaMock = {

// ─── App factory ──────────────────────────────────────────────────────────────

let mockJwtVerify = vi.fn();
const mockJwtVerify = vi.fn();

async function buildApp(): Promise<FastifyInstance> {
const app = Fastify({ logger: false });
Expand All @@ -118,7 +122,7 @@ async function createTeam(
app: FastifyInstance,
body: Record<string, unknown>,
authenticated = true,
) {
): Promise<ReturnType<typeof app.inject>> {
return app.inject({
method: 'POST',
url: '/',
Expand Down Expand Up @@ -220,6 +224,46 @@ describe('Teams API', () => {
expect(res.statusCode).toBe(500);
expect(res.json()).toMatchObject({ error: 'Failed to create team' });
});

it('201 — retries and succeeds when first attempt loses slug race to concurrent request', async () => {
// First generateUniqueSlug: base slug appears available
prismaMock.team.findUnique.mockResolvedValueOnce(null);
// First $transaction: P2002 — another request inserted first
prismaMock.$transaction.mockRejectedValueOnce(
new Prisma.PrismaClientKnownRequestError('Unique constraint failed', { code: 'P2002', clientVersion: '0' }),
);
// Second generateUniqueSlug: base slug now taken, devcard-core-1 is free
prismaMock.team.findUnique.mockResolvedValueOnce(MOCK_TEAM); // devcard-core taken
prismaMock.team.findUnique.mockResolvedValueOnce(null); // devcard-core-1 free
// Second $transaction: succeeds with suffix slug
prismaMock.$transaction.mockImplementationOnce(async (cb: any) => {
return cb({
team: { create: vi.fn().mockResolvedValue({ ...MOCK_TEAM, slug: 'devcard-core-1' }) },
teamMember: { create: vi.fn().mockResolvedValue({}) },
});
});

const res = await createTeam(app, validBody);

expect(res.statusCode).toBe(201);
expect(res.json().slug).toBe('devcard-core-1');
});

it('409 — exhausts all retry attempts when DB rejects every slug with P2002', async () => {
const p2002 = new Prisma.PrismaClientKnownRequestError(
'Unique constraint failed on the fields: (`slug`)',
{ code: 'P2002', clientVersion: '0' },
);
// Slug always appears available at the application level
prismaMock.team.findUnique.mockResolvedValue(null);
// DB always rejects with P2002 (concurrent inserts won every race)
prismaMock.$transaction.mockRejectedValue(p2002);

const res = await createTeam(app, validBody);

expect(res.statusCode).toBe(409);
expect(prismaMock.$transaction).toHaveBeenCalledTimes(5);
});
});

// ── GET /:slug — public team profile ─────────────────────────────────────
Expand Down
Loading