fix(teams): wrap updateSplits delete and save in database transaction (#375)

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<Bounty>) => Promise.resolve(b)),
     };
+    mockManager = {
+      delete: jest.fn().mockResolvedValue(undefined),
+      create: jest.fn((_, s: Partial<TeamMemberSplit>) => s),
+      save: jest.fn((_, s: Partial<TeamMemberSplit>[]) =>
+        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);

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;
   }