From 82a9e758425e4d58b3e26a4a8a406dbfdfb3a9ae Mon Sep 17 00:00:00 2001 From: Developer Date: Tue, 19 May 2026 10:06:30 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20code=20review=20=E2=80=94=207=20issues?= =?UTF-8?q?=20resolved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (C1) Add dimensionScores/radarData/passed columns to AssessmentSession (C2) Mock DataSource in service.spec.ts + app.e2e-spec.ts (C3) Mock AuditLogService in controller.spec.ts (C4) Rewrite deleteSession tests for dataSource.transaction (I1) batchDeleteSessions uses transaction with certificate cleanup (I2) extractDimensionScores reads from session property (I3/I5) PDF generator supports multi-page + newline splitting (I4) findOne inside transaction uses deleteCondition --- .../assessment/assessment.controller.spec.ts | 2 + .../src/assessment/assessment.service.spec.ts | 43 +++++- server/src/assessment/assessment.service.ts | 28 ++-- .../entities/assessment-session.entity.ts | 9 ++ .../src/assessment/services/export.service.ts | 2 +- .../src/assessment/services/pdf-generator.ts | 131 +++++++++++------- server/test/app.e2e-spec.ts | 12 ++ 7 files changed, 164 insertions(+), 63 deletions(-) diff --git a/server/src/assessment/assessment.controller.spec.ts b/server/src/assessment/assessment.controller.spec.ts index b1bd6c6..ec330f9 100644 --- a/server/src/assessment/assessment.controller.spec.ts +++ b/server/src/assessment/assessment.controller.spec.ts @@ -6,6 +6,7 @@ import { TenantService } from '../tenant/tenant.service'; import { UserService } from '../user/user.service'; import { CombinedAuthGuard } from '../auth/combined-auth.guard'; import { ExportService } from './services/export.service'; +import { AuditLogService } from './services/audit-log.service'; describe('AssessmentController', () => { let controller: AssessmentController; @@ -27,6 +28,7 @@ describe('AssessmentController', () => { { provide: UserService, useFactory: mockService }, { provide: TenantService, useFactory: mockService }, { provide: ExportService, useFactory: mockService }, + { provide: AuditLogService, useFactory: () => ({ log: jest.fn() }) }, { provide: Reflector, useFactory: mockReflector }, { provide: CombinedAuthGuard, useFactory: mockGuard }, ], diff --git a/server/src/assessment/assessment.service.spec.ts b/server/src/assessment/assessment.service.spec.ts index 4121a81..dee4f53 100644 --- a/server/src/assessment/assessment.service.spec.ts +++ b/server/src/assessment/assessment.service.spec.ts @@ -1,5 +1,6 @@ import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { DataSource } from 'typeorm'; import { AssessmentService } from './assessment.service'; import { AssessmentSession, AssessmentStatus } from './entities/assessment-session.entity'; import { AssessmentQuestion } from './entities/assessment-question.entity'; @@ -25,6 +26,7 @@ describe('AssessmentService', () => { let service: AssessmentService; let sessionRepository: any; let certificateRepository: any; + let dataSource: any; const mockRepository = () => ({ delete: jest.fn(), @@ -39,6 +41,19 @@ describe('AssessmentService', () => { const regularUser = { id: 'user-1', role: 'user' }; const adminUser = { id: 'admin-1', role: 'admin' }; + const mockManager = (overrides?: any) => ({ + findOne: jest.fn(), + delete: jest.fn().mockResolvedValue({ affected: 1 }), + save: jest.fn(), + ...overrides, + }); + + const mockDataSource = (manager?: any) => ({ + transaction: jest.fn(async (cb: any) => { + return cb(manager || mockManager()); + }), + }); + beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ providers: [ @@ -61,12 +76,14 @@ describe('AssessmentService', () => { { provide: ChatService, useFactory: mockService }, { provide: I18nService, useFactory: mockService }, { provide: TenantService, useFactory: mockService }, + { provide: DataSource, useFactory: () => mockDataSource(mockManager()) }, ], }).compile(); service = module.get(AssessmentService); sessionRepository = module.get(getRepositoryToken(AssessmentSession)); certificateRepository = module.get(getRepositoryToken(AssessmentCertificate)); + dataSource = module.get(DataSource); }); it('should be defined', () => { @@ -75,19 +92,33 @@ describe('AssessmentService', () => { describe('deleteSession', () => { it('should delete a session when non-admin user owns it', async () => { - sessionRepository.delete.mockResolvedValue({ affected: 1 }); + const manager = mockManager({ + findOne: jest.fn().mockResolvedValue({ id: 'session-id', userId: 'user-1' }), + }); + dataSource.transaction.mockImplementation(async (cb: any) => cb(manager)); + await expect(service.deleteSession('session-id', regularUser)).resolves.not.toThrow(); - expect(sessionRepository.delete).toHaveBeenCalledWith({ id: 'session-id', userId: 'user-1' }); + expect(manager.findOne).toHaveBeenCalledWith(AssessmentSession, { where: { id: 'session-id', userId: 'user-1' } }); + expect(manager.delete).toHaveBeenCalledWith(AssessmentCertificate, { sessionId: 'session-id' }); + expect(manager.delete).toHaveBeenCalledWith(AssessmentSession, { id: 'session-id' }); }); it('should delete any session when admin user', async () => { - sessionRepository.delete.mockResolvedValue({ affected: 1 }); + const manager = mockManager({ + findOne: jest.fn().mockResolvedValue({ id: 'other-session', userId: 'user-2' }), + }); + dataSource.transaction.mockImplementation(async (cb: any) => cb(manager)); + await expect(service.deleteSession('other-session', adminUser)).resolves.not.toThrow(); - expect(sessionRepository.delete).toHaveBeenCalledWith({ id: 'other-session' }); + expect(manager.findOne).toHaveBeenCalledWith(AssessmentSession, { where: { id: 'other-session' } }); }); - it('should throw NotFoundException if no session was affected', async () => { - sessionRepository.delete.mockResolvedValue({ affected: 0 }); + it('should throw NotFoundException if session not found', async () => { + const manager = mockManager({ + findOne: jest.fn().mockResolvedValue(null), + }); + dataSource.transaction.mockImplementation(async (cb: any) => cb(manager)); + await expect(service.deleteSession('non-existent', regularUser)).rejects.toThrow(NotFoundException); }); }); diff --git a/server/src/assessment/assessment.service.ts b/server/src/assessment/assessment.service.ts index daad09d..572c1a0 100644 --- a/server/src/assessment/assessment.service.ts +++ b/server/src/assessment/assessment.service.ts @@ -1145,7 +1145,7 @@ const initialState: Partial = { deleteCondition.userId = userId; } - const session = await manager.findOne(AssessmentSession, { where: { id: sessionId } }); + const session = await manager.findOne(AssessmentSession, { where: deleteCondition }); if (!session) { throw new NotFoundException('Session not found or you do not have permission to delete it'); } @@ -1726,13 +1726,25 @@ const initialState: Partial = { async batchDeleteSessions(ids: string[], user: any): Promise { const isAdmin = user.role === 'super_admin' || user.role === 'admin'; - const queryBuilder = this.sessionRepository.createQueryBuilder().delete().whereInIds(ids); - if (!isAdmin) { - queryBuilder.andWhere('user_id = :userId', { userId: user.id }); - } - const result = await queryBuilder.execute(); - this.logger.log(`[batchDeleteSessions] Deleted ${result.affected} sessions`); - return result.affected || 0; + + return this.dataSource.transaction(async (manager) => { + const query: any = { id: In(ids) }; + if (!isAdmin) { + query.userId = user.id; + } + + const sessions = await manager.find(AssessmentSession, { where: query }); + const sessionIds = sessions.map((s) => s.id); + + if (sessionIds.length === 0) { + return 0; + } + + await manager.delete(AssessmentCertificate, { sessionId: In(sessionIds) }); + const result = await manager.delete(AssessmentSession, { id: In(sessionIds) }); + this.logger.log(`[batchDeleteSessions] Deleted ${sessionIds.length} sessions`); + return result.affected || 0; + }); } async batchExportSessions(ids: string[], userId: string): Promise { diff --git a/server/src/assessment/entities/assessment-session.entity.ts b/server/src/assessment/entities/assessment-session.entity.ts index a08c65e..c30cf52 100644 --- a/server/src/assessment/entities/assessment-session.entity.ts +++ b/server/src/assessment/entities/assessment-session.entity.ts @@ -64,6 +64,15 @@ export class AssessmentSession { @Column({ type: 'float', name: 'original_score', nullable: true }) originalScore: number; + @Column({ type: 'simple-json', nullable: true, name: 'dimension_scores' }) + dimensionScores: Record; + + @Column({ type: 'simple-json', nullable: true, name: 'radar_data' }) + radarData: any; + + @Column({ nullable: true }) + passed: boolean; + @Column({ type: 'text', name: 'final_report', nullable: true }) finalReport: string; diff --git a/server/src/assessment/services/export.service.ts b/server/src/assessment/services/export.service.ts index c4971c7..8c7a62b 100644 --- a/server/src/assessment/services/export.service.ts +++ b/server/src/assessment/services/export.service.ts @@ -96,7 +96,7 @@ export class ExportService { } private extractDimensionScores(session: AssessmentSession): any[][] { - const scores = session.templateJson?.dimensionScores || session.finalReport; + const scores = (session as any).dimensionScores; if (!scores) return [['未找到维度分数']]; if (typeof scores === 'string') { diff --git a/server/src/assessment/services/pdf-generator.ts b/server/src/assessment/services/pdf-generator.ts index 4a13cdf..cd86666 100644 --- a/server/src/assessment/services/pdf-generator.ts +++ b/server/src/assessment/services/pdf-generator.ts @@ -47,69 +47,105 @@ function textToHex(str: string): string { export async function generateAssessmentPdf(options: PdfReportOptions): Promise { const doc = await PDFDocument.create(); - const page = doc.addPage([595.28, 841.89]); - const ctx = doc.context; const fontBytes = findFont(); if (fontBytes.length === 0) { throw new Error('No CJK font found. Install Noto Sans SC or similar.'); } - const fontProgRef = ctx.nextRef(); - ctx.assign(fontProgRef, ctx.flateStream(fontBytes)); + const FONT_DESC_REF = { ref: null as any }; + const FONT_REF = { ref: null as any }; - const fontDescRef = ctx.nextRef(); - ctx.assign(fontDescRef, ctx.obj({ - Type: 'FontDescriptor', - FontName: 'CJKFont', - Flags: 4, - FontBBox: [0, -300, 1000, 1000], - ItalicAngle: 0, - Ascent: 900, - Descent: -200, - CapHeight: 800, - StemV: 80, - FontFile2: fontProgRef, - })); + function prepareFont(page: any) { + const fontProgRef = ctx.nextRef(); + ctx.assign(fontProgRef, ctx.flateStream(fontBytes)); - const cidFontRef = ctx.nextRef(); - ctx.assign(cidFontRef, ctx.obj({ - Type: 'Font', - Subtype: 'CIDFontType2', - BaseFont: 'CJKFont', - CIDSystemInfo: ctx.obj({ - Registry: 'Adobe', - Ordering: 'Identity', - Supplement: 0, - }), - FontDescriptor: fontDescRef, - W: [0, [500]], - })); + const fontDescRef = ctx.nextRef(); + ctx.assign(fontDescRef, ctx.obj({ + Type: 'FontDescriptor', + FontName: 'CJKFont', + Flags: 4, + FontBBox: [0, -300, 1000, 1000], + ItalicAngle: 0, + Ascent: 900, + Descent: -200, + CapHeight: 800, + StemV: 80, + FontFile2: fontProgRef, + })); + FONT_DESC_REF.ref = fontDescRef; - const fontRef = ctx.nextRef(); - ctx.assign(fontRef, ctx.obj({ - Type: 'Font', - Subtype: 'Type0', - BaseFont: 'CJKFont', - Encoding: 'Identity-H', - DescendantFonts: [cidFontRef], - })); + const cidFontRef = ctx.nextRef(); + ctx.assign(cidFontRef, ctx.obj({ + Type: 'Font', + Subtype: 'CIDFontType2', + BaseFont: 'CJKFont', + CIDSystemInfo: ctx.obj({ + Registry: 'Adobe', + Ordering: 'Identity', + Supplement: 0, + }), + FontDescriptor: fontDescRef, + W: [0, [500]], + })); - const fontKey = page.node.newFontDictionaryKey('F1'); - page.node.setFontDictionary(fontKey, fontRef); + const fontRef = ctx.nextRef(); + ctx.assign(fontRef, ctx.obj({ + Type: 'Font', + Subtype: 'Type0', + BaseFont: 'CJKFont', + Encoding: 'Identity-H', + DescendantFonts: [cidFontRef], + })); + FONT_REF.ref = fontRef; + + const fontKey = page.node.newFontDictionaryKey('F1'); + page.node.setFontDictionary(fontKey, fontRef); + } - let contentOps = ''; - let y = 800; const margin = 50; const pageWidth = 595.28; + const pageHeight = 841.89; + + const ctx = doc.context as any; + let currentPage = doc.addPage([pageWidth, pageHeight]); + let contentOps = ''; + let y = pageHeight - 50; + + prepareFont(currentPage); + + function addFontToPage(page: any) { + const fontKey = page.node.newFontDictionaryKey('F1'); + page.node.setFontDictionary(fontKey, FONT_REF.ref); + } + + function ensureSpace(needed: number) { + const bottomMargin = 60; + if (y - needed < bottomMargin) { + const contentObj = ctx.flateStream(contentOps); + const contentRef = ctx.nextRef(); + ctx.assign(contentRef, contentObj); + currentPage.node.addContentStream(contentRef); + + currentPage = doc.addPage([pageWidth, pageHeight]); + addFontToPage(currentPage); + contentOps = ''; + y = pageHeight - 50; + } + } function addLine(text: string, size: number, bold: boolean = false) { - const hex = textToHex(text); - contentOps += `BT\n/F1 ${size} Tf\n1 0 0 1 ${margin} ${y} Tm\n<${hex}> Tj\nET\n`; - y -= size * 1.5; + const lines = text.split('\n'); + for (const line of lines) { + ensureSpace(size * 1.5); + const hex = textToHex(line); + contentOps += `BT\n/F1 ${size} Tf\n1 0 0 1 ${margin} ${y} Tm\n<${hex}> Tj\nET\n`; + y -= size * 1.5; + } } function addSeparator() { + ensureSpace(12); let hex = ''; for (let i = 0; i < 55; i++) hex += '002D'; contentOps += `BT\n/F1 8 Tf\n1 0 0 1 ${margin} ${y} Tm\n<${hex}> Tj\nET\n`; @@ -125,10 +161,9 @@ export async function generateAssessmentPdf(options: PdfReportOptions): Promise< addSeparator(); for (const section of options.sections) { - if (y < 60) break; + ensureSpace(30); addLine(section.title, 12); for (const line of section.lines) { - if (y < 40) break; addLine(line, 10); } y -= 6; @@ -140,7 +175,7 @@ export async function generateAssessmentPdf(options: PdfReportOptions): Promise< const contentObj = ctx.flateStream(contentOps); const contentRef = ctx.nextRef(); ctx.assign(contentRef, contentObj); - page.node.addContentStream(contentRef); + currentPage.node.addContentStream(contentRef); return Buffer.from(await doc.save()); } diff --git a/server/test/app.e2e-spec.ts b/server/test/app.e2e-spec.ts index 5bf8149..e714204 100644 --- a/server/test/app.e2e-spec.ts +++ b/server/test/app.e2e-spec.ts @@ -1,5 +1,6 @@ import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { DataSource } from 'typeorm'; import { AssessmentService } from '../src/assessment/assessment.service'; import { AssessmentSession } from '../src/assessment/entities/assessment-session.entity'; import { AssessmentQuestion } from '../src/assessment/entities/assessment-question.entity'; @@ -20,6 +21,16 @@ import { ChatService } from '../src/chat/chat.service'; import { I18nService } from '../src/i18n/i18n.service'; import { TenantService } from '../src/tenant/tenant.service'; +const mockManager = () => ({ + findOne: jest.fn(), + delete: jest.fn().mockResolvedValue({ affected: 1 }), + save: jest.fn(), +}); + +const mockDataSource = () => ({ + transaction: jest.fn(async (cb: any) => cb(mockManager())), +}); + /** * Certificate integration tests — verify the full certificate lifecycle * through the AssessmentService with mocked repositories. @@ -61,6 +72,7 @@ describe('Certificate (integration)', () => { { provide: ChatService, useFactory: mockSvc }, { provide: I18nService, useFactory: mockSvc }, { provide: TenantService, useFactory: mockSvc }, + { provide: DataSource, useFactory: mockDataSource }, ], }).compile();