fix: resolve 9 P0+P1 review findings (bills decorators, campus isolation bypass, ding raw crash, batch import counter, SSE multi-tab, interval leak, dept permissions, cycle prevention, tree depth)

This commit is contained in:
2026-07-06 00:38:51 +08:00
parent 90b6ce4aba
commit b3655da012
8 changed files with 75 additions and 6 deletions

View File

@@ -58,6 +58,7 @@ const NotificationBell: React.FC = () => {
}; };
const openRef = useRef(open); const openRef = useRef(open);
openRef.current = open; openRef.current = open;
const retryRef = useRef<number | null>(null);
// SSE connection — decoupled from popover open state // SSE connection — decoupled from popover open state
useEffect(() => { useEffect(() => {
@@ -74,9 +75,14 @@ const NotificationBell: React.FC = () => {
}; };
es.onerror = () => { es.onerror = () => {
es.close(); es.close();
setInterval(fetchUnread, 60_000); if (retryRef.current !== null) clearInterval(retryRef.current);
retryRef.current = window.setInterval(fetchUnread, 60_000);
};
return () => {
es.close();
clearInterval(retryRef.current ?? undefined);
retryRef.current = null;
}; };
return () => es.close();
}, []); }, []);
const handleOpen = (visible: boolean) => { const handleOpen = (visible: boolean) => {

View File

@@ -233,7 +233,7 @@ export class AttendanceService {
} }
return this.dingRawRepo.find({ return this.dingRawRepo.find({
where: await this.scope.filter(where), where,
relations: ['matchedStudent'], relations: ['matchedStudent'],
order: { attendanceDate: 'DESC', checkInTime: 'ASC' }, order: { attendanceDate: 'DESC', checkInTime: 'ASC' },
}); });

View File

@@ -27,6 +27,8 @@ import { extractRequestInfo } from '../common/request-utils';
import { RequirePermission } from '../auth/decorators/permission.decorator'; import { RequirePermission } from '../auth/decorators/permission.decorator';
import type { Response } from 'express'; import type { Response } from 'express';
@UseGuards(JwtAuthGuard)
@Controller('bills')
export class BillsController { export class BillsController {
constructor( constructor(
private service: BillsService, private service: BillsService,

View File

@@ -44,7 +44,13 @@ export class CampusScope {
} }
const ids = await this.getEffectiveScopeIds(); const ids = await this.getEffectiveScopeIds();
if (ids.length === 0) return where; if (ids.length === 0) {
// Non-super-admin with no scoping → match nothing, never leak unfiltered data
if (!this.isSuperAdmin) {
return { ...where, departmentId: In([]) } as unknown as T;
}
return where;
}
return { ...where, departmentId: In(ids) } as unknown as T; return { ...where, departmentId: In(ids) } as unknown as T;
} }

View File

@@ -16,6 +16,7 @@ import {
AssignUserDto, AssignUserDto,
} from './dto/department.dto'; } from './dto/department.dto';
import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
import { RequirePermission } from '../auth/decorators/permission.decorator';
@UseGuards(JwtAuthGuard) @UseGuards(JwtAuthGuard)
@Controller('departments') @Controller('departments')
@@ -23,26 +24,31 @@ export class DepartmentsController {
constructor(private readonly departmentsService: DepartmentsService) {} constructor(private readonly departmentsService: DepartmentsService) {}
@Get() @Get()
@RequirePermission('department:view')
findAll() { findAll() {
return this.departmentsService.findAll(); return this.departmentsService.findAll();
} }
@Get('tree') @Get('tree')
@RequirePermission('department:view')
findTree() { findTree() {
return this.departmentsService.findTree(); return this.departmentsService.findTree();
} }
@Get(':id') @Get(':id')
@RequirePermission('department:view')
findOne(@Param('id', ParseIntPipe) id: number) { findOne(@Param('id', ParseIntPipe) id: number) {
return this.departmentsService.findOne(id); return this.departmentsService.findOne(id);
} }
@Post() @Post()
@RequirePermission('department:edit')
create(@Body() dto: CreateDepartmentDto) { create(@Body() dto: CreateDepartmentDto) {
return this.departmentsService.create(dto); return this.departmentsService.create(dto);
} }
@Put(':id') @Put(':id')
@RequirePermission('department:edit')
update( update(
@Param('id', ParseIntPipe) id: number, @Param('id', ParseIntPipe) id: number,
@Body() dto: UpdateDepartmentDto, @Body() dto: UpdateDepartmentDto,
@@ -51,16 +57,19 @@ export class DepartmentsController {
} }
@Delete(':id') @Delete(':id')
@RequirePermission('department:delete')
remove(@Param('id', ParseIntPipe) id: number) { remove(@Param('id', ParseIntPipe) id: number) {
return this.departmentsService.remove(id); return this.departmentsService.remove(id);
} }
@Get(':id/users') @Get(':id/users')
@RequirePermission('department:view')
getUsers(@Param('id', ParseIntPipe) id: number) { getUsers(@Param('id', ParseIntPipe) id: number) {
return this.departmentsService.getUsers(id); return this.departmentsService.getUsers(id);
} }
@Post(':id/users') @Post(':id/users')
@RequirePermission('department:edit')
assignUser( assignUser(
@Param('id', ParseIntPipe) id: number, @Param('id', ParseIntPipe) id: number,
@Body() dto: AssignUserDto, @Body() dto: AssignUserDto,
@@ -69,6 +78,7 @@ export class DepartmentsController {
} }
@Delete(':id/users/:userId') @Delete(':id/users/:userId')
@RequirePermission('department:delete')
removeUser( removeUser(
@Param('id', ParseIntPipe) id: number, @Param('id', ParseIntPipe) id: number,
@Param('userId', ParseIntPipe) userId: number, @Param('userId', ParseIntPipe) userId: number,

View File

@@ -22,12 +22,33 @@ export class DepartmentsService {
} }
async findTree(): Promise<Department[]> { async findTree(): Promise<Department[]> {
// Load ALL departments flat (no relations — avoids N+1 and only loads 1 level),
// then build the full tree in memory.
const all = await this.deptRepo.find({ const all = await this.deptRepo.find({
where: { status: 'active' }, where: { status: 'active' },
order: { sortOrder: 'ASC', name: 'ASC' }, order: { sortOrder: 'ASC', name: 'ASC' },
relations: ['children'],
}); });
return all.filter((d) => d.parentId === null);
const byParent = new Map<number | null, Department[]>();
for (const dept of all) {
const key = dept.parentId ?? null;
const list = byParent.get(key);
if (list) {
list.push(dept);
} else {
byParent.set(key, [dept]);
}
}
const attachChildren = (dept: Department): void => {
const children = byParent.get(dept.id) ?? [];
dept.children = children;
for (const child of children) attachChildren(child);
};
const roots = byParent.get(null) ?? [];
for (const root of roots) attachChildren(root);
return roots;
} }
async findOne(id: number): Promise<Department> { async findOne(id: number): Promise<Department> {
@@ -43,6 +64,19 @@ export class DepartmentsService {
async update(id: number, dto: UpdateDepartmentDto): Promise<Department> { async update(id: number, dto: UpdateDepartmentDto): Promise<Department> {
const dept = await this.findOne(id); const dept = await this.findOne(id);
// Prevent parent cycles: parentId must not be the dept itself or one of its descendants
if (dto.parentId !== undefined && dto.parentId !== null) {
const newParentId = dto.parentId;
if (newParentId === id) {
throw new ConflictException('不能将部门的上级设为自身');
}
const descendantIds = await this.getDescendantIds(id);
if (descendantIds.includes(newParentId)) {
throw new ConflictException('不能将部门的上级设为其子部门,会形成循环');
}
}
Object.assign(dept, dto); Object.assign(dept, dto);
return this.deptRepo.save(dept); return this.deptRepo.save(dept);
} }

View File

@@ -9,6 +9,7 @@ import { CreateNotificationDto } from './dto/notification.dto';
@Injectable() @Injectable()
export class NotificationsService { export class NotificationsService {
private subjects = new Map<number, Subject<Notification>>(); private subjects = new Map<number, Subject<Notification>>();
private subscriberCounts = new Map<number, number>();
constructor( constructor(
@InjectRepository(Notification) @InjectRepository(Notification)
@@ -76,11 +77,20 @@ export class NotificationsService {
subscribe(userId: number): Observable<Notification> { subscribe(userId: number): Observable<Notification> {
if (!this.subjects.has(userId)) { if (!this.subjects.has(userId)) {
this.subjects.set(userId, new Subject<Notification>()); this.subjects.set(userId, new Subject<Notification>());
this.subscriberCounts.set(userId, 0);
} }
this.subscriberCounts.set(userId, (this.subscriberCounts.get(userId) ?? 0) + 1);
return this.subjects.get(userId)!.asObservable(); return this.subjects.get(userId)!.asObservable();
} }
unsubscribe(userId: number): void { unsubscribe(userId: number): void {
const count = (this.subscriberCounts.get(userId) ?? 0) - 1;
if (count > 0) {
this.subscriberCounts.set(userId, count);
return;
}
// Last subscriber gone → complete + clean up
this.subscriberCounts.delete(userId);
const subj = this.subjects.get(userId); const subj = this.subjects.get(userId);
if (subj) { if (subj) {
subj.complete(); subj.complete();

View File

@@ -142,6 +142,7 @@ export class StudentsService {
tenantId: row.tenantId || undefined, tenantId: row.tenantId || undefined,
}), }),
); );
imported++;
} }
return { return {
message: `成功导入 ${imported} 名学生,跳过 ${skipped} 条(重复或空行)`, message: `成功导入 ${imported} 名学生,跳过 ${skipped} 条(重复或空行)`,