goodgo-platform/docs/audits/CODE_QUALITY_AUDIT.md

GoodGo Platform - Code Quality Audit Report

Depth Level: Very Thorough
Audit Date: April 9, 2026
Codebase: /Users/velikho/Desktop/WORKING/goodgo-platform-ai/

---

1. ERROR HANDLING

✅ STRENGTHS

• DomainException Pattern Properly Implemented: Centralized exception hierarchy in /modules/shared/domain/domain-exception.ts (Lines 13-56)

  • DomainException, NotFoundException, ValidationException, ConflictException, UnauthorizedException, ForbiddenException
  • All extend HttpException with proper status codes

• Global Exception Filter: /modules/shared/infrastructure/filters/global-exception.filter.ts (Lines 1-84)

  • Catches all exceptions at application boundary
  • Converts to standard ErrorResponseBody format
  • Proper logging with correlation IDs

• Result<T, E> Pattern: /modules/shared/domain/result.ts (Lines 1-56)

  • Functional Result type with ok(), err(), map(), andThen(), match() methods
  • Good for domain-level error handling

⚠️ ISSUES FOUND

[CRITICAL] Domain entities throwing plain Error instead of domain exceptions:

• payments/domain/entities/payment.entity.ts (Lines 94, 107, 134)

  • throw new Error('Cannot complete payment in status ${this._status}')
  • throw new Error('Cannot fail payment in status ${this._status}')
  • throw new Error('Chỉ có thể hoàn tiền cho thanh toán đã hoàn tất')

• subscriptions/domain/entities/subscription.entity.ts (Lines 75, 90, 104, 112)

  • throw new Error('Không thể nâng cấp subscription...')
  • throw new Error('Subscription đã bị hủy')
  • Multiple similar instances

Fix: Domain entities should NOT throw; should return Result<T, E> instead.

[HIGH] Infrastructure services throwing plain Error for env validation:

• payments/infrastructure/services/vnpay.service.ts (Line 16)
• payments/infrastructure/services/momo.service.ts (Line 16)
• payments/infrastructure/services/zalopay.service.ts (Line 16)
• auth/infrastructure/strategies/google-oauth.strategy.ts (Line 22)
• auth/auth.module.ts (Line 39)

Fix: Use configuration validation at module bootstrap, not service instantiation.

[MEDIUM] Some controllers throw directly instead of via DomainException:

• auth/presentation/controllers/oauth.controller.ts (Lines 74, 101)
  • throw new UnauthorizedException(...) - OK, but pattern inconsistent
  • Should use Result pattern in handlers instead

---

2. IMPORT ORDER & PATH ALIASES

✅ STRENGTHS

• Path Alias Configuration Correct: tsconfig.base.json enables @modules/* path (tsconfig.json Line 14)

• ESLint Import Rules Well-Configured: eslint.config.mjs (Lines 64-71)

  • Groups: ['builtin', 'external', 'internal', 'parent', 'sibling', 'index']
  • import-x/no-duplicates: enforced
  • Alphabetical sorting enforced

• Dependencies Properly Exported: All modules have index.ts barrel exports

  • auth/index.ts exports: AuthModule, guards, decorators, JwtPayload type
  • payments/index.ts exports: PaymentsModule, repository token, gateway interface

⚠️ ISSUES FOUND

[HIGH] Cross-module internal imports NOT respecting barrel pattern:

158 instances of direct internal imports found:

1. @modules/auth/infrastructure/services/token.service imported directly:

   • payments/presentation/controllers/payments.controller.ts (Line 21)
   • Should import from @modules/auth (barrel) but TokenService/JwtPayload is exported in index.ts

2. Infrastructure services imported from multiple modules:

   • auth/application/commands/refresh-token/refresh-token.handler.ts (Line ?)
     • Imports TokenService from '../../../infrastructure/services/token.service'
   • auth/application/commands/login-user/login-user.handler.ts
     • Same pattern

3. CacheService imported directly:

   • auth/application/queries/get-profile/get-profile.handler.ts
     • from '@modules/shared/infrastructure/cache.service'
     • Should use barrel @modules/shared

Fix:

1. Update auth/index.ts to export TokenService (not just type)
2. Update shared/index.ts to export CacheService
3. Replace all direct infrastructure imports with barrel imports

---

3. TYPESCRIPT STRICTNESS

✅ STRENGTHS

• Strict Mode Enabled: tsconfig.base.json Line 7: "strict": true

• Advanced Flags Set:

  • noUncheckedIndexedAccess: true (Line 15)
  • noImplicitOverride: true (Line 16)
  • noPropertyAccessFromIndexSignature: true (Line 17)
  • forceConsistentCasingInFileNames: true (Line 10)
  • declaration: true, declarationMap: true, sourceMap: true

• ESLint Enforcement:

  • @typescript-eslint/no-explicit-any: warn (Lines 57)
  • @typescript-eslint/no-unused-vars with pattern ^_ (Lines 53-55)
  • Type import enforcement: inline type imports (Lines 58-60)

⚠️ ISSUES FOUND

[MEDIUM] No noImplicitAny explicitly set (defaults to true with strict):

• Some files may have relaxed type checking in test files
• ESLint allows any in test files (eslint.config.mjs Lines 108-109)
• Consider: Add @typescript-eslint/no-explicit-any: error for non-test files

---

4. CODE DUPLICATION

⚠️ ISSUES FOUND

[MEDIUM] Repeated Logger Pattern (50+ instances):

private readonly logger = new Logger(ClassName.name);

Found in:

• payments/application/commands/handle-callback/handle-callback.handler.ts
• payments/application/commands/create-payment/create-payment.handler.ts
• payments/application/commands/refund-payment/refund-payment.handler.ts
• payments/infrastructure/services/zalopay.service.ts
• payments/infrastructure/services/momo.service.ts
• And 45+ more

Fix: Create a base handler class or injectable factory for logger initialization:

@Injectable()
export abstract class BaseCommandHandler {
  protected readonly logger = this.getLoggerService();
  constructor(protected readonly loggerService: LoggerService) {}
  protected getLoggerService() { return this.loggerService; }
}

[MEDIUM] Prisma Service Injection Pattern (50+ instances):

constructor(private readonly prisma: PrismaService) {}

All repositories follow this, but no base repository class to reduce duplication.

Fix: Create base repository class:

@Injectable()
export abstract class BasePrismaRepository {
  constructor(protected readonly prisma: PrismaService) {}
  protected buildPaginationParams(page: number, limit: number) {
    return { skip: (page - 1) * limit, take: limit };
  }
}

[LOW] Error message formatting duplicated:

• Multiple services manually format bigint to string
• No shared utility for currency/number formatting
• admin/infrastructure/repositories/prisma-admin-query.repository.ts Line 56: Math.ceil(total / limit)
• listings/infrastructure/repositories/prisma-listing.repository.ts: Similar pagination logic

---

5. DEPENDENCY INJECTION

✅ STRENGTHS

• Module Pattern Correct: All modules properly structured with @Module() decorator
• CQRS Integration: CqrsModule imported in all command/query handler modules
• Provider Registration Clear: Handlers registered in arrays (CommandHandlers, QueryHandlers)
• Exports Explicit: Module exports define public API

Example - payments.module.ts (Lines 1-44):

@Module({
  imports: [CqrsModule],
  controllers: [PaymentsController],
  providers: [
    { provide: PAYMENT_REPOSITORY, useClass: PrismaPaymentRepository },
    VnpayService, MomoService, ZalopayService,
    { provide: PAYMENT_GATEWAY_FACTORY, useClass: PaymentGatewayFactory },
    ...CommandHandlers, ...QueryHandlers,
  ],
  exports: [PAYMENT_REPOSITORY, PAYMENT_GATEWAY_FACTORY],
})

Example - auth.module.ts (Lines 33-70):

• JWT configuration with environment variable validation ✅
• Passport & JWT strategy registration ✅
• Repository and service providers ✅
• Exports: TokenService, OAuthService, USER_REPOSITORY ✅

⚠️ ISSUES FOUND

[LOW] Module imports not using barrel exports:

• Inside modules, components import directly from internal paths (acceptable, but inconsistent with inter-module rules)
• Example: payments.module.ts Line 3: import { CreatePaymentHandler } from './application/commands/...'
• Could simplify with import { CreatePaymentHandler } from './application' if using barrel exports

[LOW] Missing SharedModule export:

• shared.module.ts needs to explicitly export LoggerService
• Currently some tests import directly: auth/__tests__/auth.integration.spec.ts (Line 18)
  • import { PrismaService } from '@modules/shared/infrastructure/prisma.service'
  • Should be import { PrismaService } from '@modules/shared'

---

6. EVENT HANDLING (@OnEvent Pattern)

✅ STRENGTHS

• Event Pattern Properly Implemented:
  • 10 event listeners found (all in notifications module)
  • Using @OnEvent('event.name', { async: true })

Example - notifications/application/listeners/payment-completed.listener.ts (Lines 17-43):

@OnEvent('payment.completed', { async: true })
async handle(event: PaymentCompletedEvent): Promise<void> {
  // Proper async handling
  const user = await this.prisma.user.findUnique({...});
  await this.commandBus.execute(new SendNotificationCommand(...));
}

✅ DOMAIN EVENTS FOUND

• payments/domain/events/payment-created.event.ts
• payments/domain/events/payment-completed.event.ts
• payments/domain/events/payment-failed.event.ts
• Events implement DomainEvent interface

⚠️ ISSUES FOUND

[MEDIUM] Event publishing not found in domain entities:

• Events are defined but no evidence of publishEvent() or getUncommittedEvents()
• Entities don't publish events when state changes
• Example: payments/domain/entities/payment.entity.ts (188 lines) - no event publishing

Fix: Implement event sourcing pattern:

export class PaymentEntity extends AggregateRoot {
  private events: DomainEvent[] = [];
  
  complete(): void {
    if (this._status !== PaymentStatus.PENDING) throw new Error(...);
    this._status = PaymentStatus.COMPLETED;
    this.events.push(new PaymentCompletedEvent(...));
  }
  
  getUncommittedEvents(): DomainEvent[] { return this.events; }
}

[MEDIUM] Only 10 event listeners for entire platform:

• 2 Listings module events (listing-created usage tracking)
• 2 Payments module events
• 8 Notifications module listeners

Expected improvements:

• Auth events: user.registered, user.verified, user.banned
• Listings events: listing.created, listing.approved, listing.rejected, listing.expired
• Subscriptions events: subscription.expired, subscription.upgraded
• Currently only notifications react to events

---

7. VALIDATION

✅ STRENGTHS

• DTO Pattern with class-validator: All presentation DTOs use decorators
• Global Validation Pipe: main.ts (Lines 90-98)
  • whitelist: true, forbidNonWhitelisted: true
  • transform: true, transformOptions: { enableImplicitConversion: true }

Example - auth/presentation/dto/register.dto.ts (Lines 1-23):

@IsString()
@MinLength(8)
password!: string;

@IsOptional()
@IsEmail()
email?: string;

Example - listings/presentation/dto/create-listing.dto.ts (Lines 1-50+):

• 163 lines with comprehensive validation
• Proper enum validation, min/max length, number ranges
• @Transform(({ value }) => BigInt(value)) for bigint handling

⚠️ ISSUES FOUND

[LOW] Missing validation in some DTOs:

• payments/presentation/dto/refund-payment.dto.ts - basic but minimal validation
• notifications/presentation/dto - not found (notifications controller may skip DTOs)

[MEDIUM] BigInt handling inconsistent:

• listings/presentation/dto/create-listing.dto.ts uses @Transform(({ value }) => BigInt(value))
• But not all price/amount fields in other modules use this pattern
• payments/presentation/dto/create-payment.dto.ts - check if bigint amounts validated

[LOW] Custom validators not extracted:

• Vietnam phone validation in auth/infrastructure/strategies/local.strategy.ts
• No reusable @IsVietnamPhone() decorator found
• Should create: shared/decorators/vietnam-phone.decorator.ts

---

8. LOGGING

✅ STRENGTHS

• Custom LoggerService: /modules/shared/infrastructure/logger.service.ts (Lines 1-52)

  • Uses Pino logger with environment-based transport
  • Pretty printing in non-production, structured JSON in production
  • PII masking via maskPii() function
  • Support for context and trace parameters

• Logger Injection Pattern: Services properly inject LoggerService

  • PaymentCompletedListener constructor (Line 14)
  • All handlers have access to centralized logging

⚠️ ISSUES FOUND

[MEDIUM] Direct Logger from @nestjs/common still used in 50+ places:

private readonly logger = new Logger(ClassName.name);

Should use:

constructor(private readonly logger: LoggerService) {}

Found in:

• payments/infrastructure/services/zalopay.service.ts (Line 11)
• payments/infrastructure/services/momo.service.ts (Line 11)
• payments/infrastructure/services/vnpay.service.ts (Line 11)
• All payment handlers
• All OAuth strategies

[MEDIUM] LoggerService not registered in SharedModule:

• Need to verify shared.module.ts exports LoggerService
• If not, this explains why handlers use direct Logger import

[LOW] Log levels inconsistent:

• Some use .log(), some use .warn(), some use .error()
• No ERROR recovery logging (just error reporting)
• Consider adding .verbose() for debugging

---

9. API VERSIONING

⚠️ CRITICAL ISSUE

[HIGH] No API versioning found:

• main.ts Line 40: SwaggerModule.setup('api/docs', app, document)
• Controllers use @Controller('payments'), @Controller('auth'), etc.
• No /api/v1/ prefix found

Expected structure:

@Controller('api/v1/payments')  // Current: 'payments'
@Controller('api/v1/auth')       // Current: 'auth'

Or via global prefix:

app.setGlobalPrefix('api/v1'); // In main.ts bootstrap

Fix: Add in main.ts after app creation:

app.setGlobalPrefix('api/v1');

This ensures:

• All routes become /api/v1/*
• Swagger docs at /api/v1/docs
• Future-proof for v2 support

---

10. FILE SIZE VIOLATIONS (>200 lines)

⚠️ ISSUES FOUND

[MEDIUM] Files exceeding 200-line convention:

1. admin/infrastructure/repositories/prisma-admin-query.repository.ts (313 lines)

   • Multiple query methods (getModerationQueue, getDashboardStats, getRevenueStats, etc.)
   • Fix: Split into separate query repositories by domain

2. admin/presentation/controllers/admin.controller.ts (289 lines)

   • All admin endpoints in single controller
   • Fix: Split into admin-listings, admin-users, admin-subscriptions controllers

3. listings/infrastructure/repositories/prisma-listing.repository.ts (274 lines)

   • Too many methods (findById, findByIdWithProperty, search, save, etc.)
   • Fix: Split read/write operations

4. analytics/infrastructure/tests/prisma-market-index.repository.spec.ts (254 lines)

   • Large test file, acceptable

5. listings/domain/tests/property.entity.spec.ts (234 lines)

   • Large test file, acceptable

6. listings/presentation/controllers/listings.controller.ts (213 lines)

   • Multiple endpoints (create, update, delete, search, etc.)
   • Fix: Extract into separate action classes or slim down

7. payments/infrastructure/services/zalopay.service.ts (211 lines)

   • Payment gateway service handling multiple operations
   • Acceptable but should consider refactoring

8. payments/infrastructure/services/momo.service.ts (209 lines)

   • Similar to ZaloPay service
   • Acceptable but consider extraction

9. auth/presentation/controllers/auth.controller.ts (200 lines)

   • Boundary, acceptable but near limit
   • Monitor for growth

Total files >200 lines: 9 files (3 critical, 6 acceptable)

---

11. ESLINT CONFIGURATION

✅ STRENGTHS

• Modern Flat Config Format: eslint.config.mjs (ESLint v9+)

• Comprehensive Rule Coverage (Lines 8-122):

  • TypeScript recommended rules ✅
  • Import plugin (builtin, external, internal ordering) ✅
  • Prettier integration ✅
  • Unused variables with ^_ pattern ✅
  • Type import enforcement ✅

• Specific Overrides:

  • NestJS module rules: @typescript-eslint/no-extraneous-class: off (Line 85)
  • React/Next overrides (Lines 92-102)
  • Test file relaxations (Lines 105-112)
  • Script file relaxations (Lines 114-121)

⚠️ MISSING RULES

[MEDIUM] Missing important linting rules:

1. No no-restricted-imports to prevent direct infrastructure imports
2. No @typescript-eslint/explicit-function-return-types enforcement
3. No @typescript-eslint/explicit-module-boundary-types
4. No sonarjs plugin for cognitive complexity
5. No eslint-plugin-decorator-frame for NestJS-specific rules

Recommendation: Add to eslint.config.mjs:

{
  files: ['apps/api/**/*.ts'],
  rules: {
    'no-restricted-imports': [
      'error',
      {
        patterns: [
          '@modules/*/infrastructure/*',
          '@modules/*/application/*',
          '@modules/*/presentation/*'
        ]
      }
    ],
    '@typescript-eslint/explicit-function-return-types': ['warn', {
      allowExpressions: true,
      allowTypedFunctionExpressions: true
    }]
  }
}

---

12. PERFORMANCE PATTERNS

✅ STRENGTHS

• Pagination Implemented: Repositories include pagination logic
  • admin/infrastructure/repositories/prisma-admin-query.repository.ts (Lines 18-52)
  • listings/infrastructure/repositories/prisma-listing.repository.ts
• Query Optimization: Using select and include properly in many places
  • Example: listings/infrastructure/repositories/prisma-listing.repository.ts (Lines 21-29)
    • Limits media to 10 items with take: 10

⚠️ ISSUES FOUND

[MEDIUM] Potential N+1 Query Risks:

1. admin/infrastructure/repositories/prisma-admin-query.repository.ts:

   • Line 21-32: findMany with include on property, seller ✅ (Good)
   • Lines 69-77: Multiple sequential .count() calls ✅ (Using Promise.all - Good)

2. payments/application/commands/handle-callback/handle-callback.handler.ts:

   • Need to verify if payment.findUnique() includes all related data
   • Listeners may do additional queries after payment completion

3. listings/infrastructure/repositories/prisma-listing.repository.ts:

   • Line 24: media: { orderBy: { order: 'asc' }, take: 10 } ✅ (Limited)
   • But other methods may not include all necessary relations

[LOW] Missing database indexes:

• Prisma schema should define indexes for:
  • listing.status (PENDING_REVIEW, ACTIVE, etc.)
  • payment.status and timestamp ranges
  • user.createdAt for date ranges
  • Check schema.prisma for index definitions

[LOW] No query result caching visible:

• CacheService exists but usage limited to:
  • auth/application/queries/get-profile (Line ?)
  • Should cache:
    • User profiles (5 min TTL)
    • Listings (1 min TTL)
    • Payment status (30 sec TTL)

---

DEPENDENCY CRUISER CONFIGURATION

✅ STRENGTHS

• Well-configured rules: .dependency-cruiser.cjs (Lines 1-79)
  • Circular dependency detection ✅
  • Cross-module internal imports forbidden ✅
  • App-to-module internals forbidden ✅
  • Orphan module detection ✅

NOTES

• These rules should catch the import violations found in section 2
• Run pnpx depcruise to validate compliance

---

SUMMARY OF FINDINGS

Critical Issues (Must Fix)

1. Domain entities throwing plain Error - Should return Result or throw DomainException
2. No API versioning - Add /api/v1/ prefix
3. Cross-module internal imports - Update barrel exports

High Priority Issues

1. Infrastructure services throwing Error for env validation - Move to module factory
2. Event publishing not implemented - Add to aggregate roots
3. Logger pattern inconsistent - 50+ direct Logger imports instead of injection

Medium Priority Issues

1. Code duplication - Logger, Prisma service, pagination logic
2. Large file violations - 3 files significantly >200 lines
3. Missing custom validators - No @IsVietnamPhone() decorator
4. N+1 query risks - Some repositories need optimization

Low Priority Issues

1. ESLint rule gaps - Missing explicit function return types
2. Module exports incomplete - SharedModule not exporting all services
3. No caching strategy - Consider implementing for frequent queries
4. Test files use direct Logger - Not critical but inconsistent

---

RECOMMENDATIONS

Quick Wins (1-2 days)

• Add /api/v1/ global prefix to main.ts
• Export missing services in module barrels
• Update 10 files to import from barrels instead of direct paths

Medium Term (1 week)

• Create BaseRepository and BaseHandler for DI consistency
• Add @IsVietnamPhone() and other custom validators
• Split large controller/repository files
• Replace direct Logger imports with injection

Long Term (2+ weeks)

• Implement event publishing in domain entities
• Add event handlers for more domain events
• Implement result-based error handling in handlers
• Add comprehensive caching strategy
• Extended ESLint rules for architecture enforcement