fix: add status guards to prevent data loss

- create: auto-delete REJECTED→throw error; add tenantId filter - remove: forbid PUBLISHED bank deletion - removeItem: forbid PUBLISHED item deletion - generateQuestions: restrict to DRAFT status only - frontend: render MULTIPLE_CHOICE options/judgment/followupHints - frontend: add judgment and followupHints to QuestionBankItem type - add 12 service guard tests (109 total)
2026-05-21 08:55:35 +08:00
parent e782d180d7
commit 57898f939c
4 changed files with 210 additions and 5 deletions
@@ -1,6 +1,11 @@
+import { Test, TestingModule } from '@nestjs/testing';
+import { getRepositoryToken } from '@nestjs/typeorm';
+import { ConfigService } from '@nestjs/config';
+import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
 import {
  GENERATE_QUESTIONS_SYSTEM_PROMPT,
  parseGeneratedQuestion,
+  QuestionBankService,
 } from './question-bank.service';
 import {
  QuestionBankItem,
@@ -9,8 +14,13 @@ import {
  QuestionDimension,
  QuestionBankItemStatus,
 } from '../entities/question-bank-item.entity';
+import { QuestionBank, QuestionBankStatus } from '../entities/question-bank.entity';
+import { ModelConfigService } from '../../model-config/model-config.service';

 const BANK_ID = 'test-bank-id';
+const TEMPLATE_ID = 'test-template-id';
+const USER_ID = 'user-1';
+const TENANT_ID = 'default';

 describe('GENERATE_QUESTIONS_SYSTEM_PROMPT', () => {
  it('should require both choice and open question types', () => {
@@ -259,3 +269,161 @@ describe('parseGeneratedQuestion', () => {
    });
  });
 });
+
+describe('QuestionBankService - status guards', () => {
+  let service: QuestionBankService;
+  let bankRepo: any;
+  let itemRepo: any;
+
+  const mockRepository = () => ({
+    findOne: jest.fn(),
+    find: jest.fn(),
+    save: jest.fn().mockImplementation((entity: any) => Promise.resolve(entity)),
+    create: jest.fn((dto: any) => dto as any),
+    remove: jest.fn().mockResolvedValue(undefined),
+    createQueryBuilder: jest.fn().mockReturnValue({
+      leftJoinAndSelect: jest.fn().mockReturnThis(),
+      where: jest.fn().mockReturnThis(),
+      orderBy: jest.fn().mockReturnThis(),
+      skip: jest.fn().mockReturnThis(),
+      take: jest.fn().mockReturnThis(),
+      getManyAndCount: jest.fn().mockResolvedValue([[], 0]),
+      getMany: jest.fn().mockResolvedValue([]),
+    }),
+  });
+
+  const mockModelConfig = () => ({
+    findDefaultByType: jest.fn().mockResolvedValue({
+      apiKey: 'sk-test',
+      modelId: 'deepseek-chat',
+      baseUrl: 'https://api.deepseek.com/v1',
+    }),
+  });
+
+  const makeBank = (overrides?: Partial<QuestionBank>): QuestionBank =>
+    ({
+      id: 'bank-1',
+      name: 'Test Bank',
+      status: QuestionBankStatus.DRAFT,
+      templateId: TEMPLATE_ID,
+      tenantId: TENANT_ID,
+      ...overrides,
+    }) as QuestionBank;
+
+  const makeItem = (overrides?: Partial<QuestionBankItem>): QuestionBankItem =>
+    ({
+      id: 'item-1',
+      bankId: BANK_ID,
+      questionText: 'Question?',
+      questionType: QuestionType.SHORT_ANSWER,
+      keyPoints: ['kp1'],
+      difficulty: QuestionDifficulty.STANDARD,
+      dimension: QuestionDimension.PROMPT,
+      status: QuestionBankItemStatus.PENDING_REVIEW,
+      ...overrides,
+    }) as QuestionBankItem;
+
+  beforeEach(async () => {
+    const module: TestingModule = await Test.createTestingModule({
+      providers: [
+        QuestionBankService,
+        { provide: getRepositoryToken(QuestionBank), useFactory: mockRepository },
+        { provide: getRepositoryToken(QuestionBankItem), useFactory: mockRepository },
+        { provide: ModelConfigService, useFactory: mockModelConfig },
+        { provide: ConfigService, useFactory: () => ({}) },
+      ],
+    }).compile();
+
+    service = module.get<QuestionBankService>(QuestionBankService);
+    bankRepo = module.get(getRepositoryToken(QuestionBank));
+    itemRepo = module.get(getRepositoryToken(QuestionBankItem));
+  });
+
+  describe('create', () => {
+    const createDto = { name: 'New Bank', templateId: TEMPLATE_ID };
+
+    it('create: should allow cross-tenant when DRAFT exists for another tenant', async () => {
+      bankRepo.findOne.mockResolvedValue(null);
+
+      const result = await service.create(createDto, USER_ID, TENANT_ID);
+      expect(result).toBeDefined();
+    });
+
+    it('create: DRAFT exists same tenant → BadRequestException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.DRAFT }));
+
+      await expect(service.create(createDto, USER_ID, TENANT_ID)).rejects.toThrow(BadRequestException);
+    });
+
+    it('create: REJECTED exists same tenant → BadRequestException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.REJECTED }));
+
+      await expect(service.create(createDto, USER_ID, TENANT_ID)).rejects.toThrow(BadRequestException);
+    });
+
+    it('create: PUBLISHED exists same tenant → BadRequestException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.PUBLISHED }));
+
+      await expect(service.create(createDto, USER_ID, TENANT_ID)).rejects.toThrow(BadRequestException);
+    });
+
+    it('create: no existing bank → success', async () => {
+      bankRepo.findOne.mockResolvedValue(null);
+
+      const result = await service.create(createDto, USER_ID, TENANT_ID);
+      expect(result).toBeDefined();
+    });
+  });
+
+  describe('remove', () => {
+    it('remove: DRAFT → success', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.DRAFT }));
+
+      await expect(service.remove('bank-1')).resolves.toBeUndefined();
+    });
+
+    it('remove: REJECTED → success', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.REJECTED }));
+
+      await expect(service.remove('bank-1')).resolves.toBeUndefined();
+    });
+
+    it('remove: PUBLISHED → ForbiddenException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.PUBLISHED }));
+
+      await expect(service.remove('bank-1')).rejects.toThrow(ForbiddenException);
+    });
+  });
+
+  describe('removeItem', () => {
+    it('removeItem: PENDING_REVIEW item → success', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank());
+      itemRepo.findOne.mockResolvedValue(makeItem({ status: QuestionBankItemStatus.PENDING_REVIEW }));
+
+      await expect(service.removeItem(BANK_ID, 'item-1')).resolves.toBeUndefined();
+    });
+
+    it('removeItem: PUBLISHED item → ForbiddenException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank());
+      itemRepo.findOne.mockResolvedValue(makeItem({ status: QuestionBankItemStatus.PUBLISHED }));
+
+      await expect(service.removeItem(BANK_ID, 'item-1')).rejects.toThrow(ForbiddenException);
+    });
+  });
+
+  describe('generateQuestions', () => {
+    it('generateQuestions: PUBLISHED bank → ForbiddenException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.PUBLISHED }));
+
+      await expect(service.generateQuestions('bank-1', 1, 'some content', TENANT_ID))
+        .rejects.toThrow(ForbiddenException);
+    });
+
+    it('generateQuestions: PENDING_REVIEW bank → ForbiddenException', async () => {
+      bankRepo.findOne.mockResolvedValue(makeBank({ status: QuestionBankStatus.PENDING_REVIEW }));
+
+      await expect(service.generateQuestions('bank-1', 1, 'some content', TENANT_ID))
+        .rejects.toThrow(ForbiddenException);
+    });
+  });
+});
@@ -266,13 +266,11 @@ export class QuestionBankService {
    }
    if (createDto.templateId) {
      const existing = await this.bankRepository.findOne({
-        where: { templateId: createDto.templateId },
+        where: { templateId: createDto.templateId, tenantId: tenantId || undefined as any },
      });
      if (existing) {
-        if (existing.status === QuestionBankStatus.DRAFT || existing.status === QuestionBankStatus.REJECTED) {
-          await this.bankRepository.remove(existing);
-        } else {
-          throw new BadRequestException('该模板已关联有效题库，请编辑已有题库');
+        if (existing.status === QuestionBankStatus.DRAFT || existing.status === QuestionBankStatus.REJECTED || existing.status === QuestionBankStatus.PUBLISHED) {
+          throw new BadRequestException('该模板已关联题库，请编辑已有题库或删除后重建');
        }
      }
    }
@@ -349,6 +347,9 @@ export class QuestionBankService {

  async remove(id: string): Promise<void> {
    const bank = await this.findOne(id);
+    if (bank.status === QuestionBankStatus.PUBLISHED) {
+      throw new ForbiddenException('已发布的题库不可删除');
+    }
    await this.bankRepository.remove(bank);
  }

@@ -441,6 +442,9 @@ export class QuestionBankService {
    if (!item) {
      throw new NotFoundException(`QuestionBankItem with ID "${itemId}" not found`);
    }
+    if (item.status === QuestionBankItemStatus.PUBLISHED) {
+      throw new ForbiddenException('已发布的题目不可删除');
+    }
    await this.itemRepository.remove(item);
  }

@@ -452,6 +456,10 @@ export class QuestionBankService {
  ): Promise<QuestionBankItem[]> {
    const bank = await this.findOne(bankId);

+    if (bank.status !== QuestionBankStatus.DRAFT) {
+      throw new ForbiddenException('仅草稿状态的题库可生成题目');
+    }
+
    if (count <= 0 || count > 50) {
      throw new BadRequestException('生成数量必须在 1-50 之间');
    }