fix(#1013): Eliminate N+1 count queries in KpiService

src/utils/masking/kpi.service.spec.ts

@@ -0,0 +1,79 @@
+import { Test, TestingModule } from '@nestjs/testing';
+import { KpiService } from './kpi.service';
+import { getRepositoryToken } from '@nestjs/typeorm';
+import { Course } from '../../courses/entities/course.entity';
+import { Enrollment } from '../../courses/entities/enrollment.entity';
+import { User } from '../../users/entities/user.entity';
+import { MetricsService } from '../../observability/metrics.service';
+
+describe('KpiService', () => {
+  let service: KpiService;
+  let mockCourseRepository: any;
+  let mockEnrollmentRepository: any;
+  let mockUserRepository: any;
+  let mockMetricsService: any;
+
+  beforeEach(async () => {
+    mockCourseRepository = {
+      find: jest.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]),
+    };
+
+    mockEnrollmentRepository = {
+      createQueryBuilder: jest.fn().mockReturnValue({
+        select: jest.fn().mockReturnThis(),
+        addSelect: jest.fn().mockReturnThis(),
+        groupBy: jest.fn().mockReturnThis(),
+        getRawMany: jest.fn().mockResolvedValue([{ courseId: 1, count: '5' }]),
+      }),
+    };
+
+    mockUserRepository = {
+      createQueryBuilder: jest.fn().mockReturnValue({
+        select: jest.fn().mockReturnThis(),
+        where: jest.fn().mockReturnThis(),
+        getRawOne: jest.fn().mockResolvedValue({ count: '10' }),
+      }),
+    };
+
+    mockMetricsService = {
+      recordMetric: jest.fn(),
+    };
+
+    const module: TestingModule = await Test.createTestingModule({
+      providers: [
+        KpiService,
+        { provide: getRepositoryToken(Course), useValue: mockCourseRepository },
+        { provide: getRepositoryToken(Enrollment), useValue: mockEnrollmentRepository },
+        { provide: getRepositoryToken(User), useValue: mockUserRepository },
+        { provide: MetricsService, useValue: mockMetricsService },
+      ],
+    }).compile();
+
+    service = module.get<KpiService>(KpiService);
+  });
+
+  it('should be defined', () => {
+    expect(service).toBeDefined();
+  });
+
+  it('calculateEnrollmentConversionRate should issue a constant number of queries regardless of course count', async () => {
+    await service.calculateEnrollmentConversionRate();
+
+    // Assert that find is called once for courses
+    expect(mockCourseRepository.find).toHaveBeenCalledTimes(1);
+
+    // Assert that createQueryBuilder (for grouping) is called exactly once, 
+    // regardless of the 2 courses returned.
+    expect(mockEnrollmentRepository.createQueryBuilder).toHaveBeenCalledTimes(1);
+
+    // Verify metric was recorded
+    expect(mockMetricsService.recordMetric).toHaveBeenCalledWith('kpi_job_duration_ms', expect.any(Number));
+  });
+
+  it('calculateUserRetention should not load individual user rows', async () => {
+    await service.calculateUserRetention('2023-01');
+
+    expect(mockUserRepository.createQueryBuilder).toHaveBeenCalledTimes(1);
+    expect(mockMetricsService.recordMetric).toHaveBeenCalledWith('kpi_job_duration_ms', expect.any(Number));
+  });
+});

src/utils/masking/kpi.service.ts

@@ -0,0 +1,69 @@
+import { Injectable } from '@nestjs/common';
+import { InjectRepository } from '@nestjs/typeorm';
+import { Repository } from 'typeorm';
+import { Course } from '../../courses/entities/course.entity';
+import { Enrollment } from '../../courses/entities/enrollment.entity';
+import { User } from '../../users/entities/user.entity';
+import { MetricsService } from '../../observability/metrics.service'; // Assuming observability exists
+
+@Injectable()
+export class KpiService {
+  constructor(
+    @InjectRepository(Course)
+    private readonly courseRepository: Repository<Course>,
+    @InjectRepository(Enrollment)
+    private readonly enrollmentRepository: Repository<Enrollment>,
+    @InjectRepository(User)
+    private readonly userRepository: Repository<User>,
+    private readonly metricsService: MetricsService,
+  ) {}
+
+  async calculateEnrollmentConversionRate() {
+    const startTime = Date.now();
+    try {
+      const courses = await this.courseRepository.find();
+
+      const enrollmentCounts = await this.enrollmentRepository
+        .createQueryBuilder('enrollment')
+        .select('enrollment.courseId', 'courseId')
+        .addSelect('COUNT(*)', 'count')
+        .groupBy('enrollment.courseId')
+        .getRawMany();
+
+      const countMap = new Map<number, number>();
+      enrollmentCounts.forEach((row) => {
+        countMap.set(row.courseId, parseInt(row.count, 10));
+      });
+
+      return courses.map(course => ({
+        courseId: course.id,
+        enrollmentCount: countMap.get(course.id) || 0,
+        // Conversion rate logic here (mocked for this issue)
+        conversionRate: 0 
+      }));
+    } finally {
+      const duration = Date.now() - startTime;
+      this.metricsService.recordMetric('kpi_job_duration_ms', duration);
+    }
+  }
+
+  async calculateUserRetention(cohortMonth: string) {
+    const startTime = Date.now();
+    try {
+      // Replaced userRepository.find() with a COUNT aggregate per cohort window
+      const result = await this.userRepository
+        .createQueryBuilder('user')
+        .select('COUNT(*)', 'count')
+        .where('user.cohortMonth = :cohortMonth', { cohortMonth })
+        .getRawOne();
+
+      return {
+        cohortMonth,
+        retentionCount: parseInt(result.count, 10) || 0
+      };
+    } finally {
+      const duration = Date.now() - startTime;
+      this.metricsService.recordMetric('kpi_job_duration_ms', duration);
+    }
+  }
+}