diff --git a/src/teams/teams.service.spec.ts b/src/teams/teams.service.spec.ts index b66b0a9..560ef88 100644 --- a/src/teams/teams.service.spec.ts +++ b/src/teams/teams.service.spec.ts @@ -1,5 +1,5 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { getRepositoryToken } from '@nestjs/typeorm'; +import { getDataSourceToken, getRepositoryToken } from '@nestjs/typeorm'; import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { TeamsService } from './teams.service'; import { Bounty, Team, TeamMemberSplit, User } from '../common/entities'; @@ -10,6 +10,8 @@ describe('TeamsService', () => { let teamRepo: { findOne: jest.Mock; save: jest.Mock; create: jest.Mock }; let splitRepo: { save: jest.Mock; create: jest.Mock; delete: jest.Mock }; let bountyRepo: { findOne: jest.Mock; save: jest.Mock }; + let mockManager: { delete: jest.Mock; save: jest.Mock; create: jest.Mock }; + let dataSource: { transaction: jest.Mock }; let userRepo: { findOne: jest.Mock }; beforeEach(async () => { @@ -34,6 +36,16 @@ describe('TeamsService', () => { findOne: jest.fn(), save: jest.fn((b: Partial) => Promise.resolve(b)), }; + mockManager = { + delete: jest.fn().mockResolvedValue(undefined), + create: jest.fn((_, s: Partial) => s), + save: jest.fn((_, s: Partial[]) => + Promise.resolve(s.map((x) => ({ id: `split-${x.userId}`, ...x }))), + ), + }; + dataSource = { + transaction: jest.fn().mockImplementation((cb) => cb(mockManager)), + }; userRepo = { findOne: jest.fn(), }; @@ -44,6 +56,7 @@ describe('TeamsService', () => { { provide: getRepositoryToken(Team), useValue: teamRepo }, { provide: getRepositoryToken(TeamMemberSplit), useValue: splitRepo }, { provide: getRepositoryToken(Bounty), useValue: bountyRepo }, + { provide: getDataSourceToken(), useValue: dataSource }, { provide: getRepositoryToken(User), useValue: userRepo }, ], }).compile(); @@ -132,6 +145,62 @@ describe('TeamsService', () => { }); }); + describe('updateSplits', () => { + it('rejects via validateSplitPercentages when splits do not sum to 100', async () => { + teamRepo.findOne.mockResolvedValue({ id: 't1', splits: [] }); + + await expect( + service.updateSplits('t1', [{ userId: 'u1', percentage: 60 }]), + ).rejects.toThrow( + 'team member split percentages must sum to 100, got 60.00', + ); + + expect(dataSource.transaction).not.toHaveBeenCalled(); + }); + + it('replaces member splits atomically within a database transaction', async () => { + teamRepo.findOne.mockResolvedValue({ id: 't1', splits: [] }); + + const updated = await service.updateSplits('t1', [ + { userId: 'u1', role: 'lead', percentage: 70 }, + { userId: 'u2', percentage: 30 }, + ]); + + expect(dataSource.transaction).toHaveBeenCalledTimes(1); + expect(mockManager.delete).toHaveBeenCalledWith(TeamMemberSplit, { + teamId: 't1', + }); + expect(mockManager.save).toHaveBeenCalledWith( + TeamMemberSplit, + expect.arrayContaining([ + expect.objectContaining({ + teamId: 't1', + userId: 'u1', + role: 'lead', + percentage: '70.00', + }), + expect.objectContaining({ + teamId: 't1', + userId: 'u2', + role: null, + percentage: '30.00', + }), + ]), + ); + expect(updated.splits).toHaveLength(2); + }); + + it('throws NotFoundException when the team does not exist without entering transaction', async () => { + teamRepo.findOne.mockResolvedValue(null); + + await expect( + service.updateSplits('missing-team', [{ userId: 'u1', percentage: 100 }]), + ).rejects.toThrow(NotFoundException); + + expect(dataSource.transaction).not.toHaveBeenCalled(); + }); + }); + describe('assignToBounty', () => { it('throws NotFoundException when the team does not exist', async () => { teamRepo.findOne.mockResolvedValue(null); diff --git a/src/teams/teams.service.ts b/src/teams/teams.service.ts index 7e0a656..2470060 100644 --- a/src/teams/teams.service.ts +++ b/src/teams/teams.service.ts @@ -4,8 +4,8 @@ import { Injectable, NotFoundException, } from '@nestjs/common'; -import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; +import { DataSource, Repository } from 'typeorm'; import { Bounty, Team, TeamMemberSplit, User } from '../common/entities'; import { BountyStatus, UserRole } from '../common/enums'; import { CreateTeamDto, TeamMemberSplitDto } from './dto/create-team.dto'; @@ -80,22 +80,22 @@ export class TeamsService { throw new NotFoundException(`User ${member.userId} not found`); } } - - // Remove existing splits - await this.splitRepo.delete({ teamId: team.id }); - - // Create new splits — one batched save, same rationale as create() (#150). - team.splits = await this.splitRepo.save( - members.map((m) => - this.splitRepo.create({ - teamId: team.id, - userId: m.userId, - role: m.role ?? null, - percentage: m.percentage.toFixed(2), - }), - ), - ); - + // Delete existing splits and insert new splits within a single database transaction (#375). + // If saving the new splits fails, the rollback preserves the existing valid split configuration. + team.splits = await this.dataSource.transaction(async (mgr) => { + await mgr.delete(TeamMemberSplit, { teamId: team.id }); + return mgr.save( + TeamMemberSplit, + members.map((m) => + mgr.create(TeamMemberSplit, { + teamId: team.id, + userId: m.userId, + role: m.role ?? null, + percentage: m.percentage.toFixed(2), + }), + ), + ); + }); return team; }