fix: code review — 7 issues resolved
(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
This commit is contained in:
@@ -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 },
|
||||
],
|
||||
|
||||
@@ -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>(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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1145,7 +1145,7 @@ const initialState: Partial<EvaluationState> = {
|
||||
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<EvaluationState> = {
|
||||
|
||||
async batchDeleteSessions(ids: string[], user: any): Promise<number> {
|
||||
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<any[]> {
|
||||
|
||||
@@ -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<string, number>;
|
||||
|
||||
@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;
|
||||
|
||||
|
||||
@@ -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') {
|
||||
|
||||
@@ -47,69 +47,105 @@ function textToHex(str: string): string {
|
||||
|
||||
export async function generateAssessmentPdf(options: PdfReportOptions): Promise<Buffer> {
|
||||
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());
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user