From 3551641a8587633518e63a16277617b6d0b4ddba Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 15:42:09 +0200 Subject: [PATCH] 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 --- .../logging/model/logging-event.type.ts | 2 + .../src/teams/dto/player-management.dto.ts | 19 ++ .../teams/dto/update-player-profile.dto.ts | 14 ++ .../src/teams/team-members.service.spec.ts | 221 ++++++++++++++++++ .../src/teams/team-members.service.ts | 181 ++++++++++++++ .../src/teams/teams.controller.ts | 47 +++- .../src/teams/teams.module.ts | 10 +- .../src/teams/teams.service.ts | 27 +-- myteamwallet_backend/test/jest-e2e.json | 3 + .../test/team-members-rollback.e2e-spec.ts | 85 +++++++ .../src/app/core/team/teams-api.spec.ts | 16 ++ .../src/app/core/team/teams-api.ts | 14 ++ .../features/team/members/player-detail.html | 24 ++ .../features/team/members/player-detail.scss | 10 + .../team/members/player-detail.spec.ts | 115 ++++++++- .../features/team/members/player-detail.ts | 77 ++++++ 16 files changed, 830 insertions(+), 35 deletions(-) create mode 100644 myteamwallet_backend/src/teams/dto/player-management.dto.ts create mode 100644 myteamwallet_backend/src/teams/dto/update-player-profile.dto.ts create mode 100644 myteamwallet_backend/src/teams/team-members.service.spec.ts create mode 100644 myteamwallet_backend/src/teams/team-members.service.ts create mode 100644 myteamwallet_backend/test/team-members-rollback.e2e-spec.ts diff --git a/myteamwallet_backend/src/database/logging/model/logging-event.type.ts b/myteamwallet_backend/src/database/logging/model/logging-event.type.ts index bfd1002..5e8073f 100644 --- a/myteamwallet_backend/src/database/logging/model/logging-event.type.ts +++ b/myteamwallet_backend/src/database/logging/model/logging-event.type.ts @@ -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' diff --git a/myteamwallet_backend/src/teams/dto/player-management.dto.ts b/myteamwallet_backend/src/teams/dto/player-management.dto.ts new file mode 100644 index 0000000..7dd6044 --- /dev/null +++ b/myteamwallet_backend/src/teams/dto/player-management.dto.ts @@ -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; +} diff --git a/myteamwallet_backend/src/teams/dto/update-player-profile.dto.ts b/myteamwallet_backend/src/teams/dto/update-player-profile.dto.ts new file mode 100644 index 0000000..ee4f6b7 --- /dev/null +++ b/myteamwallet_backend/src/teams/dto/update-player-profile.dto.ts @@ -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; +} diff --git a/myteamwallet_backend/src/teams/team-members.service.spec.ts b/myteamwallet_backend/src/teams/team-members.service.spec.ts new file mode 100644 index 0000000..f27ccdb --- /dev/null +++ b/myteamwallet_backend/src/teams/team-members.service.spec.ts @@ -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) { + const query: Record = {}; + [ + 'innerJoin', + 'innerJoinAndSelect', + 'leftJoinAndSelect', + 'where', + 'andWhere', + 'setLock', + 'orderBy', + ].forEach((method) => (query[method] = jest.fn(() => query))); + return Object.assign(query, overrides); + } +}); diff --git a/myteamwallet_backend/src/teams/team-members.service.ts b/myteamwallet_backend/src/teams/team-members.service.ts new file mode 100644 index 0000000..ce27dfa --- /dev/null +++ b/myteamwallet_backend/src/teams/team-members.service.ts @@ -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 { + 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 { + 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 { + 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 { + 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 { + 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, + playerId: number, + teamId: number, + ): Promise { + 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[0]['event'], + userId: number, + details: string, + ) { + return this.logger.info({ event, userId, details }, manager); + } +} diff --git a/myteamwallet_backend/src/teams/teams.controller.ts b/myteamwallet_backend/src/teams/teams.controller.ts index da73de9..d39a5e0 100644 --- a/myteamwallet_backend/src/teams/teams.controller.ts +++ b/myteamwallet_backend/src/teams/teams.controller.ts @@ -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: diff --git a/myteamwallet_backend/src/teams/teams.module.ts b/myteamwallet_backend/src/teams/teams.module.ts index 3f02e5a..b859524 100644 --- a/myteamwallet_backend/src/teams/teams.module.ts +++ b/myteamwallet_backend/src/teams/teams.module.ts @@ -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 {} diff --git a/myteamwallet_backend/src/teams/teams.service.ts b/myteamwallet_backend/src/teams/teams.service.ts index 0d42752..3bac329 100644 --- a/myteamwallet_backend/src/teams/teams.service.ts +++ b/myteamwallet_backend/src/teams/teams.service.ts @@ -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); } } diff --git a/myteamwallet_backend/test/jest-e2e.json b/myteamwallet_backend/test/jest-e2e.json index e9d912f..54be8a3 100644 --- a/myteamwallet_backend/test/jest-e2e.json +++ b/myteamwallet_backend/test/jest-e2e.json @@ -5,5 +5,8 @@ "testRegex": ".e2e-spec.ts$", "transform": { "^.+\\.(t|j)s$": "ts-jest" + }, + "moduleNameMapper": { + "^src/(.*)$": "/../src/$1" } } diff --git a/myteamwallet_backend/test/team-members-rollback.e2e-spec.ts b/myteamwallet_backend/test/team-members-rollback.e2e-spec.ts new file mode 100644 index 0000000..f4c320a --- /dev/null +++ b/myteamwallet_backend/test/team-members-rollback.e2e-spec.ts @@ -0,0 +1,85 @@ +import 'reflect-metadata'; +import { DataSource, DataSourceOptions } from 'typeorm'; +import { Player } from '../src/players/entities/player.entity'; +import { Team } from '../src/teams/entities/team.entity'; +import { Transaction } from '../src/transactions/entitites/transaction.entity'; +import { TransactionTypeEnum } from '../src/transactions/transaction-type.enum'; +import { DEACTIVATION_ADJUSTMENT_NOTE_PREFIX } from '../src/teams/team-members.service'; + +// Eigene DataSource statt der geteilten `AppDataSource`: deren `migrations`-Glob +// (`src/database/migrations/**/*`) matcht auch `.spec.ts`-Dateien in diesem Ordner +// (z.B. `AddPlayerLookupIndexes.spec.ts`) und TypeORM requiret sie beim +// Metadaten-Aufbau -- das registriert `describe`/`it` NACH Testlauf-Start und lässt +// Jest mit "Cannot add a test after tests have started running" abbrechen. Diese +// Ausgleichsbuchung braucht keine Migrationen, nur Entities -- migrations: [] umgeht das. +describe('TeamMembers rollback atomicity (real MySQL)', () => { + let dataSource: DataSource; + let team: Team; + let player: Player; + const marker = `rollback-test-${Date.now()}`; + + beforeAll(async () => { + dataSource = new DataSource({ + type: process.env.DATABASE_TYPE, + url: process.env.DATABASE_URL, + host: process.env.DATABASE_HOST, + port: parseInt(process.env.DATABASE_PORT, 10) || 5432, + username: process.env.DATABASE_USERNAME, + password: process.env.DATABASE_PASSWORD, + database: process.env.DATABASE_NAME, + synchronize: false, + entities: [__dirname + '/../src/**/*.entity{.ts,.js}'], + } as DataSourceOptions); + await dataSource.initialize(); + team = await dataSource.getRepository(Team).save({ + name: marker, + alias: marker, + } as Team); + player = await dataSource.getRepository(Player).save({ + firstName: 'Rollback', + lastName: 'Test', + team, + teamRole: { id: 4 }, // treasurer + balance: 42, + active: true, + } as unknown as Player); + }); + + afterAll(async () => { + await dataSource + .getRepository(Transaction) + .delete({ player: { id: player.id } }); + await dataSource.getRepository(Player).delete({ id: player.id }); + await dataSource.getRepository(Team).delete({ id: team.id }); + await dataSource.destroy(); + }); + + it('rolls back BOTH the inserted adjustment transaction AND the explicit balance write when a later error is thrown in the same manager transaction', async () => { + await expect( + dataSource.transaction(async (manager) => { + await manager.getRepository(Transaction).insert({ + amount: -42, + date: new Date().toISOString(), + note: `${DEACTIVATION_ADJUSTMENT_NOTE_PREFIX} #${player.id}`, + player: { id: player.id } as Player, + type: { id: TransactionTypeEnum.credit }, + }); + await manager + .getRepository(Player) + .save({ ...player, balance: 0, active: false }); + throw new Error('forced failure after adjustment transaction would have been created'); + }), + ).rejects.toThrow('forced failure'); + + const persistedTransaction = await dataSource.getRepository(Transaction).findOne({ + where: { player: { id: player.id } }, + }); + expect(persistedTransaction).toBeNull(); + + const reloadedPlayer = await dataSource.getRepository(Player).findOne({ + where: { id: player.id }, + }); + expect(reloadedPlayer?.active).toBe(true); + expect(Number(reloadedPlayer?.balance)).toBe(42); + }); +}); diff --git a/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts b/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts index de482ce..1ba1e5e 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts @@ -76,4 +76,20 @@ describe('TeamsApi', () => { expect(request.request.method).toBe('GET'); request.flush([]); }); + + it('sets a player active/inactive', () => { + service.setPlayerActive(5, 7, false).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}teams/5/players/7/active`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ active: false }); + request.flush({ id: 7, active: false }); + }); + + it('changes a player team-role', () => { + service.setPlayerTeamRole(5, 7, 4).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}teams/5/players/7/team-role`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ teamRoleId: 4 }); + request.flush({ id: 7, teamRole: { id: 4 } }); + }); }); diff --git a/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts b/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts index b76ba49..c54cc5b 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts @@ -33,6 +33,20 @@ export class TeamsApi { return this.http.put(`${environment.apiUrl}teams/${teamId}/players`, player); } + setPlayerActive(teamId: number, playerId: number, active: boolean): Observable { + return this.http.patch( + `${environment.apiUrl}teams/${teamId}/players/${playerId}/active`, + { active }, + ); + } + + setPlayerTeamRole(teamId: number, playerId: number, teamRoleId: number): Observable { + return this.http.patch( + `${environment.apiUrl}teams/${teamId}/players/${playerId}/team-role`, + { teamRoleId }, + ); + } + loadPlayerTransactions(teamId: number, playerId: number): Observable { return this.http.get( `${environment.apiUrl}teams/${teamId}/players/${playerId}/transactions`, diff --git a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.html b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.html index 17a5a74..fbb9063 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.html +++ b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.html @@ -10,6 +10,30 @@ {{ player.balance | currency: 'EUR' }} + @if (canManage()) { +
+ + + Rolle + + Spieler + 2. Kassenwart + Kapitän + Kassenwart + Trainer + + +
+ } + @if (mutationError(); as error) { +

{{ error }}

+ }

Buchungsverlauf

@if (loading()) {
diff --git a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.scss b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.scss index d9c179f..4e759b2 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.scss +++ b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.scss @@ -47,6 +47,16 @@ header span { .negative { color: var(--mat-sys-error); } +.manage-actions { + display: flex; + align-items: center; + gap: 1rem; + flex-wrap: wrap; + margin: -1rem 0 1.5rem; +} +.error { + color: var(--mat-sys-error); +} .state { min-height: 180px; display: grid; diff --git a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.spec.ts b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.spec.ts index 7f6a434..c369aca 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.spec.ts @@ -2,14 +2,24 @@ import { signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { provideHttpClient } from '@angular/common/http'; import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { MatDialog } from '@angular/material/dialog'; import { ActivatedRoute, convertToParamMap, provideRouter } from '@angular/router'; +import { Subject } from 'rxjs'; import { PlayerDetail } from './player-detail'; +import { AuthStore } from '../../../core/auth/auth-store'; import { TeamStore } from '../../../core/team/team-store'; import { environment } from '../../../../environments/environment'; describe('PlayerDetail', () => { - it('renders the selected player and transaction history', async () => { - const team = { + const teamsApiUrl = `${environment.apiUrl}teams/5`; + let httpMock: HttpTestingController; + let refreshTeam: ReturnType; + let currentUser: ReturnType; + let closeDialog: Subject; + let dialog: { open: ReturnType }; + + function makeTeam(playerOverrides: Partial> = {}) { + return { id: 5, name: 'Team A', alias: 'team-a', @@ -22,16 +32,27 @@ describe('PlayerDetail', () => { balance: -12, active: true, teamRole: { id: 1, name: 'player' }, + ...playerOverrides, }, ], }; + } + + async function create(team: ReturnType, user: unknown) { + refreshTeam = vi.fn(); + currentUser = signal(user); + closeDialog = new Subject(); + dialog = { open: vi.fn(() => ({ afterClosed: () => closeDialog.asObservable() })) }; + await TestBed.configureTestingModule({ imports: [PlayerDetail], providers: [ provideHttpClient(), provideHttpClientTesting(), provideRouter([]), - { provide: TeamStore, useValue: { team: signal(team), loading: signal(false) } }, + { provide: TeamStore, useValue: { team: signal(team), loading: signal(false), refreshTeam } }, + { provide: AuthStore, useValue: { currentUser } }, + { provide: MatDialog, useValue: dialog }, { provide: ActivatedRoute, useValue: { snapshot: { paramMap: convertToParamMap({ playerId: '7' }) } }, @@ -40,15 +61,91 @@ describe('PlayerDetail', () => { }).compileComponents(); const fixture = TestBed.createComponent(PlayerDetail); fixture.detectChanges(); - TestBed.inject(HttpTestingController) - .expectOne(`${environment.apiUrl}teams/5/players/7/transactions`) - .flush([ - { id: 1, date: '2026-07-31', amount: 12, note: 'Beitrag', type: { id: 11, name: 'fine' } }, - ]); + httpMock = TestBed.inject(HttpTestingController); + httpMock.expectOne(`${teamsApiUrl}/players/7/transactions`).flush([]); await fixture.whenStable(); fixture.detectChanges(); + return fixture; + } + + afterEach(() => httpMock.verify()); + + it('renders the selected player and transaction history', async () => { + const fixture = await create( + makeTeam({ balance: -12 }), + { id: 99, role: { id: 2 } }, + ); expect(fixture.nativeElement.textContent).toContain('Alex Muster'); - expect(fixture.nativeElement.textContent).toContain('Beitrag'); expect(fixture.nativeElement.textContent).toContain('-12,00'); }); + + it('hides the manage controls for a user without team-manager rights', async () => { + const fixture = await create(makeTeam(), { id: 99, role: { id: 2 } }); + const button = [...fixture.nativeElement.querySelectorAll('button')].find((b: HTMLButtonElement) => + b.textContent?.includes('Deaktivieren'), + ); + expect(button).toBeUndefined(); + }); + + it('shows the manage controls for a global admin', async () => { + const fixture = await create(makeTeam(), { id: 1, role: { id: 1 } }); + const button = [...fixture.nativeElement.querySelectorAll('button')].find((b: HTMLButtonElement) => + b.textContent?.includes('Deaktivieren'), + ); + expect(button).toBeDefined(); + }); + + it('shows the manage controls for a captain-and-above team member', async () => { + const fixture = await create( + makeTeam({ user: { id: 42 }, teamRole: { id: 3, name: 'captain' } }), + { id: 42, role: { id: 2 } }, + ); + const button = [...fixture.nativeElement.querySelectorAll('button')].find((b: HTMLButtonElement) => + b.textContent?.includes('Deaktivieren'), + ); + expect(button).toBeDefined(); + }); + + it('deactivates the player on confirm and refreshes the team', async () => { + const fixture = await create(makeTeam(), { id: 1, role: { id: 1 } }); + const button = [...fixture.nativeElement.querySelectorAll('button')].find((b: HTMLButtonElement) => + b.textContent?.includes('Deaktivieren'), + ) as HTMLButtonElement; + button.click(); + + expect(dialog.open).toHaveBeenCalled(); + expect(dialog.open.mock.calls[0][1].data.message).toContain('Alex Muster'); + httpMock.expectNone(`${teamsApiUrl}/players/7/active`); + + closeDialog.next(true); + const request = httpMock.expectOne(`${teamsApiUrl}/players/7/active`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ active: false }); + request.flush({ id: 7, active: false }); + + expect(refreshTeam).toHaveBeenCalled(); + }); + + it('does not call the API when the confirmation dialog is dismissed', async () => { + const fixture = await create(makeTeam(), { id: 1, role: { id: 1 } }); + const button = [...fixture.nativeElement.querySelectorAll('button')].find((b: HTMLButtonElement) => + b.textContent?.includes('Deaktivieren'), + ) as HTMLButtonElement; + button.click(); + closeDialog.next(false); + httpMock.expectNone(`${teamsApiUrl}/players/7/active`); + expect(refreshTeam).not.toHaveBeenCalled(); + }); + + it('changes the team-role and refreshes the team', async () => { + const fixture = await create(makeTeam(), { id: 1, role: { id: 1 } }); + fixture.componentInstance['changeTeamRole'](4); + + const request = httpMock.expectOne(`${teamsApiUrl}/players/7/team-role`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ teamRoleId: 4 }); + request.flush({ id: 7, teamRole: { id: 4 } }); + + expect(refreshTeam).toHaveBeenCalled(); + }); }); diff --git a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.ts b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.ts index 156ef24..a6b82c1 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.ts +++ b/myteamwallet_frontend_modern/src/app/features/team/members/player-detail.ts @@ -1,15 +1,22 @@ +import { HttpErrorResponse } from '@angular/common/http'; import { CurrencyPipe, DatePipe, registerLocaleData } from '@angular/common'; import localeDe from '@angular/common/locales/de'; import { Component, LOCALE_ID, computed, effect, inject, signal } from '@angular/core'; import { ActivatedRoute, RouterLink } from '@angular/router'; import { MatButtonModule } from '@angular/material/button'; import { MatCardModule } from '@angular/material/card'; +import { MatDialog } from '@angular/material/dialog'; +import { MatFormFieldModule } from '@angular/material/form-field'; import { MatIconModule } from '@angular/material/icon'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { MatSelectModule } from '@angular/material/select'; +import { filter, finalize, take } from 'rxjs'; +import { AuthStore } from '../../../core/auth/auth-store'; import { TeamStore } from '../../../core/team/team-store'; import { TeamsApi } from '../../../core/team/teams-api'; import { PlayerTransaction } from '../../../models/transaction.model'; import { signedTransactionAmount } from '../../../models/transaction-amount'; +import { ConfirmDialog } from '../../../shared/confirm-dialog/confirm-dialog'; registerLocaleData(localeDe); @@ -21,8 +28,10 @@ registerLocaleData(localeDe); RouterLink, MatButtonModule, MatCardModule, + MatFormFieldModule, MatIconModule, MatProgressSpinnerModule, + MatSelectModule, ], providers: [{ provide: LOCALE_ID, useValue: 'de-DE' }], templateUrl: './player-detail.html', @@ -32,6 +41,8 @@ export class PlayerDetail { private readonly route = inject(ActivatedRoute); private readonly teamsApi = inject(TeamsApi); private readonly teamStore = inject(TeamStore); + private readonly authStore = inject(AuthStore); + private readonly dialog = inject(MatDialog); private readonly playerId = Number(this.route.snapshot.paramMap.get('playerId')); private readonly loadedKey = signal(null); protected readonly team = this.teamStore.team; @@ -40,6 +51,19 @@ export class PlayerDetail { ); protected readonly transactions = signal([]); protected readonly loading = signal(true); + protected readonly savingActive = signal(false); + protected readonly savingRole = signal(false); + protected readonly mutationError = signal(null); + + protected readonly canManage = computed(() => { + const user = this.authStore.currentUser(); + if (user?.role?.id === 1) return true; + return ( + this.team()?.players?.some( + (player) => player.user?.id === user?.id && (player.teamRole?.id ?? 0) >= 3, + ) ?? false + ); + }); constructor() { effect(() => { @@ -67,4 +91,57 @@ export class PlayerDetail { protected displayAmount(transaction: PlayerTransaction): number { return signedTransactionAmount(transaction.amount, transaction.type); } + + protected changeActive(): void { + const team = this.team(); + const player = this.player(); + if (!team || !player || this.savingActive()) return; + const isActive = player.active; + this.dialog + .open(ConfirmDialog, { + data: { + title: isActive ? 'Spieler deaktivieren?' : 'Spieler aktivieren?', + message: `${player.firstName} ${player.lastName} wird ${isActive ? 'deaktiviert' : 'aktiviert'}.`, + confirmLabel: isActive ? 'Deaktivieren' : 'Aktivieren', + }, + restoreFocus: true, + }) + .afterClosed() + .pipe(filter(Boolean), take(1)) + .subscribe(() => { + this.savingActive.set(true); + this.mutationError.set(null); + this.teamsApi + .setPlayerActive(team.id, player.id, !isActive) + .pipe(finalize(() => this.savingActive.set(false))) + .subscribe({ + next: () => this.teamStore.refreshTeam(), + error: (error: HttpErrorResponse) => + this.mutationError.set(this.errorMessage(error, 'Status konnte nicht geändert werden.')), + }); + }); + } + + protected changeTeamRole(teamRoleId: number): void { + const team = this.team(); + const player = this.player(); + if (!team || !player || this.savingRole() || player.teamRole?.id === teamRoleId) return; + this.savingRole.set(true); + this.mutationError.set(null); + this.teamsApi + .setPlayerTeamRole(team.id, player.id, teamRoleId) + .pipe(finalize(() => this.savingRole.set(false))) + .subscribe({ + next: () => this.teamStore.refreshTeam(), + error: (error: HttpErrorResponse) => + this.mutationError.set(this.errorMessage(error, 'Rolle konnte nicht geändert werden.')), + }); + } + + private errorMessage(error: HttpErrorResponse, fallback: string): string { + const detail = typeof error.error?.message === 'string' ? error.error.message : ''; + if (error.status === 403) return `Keine Berechtigung. ${detail}`.trim(); + if (error.status === 409) return detail || fallback; + return detail ? `${fallback} ${detail}` : fallback; + } }