feat(teams): manage player active status and team-role with treasurer safeguard

Team managers (captain and above) can now deactivate/reactivate a player and
change their team-role from the player detail page. Deactivation zeroes the
open balance via an auditable adjustment transaction instead of overwriting
the balance field, and both actions are blocked if they would leave a team
without an active treasurer. Also hardens the existing PUT teams/:id/players
endpoint down to profile-only fields, fixing a typo bug and closing a gap
where any authenticated user could mutate a player's active/role/balance in
any team.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Bastian Wagner
2026-08-01 15:42:09 +02:00
parent 2424a8c025
commit 3551641a85
16 changed files with 830 additions and 35 deletions

View File

@@ -20,6 +20,8 @@ export type LOGEVENT =
| 'admin_user_status_update'
| 'admin_player_assign'
| 'admin_player_unlink'
| 'player_active_update'
| 'player_team_role_update'
| 'penalty_catalog_create'
| 'penalty_catalog_update'
| 'penalty_catalog_delete'

View File

@@ -0,0 +1,19 @@
import { IsBoolean, IsIn, IsInt } from 'class-validator';
import { TeamRolesEnum } from '../../team-roles/team-roles.enum';
export class PlayerActiveDto {
@IsBoolean()
active: boolean;
}
export class PlayerTeamRoleDto {
@IsInt()
@IsIn([
TeamRolesEnum.player,
TeamRolesEnum.scnd_treasurer,
TeamRolesEnum.captain,
TeamRolesEnum.treasurer,
TeamRolesEnum.coach,
])
teamRoleId: TeamRolesEnum;
}

View File

@@ -0,0 +1,14 @@
import { IsInt, IsString, MaxLength } from 'class-validator';
export class UpdatePlayerProfileDto {
@IsInt()
id: number;
@IsString()
@MaxLength(255)
firstName: string;
@IsString()
@MaxLength(255)
lastName: string;
}

View File

@@ -0,0 +1,221 @@
import { ConflictException, NotFoundException } from '@nestjs/common';
import { TeamRolesEnum } from '../team-roles/team-roles.enum';
import { TransactionTypeEnum } from '../transactions/transaction-type.enum';
import {
DEACTIVATION_ADJUSTMENT_NOTE_PREFIX,
TeamMembersService,
} from './team-members.service';
describe('TeamMembersService', () => {
const teamId = 10;
let player: any;
let treasurers: any[];
let treasurerLockQuery: any;
let lockedPlayerQuery: any;
let playerRepository: any;
let transactionRepository: any;
let manager: any;
let dataSource: any;
let logger: any;
let access: any;
let service: TeamMembersService;
beforeEach(() => {
player = makePlayer(101, true, TeamRolesEnum.treasurer, 0);
treasurers = [player];
treasurerLockQuery = chain({ getMany: jest.fn(() => treasurers) });
lockedPlayerQuery = chain({ getOne: jest.fn(() => player) });
playerRepository = {
createQueryBuilder: jest.fn((alias: string) =>
alias === 'lockedPlayer' ? lockedPlayerQuery : treasurerLockQuery,
),
save: jest.fn((value) => Promise.resolve(value)),
findOne: jest.fn(() => player),
};
transactionRepository = {
insert: jest.fn(() => Promise.resolve({ identifiers: [{ id: 999 }] })),
find: jest.fn(() => Promise.resolve([])),
};
manager = {
getRepository: jest.fn((entity) =>
entity.name === 'Player' ? playerRepository : transactionRepository,
),
};
dataSource = { transaction: jest.fn((work) => work(manager)) };
logger = { info: jest.fn() };
access = { assertManager: jest.fn(() => Promise.resolve()) };
service = new TeamMembersService(dataSource, logger, access as any);
});
it('checks the team-manager permission before touching the database', async () => {
access.assertManager.mockRejectedValue(new Error('forbidden'));
await expect(service.setActive(5, teamId, player.id, false)).rejects.toThrow(
'forbidden',
);
expect(dataSource.transaction).not.toHaveBeenCalled();
});
it('rejects deactivating the last active treasurer and writes nothing', async () => {
player.balance = 42;
treasurers = [player];
await expect(
service.setActive(5, teamId, player.id, false),
).rejects.toBeInstanceOf(ConflictException);
expect(treasurerLockQuery.setLock).toHaveBeenCalledWith('pessimistic_write');
expect(transactionRepository.insert).not.toHaveBeenCalled();
expect(playerRepository.save).not.toHaveBeenCalled();
expect(logger.info).not.toHaveBeenCalled();
});
it('allows deactivation when another active treasurer remains, and zeroes the balance via a credit transaction', async () => {
player.balance = 42;
treasurers = [player, makePlayer(102, true, TeamRolesEnum.treasurer, 0)];
const result = await service.setActive(5, teamId, player.id, false);
expect(transactionRepository.insert).toHaveBeenCalledWith(
expect.objectContaining({
amount: -42,
note: expect.stringContaining(DEACTIVATION_ADJUSTMENT_NOTE_PREFIX),
type: { id: TransactionTypeEnum.credit },
}),
);
expect(playerRepository.save).toHaveBeenCalledWith(
expect.objectContaining({ active: false, balance: 0 }),
);
expect(logger.info).toHaveBeenCalledWith(
{
event: 'player_active_update',
details: `teamId=${teamId} playerId=${player.id} active=false`,
userId: 5,
},
manager,
);
expect(result.active).toBe(false);
});
it('deactivating a non-treasurer with zero balance never inserts an adjustment transaction', async () => {
player = makePlayer(101, true, TeamRolesEnum.player, 0);
lockedPlayerQuery = chain({ getOne: jest.fn(() => player) });
playerRepository.createQueryBuilder = jest.fn((alias: string) =>
alias === 'lockedPlayer' ? lockedPlayerQuery : treasurerLockQuery,
);
treasurers = [];
await expect(
service.setActive(5, teamId, player.id, false),
).resolves.toBeDefined();
expect(transactionRepository.insert).not.toHaveBeenCalled();
});
it('recomputes balance on reactivation, excluding deactivation adjustment rows across multiple cycles', async () => {
player = makePlayer(101, false, TeamRolesEnum.player, 0);
lockedPlayerQuery = chain({ getOne: jest.fn(() => player) });
playerRepository.createQueryBuilder = jest.fn((alias: string) =>
alias === 'lockedPlayer' ? lockedPlayerQuery : treasurerLockQuery,
);
transactionRepository.find.mockResolvedValue([
{ amount: 50, type: { id: TransactionTypeEnum.credit }, note: 'Zahlung' },
{ amount: -20, note: `${DEACTIVATION_ADJUSTMENT_NOTE_PREFIX} #101` },
{ amount: 10, type: { id: TransactionTypeEnum.credit }, note: 'Zahlung 2' },
{ amount: -40, note: `${DEACTIVATION_ADJUSTMENT_NOTE_PREFIX} #101` },
{ amount: 15, type: { id: 11 }, note: 'Strafe' },
]);
const result = await service.setActive(5, teamId, player.id, true);
// erwarteter Saldo: 50 + 10 - 15 = 45 (beide Ausgleichsbuchungen ausgeschlossen)
expect(playerRepository.save).toHaveBeenCalledWith(
expect.objectContaining({ active: true, balance: 45 }),
);
expect(result.balance).toBe(45);
});
it('no-ops when the requested active state already matches (idempotent, no writes)', async () => {
player = makePlayer(101, true, TeamRolesEnum.player, 0);
lockedPlayerQuery = chain({ getOne: jest.fn(() => player) });
playerRepository.createQueryBuilder = jest.fn((alias: string) =>
alias === 'lockedPlayer' ? lockedPlayerQuery : treasurerLockQuery,
);
await service.setActive(5, teamId, player.id, true);
expect(playerRepository.save).not.toHaveBeenCalled();
expect(logger.info).not.toHaveBeenCalled();
});
it('rejects changing team-role away from treasurer for the last active treasurer', async () => {
treasurers = [player];
await expect(
service.setTeamRole(5, teamId, player.id, TeamRolesEnum.captain),
).rejects.toBeInstanceOf(ConflictException);
expect(playerRepository.save).not.toHaveBeenCalled();
});
it('allows a team-role change when another active treasurer remains, and never touches balance/transactions', async () => {
treasurers = [player, makePlayer(102, true, TeamRolesEnum.treasurer, 0)];
const result = await service.setTeamRole(
5,
teamId,
player.id,
TeamRolesEnum.captain,
);
expect(transactionRepository.insert).not.toHaveBeenCalled();
expect(playerRepository.save).toHaveBeenCalledWith(
expect.objectContaining({ teamRole: { id: TeamRolesEnum.captain } }),
);
expect(logger.info).toHaveBeenCalledWith(
{
event: 'player_team_role_update',
details: `teamId=${teamId} playerId=${player.id} teamRoleId=${TeamRolesEnum.captain}`,
userId: 5,
},
manager,
);
expect(result.teamRole).toEqual({ id: TeamRolesEnum.captain });
});
it('throws NotFoundException when the locked player does not belong to the given team', async () => {
player.team = { id: 999 };
await expect(
service.setActive(5, teamId, player.id, false),
).rejects.toBeInstanceOf(NotFoundException);
});
function makePlayer(
id: number,
active: boolean,
teamRoleId: number,
balance: number,
) {
return {
id,
firstName: 'Pat',
lastName: 'Player',
active,
balance,
team: { id: teamId },
teamRole: { id: teamRoleId },
};
}
function chain(overrides: Record<string, jest.Mock>) {
const query: Record<string, jest.Mock> = {};
[
'innerJoin',
'innerJoinAndSelect',
'leftJoinAndSelect',
'where',
'andWhere',
'setLock',
'orderBy',
].forEach((method) => (query[method] = jest.fn(() => query)));
return Object.assign(query, overrides);
}
});

View File

@@ -0,0 +1,181 @@
import { ConflictException, Injectable, NotFoundException } from '@nestjs/common';
import { DataSource, EntityManager, Repository } from 'typeorm';
import { LoggingService } from '../database/logging/logging.service';
import { Player } from '../players/entities/player.entity';
import { TeamRole } from '../team-roles/entities/team-roles.entity';
import { TeamRolesEnum } from '../team-roles/team-roles.enum';
import { Transaction } from '../transactions/entitites/transaction.entity';
import { TransactionType } from '../transactions/entitites/transaction-type.entity';
import { TransactionTypeEnum } from '../transactions/transaction-type.enum';
import { TeamAccessService } from './team-access.service';
export const DEACTIVATION_ADJUSTMENT_NOTE_PREFIX =
'Ausgleichsbuchung (Deaktivierung)';
@Injectable()
export class TeamMembersService {
constructor(
private readonly dataSource: DataSource,
private readonly logger: LoggingService,
private readonly access: TeamAccessService,
) {}
async setActive(
actorUserId: number,
teamId: number,
playerId: number,
active: boolean,
): Promise<Player> {
await this.access.assertManager(actorUserId, teamId);
return this.dataSource.transaction(async (manager) => {
const activeTreasurers = await this.lockActiveTreasurers(manager, teamId);
const playerRepository = manager.getRepository(Player);
const player = await this.findLockedPlayer(playerRepository, playerId, teamId);
if (player.active === active) return player;
const isDeactivation = player.active && !active;
if (
isDeactivation &&
player.teamRole?.id === TeamRolesEnum.treasurer &&
activeTreasurers.length <= 1
) {
throw new ConflictException(
'Mindestens ein aktiver Kassenwart muss im Team verbleiben.',
);
}
player.active = active;
if (isDeactivation) {
await this.zeroBalance(manager, player);
} else {
await this.recomputeBalance(manager, player);
}
await playerRepository.save(player);
await this.log(
manager,
'player_active_update',
actorUserId,
`teamId=${teamId} playerId=${playerId} active=${active}`,
);
return player;
});
}
async setTeamRole(
actorUserId: number,
teamId: number,
playerId: number,
teamRoleId: TeamRolesEnum,
): Promise<Player> {
await this.access.assertManager(actorUserId, teamId);
return this.dataSource.transaction(async (manager) => {
const activeTreasurers = await this.lockActiveTreasurers(manager, teamId);
const playerRepository = manager.getRepository(Player);
const player = await this.findLockedPlayer(playerRepository, playerId, teamId);
if (player.teamRole?.id === teamRoleId) return player;
const isDemotionFromTreasurer =
player.active &&
player.teamRole?.id === TeamRolesEnum.treasurer &&
teamRoleId !== TeamRolesEnum.treasurer;
if (isDemotionFromTreasurer && activeTreasurers.length <= 1) {
throw new ConflictException(
'Mindestens ein aktiver Kassenwart muss im Team verbleiben.',
);
}
player.teamRole = { id: teamRoleId } as TeamRole;
await playerRepository.save(player);
await this.log(
manager,
'player_team_role_update',
actorUserId,
`teamId=${teamId} playerId=${playerId} teamRoleId=${teamRoleId}`,
);
return player;
});
}
// insert() statt save(): umgeht bewusst @BeforeInsert setBalance() auf Transaction,
// das intern this.player.save() (ActiveRecord/globale DataSource, NICHT `manager`)
// aufruft -- würde zusammen mit dem pessimistic_write-Lock oben zur Selbstblockade führen.
// Saldo wird stattdessen explizit über den transaktionalen `manager` gesetzt.
private async zeroBalance(manager: EntityManager, player: Player): Promise<void> {
const currentBalance = Number(player.balance);
if (currentBalance === 0) return;
await manager.getRepository(Transaction).insert({
amount: -currentBalance,
date: new Date().toISOString(),
note: `${DEACTIVATION_ADJUSTMENT_NOTE_PREFIX} #${player.id}`,
player: { id: player.id } as Player,
type: { id: TransactionTypeEnum.credit } as TransactionType,
});
player.balance = 0;
}
private async recomputeBalance(manager: EntityManager, player: Player): Promise<void> {
const transactions = await manager.getRepository(Transaction).find({
where: { player: { id: player.id } },
relations: ['type'],
});
player.balance = transactions
.filter((t) => !t.note?.startsWith(DEACTIVATION_ADJUSTMENT_NOTE_PREFIX))
.reduce((acc, t) => {
let amount = Number(t.amount);
if (t.type && t.type.id > 10 && amount > 0) amount = -amount;
return acc + amount;
}, 0);
}
private lockActiveTreasurers(
manager: EntityManager,
teamId: number,
): Promise<Player[]> {
return manager
.getRepository(Player)
.createQueryBuilder('player')
.innerJoin('player.team', 'team')
.innerJoinAndSelect('player.teamRole', 'teamRole')
.where('team.id = :teamId', { teamId })
.andWhere('teamRole.id = :treasurerRole', {
treasurerRole: TeamRolesEnum.treasurer,
})
.andWhere('player.active = :active', { active: true })
.setLock('pessimistic_write')
.orderBy('player.id', 'ASC')
.getMany();
}
private async findLockedPlayer(
repository: Repository<Player>,
playerId: number,
teamId: number,
): Promise<Player> {
const player = await repository
.createQueryBuilder('lockedPlayer')
.leftJoinAndSelect('lockedPlayer.teamRole', 'teamRole')
.leftJoinAndSelect('lockedPlayer.team', 'team')
.where('lockedPlayer.id = :playerId', { playerId })
.setLock('pessimistic_write')
.getOne();
if (!player || player.team?.id !== teamId) {
throw new NotFoundException('Spieler nicht gefunden.');
}
return player;
}
private log(
manager: EntityManager,
event: Parameters<LoggingService['info']>[0]['event'],
userId: number,
details: string,
) {
return this.logger.info({ event, userId, details }, manager);
}
}

View File

@@ -21,6 +21,9 @@ import { CreateTeamDTO } from './dto/create-team.dto';
import { TeamsService } from './teams.service';
import { PublicTeamAccessService } from './public-team-access.service';
import { UpdatePublicAccessDto } from './dto/public-access.dto';
import { UpdatePlayerProfileDto } from './dto/update-player-profile.dto';
import { PlayerActiveDto, PlayerTeamRoleDto } from './dto/player-management.dto';
import { TeamMembersService } from './team-members.service';
@ApiTags('Teams')
@Controller({
@@ -31,6 +34,7 @@ export class TeamsController {
constructor(
private service: TeamsService,
private publicAccess: PublicTeamAccessService,
private teamMembers: TeamMembersService,
) {}
@ApiBearerAuth()
@@ -119,10 +123,51 @@ export class TeamsController {
})
@Put(':id/players')
@HttpCode(HttpStatus.CREATED)
updatePlayer(@Param('id') id: string, @Body() playerDto: any) {
updatePlayer(@Body() playerDto: UpdatePlayerProfileDto) {
return this.service.updatePlayer(playerDto);
}
@ApiOperation({
summary: 'Spieler aktivieren/deaktivieren',
description:
'Setzt den Aktiv-Status eines Spielers. Bei Deaktivierung wird die offene Balance über eine Ausgleichsbuchung genullt.',
})
@ApiBearerAuth()
@UseGuards(AuthGuard('jwt'))
@Patch(':id/players/:playerId/active')
@HttpCode(HttpStatus.OK)
setPlayerActive(
@Req() req,
@Param('id', ParseIntPipe) id: number,
@Param('playerId', ParseIntPipe) playerId: number,
@Body() dto: PlayerActiveDto,
) {
return this.teamMembers.setActive(Number(req.user.id), id, playerId, dto.active);
}
@ApiOperation({
summary: 'Team-Rolle eines Spielers ändern',
description:
'Ändert die Team-Rolle eines Spielers (z.B. Spieler -> Kassenwart). Blockiert, wenn dadurch kein aktiver Kassenwart mehr im Team verbleiben würde.',
})
@ApiBearerAuth()
@UseGuards(AuthGuard('jwt'))
@Patch(':id/players/:playerId/team-role')
@HttpCode(HttpStatus.OK)
setPlayerTeamRole(
@Req() req,
@Param('id', ParseIntPipe) id: number,
@Param('playerId', ParseIntPipe) playerId: number,
@Body() dto: PlayerTeamRoleDto,
) {
return this.teamMembers.setTeamRole(
Number(req.user.id),
id,
playerId,
dto.teamRoleId,
);
}
@ApiOperation({
summary: 'Neues Team anlegen',
description:

View File

@@ -6,6 +6,7 @@ import { TeamRole } from 'src/team-roles/entities/team-roles.entity';
import { TeamSetting } from 'src/team-settings/entities/team-setting.entity';
import { TeamWalletTransaction } from 'src/team-wallet-transactions/entities/team-wallet-transaction.entity';
import { Transaction } from 'src/transactions/entitites/transaction.entity';
import { TransactionType } from 'src/transactions/entitites/transaction-type.entity';
import { Team } from './entities/team.entity';
import { TeamsController } from './teams.controller';
import { TeamsService } from './teams.service';
@@ -14,6 +15,7 @@ import { TeamAccessService } from './team-access.service';
import { PublicTeamAccessService } from './public-team-access.service';
import { PublicTeamsController } from './public-teams.controller';
import { PenaltyEntity } from '../penalty/entities/penalty.entity';
import { TeamMembersService } from './team-members.service';
@Module({
imports: [
@@ -21,6 +23,7 @@ import { PenaltyEntity } from '../penalty/entities/penalty.entity';
Team,
Player,
Transaction,
TransactionType,
TeamRole,
TeamSetting,
TeamWalletTransaction,
@@ -30,7 +33,12 @@ import { PenaltyEntity } from '../penalty/entities/penalty.entity';
LoggingModule,
],
controllers: [TeamsController, PublicTeamsController],
providers: [TeamsService, TeamAccessService, PublicTeamAccessService],
providers: [
TeamsService,
TeamAccessService,
PublicTeamAccessService,
TeamMembersService,
],
exports: [TeamAccessService],
})
export class TeamsModule {}

View File

@@ -9,6 +9,7 @@ import { TeamWalletTransaction } from 'src/team-wallet-transactions/entities/tea
import { Transaction } from 'src/transactions/entitites/transaction.entity';
import { Repository } from 'typeorm';
import { CreateTeamDTO } from './dto/create-team.dto';
import { UpdatePlayerProfileDto } from './dto/update-player-profile.dto';
import { Team } from './entities/team.entity';
@Injectable()
@@ -217,38 +218,16 @@ export class TeamsService {
return result;
}
async updatePlayer(playerDTO: any) {
async updatePlayer(playerDTO: UpdatePlayerProfileDto) {
const player = await this.playerRepository.findOneOrFail({
where: {
id: playerDTO.id,
},
relations: ['teamRole', 'transactions', 'team', 'transactions.type'],
});
player.active = playerDTO.active;
player.balance = playerDTO.balance;
player.firstName = playerDTO.firstName;
player.lastName = playerDTO.lastName;
if (player.teamRole.id != playerDTO.teamRole.id) {
player.teamRole = await this.rolesRepository.findOneByOrFail({
id: playerDTO.reamRole.id,
});
}
let balance = player.balance;
if (!player.active && player.balance != 0) {
player.balance = 0;
} else if (player.active && player.balance == 0) {
balance = player.transactions.reduce((acc, succ) => {
let b = succ.amount;
if (succ.type?.id > 10 && succ.amount > 0) {
b = b * -1;
}
return acc + b;
}, 0);
player.balance = balance;
}
const res = await this.playerRepository.save(player);
return res;
return this.playerRepository.save(player);
}
}