From 5b5f14674d45a643dc9a28f28857ce5163ab936d Mon Sep 17 00:00:00 2001 From: Developer Date: Tue, 19 May 2026 10:22:18 +0800 Subject: [PATCH] fix: minor issues from code review (M1) DTO: @IsObject({ each: true }) on dimensions array (M2) audit log: add missing tenantId in submitAnswer (M3) console.log -> this.logger in controller + service --- .../src/assessment/assessment.controller.ts | 39 ++++++++++--------- server/src/assessment/assessment.service.ts | 32 +++++++-------- .../src/assessment/dto/create-template.dto.ts | 2 + 3 files changed, 39 insertions(+), 34 deletions(-) diff --git a/server/src/assessment/assessment.controller.ts b/server/src/assessment/assessment.controller.ts index 413353f..827d0bc 100644 --- a/server/src/assessment/assessment.controller.ts +++ b/server/src/assessment/assessment.controller.ts @@ -13,6 +13,7 @@ import { Delete, Put, ForbiddenException, + Logger, } from '@nestjs/common'; import { map } from 'rxjs/operators'; import { AssessmentService } from './assessment.service'; @@ -26,6 +27,8 @@ import { ApiTags, ApiOperation, ApiResponse } from '@nestjs/swagger'; @Controller('assessment') @UseGuards(CombinedAuthGuard) export class AssessmentController { + private readonly logger = new Logger(AssessmentController.name); + constructor( private readonly assessmentService: AssessmentService, private readonly exportService: ExportService, @@ -40,8 +43,8 @@ export class AssessmentController { body: { knowledgeBaseId?: string; language?: string; templateId?: string }, ) { const { id: userId, tenantId } = req.user; - console.log( - `[AssessmentController] startSession: user=${userId}, tenant=${tenantId}, templateId=${body.templateId}, kbId=${body.knowledgeBaseId}`, + this.logger.log( + `startSession: user=${userId}, tenant=${tenantId}, templateId=${body.templateId}, kbId=${body.knowledgeBaseId}`, ); const session = await this.assessmentService.startSession( userId, @@ -61,9 +64,9 @@ export class AssessmentController { @Param('id') sessionId: string, @Body() body: { answer: string; language?: string }, ) { - const { id: userId } = req.user; - console.log( - `[AssessmentController] >>> submitAnswer CALLED: user=${userId}, session=${sessionId}, answerLen=${body.answer?.length}`, + const { id: userId, tenantId } = req.user; + this.logger.log( + `submitAnswer: user=${userId}, session=${sessionId}, answerLen=${body.answer?.length}`, ); const result = await this.assessmentService.submitAnswer( sessionId, @@ -71,7 +74,7 @@ export class AssessmentController { body.answer, body.language, ); - this.auditLog.log({ userId, action: 'session.answer', resourceType: 'assessment_session', resourceId: sessionId, details: { answerLength: body.answer?.length } }); + this.auditLog.log({ userId, tenantId, action: 'session.answer', resourceType: 'assessment_session', resourceId: sessionId, details: { answerLength: body.answer?.length } }); return result; } @@ -79,8 +82,8 @@ export class AssessmentController { @ApiOperation({ summary: 'Stream initial session generation' }) startSessionStream(@Request() req: any, @Param('id') sessionId: string) { const { id: userId } = req.user; - console.log( - `[AssessmentController] startSessionStream: user=${userId}, session=${sessionId}`, + this.logger.log( + `startSessionStream: user=${userId}, session=${sessionId}`, ); return this.assessmentService .startSessionStream(sessionId, userId) @@ -98,8 +101,8 @@ export class AssessmentController { @Query('language') language?: string, ) { const { id: userId } = req.user; - console.log( - `[AssessmentController] >>> submitAnswerStream CALLED: user=${userId}, session=${sessionId}, answerLen=${answer?.length}, lang=${language}`, + this.logger.log( + `submitAnswerStream: user=${userId}, session=${sessionId}, answerLen=${answer?.length}, lang=${language}`, ); return this.assessmentService .submitAnswerStream(sessionId, userId, answer, language) @@ -110,8 +113,8 @@ export class AssessmentController { @ApiOperation({ summary: 'Get the current state of an assessment session' }) async getSessionState(@Request() req: any, @Param('id') sessionId: string) { const { id: userId } = req.user; - console.log( - `[AssessmentController] getSessionState: user=${userId}, session=${sessionId}`, + this.logger.log( + `getSessionState: user=${userId}, session=${sessionId}`, ); return this.assessmentService.getSessionState(sessionId, userId); } @@ -120,8 +123,8 @@ export class AssessmentController { @ApiOperation({ summary: 'Delete an assessment session' }) async deleteSession(@Request() req: any, @Param('id') sessionId: string) { const user = req.user; - console.log( - `[AssessmentController] deleteSession: user=${user.id}, role=${user.role}, session=${sessionId}`, + this.logger.log( + `deleteSession: user=${user.id}, role=${user.role}, session=${sessionId}`, ); await this.assessmentService.deleteSession(sessionId, user); this.auditLog.log({ userId: user.id, tenantId: user.tenantId, action: 'session.delete', resourceType: 'assessment_session', resourceId: sessionId }); @@ -135,8 +138,8 @@ export class AssessmentController { @Param('id') sessionId: string, ) { const { id: userId, tenantId } = req.user; - console.log( - `[AssessmentController] getCertificate: user=${userId}, session=${sessionId}`, + this.logger.log( + `getCertificate: user=${userId}, session=${sessionId}`, ); return this.assessmentService.generateCertificate(sessionId, userId, tenantId); } @@ -178,8 +181,8 @@ export class AssessmentController { @Query('knowledgeGroupId') knowledgeGroupId?: string, ) { const { id: userId, tenantId, role } = req.user; - console.log( - `[AssessmentController] getStats: user=${userId}, role=${role}, tenant=${tenantId}`, + this.logger.log( + `getStats: user=${userId}, role=${role}, tenant=${tenantId}`, ); return this.assessmentService.getStats( userId, diff --git a/server/src/assessment/assessment.service.ts b/server/src/assessment/assessment.service.ts index 572c1a0..e7be0a6 100644 --- a/server/src/assessment/assessment.service.ts +++ b/server/src/assessment/assessment.service.ts @@ -142,7 +142,7 @@ private async getModel(tenantId: string): Promise { scores: Record, weightConfig: { prompt: number; other: number }, ): { finalScore: number; dimensionScores: Record; radarData: Record } { - console.log('[calculateScores] Input:', { + this.logger.debug('[calculateScores] Input:', { questionsCount: questions.length, scores, weightConfig, @@ -180,7 +180,7 @@ private async getModel(tenantId: string): Promise { ? otherDimsWithScores.reduce((sum, dim) => sum + (dimensionAverages[dim] || 0), 0) / otherDimsWithScores.length : 0; - console.log('[calculateScores] Scoring debug:', { promptAvg, otherDimsWithScores, otherAvg, workCapability: dimensionAverages.workCapability }); + this.logger.debug('[calculateScores] Scoring debug:', { promptAvg, otherDimsWithScores, otherAvg, workCapability: dimensionAverages.workCapability }); const finalScore = promptAvg * (weightConfig.prompt / 100) + otherAvg * (weightConfig.other / 100); @@ -189,7 +189,7 @@ private async getModel(tenantId: string): Promise { radarData[dim] = Math.round(dimensionAverages[dim] * 10) / 10; }); - console.log('[calculateScores] Result:', { + this.logger.debug('[calculateScores] Result:', { finalScore: Math.round(finalScore * 10) / 10, dimensionScores: dimensionAverages, promptAvg, @@ -709,7 +709,7 @@ const initialState: Partial = { const finalData = fullState.values as EvaluationState; if (finalData && finalData.messages) { - console.log( + this.logger.debug( `[AssessmentService] startSessionStream Final Authoritative State messages:`, finalData.messages.length, ); @@ -903,13 +903,13 @@ const initialState: Partial = { answer: string, language: string = 'en', ): Observable { - console.log('[submitAnswerStream] START - sessionId:', sessionId, 'answer length:', answer?.length); + this.logger.debug('[submitAnswerStream] START - sessionId:', sessionId, 'answer length:', answer?.length); let emittedNextQuestion = false; let hasEmittedNodes = false; return new Observable((observer) => { (async () => { try { - console.log('[submitAnswerStream] After Observable - sessionId:', sessionId); + this.logger.debug('[submitAnswerStream] After Observable - sessionId:', sessionId); const session = await this.sessionRepository.findOne({ where: { id: sessionId, userId }, }); @@ -928,7 +928,7 @@ const initialState: Partial = { graphState && graphState.values && Object.keys(graphState.values).length > 0; - console.log( + this.logger.debug( `[AssessmentService] submitAnswerStream: sessionId=${sessionId}, hasState=${hasState}, nextNodes=[${graphState.next || ''}]`, ); @@ -954,8 +954,8 @@ const initialState: Partial = { let hasEmittedNodes = false; for await (const [mode, data] of stream) { streamCount++; - console.log('[submitAnswerStream] Stream event:', streamCount, mode, Object.keys(data || {})); - console.log('[submitAnswerStream] Data detail:', JSON.stringify(data).substring(0, 500)); + this.logger.debug('[submitAnswerStream] Stream event:', streamCount, mode, Object.keys(data || {})); + this.logger.debug('[submitAnswerStream] Data detail:', JSON.stringify(data).substring(0, 500)); if (mode === 'updates') { hasEmittedNodes = true; const node = Object.keys(data)[0]; @@ -963,17 +963,17 @@ const initialState: Partial = { // Skip interrupt nodes - they have no useful data if (node === '__interrupt__' || !updateData || Object.keys(updateData).length === 0) { - console.log('[submitAnswerStream] Skipping empty interrupt node'); + this.logger.debug('[submitAnswerStream] Skipping empty interrupt node'); continue; } - console.log('[submitAnswerStream] Node update:', node, { + this.logger.debug('[submitAnswerStream] Node update:', node, { hasMessages: !!updateData.messages, messageCount: updateData.messages?.length, currentIndex: updateData.currentQuestionIndex, dataKeys: Object.keys(updateData).join(',') }); - console.log('[submitAnswerStream] Sending to frontend:', JSON.stringify(updateData).substring(0, 500)); + this.logger.debug('[submitAnswerStream] Sending to frontend:', JSON.stringify(updateData).substring(0, 500)); if (updateData.messages) { updateData.messages = this.mapMessages(updateData.messages); } @@ -984,7 +984,7 @@ const initialState: Partial = { } observer.next({ type: 'node', node, data: updateData }); } else if (mode === 'values') { - console.log('[submitAnswerStream] Values update - keys:', Object.keys(data || {})); + this.logger.debug('[submitAnswerStream] Values update - keys:', Object.keys(data || {})); } } @@ -995,13 +995,13 @@ const initialState: Partial = { const finalData = fullState.values as EvaluationState; // Force emit the next question if stream didn't emit updates (hasEmittedNodes is false) - console.log('[submitAnswerStream] Force check:', { hasEmittedNodes, hasFinalData: !!finalData, hasQuestions: !!finalData?.questions, qLen: finalData?.questions?.length, emittedNextQuestion }); + this.logger.debug('[submitAnswerStream] Force check:', { hasEmittedNodes, hasFinalData: !!finalData, hasQuestions: !!finalData?.questions, qLen: finalData?.questions?.length, emittedNextQuestion }); if (!hasEmittedNodes && finalData && finalData.questions && finalData.questions.length > 0 && !emittedNextQuestion) { const currentIndex = finalData.currentQuestionIndex || 0; const nextQuestion = finalData.questions[currentIndex]; if (nextQuestion) { const questionText = nextQuestion.questionText || ''; - console.log('[submitAnswerStream] Forcing emit next question:', { + this.logger.debug('[submitAnswerStream] Forcing emit next question:', { currentIndex, questionPreview: questionText.substring(0, 50) }); @@ -1021,7 +1021,7 @@ const initialState: Partial = { } if (finalData && finalData.messages) { - console.log( + this.logger.debug( `[AssessmentService] submitAnswerStream Final Authoritative State messages:`, finalData.messages.length, ); diff --git a/server/src/assessment/dto/create-template.dto.ts b/server/src/assessment/dto/create-template.dto.ts index 710c8f8..8523cd0 100644 --- a/server/src/assessment/dto/create-template.dto.ts +++ b/server/src/assessment/dto/create-template.dto.ts @@ -8,6 +8,7 @@ import { Max, IsObject, IsBoolean, + IsNumber, } from 'class-validator'; export class CreateTemplateDto { @@ -60,6 +61,7 @@ export class CreateTemplateDto { linkedGroupIds?: string[]; @IsArray() + @IsObject({ each: true }) @IsOptional() dimensions?: Array<{ name: string; label: string; weight: number }>;