Code Review Guidelines

Overview

This document is the code review standard for NestJS microservice projects built on this blueprint. Use it as a reference before opening a PR, during review, and when onboarding new engineers.

The goal of every review is the same: make the code better, the architecture stronger, and the reviewer’s intent clear — while keeping the author’s confidence intact.

---

1. Naming Conventions

Microservices

• Rule: Use kebab-case and suffix bounded domain services with -bc
• Examples:
  • ✅ orders-bc, notifications-bc, scheduling-bc
  • ❌ ordersBC, orders_bc, Orders-BC

Modules

• Rule: Singular + Module
• Examples:
  • ✅ OrderModule, ConfigModule
  • ❌ OrdersModule, orderModule

Controllers

• Rule: Plural + Controller
• Examples:
  • ✅ OrdersController, UsersController
  • ❌ OrderController, orderController

Services

• Rule: Plural + Service, or action-based for complex workflows
• Examples:
  • ✅ OrdersService, AppointmentCheckInService
  • ❌ OrderService (too generic if domain is complex)

Files

• Rule: kebab-case
• Examples:
  • ✅ create-order.dto.ts, appointment-check-in.service.ts
  • ❌ CreateOrder.dto.ts, appointmentCheckIn.service.ts

Folders (Module Directories)

• Rule: Singular + kebab-case (mirrors the Module naming convention)
• Examples:
  • ✅ modules/order/, modules/notification/
  • ❌ modules/orders/, modules/notifications/

File Naming by Type

Type	Convention	Example
Module	Singular	order.module.ts
Controller	Plural	orders.controller.ts
Service	Plural	orders.service.ts
Entity	Singular	order.entity.ts
DTO	Singular resource	create-order.dto.ts

Import Paths

Use path aliases — never relative imports that cross module boundaries:

// ✅ Correct
import { CreateOrderDTO } from '@apps/orders-bc/src/modules/order/dto/create-order.dto';

// ❌ Wrong
import { CreateOrderDTO } from '../../../../dto/create-order.dto';

Database Columns (Entity Properties)

• Critical: ❌ NEVER use camelCase in Entity properties
• Rule: MUST use snake_case to map directly to PostgreSQL
• Examples:
  • ✅ first_name, created_at, is_active
  • ❌ firstName, createdAt, isActive

Variables / Functions

• Rule: camelCase (standard TypeScript convention)
• Examples:
  • ✅ const orderTotal, function calculateDiscount()
  • ❌ const order_total, function CalculateDiscount()

---

2. Architecture & Structure

Bounded Contexts

• Critical Rule: ❌ NO cross-database access
• A service must not query tables from another service’s database directly
• Inter-service communication must go through:
  • REST API calls
  • Event messaging via EventsController

Business Logic Location

• Rule: Business logic belongs in apps/<service>/src/modules/ only
• ❌ Never put domain-specific logic in libs/
• libs/ is for shared infrastructure: Guards, Decorators, generic DTOs, utilities

Inter-Service Communication

Separate EventsController

orders/
├── orders.controller.ts        # HTTP endpoints (external)
├── orders-events.controller.ts # TCP / event handlers (internal)
└── orders.service.ts

Payload Standard

• Rule: Use the shared IMicroservicePayload type for all inter-service messages
• Purpose: Enables distributed tracing and consistent user context propagation

---

3. Database & Entities

Swagger Decorators in Entities

• Critical: ❌ NEVER use @ApiProperty or any Swagger decorator in Entity files
• Rule: Swagger decorators belong in DTOs only

Database Specification on @Entity

Every entity must declare which database it belongs to:

// ✅ Correct
@Entity({
    name: 'orders',
    database: AppDatabases.APP_ORDERS,
})
export class Order {}

// ❌ Wrong: no database specified
@Entity({ name: 'orders' })
export class Order {}

Column Comments

All columns must have descriptive comments:

@Column({
    type: 'varchar',
    comment: 'Customer first name as provided at registration',
})
first_name: string;

Date/Time Columns

• Rule: Must use timestamptz type (UTC with timezone)
• ❌ Never use plain timestamp without timezone

@CreateDateColumn({
    type: 'timestamptz',
    comment: 'Record creation timestamp in UTC',
})
created_at: Date;

Nullable Columns

If @Column has nullable: true, the TypeScript type must include | null:

// ✅ Correct
@Column({ type: 'varchar', length: 255, nullable: true, comment: 'Optional display name' })
display_name: string | null;

// ❌ Wrong: missing | null on nullable column
@Column({ type: 'varchar', nullable: true, comment: 'Optional display name' })
display_name: string;

Entity-DTO Type Consistency

DTO field types must match their Entity definitions:

// Entity: nullable
@Column({ nullable: true })
note: string | null;

// ✅ Correct DTO: optional
@IsString()
@IsOptional()
note?: string;

// Entity: NOT nullable
@Column()
reference_number: string;

// ✅ Correct DTO: required
@IsString()
@IsNotEmpty()
reference_number: string;

Migration Immutability

• Critical: ❌ NEVER modify a migration file that has already been merged
• Rule: Always create a new migration for schema changes
• Reason: Modifying merged migrations causes irreproducible database states across environments

---

4. API Design & DTOs

@ResourceType Decorator

All controllers returning data must declare their resource type:

@ResourceType('orders')
@Controller('orders')
export class OrdersController {}

This enables automatic response transformation via the global interceptor.

JSON:API Response Decorators

Use the project’s custom JSON:API decorators instead of generic @ApiResponse:

@Get()
@ApiJsonApiCollectionResponse('orders', 200, OrderResponseDTO, { includePagination: true })
findPaginated(...) { ... }

@Post()
@ApiJsonApiCreatedResponse('orders', OrderResponseDTO, { description: 'Order created successfully' })
create(...) { ... }

@Get(':id')
@ApiJsonApiResponse('orders', 200, OrderResponseDTO)
findOne(...) { ... }

Mandatory Endpoint Decorators

Every endpoint must include all of the following:

@Post()
@RequirePermission('orders:create')
@HttpCode(HttpStatus.CREATED)
@ApiOperation({ summary: 'Create a new order', description: '...' })
@ApiBody({ type: CreateOrderDTO })
@ApiJsonApiCreatedResponse('orders', OrderResponseDTO)
create(...) { ... }

DTO Validation Rules

• ❌ Never use any type — always create properly typed DTOs
• Date fields must use @IsISO8601() to enforce timezone-aware input:

@IsISO8601()
@ApiProperty({ example: '2026-06-15T09:00:00+07:00' })
scheduled_at: string;

• Update DTOs must extend Create DTOs via PartialType:

export class UpdateOrderDTO extends PartialType(CreateOrderDTO) {}

URI Naming

• Rule: Plural nouns, no verbs in URLs

• ✅ GET /orders, POST /appointments

• ❌ GET /getOrders, POST /createAppointment

• For singleton resources (single config record per context): use URL without :id

• ✅ GET /configs (not /configs/:id)

---

5. Security & Error Handling

Specific Exception Types

• Critical: ❌ Never use throw new Error()
• Rule: Use NestJS-specific or custom exceptions

// ✅ Correct
throw new NotFoundException('Order not found');
throw new BadRequestException('Invalid date range');
throw new ConflictException('Order reference already exists');

// ❌ Wrong
throw new Error('Something went wrong');

Sensitive Data in Logs and Responses

• Critical: ❌ Never log or return passwords, tokens, or PII
• Sanitize all error messages before sending to the client
• Do not expose internal stack traces or database error messages in API responses

IAM Authorization

• Use policy-based authorization for complex permission scenarios
• Avoid simple role checks for fine-grained access decisions
• Prefer @RequirePermission('resource:action') over @Roles('admin')

---

6. Cross-Service Data: Lookup Tables

The Problem

Services must not query each other’s databases directly. But they often need data from other services (e.g., a user’s name when creating an order audit record).

The Solution: Synced Lookup Tables

Each bounded context maintains a local, event-driven copy of the data it needs from other services:

// ✅ Correct: join with a locally-synced lookup table
@ManyToOne(() => LookupUser)
@JoinColumn({ name: 'created_by' })
lookup_created_by: LookupUser;

// ❌ Wrong: join directly with another service's entity
@ManyToOne(() => UserFromIAM) // This crosses a database boundary

Lookup tables are kept in sync via BullMQ events. See Event-Driven Lookup Sync for the full pattern.

---

Review Checklist

Use this checklist for every pull request.

Entity Files (.entity.ts)

• No @ApiProperty decorators
• database: specified in @Entity decorator
• All column names use snake_case
• All columns have descriptive comment: strings
• Date/time columns use timestamptz
• Nullable columns declare type | null in TypeScript

DTO Files (.dto.ts)

• No any types
• Date fields use @IsISO8601()
• Update DTOs extend Create DTOs via PartialType
• All fields have Swagger @ApiProperty decorators

Controller Files (.controller.ts)

• Plural naming (OrdersController)
• @ResourceType decorator present
• JSON:API response decorators used (not generic @ApiResponse)
• No verbs in route URLs
• All endpoints have @RequirePermission, @HttpCode, @ApiOperation

Service Files (.service.ts)

• Plural or action-based naming
• No queries to other services’ databases
• NestJS exceptions used (no generic new Error())
• No sensitive data in log calls

Migration Files

• No modifications to previously merged migrations
• New migration created for any schema change

Module Organization

• Business logic in apps/<service>/src/modules/
• Shared infrastructure in libs/
• EventsController separate from HTTP Controller

---

Anti-Pattern Gallery

❌ Entity with Swagger Decorators + camelCase

@Entity({ name: 'orders' })
export class Order {
    @ApiProperty()      // ❌ No Swagger in entities
    @Column()
    orderId: string;    // ❌ Should be snake_case: order_id
}

✅ Correct Entity Structure

@Entity({
    name: 'orders',
    database: AppDatabases.APP_ORDERS,
})
export class Order {
    @Column({
        type: 'varchar',
        comment: 'External order reference number',
    })
    reference_number: string;
}

❌ Cross-Service Database Query

// In orders-bc service — WRONG
const users = await this.iamUserRepository.find(); // Queries IAM's database

✅ Using a Synced Lookup Table

// In orders-bc service — CORRECT
const users = await this.lookupUserRepository.find(); // Local, synced copy

---

AI-Assisted Code Review Prompts

These prompts are ready to use with any AI coding assistant.

Full Module Review

Review the [MODULE_NAME] module against these coding standards:

Standards: [link to this document or paste relevant sections]
Module Path: apps/[SERVICE_NAME]/src/modules/[MODULE_NAME]

Focus on:
1. Naming conventions (especially snake_case in entities)
2. Architecture compliance (no cross-database queries)
3. Entity setup (database specification, column comments, no Swagger decorators)
4. DTO validation and structure
5. Controller patterns (@ResourceType, plural naming)
6. Exception handling (no generic Error)

Output:
- ✅ Compliant areas
- ⚠️ Issues with file:line references
- 💡 Suggested fix for each issue

Entity & DTO Quick Check

Review Entity and DTO files in [MODULE_NAME].

Paths:
- apps/[SERVICE]/src/modules/[MODULE]/entities/*.entity.ts
- apps/[SERVICE]/src/modules/[MODULE]/dto/*.dto.ts

Check:
✓ Entity columns use snake_case (not camelCase)
✓ No @ApiProperty in entities
✓ database: specified in @Entity
✓ All columns have comments
✓ timestamptz for date/time fields
✓ DTOs have proper validation decorators
✓ Update DTOs extend Create DTOs via PartialType

Report violations with file:line numbers.

PR / Git Diff Review

Review all modified files in this PR against our coding standards.

Steps:
1. Examine each changed file
2. Check all 6 categories: Naming, Architecture, Database, API Design, Security, Lookup Tables
3. Report violations by category with file:line references

Format:
## PR Review

### ✅ Compliant
[summary]

### ⚠️ Issues by Category
#### Naming Conventions
- [ ] Issue (file.ts:line)
#### Architecture
- [ ] Issue (file.ts:line)
[... continue for each category ...]

Architecture Compliance Check

Analyze [MODULE_NAME] for bounded context violations.

Critical checks:
1. No cross-database queries (check all @ManyToOne, @OneToMany, @JoinColumn)
2. Business logic only in apps/, not in libs/
3. EventsController separate from HTTP controllers
4. IMicroservicePayload used for inter-service calls
5. Lookup tables used for cross-service data (not direct joins)

Report any violations immediately with file and line number.

---

Summary

Consistent code review practice is what transforms a collection of individual commits into a coherent, maintainable system. By checking every PR against the same checklist, the team builds:

• Consistency: identical standards across all bounded contexts
• Maintainability: predictable structure that any engineer can navigate
• Security: sensitive data protected at the code review gate
• Scalability: architectural boundaries enforced before they can drift