diff --git a/.agents/skills/nestjs-jest-prisma-testing/SKILL.md b/.agents/skills/nestjs-jest-prisma-testing/SKILL.md new file mode 100644 index 0000000..5affd03 --- /dev/null +++ b/.agents/skills/nestjs-jest-prisma-testing/SKILL.md @@ -0,0 +1,213 @@ +--- +name: nestjs-jest-prisma-testing +description: Write strictly-typed Jest tests for NestJS services and controllers that use Prisma directly — no `any`, correctly mocked Prisma delegates, real model shapes. +--- + +NestJS, Prisma, and Jest are a common stack — but the combination has a sharp edge. Prisma's generated client types are large and strict, and the moment a test reaches for a shortcut (`as any`, a partial mock object, an untyped `jest.fn()`), TypeScript either stops catching real bugs or starts throwing confusing `never` errors that look unrelated to the actual mistake. + +This skill exists because both problems are avoidable with a small, consistent set of rules. Apply them and Prisma-backed NestJS tests stay fully typed, fully readable, and free of escape hatches. + +## The Four Rules + +**1. Type the mock against the real class, not method by method.** +Don't try to type each `jest.fn()` individually with generics — it's fragile and commonly collapses to `never`. Instead, register the mock through `Test.createTestingModule`'s `useValue`, then retrieve it typed as `jest.Mocked`. The type comes from the retrieval, not from the mock declaration. + +**2. Mock data must match the full Prisma model shape.** +`findUnique`'s real return type is the entire model — every field, not just the ones you care about. `{ id: "task-1" }` will fail to typecheck against Prisma's generated `Task` type. Build full fixtures once and reuse them. + +**3. Enum and literal fields need `as const`.** +Any field backed by a Prisma enum (`role`, `status`, `priority`, etc.) gets widened to plain `string` by TypeScript unless pinned with `as const`. Without it, a perfectly valid value like `"TODO"` fails to satisfy a Prisma-generated return type that expects the literal union. + +**4. `as unknown as T` is the only acceptable cast — and only when nothing else works.** +A handwritten mock of a Prisma delegate (`prisma.task`) will never structurally satisfy the real type, which has 15+ methods (`aggregate`, `groupBy`, `findFirst`, and more) beyond the few you're using. TypeScript correctly refuses a direct cast here because there's too little overlap. Going through `unknown` first is the deliberate, narrow way to say "trust me, this is enough for what I'm testing" — without reaching for `any`, which throws away type checking everywhere, not just on this one object. + +## Mocking a Prisma Delegate + +```typescript +import type { PrismaClient } from "../generated/prisma/client.js"; + +const mockPrisma = { + task: { + findMany: jest.fn(), + findUnique: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + } as unknown as jest.Mocked, +}; +``` + +Only declare the methods your service actually calls. The `as unknown as` cast is expected and correct here — it's not a sign something is wrong. + +## Mocking `$transaction` + +This is the part of Prisma mocking that trips people up most, because `$transaction` with a callback doesn't return a value directly — it receives a transaction client and expects your code to run queries against *that* client: + +```typescript +const mockPrisma = { + $transaction: jest.fn((callback) => callback(mockPrisma)), + task: { /* ... */ } as unknown as jest.Mocked, + user: { /* ... */ } as unknown as jest.Mocked, +}; +``` + +The mock simply invokes the callback immediately with the same mock object. Since `task`/`user` are already mocked on that object, any "inside the transaction" queries your service runs hit the exact same mocked methods you've already configured — no real transactional behavior needed for a unit test. + +Stop here. If a test needs to verify actual rollback or isolation behavior, that's not a unit test's job — write an E2E test against a real test database instead. Trying to simulate rollback semantics inside a mock is a sign you're testing the wrong layer. + +## Service Unit Tests + +Build fixtures once, reuse everywhere a model is mocked more than once in a file — a common bug is one inline mock missing fields while four others have them right, simply because retyping the same object by hand at five call sites is easy to get inconsistent. + +```typescript +import { jest } from "@jest/globals"; +import { NotFoundException } from "@nestjs/common"; +import { Test, TestingModule } from "@nestjs/testing"; +import type { PrismaClient } from "../generated/prisma/client.js"; +import { PrismaService } from "../lib/database/prisma.service.js"; +import { TaskService } from "./task.service.js"; + +const mockTask = { + id: "task-1", + title: "Test Task", + description: "A test task", + status: "TODO" as const, + priority: "LOW" as const, + projectId: "proj-1", + assigneeId: "user-1", + createdAt: new Date(), + updatedAt: new Date(), +}; + +const mockPrisma = { + task: { + findUnique: jest.fn(), + findMany: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + } as unknown as jest.Mocked, +}; + +describe("TaskService", () => { + let service: TaskService; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [TaskService, { provide: PrismaService, useValue: mockPrisma }], + }).compile(); + + service = module.get(TaskService) as jest.Mocked; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("findById", () => { + it("returns the task when found", async () => { + mockPrisma.task.findUnique.mockResolvedValue(mockTask); + + const result = await service.findById("task-1"); + + expect(result).toEqual(mockTask); + }); + + it("throws NotFoundException when not found", async () => { + mockPrisma.task.findUnique.mockResolvedValue(null); + + await expect(service.findById("nonexistent")).rejects.toThrow(NotFoundException); + }); + }); +}); +``` + +## Controller Unit Tests + +Controllers don't touch Prisma directly — they depend on services. Mock the service the same way, via DI retrieval typed as `jest.Mocked`: + +```typescript +import { jest } from "@jest/globals"; +import { Test, TestingModule } from "@nestjs/testing"; +import type { Request } from "express"; +import { TaskController } from "./task.controller.js"; +import { TaskService } from "./task.service.js"; + +describe("TaskController", () => { + let controller: TaskController; + let service: jest.Mocked; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [TaskController], + providers: [ + { + provide: TaskService, + useValue: { findById: jest.fn(), create: jest.fn() }, + }, + ], + }).compile(); + + controller = module.get(TaskController); + service = module.get(TaskService) as jest.Mocked; + }); + + it("returns a task for an authorized user", async () => { + const req = { + user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, + } as unknown as Request; + + service.findById.mockResolvedValue(mockTask); + + await controller.findById("task-1", req); + + expect(service.findById).toHaveBeenCalledWith("task-1"); + }); +}); +``` + +If your project augments Express's `Request` type with a `user` field (common with custom auth), `as unknown as Request` is the correct cast for a partial request object — `Request`'s real shape has dozens of required properties no test object will ever fully provide. + +## E2E Tests + +- Use a **real test database**. Never mock Prisma in E2E — that turns an E2E test into a slower, more complicated unit test and defeats its actual purpose: verifying the HTTP layer, the DI container, and the database integration together. +- Bootstrap with `createNestApplication()` + `app.init()` in `beforeAll`, and always `app.close()` in `afterAll` — skipping this leaks open handles and hangs the test runner. +- Clean up between tests with transaction rollback or table truncation in `afterEach`. Don't rely on test execution order for isolation. + +### The `useClass` vs `useExisting` guard gotcha + +If your app registers a global guard like this: + +```typescript +providers: [{ provide: APP_GUARD, useClass: SomeGuard }], +``` + +**you cannot override it in tests.** `useClass` lets Nest instantiate the guard privately and internally — it's invisible to `overrideProvider`. To make a globally-registered guard overridable, register it with `useExisting` instead, and also list it as its own provider: + +```typescript +providers: [ + { provide: APP_GUARD, useExisting: SomeGuard }, + SomeGuard, +], +``` + +Now it's visible to Nest as a regular provider, and your E2E setup can disable it cleanly: + +```typescript +const moduleRef = await Test.createTestingModule({ imports: [AppModule] }) + .overrideProvider(SomeGuard) + .useValue({ canActivate: jest.fn(() => true) }) + .compile(); +``` + +This is an easy thing to miss, since the app runs fine in production either way — the difference only shows up the moment you try to test around the guard. + +## Anti-Patterns + +- `as any` anywhere in a test file. If a cast feels unavoidable, it's almost always `as unknown as T` you actually need. +- Partial mock objects (`{ id: "x" }`) standing in for full Prisma models. Build the full fixture once. +- Retyping the same model's mock object inline at multiple call sites instead of extracting a shared fixture. +- Mocking the database in E2E tests. +- Testing private methods directly (`service["privateMethod"]()`). Test through the public API. +- Leaving `app.close()` out of an E2E suite's `afterAll`. +- Assuming a `useClass`-registered global guard can be overridden in tests without changing its registration first. diff --git a/.agents/skills/nestjs-patterns/SKILL.md b/.agents/skills/nestjs-patterns/SKILL.md new file mode 100644 index 0000000..88b72ed --- /dev/null +++ b/.agents/skills/nestjs-patterns/SKILL.md @@ -0,0 +1,603 @@ +--- +name: nestjs-patterns +description: Build production NestJS applications correctly — modules, DI, controllers, services, DTOs, guards, filters, and interceptors following framework-native patterns. +--- + +NestJS hands you an architecture. The mistake most teams make is ignoring it — reaching for generic Node.js patterns, instantiating services directly, or putting business logic in controllers. This skill is about using what NestJS gives you, not working around it. + +Every pattern here is framework-native. None of them require a specific ORM, auth library, or third-party package. They work regardless of what you plug in underneath. + +## Project Structure + +Group by feature, not by technical layer. A folder called `controllers/` that holds every controller in the app is a maintenance problem at 10 endpoints and a disaster at 50. A folder called `users/` that holds the controller, service, DTOs, and tests for the users feature stays navigable as the app grows. + +``` +src/ +├── app.module.ts +├── main.ts +├── common/ # Cross-cutting concerns shared across features +│ ├── decorators/ # @SetMetadata-based custom decorators +│ ├── filters/ # Exception filters +│ ├── guards/ # Auth, roles, rate-limit guards +│ ├── interceptors/ # Response transformation, logging +│ └── pipes/ # Custom validation pipes +├── lib/ # Infrastructure modules (database, mail, cache, etc.) +│ └── database/ # e.g. prisma.module.ts + prisma.service.ts +└── module/ # Feature modules + ├── users/ + │ ├── dto/ + │ ├── users.controller.ts + │ ├── users.service.ts + │ ├── users.module.ts + │ └── users.service.spec.ts + └── posts/ +``` + +**Rule:** Feature modules go in `src/module//`. Infrastructure (database, mail, cache) goes in `src/lib//`. Shared guards, interceptors, decorators, and filters go in `src/common//`. + +## Module Patterns + +### Feature module + +```typescript +@Module({ + controllers: [UsersController], + providers: [UsersService], + exports: [UsersService], // only if another module needs to inject this service +}) +export class UsersModule {} +``` + +Only export what other modules actually need to inject. Exporting everything by default creates accidental coupling and makes boundaries meaningless. + +### Infrastructure module — global, imported once + +```typescript +@Global() +@Module({ + providers: [DatabaseService], + exports: [DatabaseService], +}) +export class DatabaseModule {} +``` + +`@Global()` makes the module's exports available everywhere without re-importing. Use it only for infrastructure that genuinely belongs everywhere (database, logger, config). Import it once in `AppModule` — never in feature modules. + +### Feature module importing another feature + +```typescript +@Module({ + imports: [TasksModule], // import when you need to inject TasksService + controllers: [CommentsController], + providers: [CommentsService], +}) +export class CommentsModule {} +``` + +Import the module, not the service directly. NestJS resolves providers through module boundaries — if `CommentsService` needs `TasksService`, import `TasksModule` (and ensure `TasksModule` exports `TasksService`). + +### AppModule — root composition + +```typescript +@Module({ + imports: [ + ConfigModule.forRoot({ isGlobal: true, validationSchema }), + DatabaseModule, // @Global infrastructure, imported once + UsersModule, + PostsModule, + CommentsModule, + ], + providers: [ + // Global enhancers with DI live here — see Bootstrap section + ], + controllers: [AppController], +}) +export class AppModule {} +``` + +`AppModule` composes the application. It doesn't own business logic. + +## Dependency Injection Rules + +**Never instantiate services directly.** `new UsersService()` bypasses NestJS's DI container entirely — the instance has no injected dependencies, no lifecycle hooks, and can't be mocked in tests. + +```typescript +// Wrong +const service = new UsersService(new DatabaseService()); + +// Correct +@Injectable() +export class PostsService { + constructor(private readonly usersService: UsersService) {} +} +``` + +**Use `@Inject()` for custom tokens.** When a dependency is bound to a string or Symbol token (common with repository interfaces or third-party modules), `@Inject()` is required: + +```typescript +constructor(@Inject(DATABASE_TOKEN) private readonly db: DatabaseClient) {} +``` + +**Use `forRootAsync()` for runtime config.** When a module needs injected services to initialize, `forRootAsync` with `useFactory` is the correct pattern — it defers initialization until the DI container is ready: + +```typescript +SomeModule.forRootAsync({ + useFactory: (configService: ConfigService) => ({ + apiKey: configService.getOrThrow("SOME_API_KEY"), + }), + inject: [ConfigService], +}); +``` + +## Controllers + +Controllers are thin. They receive a request, call a service, and return the result. Business logic — validation beyond DTO constraints, access control decisions, data transformation — belongs in services. + +### Standard controller shape + +```typescript +@Controller("users") +export class UsersController { + constructor(private readonly usersService: UsersService) {} + + @Get() + findAll() { + return this.usersService.findAll(); + } + + @Get(":id") + findById(@Param("id") id: string) { + return this.usersService.findById(id); + } + + @Post() + @UseGuards(RolesGuard) + @Roles("ADMIN") + @ResponseMessage("User created successfully") + create(@Body() dto: CreateUserDto) { + return this.usersService.create(dto); + } + + @Patch(":id") + @UseGuards(RolesGuard) + @Roles("ADMIN") + @ResponseMessage("User updated successfully") + update(@Param("id") id: string, @Body() dto: UpdateUserDto) { + return this.usersService.update(id, dto); + } + + @Delete(":id") + @UseGuards(RolesGuard) + @Roles("ADMIN") + @ResponseMessage("User deleted successfully") + delete(@Param("id") id: string) { + return this.usersService.delete(id); + } +} +``` + +### Route Ordering — Static before Dynamic +NestJS evaluates route handlers top-to-bottom in the order they are defined. Always put static routes before parameterized dynamic routes. + +```typescript +@Controller("users") +export class UsersController { + // 1. Static routes go first + @Get("all") + findAll() { + return this.usersService.findAll(); + } + + // 2. Dynamic/parameterized routes go last + @Get(":id") + findById(@Param("id") id: string) { + return this.usersService.findById(id); + } +} + +### Scoped access — the `@CurrentUser()` decorator + +When handlers need the authenticated user, don't read `req.user` directly. +`req.user` is typed as `Express.User | undefined` — accessing it with `!` +is a non-null assertion that Biome flags and strict TypeScript rejects. +Repeating `if (!user) throw new UnauthorizedException()` across every +handler is boilerplate that belongs in one place. + +The correct pattern is a `@CurrentUser()` param decorator: + +```typescript +// src/common/decorators/current-user.decorator.ts +import { createParamDecorator, ExecutionContext, UnauthorizedException } from "@nestjs/common"; +import type { Request } from "express"; + +export const CurrentUser = createParamDecorator( + (_data: unknown, ctx: ExecutionContext) => { + const request = ctx.switchToHttp().getRequest(); + if (!request.user) throw new UnauthorizedException(); + return request.user; + }, +); +``` + +Use it in controllers — fully typed, no assertion, no repeated null check: + +```typescript +@Get() +findAll( + @Param("projectId") projectId: string, + @CurrentUser() user: Express.User, +) { + return this.taskService.findByProject(projectId, user.id, user.role); +} +``` + +TypeScript knows `user` is `Express.User` — not `Express.User | undefined`. +The decorator handles the unauthorized case in one place. If a `@Public()` +route accidentally tries to use `@CurrentUser()`, it throws a clean 401 +instead of crashing on an undefined access. + +### Nested route params + +```typescript +@Controller("projects/:projectId/tasks") +export class TasksController { + @Get() + findAll(@Param("projectId") projectId: string) { + return this.tasksService.findByProject(projectId); + } + + @Get(":id") + findById( + @Param("projectId") _projectId: string, // prefix unused params with _ + @Param("id") id: string, + ) { + return this.tasksService.findById(id); + } +} +``` + +Unused route params that are structurally required by the path get prefixed with `_` to signal intent without triggering lint warnings. + +## Services + +Services own business logic. They validate references, enforce access rules, and throw NestJS exception classes — never raw `Error` or HTTP status codes. + +### Consistent method shape + +```typescript +@Injectable() +export class TasksService { + async findAll(): Promise // public list, no auth filter + async findById(id: string): Promise // throws NotFoundException if missing + async findByIdScoped(id, userId, role) // ownership check before returning + async create(dto, ...context): Promise // validates refs, returns created + async update(id, dto): Promise // validates existence first + async delete(id): Promise // validates existence first +``` + +`findById` always throws `NotFoundException` when not found — never returns `null`. Callers that need to handle the missing case differently call it inside a try/catch; callers that just need the entity call it directly and trust it throws. + +### Reusable query objects + +```typescript +@Injectable() +export class TasksService { + private include = { + project: { select: { id: true, name: true } }, + assignee: { select: { id: true, name: true, email: true } }, + } as const; + + async findById(id: string) { + const task = await this.db.task.findUnique({ where: { id }, include: this.include }); + if (!task) throw new NotFoundException(`Task ${id} not found`); + return task; + } +} +``` + +Define `include`, `select`, or `where` fragments as class properties with `as const`. Reuse them across methods to keep the returned shape consistent and prevent drift between read paths. + +### Throw NestJS exceptions, never raw errors + +```typescript +// Wrong +throw new Error("Not found"); +throw { statusCode: 404, message: "Not found" }; + +// Correct +throw new NotFoundException("Task not found"); +throw new ForbiddenException("You do not have access to this resource"); +throw new ConflictException("Email already in use"); +``` + +NestJS's `HttpException` subclasses carry the status code, integrate with exception filters, and produce consistent error shapes. Raw errors bypass all of this. + +## DTOs + +DTOs are classes, not interfaces. Class-based DTOs survive TypeScript compilation and can carry runtime decorator metadata — interfaces don't. + +### Create DTO + +```typescript +import { IsString, IsNotEmpty, IsOptional, IsEnum } from "class-validator"; + +export class CreateTaskDto { + @IsString() + @IsNotEmpty() + title: string; + + @IsString() + @IsOptional() + description?: string; + + @IsEnum(TaskStatus) + @IsOptional() + status?: TaskStatus; + + @IsString() + @IsOptional() + assigneeId?: string; +} +``` + +Every field gets a `class-validator` decorator. Required fields get `@IsNotEmpty()`. Optional fields get `@IsOptional()` — without it, `undefined` fields fail validation even when absent. + +### Update DTO — always extend, never recreate + +```typescript +// Use @nestjs/swagger's PartialType — it preserves Swagger metadata automatically. +// @nestjs/mapped-types works but strips it. +import { PartialType } from "@nestjs/swagger"; +import { CreateTaskDto } from "./create-task.dto.js"; + +export class UpdateTaskDto extends PartialType(CreateTaskDto) {} +``` + +`PartialType` makes every field optional and keeps validation decorators in sync automatically. Never create update DTOs from scratch — you'll inevitably diverge from the create DTO and introduce inconsistencies that are hard to track down. + +If the update DTO needs additional fields not present in the create DTO: + +```typescript +export class UpdateTaskDto extends PartialType(CreateTaskDto) { + @IsString() + @IsOptional() + projectId?: string; +} +``` + +### ValidationPipe — always these settings + +```typescript +new ValidationPipe({ + whitelist: true, // strip properties not in the DTO + forbidNonWhitelisted: true, // throw on unknown properties instead of stripping silently + transform: true, // auto-transform payloads to DTO class instances +}); +``` + +`whitelist` alone strips unknown properties silently — callers sending garbage fields get no feedback. `forbidNonWhitelisted` turns that into an error. Both together enforce the contract strictly. + +## Custom Decorators + +The `SetMetadata` factory pattern: a function that wraps `SetMetadata` with a fixed key, read later by a guard or interceptor via `Reflector`. + +```typescript +// Define +export const Roles = (...roles: string[]) => SetMetadata("roles", roles); +export const ResponseMessage = (message: string) => SetMetadata("response-message", message); + +// Use on a handler +@Roles("ADMIN") +@ResponseMessage("Created successfully") +create(@Body() dto: CreateDto) { ... } + +// Read in a guard or interceptor +const roles = this.reflector.getAllAndOverride("roles", [ + context.getHandler(), + context.getClass(), +]); +``` + +`getAllAndOverride` checks handler metadata first, then class metadata — handler wins. This lets you set a default at the controller level and override per-method. + +## Guards + +### Role guard shape + +```typescript +@Injectable() +export class RolesGuard implements CanActivate { + constructor(private readonly reflector: Reflector) {} + + canActivate(context: ExecutionContext): boolean { + const requiredRoles = this.reflector.getAllAndOverride("roles", [context.getHandler(), context.getClass()]); + + if (!requiredRoles?.length) return true; // no roles required — allow through + + const request = context.switchToHttp().getRequest(); + const user = request.user; + + if (!user?.role) throw new ForbiddenException("No role assigned"); + + return requiredRoles.includes(user.role); + } +} +``` + +Return `true` to allow. Return `false` or throw to deny. Throwing a specific `ForbiddenException`/`UnauthorizedException` gives callers a meaningful error — returning `false` produces a generic 403. + +### Extending a third-party guard + +```typescript +@Injectable() +export class ExtendedGuard extends ThirdPartyGuard { + constructor( + @Inject(THIRD_PARTY_TOKEN) client: ThirdPartyClient, + private readonly reflector: Reflector, + ) { + super(client); + } + + override async canActivate(context: ExecutionContext): Promise { + const skip = this.reflector.getAllAndOverride("skip-guard", [context.getHandler(), context.getClass()]); + if (skip) return true; + + return super.canActivate(context); + } +} +``` + +Pattern: extend to add skip/bypass metadata logic, delegate to `super.canActivate()` for the real check. + +### Global guard registration — the `useExisting` gotcha + +```typescript +// Wrong — guard instantiated privately, cannot be overridden in tests +providers: [{ provide: APP_GUARD, useClass: SomeGuard }]; + +// Correct — guard visible as a regular provider, overridable in tests +providers: [{ provide: APP_GUARD, useExisting: SomeGuard }, SomeGuard]; +``` + +`useClass` lets Nest instantiate the guard privately — it becomes invisible to `overrideProvider` in tests, so you can never disable it in your test suite. `useExisting` references the separately-registered provider instead, keeping it visible and overridable. This difference only shows up in tests — the app behaves identically in production either way. + +When a global guard needs injected dependencies, use `useFactory`: + +```typescript +{ + provide: APP_GUARD, + useFactory: (client: SomeClient, reflector: Reflector) => + new ExtendedGuard(client, reflector), + inject: [SOME_CLIENT_TOKEN, Reflector], +} +``` + +## Exception Filters + +```typescript +@Catch() +export class GlobalExceptionFilter implements ExceptionFilter { + catch(exception: unknown, host: ArgumentsHost) { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + + // 1. NestJS HTTP exceptions (NotFoundException, ForbiddenException, etc.) + if (exception instanceof HttpException) { + const status = exception.getStatus(); + const body = exception.getResponse(); + + response.status(status).json(typeof body === "object" ? { statusCode: status, ...body } : { statusCode: status, message: body }); + return; + } + + // 2. Known ORM / infrastructure errors — map to HTTP responses here + // e.g. unique constraint violations → 409, foreign key errors → 400 + + // 3. Fallback — unexpected errors + response.status(500).json({ + statusCode: 500, + message: "Internal server error", + }); + } +} +``` + +Order matters: check the most specific exception types first, fall through to the generic handler last. `@Catch()` with no arguments catches everything — use this for your global filter, not `@Catch(HttpException)`, which silently swallows non-HTTP errors and returns a blank 500. + +## Interceptors + +### Response envelope interceptor + +```typescript +@Injectable() +export class TransformInterceptor implements NestInterceptor { + constructor(private readonly reflector: Reflector) {} + + intercept(context: ExecutionContext, next: CallHandler): Observable { + const response = context.switchToHttp().getResponse(); + const statusCode = response.statusCode ?? 200; + const message = this.reflector.get("response-message", context.getHandler()) ?? "Success"; + + return next.handle().pipe(map((data) => ({ statusCode, message, data }))); + } +} +``` + +Wraps every successful response in a consistent `{ statusCode, message, data }` shape. Per-endpoint messages come from the `@ResponseMessage()` decorator via `Reflector`. Register globally so the shape is enforced everywhere without per-controller setup. + +## Bootstrap Order + +Order matters. Each global enhancement applies to everything registered after it — getting this wrong means pipes don't run, filters miss errors, or interceptors don't wrap responses. + +```typescript +async function bootstrap() { + const app = await NestFactory.create(AppModule); + + // 1. Swagger (optional — before globals so it can inspect the full app) + const config = new DocumentBuilder().setTitle("API").setVersion("1.0").build(); + SwaggerModule.setup("docs", app, SwaggerModule.createDocument(app, config)); + + // 2. Pipes — ValidationPipe has no injected deps, fine to instantiate here + app.useGlobalPipes(new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, transform: true })); + + // 3. Shutdown hooks — graceful termination + app.enableShutdownHooks(); + + // 4. Listen + await app.listen(process.env.PORT ?? 3000); +} +``` + +### Global enhancers with dependency injection + +`app.useGlobalFilters(new MyFilter())` and `app.useGlobalInterceptors(new MyInterceptor())` work for simple enhancers with no injected dependencies. The moment a filter or interceptor needs a `LoggerService`, `ConfigService`, or anything else injected, **they must be registered in `AppModule` instead** — not instantiated with `new` in `main.ts`, which places them outside the DI container entirely. + +Use `APP_FILTER`, `APP_INTERCEPTOR`, and `APP_PIPE` tokens from `@nestjs/core`: + +```typescript +// app.module.ts +import { APP_FILTER, APP_INTERCEPTOR } from "@nestjs/core"; + +@Module({ + providers: [ + { + provide: APP_FILTER, + useClass: GlobalExceptionFilter, // can now inject LoggerService, ConfigService, etc. + }, + { + provide: APP_INTERCEPTOR, + useClass: TransformInterceptor, // Reflector injected automatically by NestJS + }, + ], +}) +export class AppModule {} +``` + +**Default to the `APP_*` token approach for filters and interceptors** — even if they don't need injected deps today, they almost always will when the codebase matures (adding structured logging to a filter is the most common example). `ValidationPipe` is the exception: it rarely needs injected services and `useGlobalPipes(new ValidationPipe({...}))` in `main.ts` is idiomatic and widely used. + +## Env Validation + +Validate environment variables at boot — before any module initializes — so the app refuses to start with missing or malformed config rather than failing at runtime when the first request hits a missing value. + +```typescript +// With any schema validator (Joi, Zod, class-validator — pick one) +ConfigModule.forRoot({ + isGlobal: true, + validationSchema: yourSchemaHere, + // or: validate: yourValidateFunctionHere +}); +``` + +Every required env var should be explicitly named in the schema. Optional vars should have `.optional()` or a `.default()`. The goal: a fresh checkout with a missing `.env` fails loudly at startup, not silently at the first database call three seconds into a request. + +## Anti-Patterns + +- **`new Service()` direct instantiation.** Bypasses DI, breaks injection, breaks tests. Always use constructor injection. +- **`new Filter()` or `new Interceptor()` in `main.ts` when they have injected deps.** Places the enhancer outside the DI container. Use `APP_FILTER`/`APP_INTERCEPTOR` tokens in `AppModule` instead. +- **Business logic in controllers.** Controllers receive requests and call services. Access checks, data transformation, and validation beyond DTO constraints belong in services. +- **`@Catch(HttpException)` as your global filter.** This silently swallows non-HTTP errors and returns a blank 500 with no logging. Use `@Catch()` — no arguments. +- **Update DTOs recreated from scratch.** They drift from the create DTO immediately. Use `PartialType` from `@nestjs/swagger`. +- **`useClass` for global guards registered via `APP_GUARD`.** The guard becomes invisible to `overrideProvider` — you can never bypass it in tests. Use `useExisting` + a separately-registered provider. +- **Exporting every provider from every module by default.** Only export what other modules actually inject. Over-exporting creates invisible coupling that breaks encapsulation. +- **Throwing raw `Error` or plain objects from services.** Throw NestJS `HttpException` subclasses. Raw errors bypass filters and produce inconsistent responses. +- **Skipping `forbidNonWhitelisted: true` on ValidationPipe.** `whitelist: true` alone strips unknown fields silently. Callers sending invalid payloads get no feedback and no error. Use both. diff --git a/.opencode/package-lock.json b/.opencode/package-lock.json new file mode 100644 index 0000000..ad72fd4 --- /dev/null +++ b/.opencode/package-lock.json @@ -0,0 +1,401 @@ +{ + "name": ".opencode", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "dependencies": { + "@opencode-ai/plugin": "1.17.12" + } + }, + "node_modules/@ai-sdk/provider": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/@ai-sdk/provider/-/provider-3.0.8.tgz", + "integrity": "sha512-oGMAgGoQdBXbZqNG0Ze56CHjDZ1IDYOwGYxYjO5KLSlz5HiNQ9udIXsPZ61VWaHGZ5XW/jyjmr6t2xz2jGVwbQ==", + "license": "Apache-2.0", + "dependencies": { + "json-schema": "^0.4.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@msgpackr-extract/msgpackr-extract-darwin-arm64": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/@msgpackr-extract/msgpackr-extract-darwin-arm64/-/msgpackr-extract-darwin-arm64-3.0.4.tgz", + "integrity": "sha512-LCkGo6JDfaBhgST7UpPWgNgLINpcpabaHfyz5OBx75nUYxBsaEPxjnyNjWpeb/xBup/682QnBfRBy2/LvPutZQ==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/@msgpackr-extract/msgpackr-extract-darwin-x64": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/@msgpackr-extract/msgpackr-extract-darwin-x64/-/msgpackr-extract-darwin-x64-3.0.4.tgz", + "integrity": "sha512-zExlW9zUJKZH/tOtVMttwjKa4Xm/3KcNjnE3dPN92uCktwavMxpgCA3MoJK/DOnTWsQgo224OaST27/mPNAf+w==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/@msgpackr-extract/msgpackr-extract-linux-arm": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/@msgpackr-extract/msgpackr-extract-linux-arm/-/msgpackr-extract-linux-arm-3.0.4.tgz", + "integrity": "sha512-Tg3yX65f5GbtXLkrYEHE5oibZG9epyYWas7FogTTEJeDEF9JlXJzKgXaNhT3UXlTOeA+AfZpYZYZ0uPj7Cfquw==", + "cpu": [ + "arm" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@msgpackr-extract/msgpackr-extract-linux-arm64": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/@msgpackr-extract/msgpackr-extract-linux-arm64/-/msgpackr-extract-linux-arm64-3.0.4.tgz", + "integrity": "sha512-dgX0P/9wGPJeHFBG+ZmhgE6bmtMt7NP5CRBGyyktpopdk/mW4POnrpQsSLtKI1dwpc+pPLuXHDh6vvskyQE/sw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@msgpackr-extract/msgpackr-extract-linux-x64": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/@msgpackr-extract/msgpackr-extract-linux-x64/-/msgpackr-extract-linux-x64-3.0.4.tgz", + "integrity": "sha512-8TNXMEjJc3QEy7R/x1INhgiU+XakDAFUzBhaz7+Rbrs8NH5UQeHQxxmzsSBJGyV6I1jW79undiQm8tOI+D+8FQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/@msgpackr-extract/msgpackr-extract-win32-x64": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/@msgpackr-extract/msgpackr-extract-win32-x64/-/msgpackr-extract-win32-x64-3.0.4.tgz", + "integrity": "sha512-CmCXPQrkbwExx3j946/PtHWHbYJiCRBRDl4BlkRQcJB/YOwQxJRTpoo7aTsortjgoJ1x7opzTSxn7C+ASSLVjQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, + "node_modules/@opencode-ai/plugin": { + "version": "1.17.12", + "resolved": "https://registry.npmjs.org/@opencode-ai/plugin/-/plugin-1.17.12.tgz", + "integrity": "sha512-hTfR1sj+EqoYY/lAphxSyt63xs/ZpHFyvP1ekf1UEysdnOEa5mTLTevDu1cq1aHLqcECRGBIjf7iBlNGLCguAg==", + "license": "MIT", + "dependencies": { + "@ai-sdk/provider": "3.0.8", + "@opencode-ai/sdk": "1.17.12", + "effect": "4.0.0-beta.83", + "zod": "4.1.8" + }, + "peerDependencies": { + "@opentui/core": ">=0.3.4", + "@opentui/keymap": ">=0.3.4", + "@opentui/solid": ">=0.3.4" + }, + "peerDependenciesMeta": { + "@opentui/core": { + "optional": true + }, + "@opentui/keymap": { + "optional": true + }, + "@opentui/solid": { + "optional": true + } + } + }, + "node_modules/@opencode-ai/sdk": { + "version": "1.17.12", + "resolved": "https://registry.npmjs.org/@opencode-ai/sdk/-/sdk-1.17.12.tgz", + "integrity": "sha512-N8kazWO0ZLCHWYFuZQt1UJM+bWxY6g1auSG6SvD1+K3+W+nw2qIhDAUGNCD0KVW3bY2LCwvfWvpG2ZbVGCHC0Q==", + "license": "MIT", + "dependencies": { + "cross-spawn": "7.0.6" + } + }, + "node_modules/@standard-schema/spec": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@standard-schema/spec/-/spec-1.1.0.tgz", + "integrity": "sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w==", + "license": "MIT" + }, + "node_modules/cross-spawn": { + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", + "integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==", + "license": "MIT", + "dependencies": { + "path-key": "^3.1.0", + "shebang-command": "^2.0.0", + "which": "^2.0.1" + }, + "engines": { + "node": ">= 8" + } + }, + "node_modules/detect-libc": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/detect-libc/-/detect-libc-2.1.2.tgz", + "integrity": "sha512-Btj2BOOO83o3WyH59e8MgXsxEQVcarkUOpEYrubB0urwnN10yQ364rsiByU11nZlqWYZm05i/of7io4mzihBtQ==", + "license": "Apache-2.0", + "optional": true, + "engines": { + "node": ">=8" + } + }, + "node_modules/effect": { + "version": "4.0.0-beta.83", + "resolved": "https://registry.npmjs.org/effect/-/effect-4.0.0-beta.83.tgz", + "integrity": "sha512-0wsak8RtgGAr9UWSbVDgJHZcUqMSvicHcvaZv1MbMM7MCGgW4Rn/137J1MHQbwYPcwYGxT/IqehFd+UbYuj78w==", + "license": "MIT", + "dependencies": { + "@standard-schema/spec": "^1.1.0", + "fast-check": "^4.8.0", + "find-my-way-ts": "^0.1.6", + "ini": "^7.0.0", + "kubernetes-types": "^1.30.0", + "msgpackr": "^2.0.1", + "multipasta": "^0.2.7", + "toml": "^4.1.1", + "uuid": "^14.0.0", + "yaml": "^2.9.0" + } + }, + "node_modules/fast-check": { + "version": "4.8.0", + "resolved": "https://registry.npmjs.org/fast-check/-/fast-check-4.8.0.tgz", + "integrity": "sha512-GOJ158CUMnN6cSahsv4+ExARvIDuzzinFjkp0E9WtiBa5zcVeLozVkWaE4IzFcc+Y48Wp1EDlUZsXRyAztQcSg==", + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "license": "MIT", + "dependencies": { + "pure-rand": "^8.0.0" + }, + "engines": { + "node": ">=12.17.0" + } + }, + "node_modules/find-my-way-ts": { + "version": "0.1.6", + "resolved": "https://registry.npmjs.org/find-my-way-ts/-/find-my-way-ts-0.1.6.tgz", + "integrity": "sha512-a85L9ZoXtNAey3Y6Z+eBWW658kO/MwR7zIafkIUPUMf3isZG0NCs2pjW2wtjxAKuJPxMAsHUIP4ZPGv0o5gyTA==", + "license": "MIT" + }, + "node_modules/ini": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/ini/-/ini-7.0.0.tgz", + "integrity": "sha512-ifK0CgjALofS5bkrcTy4RaQ9Vx2Knf/eLeIO+NaswQEpH1UblrtTSCIvN71qQDMq0PeQ/SSPojvEJp9vvvfr+w==", + "license": "ISC", + "engines": { + "node": "^22.22.2 || ^24.15.0 || >=26.0.0" + } + }, + "node_modules/isexe": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz", + "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==", + "license": "ISC" + }, + "node_modules/json-schema": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/json-schema/-/json-schema-0.4.0.tgz", + "integrity": "sha512-es94M3nTIfsEPisRafak+HDLfHXnKBhV3vU5eqPcS3flIWqcxJWgXHXiey3YrpaNsanY5ei1VoYEbOzijuq9BA==", + "license": "(AFL-2.1 OR BSD-3-Clause)" + }, + "node_modules/kubernetes-types": { + "version": "1.30.0", + "resolved": "https://registry.npmjs.org/kubernetes-types/-/kubernetes-types-1.30.0.tgz", + "integrity": "sha512-Dew1okvhM/SQcIa2rcgujNndZwU8VnSapDgdxlYoB84ZlpAD43U6KLAFqYo17ykSFGHNPrg0qry0bP+GJd9v7Q==", + "license": "Apache-2.0" + }, + "node_modules/msgpackr": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-2.0.4.tgz", + "integrity": "sha512-o1C5KRmuRt+apqMr1HuGSqWStZoRBUpEsCsl15uM9VdAF1qHLtvMOU2En747EnTyEl6c4pzPewRMFF31s1CNbA==", + "license": "MIT", + "optionalDependencies": { + "msgpackr-extract": "^3.0.4" + } + }, + "node_modules/msgpackr-extract": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/msgpackr-extract/-/msgpackr-extract-3.0.4.tgz", + "integrity": "sha512-4kmO/MdyUIkLIvTPr8VHLil4AtoKIoniWPIEk5+CDy0xnWC84azhSFmuJ7PxZdsYtiP5kEeQsORAVIeMgxT+Hw==", + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "dependencies": { + "node-gyp-build-optional-packages": "5.2.2" + }, + "bin": { + "download-msgpackr-prebuilds": "bin/download-prebuilds.js" + }, + "optionalDependencies": { + "@msgpackr-extract/msgpackr-extract-darwin-arm64": "3.0.4", + "@msgpackr-extract/msgpackr-extract-darwin-x64": "3.0.4", + "@msgpackr-extract/msgpackr-extract-linux-arm": "3.0.4", + "@msgpackr-extract/msgpackr-extract-linux-arm64": "3.0.4", + "@msgpackr-extract/msgpackr-extract-linux-x64": "3.0.4", + "@msgpackr-extract/msgpackr-extract-win32-x64": "3.0.4" + } + }, + "node_modules/multipasta": { + "version": "0.2.7", + "resolved": "https://registry.npmjs.org/multipasta/-/multipasta-0.2.7.tgz", + "integrity": "sha512-KPA58d68KgGil15oDqXjkUBEBYc00XvbPj5/X+dyzeo/lWm9Nc25pQRlf1D+gv4OpK7NM0J1odrbu9JNNGvynA==", + "license": "MIT" + }, + "node_modules/node-gyp-build-optional-packages": { + "version": "5.2.2", + "resolved": "https://registry.npmjs.org/node-gyp-build-optional-packages/-/node-gyp-build-optional-packages-5.2.2.tgz", + "integrity": "sha512-s+w+rBWnpTMwSFbaE0UXsRlg7hU4FjekKU4eyAih5T8nJuNZT1nNsskXpxmeqSK9UzkBl6UgRlnKc8hz8IEqOw==", + "license": "MIT", + "optional": true, + "dependencies": { + "detect-libc": "^2.0.1" + }, + "bin": { + "node-gyp-build-optional-packages": "bin.js", + "node-gyp-build-optional-packages-optional": "optional.js", + "node-gyp-build-optional-packages-test": "build-test.js" + } + }, + "node_modules/path-key": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/path-key/-/path-key-3.1.1.tgz", + "integrity": "sha512-ojmeN0qd+y0jszEtoY48r0Peq5dwMEkIlCOu6Q5f41lfkswXuKtYrhgoTpLnyIcHm24Uhqx+5Tqm2InSwLhE6Q==", + "license": "MIT", + "engines": { + "node": ">=8" + } + }, + "node_modules/pure-rand": { + "version": "8.4.1", + "resolved": "https://registry.npmjs.org/pure-rand/-/pure-rand-8.4.1.tgz", + "integrity": "sha512-c58R2+SPFcSIPXoU834QN/KPDDOSd8sXcSrqf6e83Me6Rrp1EYkxukkjXMVrKvKaADs1SOyNkWdfvLf6zY8qLQ==", + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "license": "MIT" + }, + "node_modules/shebang-command": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", + "integrity": "sha512-kHxr2zZpYtdmrN1qDjrrX/Z1rR1kG8Dx+gkpK1G4eXmvXswmcE1hTWBWYUzlraYw1/yZp6YuDY77YtvbN0dmDA==", + "license": "MIT", + "dependencies": { + "shebang-regex": "^3.0.0" + }, + "engines": { + "node": ">=8" + } + }, + "node_modules/shebang-regex": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/shebang-regex/-/shebang-regex-3.0.0.tgz", + "integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==", + "license": "MIT", + "engines": { + "node": ">=8" + } + }, + "node_modules/toml": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/toml/-/toml-4.1.2.tgz", + "integrity": "sha512-m0vXfHODcw3gk+KONAOlVQ5yNHc3yS3B1ybM3HS1vqDoS0RWTDDVBVVTYi8hH0k+2OM1vmo9fb1WX9EVqjqfHA==", + "license": "MIT", + "engines": { + "node": ">=20" + } + }, + "node_modules/uuid": { + "version": "14.0.1", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-14.0.1.tgz", + "integrity": "sha512-6ZxzVpzDXDa3bJWaHilVayA+BH/1zmxCJoVgvmqJnid/gPoKHxUrS/aC/T6LGQtNHT+XHG9fXPJB4d+IrU30Ew==", + "funding": [ + "https://github.com/sponsors/broofa", + "https://github.com/sponsors/ctavan" + ], + "license": "MIT", + "bin": { + "uuid": "dist-node/bin/uuid" + } + }, + "node_modules/which": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", + "integrity": "sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==", + "license": "ISC", + "dependencies": { + "isexe": "^2.0.0" + }, + "bin": { + "node-which": "bin/node-which" + }, + "engines": { + "node": ">= 8" + } + }, + "node_modules/yaml": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.0.tgz", + "integrity": "sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==", + "license": "ISC", + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } + }, + "node_modules/zod": { + "version": "4.1.8", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.1.8.tgz", + "integrity": "sha512-5R1P+WwQqmmMIEACyzSvo4JXHY5WiAFHRMg+zBZKgKS+Q1viRa0C1hmUKtHltoIFKtIdki3pRxkmpP74jnNYHQ==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + } + } +} diff --git a/AGENTS.md b/AGENTS.md index 54623e7..a6dedb3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,10 @@ Do not load any skill by default. Check the task first — only invoke a skill i - `/architect` — before building something non-trivial with no plan yet - `/review` — when a feature is done and needs a production check - `/recover` — when something is broken and the fix isn't obvious -- `/nestjs-testing` — when writing or fixing any `*.spec.ts` or `*.e2e-spec.ts` file +- `/nestjs-jest-prisma-testing` — when writing or fixing any `*.spec.ts` or `*.e2e-spec.ts` file +- `/nestjs-patterns` — when building NestJS feature modules, services, or controllers +- `/better-auth-best-practices` — when configuring Better Auth server, client, database adapters, session management, or plugins +- `/create-auth-skill` — when scaffolding or implementing Better Auth authentication - `/remember` — at the start of a new session to restore context, and at the end to save progress diff --git a/memory.md b/memory.md index 6c76c81..68f221c 100644 --- a/memory.md +++ b/memory.md @@ -1,43 +1,43 @@ -# Memory — Swagger, Global Exception Filter, Negative Tests, Layer 3 cleanup +# Memory — Code review session: NestJS patterns alignment -Last updated: 2026-06-30 +Last updated: 2026-07-01 ## What was built -- **Swagger documentation** — `@nestjs/swagger@11.4.5` installed. `DocumentBuilder` + `SwaggerModule.setup("docs", ...)` in `main.ts`. `@ApiTags`, `@ApiOperation`, `@ApiBearerAuth` on all 4 controllers. `@ApiProperty` / `@ApiPropertyOptional` on all DTOs. Bearer auth button in Swagger UI. -- **Barrel file for Prisma** — `src/common/prisma.ts` exports `Prisma` + `PrismaClient` from generated path. Used by the exception filter. -- **Global exception filter** — `src/common/filters/all-exceptions.filter.ts` with proper `instanceof Prisma.PrismaClientKnownRequestError` type guard. Handles HttpExceptions, Prisma P2002/P2025, validation error shapes, with `Logger` for unhandled errors. -- **Bearer auth plugin** — `bearer()` from `better-auth/plugins/bearer` added to `auth.config.ts`. Login now returns `set-auth-token` header for Swagger use. -- **Negative ownership tests** — `canAccessTask` test block added to `task.service.spec.ts` (4 tests: admin bypass, own task, other's task → ForbiddenException, missing → NotFoundException). -- **Layer 3 polish** — Removed unused `:userId` param from `unassign` route in `task.controller.ts`. Formatted long import line in `task.service.ts`. +- **`@CurrentUser()` decorator** — `src/common/decorators/current-user.decorator.ts`. A `createParamDecorator` that extracts `req.user` and throws `UnauthorizedException` if absent. Returns fully-typed `Express.User`. +- **`@Public()` decorator** — `src/common/decorators/public.decorator.ts`. Metadata flag for routes that bypass the global `AuthGuard`. +- **`AuthGuard`** — `src/common/guards/auth.guard.ts`. Global guard checks `req.user` exists, skips for `@Public()` routes. Registered via `APP_GUARD` with `useExisting`. -## Decisions made +## What was fixed -- **Swagger auth is Bearer, not cookie** — Better Auth's `bearer()` plugin enables `Authorization: Bearer `. The token comes from `set-auth-token` header on login. Cookie auth for Swagger UI was unreliable with HttpOnly cookies. -- **Exception filter uses barrel import** — `src/common/prisma.ts` re-exports from the generated path to avoid fragile relative paths everywhere. -- **`canAccessTask` is the single access gate** — used by `CommentService` to check task access before allowing comment reads/writes. All negative edge cases (non-admin accessing other's task, missing task) are now tested directly. +| Issue | Files | +|---|---| +| Filter/interceptor instantiated with `new` in `main.ts` — outside DI | `main.ts`, `app.module.ts` | +| `ArcjetOptionalGuard` not overridable in tests (`useFactory`) | `arcjet.module.ts` | +| `req.user!` assertions and repeated `if (!user) throw` in controllers | All 3 feature controllers → `@CurrentUser()` | +| `UpdateCommentDto` hand-written instead of `PartialType` | `update-comment.dto.ts` | +| `Record` instead of `Prisma.TaskWhereInput` | `task.service.ts` | +| Tests casting mock `Request` objects via `as unknown` | 3 controller specs — now pass user directly | +| `ValidationPipe` missing `transform: true` | `main.ts` | +| `mockUser.role` missing `as const` | `user.service.spec.ts` | -## Problems solved +## Decisions made -- Swagger Authorize button didn't work with cookies (HttpOnly). Switched to Bearer plugin — token from login response, paste into Swagger. -- Exception filter had type safety hole (`as PrismaClientError`) and no logging. Fixed with `instanceof Prisma.PrismaClientKnownRequestError` + `Logger`. -- `canAccessTask` was only tested through mocks in CommentService, never directly. Added 4 tests in TaskService spec. -- Unused route parameter `:userId` in `DELETE /:id/assign/:userId` confused the API surface. +- **`@CurrentUser()` over `@Req()`** — eliminates non-null assertions (`!`) and repeated null checks. One decorator handles the unauthorized case for every handler. TypeScript infers `Express.User`, not `Express.User | undefined`. +- **Global AuthGuard + controller-level `@CurrentUser()`** — guard is defense-in-depth + `@Public()` mechanism. Decorator is for TypeScript type narrowing. Both coexist without conflict. +- **`useExisting` for all `APP_*` providers** — keeps guards/filters/interceptors visible in DI for test overrides. ## Current state -- All 7 modules built (User, Project, Task, Comment, Auth, Database, Arcjet) -- Swagger at `/docs` with full endpoint descriptions and DTOs -- Global exception filter handles all error shapes consistently -- `bearer()` plugin enabled — login returns `set-auth-token` -- 74/74 tests pass, build clean -- PLAN.md fully checked except `scope` items (no pagination, soft-delete, etc.) +- All 5 code review issues fixed per `/nestjs-patterns` and `/nestjs-jest-prisma-testing` skills +- 81/81 tests pass, build clean +- No `as any` anywhere in test files +- All controllers use `@CurrentUser()` instead of `req.user` ## Next session starts with - `/remember restore` (required first action) -- Nothing urgent — codebase is in a complete, tested state. Possible next steps: pagination, soft-delete, or deployment setup. ## Open questions -- None currently. +- None diff --git a/skills-lock.json b/skills-lock.json index 9c8c47e..8a5ec7b 100644 --- a/skills-lock.json +++ b/skills-lock.json @@ -19,6 +19,18 @@ "skillPath": "better-auth/create-auth/SKILL.md", "computedHash": "393e8d2d795fa5797c9a4e0665f29183ea2e140233db59746d510340a10456e6" }, + "nestjs-jest-prisma-testing": { + "source": "kenneth-loto/skills", + "sourceType": "github", + "skillPath": "skills/nestjs/nestjs-jest-prisma-testing/SKILL.md", + "computedHash": "1a99a2c92ac6887ce46e17d0457dafb13b24a5df9294884ea556b916aa20f62b" + }, + "nestjs-patterns": { + "source": "kenneth-loto/skills", + "sourceType": "github", + "skillPath": "skills/nestjs/nestjs-patterns/SKILL.md", + "computedHash": "be9b60e7c0b7ffae518a020386f9ee2760b7f520b30779b15672cc6e239bd970" + }, "recover": { "source": "JavaScript-Mastery-Pro/skills", "sourceType": "github", diff --git a/src/app.controller.ts b/src/app.controller.ts index ad65348..7994e9f 100644 --- a/src/app.controller.ts +++ b/src/app.controller.ts @@ -1,11 +1,11 @@ import { Controller, Get } from "@nestjs/common"; -import { AllowAnonymous } from "@thallesp/nestjs-better-auth"; +import { Public } from "./common/decorators/public.decorator.js"; import { SkipArcjet } from "./common/guards/arcjet-optional.guard.js"; @Controller() export class AppController { @Get() - @AllowAnonymous() + @Public() @SkipArcjet() getRoot() { return { diff --git a/src/app.module.ts b/src/app.module.ts index 7fe592b..051917e 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -1,7 +1,11 @@ import { Module } from "@nestjs/common"; import { ConfigModule } from "@nestjs/config"; +import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR } from "@nestjs/core"; import { AuthModule } from "@thallesp/nestjs-better-auth"; import { AppController } from "./app.controller.js"; +import { AllExceptionsFilter } from "./common/filters/all-exceptions.filter.js"; +import { AuthGuard } from "./common/guards/auth.guard.js"; +import { TransformInterceptor } from "./common/interceptors/transform.interceptor.js"; import { createAuth } from "./lib/auth/auth.config.js"; import { envValidationSchema } from "./lib/config/env.config.js"; import { PrismaModule } from "./lib/database/prisma.module.js"; @@ -32,5 +36,11 @@ import { UserModule } from "./module/user/user.module.js"; ArcjetSecurityModule, ], controllers: [AppController], + providers: [ + AuthGuard, + { provide: APP_GUARD, useExisting: AuthGuard }, + { provide: APP_FILTER, useClass: AllExceptionsFilter }, + { provide: APP_INTERCEPTOR, useClass: TransformInterceptor }, + ], }) export class AppModule {} diff --git a/src/common/decorators/current-user.decorator.ts b/src/common/decorators/current-user.decorator.ts new file mode 100644 index 0000000..248a297 --- /dev/null +++ b/src/common/decorators/current-user.decorator.ts @@ -0,0 +1,16 @@ +import { + createParamDecorator, + ExecutionContext, + UnauthorizedException, +} from "@nestjs/common"; +import type { Request } from "express"; + +export const CurrentUser = createParamDecorator( + (_data: unknown, ctx: ExecutionContext) => { + const request = ctx.switchToHttp().getRequest(); + + if (!request.user) throw new UnauthorizedException(); + + return request.user; + }, +); diff --git a/src/common/decorators/public.decorator.ts b/src/common/decorators/public.decorator.ts new file mode 100644 index 0000000..8fa2f14 --- /dev/null +++ b/src/common/decorators/public.decorator.ts @@ -0,0 +1,4 @@ +import { SetMetadata } from "@nestjs/common"; + +export const IS_PUBLIC = "isPublic"; +export const Public = () => SetMetadata(IS_PUBLIC, true); diff --git a/src/common/guards/auth.guard.ts b/src/common/guards/auth.guard.ts new file mode 100644 index 0000000..33fcf60 --- /dev/null +++ b/src/common/guards/auth.guard.ts @@ -0,0 +1,33 @@ +import { + type CanActivate, + type ExecutionContext, + Injectable, + UnauthorizedException, +} from "@nestjs/common"; +import { Reflector } from "@nestjs/core"; +import type { Request } from "express"; +import { IS_PUBLIC } from "../decorators/public.decorator.js"; + +@Injectable() +export class AuthGuard implements CanActivate { + constructor(private readonly reflector: Reflector) {} + + canActivate(context: ExecutionContext): boolean { + const isPublic = this.reflector.getAllAndOverride(IS_PUBLIC, [ + context.getHandler(), + context.getClass(), + ]); + + if (isPublic) { + return true; + } + + const request = context.switchToHttp().getRequest(); + + if (!request.user) { + throw new UnauthorizedException(); + } + + return true; + } +} diff --git a/src/lib/security/arcjet.module.ts b/src/lib/security/arcjet.module.ts index 8246b43..ef57b0e 100644 --- a/src/lib/security/arcjet.module.ts +++ b/src/lib/security/arcjet.module.ts @@ -1,6 +1,4 @@ -import type { ArcjetNest } from "@arcjet/nest"; import { - ARCJET, ArcjetModule, cloudflare, detectBot, @@ -9,7 +7,7 @@ import { } from "@arcjet/nest"; import { Module } from "@nestjs/common"; import { ConfigService } from "@nestjs/config"; -import { APP_GUARD, Reflector } from "@nestjs/core"; +import { APP_GUARD } from "@nestjs/core"; import { ArcjetOptionalGuard } from "../../common/guards/arcjet-optional.guard.js"; @Module({ @@ -36,12 +34,8 @@ import { ArcjetOptionalGuard } from "../../common/guards/arcjet-optional.guard.j }), ], providers: [ - { - provide: APP_GUARD, - useFactory: (aj: ArcjetNest, reflector: Reflector) => - new ArcjetOptionalGuard(aj, reflector), - inject: [ARCJET, Reflector], - }, + ArcjetOptionalGuard, + { provide: APP_GUARD, useExisting: ArcjetOptionalGuard }, ], }) export class ArcjetSecurityModule {} diff --git a/src/main.ts b/src/main.ts index abbb16e..de7737c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,11 +1,9 @@ import { BadRequestException, ValidationPipe } from "@nestjs/common"; import { ConfigService } from "@nestjs/config"; -import { NestFactory, Reflector } from "@nestjs/core"; +import { NestFactory } from "@nestjs/core"; import type { NestExpressApplication } from "@nestjs/platform-express"; import { DocumentBuilder, SwaggerModule } from "@nestjs/swagger"; import { AppModule } from "./app.module.js"; -import { AllExceptionsFilter } from "./common/filters/all-exceptions.filter.js"; -import { TransformInterceptor } from "./common/interceptors/transform.interceptor.js"; async function bootstrap() { const app = await NestFactory.create(AppModule, { @@ -27,12 +25,11 @@ async function bootstrap() { const document = SwaggerModule.createDocument(app, swaggerConfig); SwaggerModule.setup("docs", app, document); - app.useGlobalFilters(new AllExceptionsFilter()); - app.useGlobalInterceptors(new TransformInterceptor(new Reflector())); app.useGlobalPipes( new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, + transform: true, exceptionFactory: (errors) => { const result = errors.map((error) => ({ field: error.property, diff --git a/src/module/comment/comment.controller.spec.ts b/src/module/comment/comment.controller.spec.ts index 1024ebf..bbca966 100644 --- a/src/module/comment/comment.controller.spec.ts +++ b/src/module/comment/comment.controller.spec.ts @@ -1,7 +1,5 @@ import { jest } from "@jest/globals"; -import { UnauthorizedException } from "@nestjs/common"; import { Test, TestingModule } from "@nestjs/testing"; -import type { Request } from "express"; import { CommentController } from "./comment.controller.js"; import { CommentService } from "./comment.service.js"; @@ -15,6 +13,12 @@ const mockComment = { author: { id: "user-1", name: "Test User", email: "test@example.com" }, }; +const mockUser: Express.User = { + id: "user-1", + email: "test@example.com", + role: "MEMBER", +}; + describe("CommentController", () => { let controller: CommentController; let commentService: jest.Mocked; @@ -37,7 +41,7 @@ describe("CommentController", () => { }).compile(); controller = module.get(CommentController); - commentService = module.get(CommentService); + commentService = module.get(CommentService) as jest.Mocked; }); afterEach(() => { @@ -46,12 +50,9 @@ describe("CommentController", () => { describe("findAll", () => { it("returns comments for the task", async () => { - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; commentService.findByTask.mockResolvedValue([mockComment]); - const result = await controller.findAll("task-1", req); + const result = await controller.findAll("task-1", mockUser); expect(result).toEqual([mockComment]); expect(commentService.findByTask).toHaveBeenCalledWith( @@ -60,24 +61,13 @@ describe("CommentController", () => { "MEMBER", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.findAll("task-1", req)).toThrow( - UnauthorizedException, - ); - }); }); describe("findById", () => { it("returns a single comment", async () => { - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; commentService.findByIdScoped.mockResolvedValue(mockComment); - const result = await controller.findById("task-1", "comment-1", req); + const result = await controller.findById("task-1", "comment-1", mockUser); expect(result).toEqual(mockComment); expect(commentService.findByIdScoped).toHaveBeenCalledWith( @@ -86,26 +76,15 @@ describe("CommentController", () => { "MEMBER", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.findById("task-1", "comment-1", req)).toThrow( - UnauthorizedException, - ); - }); }); describe("create", () => { const dto = { content: "New comment" }; it("returns the created comment", async () => { - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; commentService.create.mockResolvedValue(mockComment); - const result = await controller.create("task-1", dto, req); + const result = await controller.create("task-1", dto, mockUser); expect(result).toEqual(mockComment); expect(commentService.create).toHaveBeenCalledWith( @@ -115,14 +94,6 @@ describe("CommentController", () => { "MEMBER", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.create("task-1", dto, req)).toThrow( - UnauthorizedException, - ); - }); }); describe("update", () => { @@ -130,12 +101,9 @@ describe("CommentController", () => { it("returns the updated comment", async () => { const updated = { ...mockComment, content: "Updated" }; - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; commentService.update.mockResolvedValue(updated); - const result = await controller.update("comment-1", dto, req); + const result = await controller.update("comment-1", dto, mockUser); expect(result).toEqual(updated); expect(commentService.update).toHaveBeenCalledWith( @@ -144,22 +112,11 @@ describe("CommentController", () => { "user-1", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.update("comment-1", dto, req)).toThrow( - UnauthorizedException, - ); - }); }); describe("delete", () => { it("deletes the comment", async () => { - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; - await controller.delete("comment-1", req); + await controller.delete("comment-1", mockUser); expect(commentService.delete).toHaveBeenCalledWith( "comment-1", @@ -167,13 +124,5 @@ describe("CommentController", () => { "MEMBER", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.delete("comment-1", req)).toThrow( - UnauthorizedException, - ); - }); }); }); diff --git a/src/module/comment/comment.controller.ts b/src/module/comment/comment.controller.ts index 050f9c7..adf294a 100644 --- a/src/module/comment/comment.controller.ts +++ b/src/module/comment/comment.controller.ts @@ -6,11 +6,9 @@ import { Param, Patch, Post, - Req, - UnauthorizedException, } from "@nestjs/common"; import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"; -import type { Request } from "express"; +import { CurrentUser } from "../../common/decorators/current-user.decorator.js"; import { ResponseMessage } from "../../common/decorators/response-message.decorator.js"; import { CommentService } from "./comment.service.js"; import { CreateCommentDto } from "./dto/create-comment.dto.js"; @@ -24,13 +22,7 @@ export class CommentController { @Get() @ApiOperation({ summary: "List comments for a task" }) - findAll(@Param("taskId") taskId: string, @Req() req: Request) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - + findAll(@Param("taskId") taskId: string, @CurrentUser() user: Express.User) { return this.commentService.findByTask(taskId, user.id, user.role); } @@ -39,14 +31,8 @@ export class CommentController { findById( @Param("taskId") _taskId: string, @Param("id") id: string, - @Req() req: Request, + @CurrentUser() user: Express.User, ) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - return this.commentService.findByIdScoped(id, user.id, user.role); } @@ -56,14 +42,8 @@ export class CommentController { create( @Param("taskId") taskId: string, @Body() dto: CreateCommentDto, - @Req() req: Request, + @CurrentUser() user: Express.User, ) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - return this.commentService.create(dto, taskId, user.id, user.role); } @@ -73,27 +53,15 @@ export class CommentController { update( @Param("id") id: string, @Body() dto: UpdateCommentDto, - @Req() req: Request, + @CurrentUser() user: Express.User, ) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - return this.commentService.update(id, dto, user.id); } @Delete(":id") @ApiOperation({ summary: "Delete a comment (author or admin)" }) @ResponseMessage("Comment deleted successfully") - delete(@Param("id") id: string, @Req() req: Request) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - + delete(@Param("id") id: string, @CurrentUser() user: Express.User) { return this.commentService.delete(id, user.id, user.role); } } diff --git a/src/module/comment/dto/update-comment.dto.ts b/src/module/comment/dto/update-comment.dto.ts index 923b112..6654873 100644 --- a/src/module/comment/dto/update-comment.dto.ts +++ b/src/module/comment/dto/update-comment.dto.ts @@ -1,9 +1,4 @@ -import { ApiProperty } from "@nestjs/swagger"; -import { IsNotEmpty, IsString } from "class-validator"; +import { PartialType } from "@nestjs/swagger"; +import { CreateCommentDto } from "./create-comment.dto.js"; -export class UpdateCommentDto { - @ApiProperty() - @IsString() - @IsNotEmpty() - content: string; -} +export class UpdateCommentDto extends PartialType(CreateCommentDto) {} diff --git a/src/module/project/project.controller.spec.ts b/src/module/project/project.controller.spec.ts index 054e21a..fd22980 100644 --- a/src/module/project/project.controller.spec.ts +++ b/src/module/project/project.controller.spec.ts @@ -1,6 +1,5 @@ import { jest } from "@jest/globals"; import { Test, TestingModule } from "@nestjs/testing"; -import type { Request } from "express"; import { ProjectController } from "./project.controller.js"; import { ProjectService } from "./project.service.js"; @@ -13,6 +12,12 @@ const mockProject = { user: { id: "user-1", name: "Test User", email: "test@example.com" }, }; +const mockUser: Express.User = { + id: "user-1", + email: "test@example.com", + role: "MEMBER", +}; + describe("ProjectController", () => { let controller: ProjectController; let service: jest.Mocked; @@ -35,7 +40,7 @@ describe("ProjectController", () => { }).compile(); controller = module.get(ProjectController); - service = module.get(ProjectService); + service = module.get(ProjectService) as jest.Mocked; }); afterEach(() => { @@ -63,12 +68,9 @@ describe("ProjectController", () => { describe("create", () => { it("calls service.create with dto and req.user.id", async () => { const dto = { name: "New Project" }; - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; service.create.mockResolvedValue({ ...mockProject, name: "New Project" }); - const result = await controller.create(dto, req); + const result = await controller.create(dto, mockUser); expect(result).toEqual({ ...mockProject, name: "New Project" }); expect(service.create).toHaveBeenCalledWith(dto, "user-1"); diff --git a/src/module/project/project.controller.ts b/src/module/project/project.controller.ts index 5876198..b50f317 100644 --- a/src/module/project/project.controller.ts +++ b/src/module/project/project.controller.ts @@ -6,12 +6,10 @@ import { Param, Patch, Post, - Req, - UnauthorizedException, UseGuards, } from "@nestjs/common"; import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"; -import type { Request } from "express"; +import { CurrentUser } from "../../common/decorators/current-user.decorator.js"; import { ResponseMessage } from "../../common/decorators/response-message.decorator.js"; import { Roles } from "../../common/decorators/roles.decorator.js"; import { RolesGuard } from "../../common/guards/roles.guard.js"; @@ -42,13 +40,7 @@ export class ProjectController { @Roles("ADMIN") @ApiOperation({ summary: "Create a project (admin only)" }) @ResponseMessage("Project created successfully") - create(@Body() dto: CreateProjectDto, @Req() req: Request) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - + create(@Body() dto: CreateProjectDto, @CurrentUser() user: Express.User) { return this.projectService.create(dto, user.id); } diff --git a/src/module/task/task.controller.spec.ts b/src/module/task/task.controller.spec.ts index 13b939f..e3f575c 100644 --- a/src/module/task/task.controller.spec.ts +++ b/src/module/task/task.controller.spec.ts @@ -1,7 +1,5 @@ import { jest } from "@jest/globals"; -import { UnauthorizedException } from "@nestjs/common"; import { Test, TestingModule } from "@nestjs/testing"; -import type { Request } from "express"; import { TaskController } from "./task.controller.js"; import { TaskService } from "./task.service.js"; @@ -19,6 +17,12 @@ const mockTask = { assignee: { id: "user-1", name: "Test User", email: "test@example.com" }, }; +const mockUser: Express.User = { + id: "user-1", + email: "test@example.com", + role: "MEMBER", +}; + describe("TaskController", () => { let controller: TaskController; let taskService: jest.Mocked; @@ -44,7 +48,7 @@ describe("TaskController", () => { }).compile(); controller = module.get(TaskController); - taskService = module.get(TaskService); + taskService = module.get(TaskService) as jest.Mocked; }); afterEach(() => { @@ -53,12 +57,9 @@ describe("TaskController", () => { describe("findAll", () => { it("returns tasks for the user", async () => { - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; taskService.findByProject.mockResolvedValue([mockTask]); - const result = await controller.findAll("proj-1", req); + const result = await controller.findAll("proj-1", mockUser); expect(result).toEqual([mockTask]); expect(taskService.findByProject).toHaveBeenCalledWith( @@ -67,24 +68,13 @@ describe("TaskController", () => { "MEMBER", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.findAll("proj-1", req)).toThrow( - UnauthorizedException, - ); - }); }); describe("findById", () => { it("returns a single task for authorized user", async () => { - const req = { - user: { id: "user-1", email: "test@example.com", role: "MEMBER" }, - } as unknown as Request; taskService.findByIdScoped.mockResolvedValue(mockTask); - const result = await controller.findById("task-1", req); + const result = await controller.findById("task-1", mockUser); expect(result).toEqual(mockTask); expect(taskService.findByIdScoped).toHaveBeenCalledWith( @@ -93,14 +83,6 @@ describe("TaskController", () => { "MEMBER", ); }); - - it("throws UnauthorizedException when no user on request", () => { - const req = { user: undefined } as unknown as Request; - - expect(() => controller.findById("task-1", req)).toThrow( - UnauthorizedException, - ); - }); }); describe("create", () => { diff --git a/src/module/task/task.controller.ts b/src/module/task/task.controller.ts index e6f83ce..4f1efb5 100644 --- a/src/module/task/task.controller.ts +++ b/src/module/task/task.controller.ts @@ -6,12 +6,10 @@ import { Param, Patch, Post, - Req, - UnauthorizedException, UseGuards, } from "@nestjs/common"; import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"; -import type { Request } from "express"; +import { CurrentUser } from "../../common/decorators/current-user.decorator.js"; import { ResponseMessage } from "../../common/decorators/response-message.decorator.js"; import { Roles } from "../../common/decorators/roles.decorator.js"; import { RolesGuard } from "../../common/guards/roles.guard.js"; @@ -27,25 +25,16 @@ export class TaskController { @Get() @ApiOperation({ summary: "List tasks for a project" }) - findAll(@Param("projectId") projectId: string, @Req() req: Request) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - + findAll( + @Param("projectId") projectId: string, + @CurrentUser() user: Express.User, + ) { return this.taskService.findByProject(projectId, user.id, user.role); } @Get(":id") @ApiOperation({ summary: "Get task by ID" }) - findById(@Param("id") id: string, @Req() req: Request) { - const user = req.user; - - if (!user) { - throw new UnauthorizedException(); - } - + findById(@Param("id") id: string, @CurrentUser() user: Express.User) { return this.taskService.findByIdScoped(id, user.id, user.role); } diff --git a/src/module/task/task.service.ts b/src/module/task/task.service.ts index 54d8ab3..b8c4398 100644 --- a/src/module/task/task.service.ts +++ b/src/module/task/task.service.ts @@ -4,6 +4,7 @@ import { Injectable, NotFoundException, } from "@nestjs/common"; +import { Prisma } from "../../common/prisma.js"; import { PrismaService } from "../../lib/database/prisma.service.js"; import { CreateTaskDto } from "./dto/create-task.dto.js"; import { UpdateTaskDto } from "./dto/update-task.dto.js"; @@ -26,7 +27,7 @@ export class TaskService { throw new NotFoundException(`Project with id "${projectId}" not found`); } - const where: Record = { projectId }; + const where: Prisma.TaskWhereInput = { projectId }; if (role !== "ADMIN") { where.assigneeId = userId; } diff --git a/src/module/user/user.controller.spec.ts b/src/module/user/user.controller.spec.ts index cce242c..5fd9dae 100644 --- a/src/module/user/user.controller.spec.ts +++ b/src/module/user/user.controller.spec.ts @@ -36,7 +36,7 @@ describe("UserController", () => { }).compile(); controller = module.get(UserController); - service = module.get(UserService); + service = module.get(UserService) as jest.Mocked; }); afterEach(() => { diff --git a/src/module/user/user.service.spec.ts b/src/module/user/user.service.spec.ts index fe253c8..a25210a 100644 --- a/src/module/user/user.service.spec.ts +++ b/src/module/user/user.service.spec.ts @@ -9,7 +9,7 @@ const mockUser = { id: "user-1", email: "test@example.com", name: "Test User", - role: "MEMBER", + role: "MEMBER" as const, emailVerified: false, image: null, createdAt: new Date(), diff --git a/test/app.e2e-spec.ts b/test/app.e2e-spec.ts index 0706a19..41b7c27 100644 --- a/test/app.e2e-spec.ts +++ b/test/app.e2e-spec.ts @@ -9,7 +9,7 @@ type App = Server | string; describe("AppController (e2e)", () => { let app: INestApplication; - beforeEach(async () => { + beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule], }).compile(); @@ -28,7 +28,7 @@ describe("AppController (e2e)", () => { }); }); - afterEach(async () => { + afterAll(async () => { await app.close(); }); });