xiongyuxing/gongxue-base
1
0
Fork 0
forked from wangziqi/gongxue-base
Code Pull Requests Activity

fix: add DTO validation, transactional imports, role-not-found handling, null role guard

Browse Source
This commit is contained in:
wangziqi
2026-07-09 11:28:11 +08:00
parent 33e6cb445c
commit 0cfdab95ee
5 changed files with 241 additions and 457 deletions
Download Patch File Download Diff File Expand all files Collapse all files

183
apps/server/src/sync/sync.service.spec.ts
View File

@@ -1,6 +1,6 @@
import { Test, TestingModule } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { DataSource, Repository } from 'typeorm';
import { SyncService, ImportUserDto } from './sync.service';
import { SyncLog, SyncState, UserDingMapping } from '../entities';
import { User } from '../entities/user.entity';
@@ -11,7 +11,24 @@ import { WeComService } from '../integration/wecom.service';
import { AttendanceImportService } from '../attendance/attendance-import.service';
import { ScheduleSyncService } from './schedule-sync.service';
describe('SyncService — new methods', () => {
// ── EntityManager mock helpers ──
interface ManagerMock {
create: jest.Mock;
save: jest.Mock;
findOne: jest.Mock;
}
function mockManager(overrides: Partial<ManagerMock> = {}): ManagerMock {
return {
create: jest.fn(),
save: jest.fn(),
findOne: jest.fn(),
...overrides,
};
}
describe('SyncService — importDingTalkUsers (transactional)', () => {
let service: SyncService;
let mappingRepo: jest.Mocked<
@@ -20,10 +37,15 @@ describe('SyncService — new methods', () => {
let userRepo: jest.Mocked<Pick<Repository<User>, 'create' | 'save'>>;
let studentRepo: jest.Mocked<Pick<Repository<Student>, 'create' | 'save'>>;
let roleRepo: jest.Mocked<Pick<Repository<Role>, 'findOne'>>;
let dataSourceMock: jest.Mocked<Pick<DataSource, 'transaction'>>;
let dingTalkService: jest.Mocked<Pick<DingTalkService, 'fetchOrgTreeWithUsers' | 'syncAll' | 'fetchOrgTree'>>;
let mgr: ManagerMock;
beforeEach(async () => {
mgr = mockManager();
mappingRepo = {
findOne: jest.fn(),
create: jest.fn(),
@@ -45,6 +67,12 @@ describe('SyncService — new methods', () => {
findOne: jest.fn(),
};
dataSourceMock = {
transaction: jest.fn().mockImplementation(
async (cb: (manager: ManagerMock) => Promise<void>) => cb(mgr),
),
};
dingTalkService = {
fetchOrgTreeWithUsers: jest.fn(),
syncAll: jest.fn(),
@@ -64,6 +92,7 @@ describe('SyncService — new methods', () => {
{ provide: getRepositoryToken(User), useValue: userRepo },
{ provide: getRepositoryToken(Student), useValue: studentRepo },
{ provide: getRepositoryToken(Role), useValue: roleRepo },
{ provide: DataSource, useValue: dataSourceMock },
{ provide: DingTalkService, useValue: dingTalkService },
{ provide: WeComService, useValue: mockWeComService },
{ provide: AttendanceImportService, useValue: mockAttendanceImportService },
@@ -90,15 +119,13 @@ describe('SyncService — new methods', () => {
it('imports teacher when roleId is a number (role found)', async () => {
const mockRole = { id: 5, name: 'Teacher' } as Role;
roleRepo.findOne.mockResolvedValue(mockRole);
mgr.findOne.mockResolvedValue(mockRole);
const mockUser = { id: 10 } as User;
userRepo.create.mockReturnValue(mockUser);
userRepo.save.mockResolvedValue(mockUser);
mgr.create.mockReturnValue(mockUser);
mgr.save.mockResolvedValue(mockUser);
mappingRepo.findOne.mockResolvedValue(null);
mappingRepo.create.mockReturnValue({} as UserDingMapping);
mappingRepo.save.mockResolvedValue({} as UserDingMapping);
const users: ImportUserDto[] = [
{ dingUserId: 'user1', name: 'Zhang San', mobile: '13800001111', roleId: 5 },
@@ -109,27 +136,23 @@ describe('SyncService — new methods', () => {
expect(result.teacherCount).toBe(1);
expect(result.studentCount).toBe(0);
expect(result.skipped).toBe(0);
expect(result.warnings).toEqual([]);
expect(roleRepo.findOne).toHaveBeenCalledWith({ where: { id: 5 } });
expect(userRepo.save).toHaveBeenCalledWith(
expect(dataSourceMock.transaction).toHaveBeenCalledTimes(1);
expect(mgr.findOne).toHaveBeenCalledWith(Role, { where: { id: 5 } });
expect(mgr.save).toHaveBeenCalledWith(
expect.objectContaining({ roles: [mockRole] }),
);
});
it('imports teacher when roleId is a number but role not found (creates student as fallback)', async () => {
roleRepo.findOne.mockResolvedValue(null);
it('skips user when role not found (I1 fix — warns + skip, not silent teacher)', async () => {
mgr.findOne.mockResolvedValue(null);
const mockUser = { id: 11 } as User;
userRepo.create.mockReturnValue(mockUser);
userRepo.save.mockResolvedValue(mockUser);
const mockStudent = { id: 31 } as Student;
studentRepo.create.mockReturnValue(mockStudent);
studentRepo.save.mockResolvedValue(mockStudent);
mgr.create.mockReturnValue(mockUser);
mgr.save.mockResolvedValue(mockUser);
mappingRepo.findOne.mockResolvedValue(null);
mappingRepo.create.mockReturnValue({} as UserDingMapping);
mappingRepo.save.mockResolvedValue({} as UserDingMapping);
const users: ImportUserDto[] = [
{ dingUserId: 'user2', name: 'Li Si', mobile: '13800002222', roleId: 999 },
@@ -137,35 +160,31 @@ describe('SyncService — new methods', () => {
const result = await service.importDingTalkUsers(users);
// I1: role not found → skip with warning, NOT counted as teacher
expect(result.teacherCount).toBe(0);
expect(result.studentCount).toBe(1);
expect(result.skipped).toBe(0);
expect(result.studentCount).toBe(0);
expect(result.skipped).toBe(1);
expect(result.warnings).toEqual([
'角色 id=999 不存在，跳过用户 Li Si(user2)',
]);
// User should be saved once (no second save for role assignment)
expect(userRepo.save).toHaveBeenCalledTimes(1);
// Student record should have been created as fallback
expect(studentRepo.create).toHaveBeenCalledWith(
expect.objectContaining({
name: 'Li Si',
userId: 11,
status: 'active',
}),
);
expect(studentRepo.save).toHaveBeenCalled();
expect(dataSourceMock.transaction).toHaveBeenCalledTimes(1);
});
it('imports student when roleId is null', async () => {
const mockUser = { id: 20 } as User;
userRepo.create.mockReturnValue(mockUser);
userRepo.save.mockResolvedValue(mockUser);
mgr.create.mockReturnValueOnce(mockUser);
mgr.save.mockResolvedValueOnce(mockUser);
const mockStudent = { id: 30 } as Student;
studentRepo.create.mockReturnValue(mockStudent);
studentRepo.save.mockResolvedValue(mockStudent);
mgr.create.mockReturnValueOnce(mockStudent);
mgr.save.mockResolvedValueOnce(mockStudent);
// mapping create+save also calls create/save
mgr.create.mockReturnValueOnce({} as UserDingMapping);
mgr.save.mockResolvedValueOnce({} as UserDingMapping);
mappingRepo.findOne.mockResolvedValue(null);
mappingRepo.create.mockReturnValue({} as UserDingMapping);
mappingRepo.save.mockResolvedValue({} as UserDingMapping);
const users: ImportUserDto[] = [
{ dingUserId: 'user3', name: 'Wang Wu', mobile: '', roleId: null },
@@ -176,15 +195,20 @@ describe('SyncService — new methods', () => {
expect(result.studentCount).toBe(1);
expect(result.teacherCount).toBe(0);
expect(result.skipped).toBe(0);
expect(result.warnings).toEqual([]);
expect(studentRepo.create).toHaveBeenCalledWith(
// Verify Student was created via manager
const studentCreateCalls = mgr.create.mock.calls.filter(
([entity]) => entity === Student,
);
expect(studentCreateCalls.length).toBe(1);
expect(studentCreateCalls[0][1]).toEqual(
expect.objectContaining({
name: 'Wang Wu',
userId: 20,
status: 'active',
}),
);
expect(studentRepo.save).toHaveBeenCalled();
});
it('skips user when mapping already exists', async () => {
@@ -199,25 +223,30 @@ describe('SyncService — new methods', () => {
expect(result.skipped).toBe(1);
expect(result.teacherCount).toBe(0);
expect(result.studentCount).toBe(0);
expect(userRepo.create).not.toHaveBeenCalled();
expect(result.warnings).toEqual([]);
expect(dataSourceMock.transaction).not.toHaveBeenCalled();
});
it('handles per-user errors gracefully — one failure does not block others', async () => {
// First user fails, second succeeds
mappingRepo.findOne.mockResolvedValue(null);
mappingRepo.create.mockReturnValue({} as UserDingMapping);
mappingRepo.save.mockResolvedValue({} as UserDingMapping);
userRepo.create
.mockReturnValueOnce(new Error('DB error') as unknown as User)
.mockReturnValueOnce({ id: 40 } as User);
userRepo.save
.mockRejectedValueOnce(new Error('DB error'))
.mockResolvedValueOnce({ id: 40 } as User);
studentRepo.create.mockReturnValue({} as Student);
studentRepo.save.mockResolvedValue({} as Student);
// First transaction throws, second succeeds
dataSourceMock.transaction
.mockImplementationOnce(async () => {
throw new Error('DB error');
})
.mockImplementationOnce(async (cb) => {
const freshMgr = mockManager();
const mockUser = { id: 40 } as User;
freshMgr.create.mockReturnValueOnce(mockUser);
freshMgr.save.mockResolvedValueOnce(mockUser);
const mockStudent = { id: 30 } as Student;
freshMgr.create.mockReturnValueOnce(mockStudent);
freshMgr.save.mockResolvedValueOnce(mockStudent);
freshMgr.create.mockReturnValueOnce({} as UserDingMapping);
freshMgr.save.mockResolvedValueOnce({} as UserDingMapping);
await cb(freshMgr);
});
const users: ImportUserDto[] = [
{ dingUserId: 'fail', name: 'Fail User', mobile: '', roleId: null },
@@ -228,31 +257,41 @@ describe('SyncService — new methods', () => {
expect(result.studentCount).toBe(1);
expect(result.skipped).toBe(0);
// The first user's mapping should not be saved, but the second's should
expect(mappingRepo.save).toHaveBeenCalledTimes(1);
expect(result.warnings).toEqual([]);
expect(dataSourceMock.transaction).toHaveBeenCalledTimes(2);
});
it('counts mixed teacher/student/skipped correctly', async () => {
const mockRole = { id: 1, name: 'Teacher Role' } as Role;
roleRepo.findOne.mockResolvedValue(mockRole);
const mockUser = { id: 50 } as User;
userRepo.create.mockReturnValue(mockUser);
userRepo.save.mockResolvedValue(mockUser);
studentRepo.create.mockReturnValue({} as Student);
studentRepo.save.mockResolvedValue({} as Student);
mappingRepo.create.mockReturnValue({} as UserDingMapping);
mappingRepo.save.mockResolvedValue({} as UserDingMapping);
// First: skip (existing), second: teacher, third: student
// First: skip (existing)
mappingRepo.findOne
.mockResolvedValueOnce({ id: 99 } as UserDingMapping) // skip
.mockResolvedValueOnce(null) // teacher
.mockResolvedValueOnce({ id: 99 } as UserDingMapping)
.mockResolvedValueOnce(null) // teacher
.mockResolvedValueOnce(null); // student
// Teacher's manager operations
const teacherMgr = mockManager();
teacherMgr.findOne.mockResolvedValue(mockRole);
const teacherUser = { id: 50 } as User;
teacherMgr.create.mockReturnValue(teacherUser);
teacherMgr.save.mockResolvedValue(teacherUser);
// Student's manager operations
const studentMgr = mockManager();
const studentUser = { id: 51 } as User;
const mockStudent = { id: 50 } as Student;
studentMgr.create.mockReturnValueOnce(studentUser);
studentMgr.save.mockResolvedValueOnce(studentUser);
studentMgr.create.mockReturnValueOnce(mockStudent);
studentMgr.save.mockResolvedValueOnce(mockStudent);
studentMgr.create.mockReturnValueOnce({} as UserDingMapping);
studentMgr.save.mockResolvedValueOnce({} as UserDingMapping);
dataSourceMock.transaction
.mockImplementationOnce(async (cb) => cb(teacherMgr))
.mockImplementationOnce(async (cb) => cb(studentMgr));
const users: ImportUserDto[] = [
{ dingUserId: 'skip', name: 'Skip', mobile: '138', roleId: null },
{ dingUserId: 'teacher', name: 'Teacher', mobile: '139', roleId: 1 },
@@ -264,5 +303,7 @@ describe('SyncService — new methods', () => {
expect(result.teacherCount).toBe(1);
expect(result.studentCount).toBe(1);
expect(result.skipped).toBe(1);
expect(result.warnings).toEqual([]);
expect(dataSourceMock.transaction).toHaveBeenCalledTimes(2);
});
});