From 93fded717e8a3ec89aef8dd50a71e70100574ee8 Mon Sep 17 00:00:00 2001 From: ghzhost Date: Sun, 16 Aug 2026 02:50:02 +0000 Subject: [PATCH] fix(teams): change TeamMemberSplit.user onDelete from CASCADE to RESTRICT (#58) - Update TeamMemberSplit.user relation onDelete to 'RESTRICT' to preserve team percentage invariants - Add migration TeamMemberSplitFkRestrict1784500000000 replacing FK constraint with RESTRICT - Add unit tests for TeamsService covering team creation, validation and bounty assignment --- .../entities/team-member-split.entity.ts | 5 +- ...1784500000000-TeamMemberSplitFkRestrict.ts | 76 +++++++++++++ src/teams/teams.service.spec.ts | 102 ++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 src/database/migrations/1784500000000-TeamMemberSplitFkRestrict.ts create mode 100644 src/teams/teams.service.spec.ts diff --git a/src/common/entities/team-member-split.entity.ts b/src/common/entities/team-member-split.entity.ts index 65482ee..5435d34 100644 --- a/src/common/entities/team-member-split.entity.ts +++ b/src/common/entities/team-member-split.entity.ts @@ -21,7 +21,10 @@ export class TeamMemberSplit { @Column() teamId: string; - @ManyToOne(() => User, { onDelete: 'CASCADE' }) + // RESTRICT, not CASCADE: a TeamMemberSplit is a financial commitment (a promised + // percentage of a bounty payout). Deleting a User row must never silently delete + // their TeamMemberSplit row and desync the team's split percentage sum from 100%. See #58. + @ManyToOne(() => User, { onDelete: 'RESTRICT' }) @JoinColumn() user: User; diff --git a/src/database/migrations/1784500000000-TeamMemberSplitFkRestrict.ts b/src/database/migrations/1784500000000-TeamMemberSplitFkRestrict.ts new file mode 100644 index 0000000..aaa64c6 --- /dev/null +++ b/src/database/migrations/1784500000000-TeamMemberSplitFkRestrict.ts @@ -0,0 +1,76 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Fixes the data integrity issue described in #58: + * Re-points `team_member_splits.userId` foreign key from `ON DELETE CASCADE` + * to `ON DELETE RESTRICT`. + * + * A TeamMemberSplit represents a promised percentage of a bounty payout. + * Cascading user deletes silently removes individual member splits, causing + * the remaining split percentages to no longer sum to 100%. When such a bounty + * is merged and released, `EscrowService.splitRelease` / `assertValidSplits` + * throws a BadRequestException, permanently locking the funds and stranding + * the bounty in MERGED state. + */ +export class TeamMemberSplitFkRestrict1784500000000 implements MigrationInterface { + name = 'TeamMemberSplitFkRestrict1784500000000'; + + public async up(queryRunner: QueryRunner): Promise { + await this.replaceForeignKeyOnDelete( + queryRunner, + 'team_member_splits', + 'userId', + 'users', + 'RESTRICT', + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await this.replaceForeignKeyOnDelete( + queryRunner, + 'team_member_splits', + 'userId', + 'users', + 'CASCADE', + ); + } + + /** + * Finds the existing single-column foreign key from `table.column` and + * replaces its ON DELETE action in place, preserving whatever name + * `synchronize` (or a previous migration) originally gave it. + */ + private async replaceForeignKeyOnDelete( + queryRunner: QueryRunner, + table: string, + column: string, + refTable: string, + onDelete: 'SET NULL' | 'CASCADE' | 'RESTRICT', + ): Promise { + const rows = (await queryRunner.query( + ` + SELECT con.conname + FROM pg_constraint con + JOIN pg_class rel ON rel.oid = con.conrelid + JOIN pg_attribute att + ON att.attrelid = con.conrelid AND att.attnum = ANY(con.conkey) + WHERE con.contype = 'f' + AND rel.relname = $1 + AND att.attname = $2 + `, + [table, column], + )) as Array<{ conname: string }>; + + if (rows.length === 0) { + return; + } + + const { conname } = rows[0]; + await queryRunner.query( + `ALTER TABLE "${table}" DROP CONSTRAINT "${conname}"`, + ); + await queryRunner.query( + `ALTER TABLE "${table}" ADD CONSTRAINT "${conname}" FOREIGN KEY ("${column}") REFERENCES "${refTable}"("id") ON DELETE ${onDelete}`, + ); + } +} diff --git a/src/teams/teams.service.spec.ts b/src/teams/teams.service.spec.ts new file mode 100644 index 0000000..9a6a68f --- /dev/null +++ b/src/teams/teams.service.spec.ts @@ -0,0 +1,102 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { TeamsService } from './teams.service'; +import { Team, TeamMemberSplit, Bounty, User } from '../common/entities'; +import { NotFoundException, BadRequestException } from '@nestjs/common'; + +describe('TeamsService', () => { + let service: TeamsService; + let teamRepo: any; + let splitRepo: any; + let bountyRepo: any; + + beforeEach(async () => { + teamRepo = { + create: jest.fn((dto) => ({ id: 'team-uuid', ...dto })), + save: jest.fn((entity) => Promise.resolve({ id: 'team-uuid', ...entity })), + findOne: jest.fn(), + }; + splitRepo = { + create: jest.fn((dto) => ({ id: 'split-uuid', ...dto })), + save: jest.fn((entity) => Promise.resolve({ id: 'split-uuid', ...entity })), + }; + bountyRepo = { + findOne: jest.fn(), + save: jest.fn((entity) => Promise.resolve(entity)), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + TeamsService, + { provide: getRepositoryToken(Team), useValue: teamRepo }, + { provide: getRepositoryToken(TeamMemberSplit), useValue: splitRepo }, + { provide: getRepositoryToken(Bounty), useValue: bountyRepo }, + ], + }).compile(); + + service = module.get(TeamsService); + }); + + describe('create', () => { + it('creates a team and its splits when percentages sum to 100', async () => { + const dto = { + name: 'Alpha Team', + createdById: 'user-1', + members: [ + { userId: 'user-1', percentage: 60, role: 'lead' }, + { userId: 'user-2', percentage: 40, role: 'dev' }, + ], + }; + + const result = await service.create(dto); + expect(result.name).toBe('Alpha Team'); + expect(result.splits).toHaveLength(2); + expect(splitRepo.save).toHaveBeenCalledTimes(2); + }); + + it('rejects team creation if splits do not sum to 100', async () => { + const dto = { + name: 'Invalid Team', + members: [ + { userId: 'user-1', percentage: 50 }, + { userId: 'user-2', percentage: 30 }, + ], + }; + + await expect(service.create(dto)).rejects.toThrow(BadRequestException); + expect(teamRepo.save).not.toHaveBeenCalled(); + }); + }); + + describe('findOne', () => { + it('returns team with splits relation', async () => { + const mockTeam = { id: 'team-1', name: 'Alpha', splits: [] }; + teamRepo.findOne.mockResolvedValue(mockTeam); + + const res = await service.findOne('team-1'); + expect(res).toBe(mockTeam); + expect(teamRepo.findOne).toHaveBeenCalledWith({ + where: { id: 'team-1' }, + relations: { splits: true }, + }); + }); + + it('throws NotFoundException when team does not exist', async () => { + teamRepo.findOne.mockResolvedValue(null); + await expect(service.findOne('non-existent')).rejects.toThrow(NotFoundException); + }); + }); + + describe('assignToBounty', () => { + it('attaches team to bounty', async () => { + const mockTeam = { id: 'team-1', name: 'Alpha', splits: [] }; + const mockBounty = { id: 'bounty-1', teamId: null }; + teamRepo.findOne.mockResolvedValue(mockTeam); + bountyRepo.findOne.mockResolvedValue(mockBounty); + + const res = await service.assignToBounty('team-1', 'bounty-1'); + expect(res.teamId).toBe('team-1'); + expect(bountyRepo.save).toHaveBeenCalledWith(expect.objectContaining({ teamId: 'team-1' })); + }); + }); +});