diff --git a/docs/superpowers/plans/2026-08-01-penalty-catalog-management.md b/docs/superpowers/plans/2026-08-01-penalty-catalog-management.md new file mode 100644 index 0000000..8002f05 --- /dev/null +++ b/docs/superpowers/plans/2026-08-01-penalty-catalog-management.md @@ -0,0 +1,53 @@ +# Penalty Catalog Management Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extend the existing team penalty catalog with secure create, inline update, and confirmed delete management in the modern frontend. + +**Architecture:** Keep the versioned `/penalty` API compatible, reuse `TeamAccessService` for team-scoped authorization, and serialize writes by locking the owning team in a transaction. The Angular feature remains at `/team/:id/more/penalties` and treats backend authorization as authoritative. + +**Tech Stack:** NestJS 9, TypeORM 0.3, class-validator, Jest, Angular 21, Angular Material, signals/RxJS, Vitest. + +## Global Constraints + +- Only active captains, treasurers, coaches (`teamRole.id >= 3`) and global admins may mutate a team's catalog. +- Active team members and global admins may read a team's catalog; cross-team reads are forbidden. +- Description is trimmed and 1–120 characters; amount is EUR `0.01..10000.00` with at most two decimals. +- Normalized duplicate descriptions within one team return `409 Conflict`. +- Deletes are permanent and do not alter historical transactions. +- Mutations and audit entries share one transaction; logs contain IDs and action, not catalog content. +- Do not modify `myteamwallet_frontend` or integrate penalties into transaction booking. + +--- + +### Task 1: Secure backend catalog contract + +**Files:** +- Modify: `myteamwallet_backend/src/penalty/**` +- Modify: `myteamwallet_backend/src/teams/teams.module.ts` +- Modify: `myteamwallet_backend/src/database/logging/model/logging-event.type.ts` +- Test: `myteamwallet_backend/src/penalty/*.spec.ts` + +- [ ] Write failing DTO, service, and HTTP-boundary tests for safe mapping, team membership, manager roles, validation, duplicate conflicts, locking, audit rollback, update, and delete. +- [ ] Run focused tests and confirm failures are caused by missing behavior. +- [ ] Implement explicit DTOs, class-level authentication, `TeamAccessService` reuse, transactional create/update/delete, normalized duplicate checks, and audit events. +- [ ] Run focused tests and backend build; commit the backend slice. + +### Task 2: Modern frontend management + +**Files:** +- Modify: `myteamwallet_frontend_modern/src/app/core/team/penalty-api.ts` +- Modify: `myteamwallet_frontend_modern/src/app/models/penalty.model.ts` +- Modify: `myteamwallet_frontend_modern/src/app/features/team/more/penalties/**` + +- [ ] Write failing API and component tests for reader/manager views, inline edit/cancel, delete confirmation, pessimistic refresh, errors, retry, search retention, and accessible controls. +- [ ] Run focused tests and confirm failures are caused by missing behavior. +- [ ] Implement typed update/delete calls and the responsive inline management UI using existing Material patterns. +- [ ] Run focused tests, the full modern frontend suite, and TypeScript checks; commit the frontend slice. + +### Task 3: Integration and review + +- [ ] Run focused backend tests, backend build, full frontend tests, frontend TypeScript checks, and `git diff --check`. +- [ ] Confirm the legacy frontend has no feature-range diff and document the eight pre-existing backend placeholder failures separately. +- [ ] Request a read-only full-range code review; fix Critical/Important findings and re-verify. +- [ ] Run the branch-finishing workflow and preserve the worktree until the user chooses integration. 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 38182d2..bfd1002 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,9 @@ export type LOGEVENT = | 'admin_user_status_update' | 'admin_player_assign' | 'admin_player_unlink' + | 'penalty_catalog_create' + | 'penalty_catalog_update' + | 'penalty_catalog_delete' | 'team_create'; export type LOGLEVEL = 'FATAL' | 'ERROR' | 'WARN' | 'INFO' | 'DEBUG' | 'TRACE'; diff --git a/myteamwallet_backend/src/penalty/dto/create-penalty.dto.ts b/myteamwallet_backend/src/penalty/dto/create-penalty.dto.ts index cad0955..8f230df 100644 --- a/myteamwallet_backend/src/penalty/dto/create-penalty.dto.ts +++ b/myteamwallet_backend/src/penalty/dto/create-penalty.dto.ts @@ -1,19 +1,31 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsNotEmpty } from 'class-validator'; -import { Team } from 'src/teams/entities/team.entity'; +import { Transform } from 'class-transformer'; +import { + IsInt, + IsNumber, + IsString, + Length, + Max, + Min, +} from 'class-validator'; export class CreatePenaltyDTO { @ApiProperty({ example: 2342 }) - @IsNotEmpty() + @IsInt() + @Min(1) teamId: number; @ApiProperty({ example: 1 }) - @IsNotEmpty() + @IsNumber({ maxDecimalPlaces: 2 }) + @Min(0.01) + @Max(10000) amount: number; - team?: Team; - @ApiProperty({ example: 'Zu spät kommen' }) - @IsNotEmpty() + @Transform(({ value }) => + typeof value === 'string' ? value.trim() : value, + ) + @IsString() + @Length(1, 120) description: string; } diff --git a/myteamwallet_backend/src/penalty/dto/penalty-response.dto.ts b/myteamwallet_backend/src/penalty/dto/penalty-response.dto.ts new file mode 100644 index 0000000..618aab5 --- /dev/null +++ b/myteamwallet_backend/src/penalty/dto/penalty-response.dto.ts @@ -0,0 +1,6 @@ +export class PenaltyResponseDTO { + id: number; + description: string; + amount: number; + createdAt: Date; +} diff --git a/myteamwallet_backend/src/penalty/dto/update-penalty.dto.ts b/myteamwallet_backend/src/penalty/dto/update-penalty.dto.ts new file mode 100644 index 0000000..4d096ff --- /dev/null +++ b/myteamwallet_backend/src/penalty/dto/update-penalty.dto.ts @@ -0,0 +1,19 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; +import { IsNumber, IsString, Length, Max, Min } from 'class-validator'; + +export class UpdatePenaltyDTO { + @ApiProperty({ example: 'Zu spät kommen' }) + @Transform(({ value }) => + typeof value === 'string' ? value.trim() : value, + ) + @IsString() + @Length(1, 120) + description: string; + + @ApiProperty({ example: 5 }) + @IsNumber({ maxDecimalPlaces: 2 }) + @Min(0.01) + @Max(10000) + amount: number; +} diff --git a/myteamwallet_backend/src/penalty/penalty.controller.ts b/myteamwallet_backend/src/penalty/penalty.controller.ts index 553bca6..afdd657 100644 --- a/myteamwallet_backend/src/penalty/penalty.controller.ts +++ b/myteamwallet_backend/src/penalty/penalty.controller.ts @@ -1,45 +1,67 @@ import { Body, Controller, + Delete, Get, + HttpCode, + HttpStatus, Param, + ParseIntPipe, + Patch, Post, - Req, + Request, UseGuards, } from '@nestjs/common'; import { AuthGuard } from '@nestjs/passport'; import { ApiBearerAuth } from '@nestjs/swagger'; -import { Roles } from 'src/roles/roles.decorator'; -import { RolesGuard } from 'src/roles/roles.guard'; import { CreatePenaltyDTO } from './dto/create-penalty.dto'; +import { UpdatePenaltyDTO } from './dto/update-penalty.dto'; import { PenaltyService } from './penalty.service'; +type AuthenticatedRequest = { user: { id: number } }; + @ApiBearerAuth() -@Controller({ - path: 'penalty', - version: '1', -}) +@UseGuards(AuthGuard('jwt')) +@Controller({ path: 'penalty', version: '1' }) export class PenaltyController { - constructor(private service: PenaltyService) {} + constructor(private readonly service: PenaltyService) {} - @Roles([]) @Get() - getIt(@Req() req: any) { - const userId = req.user?.id; - return this.service.getAll(userId); + getUserPenalties(@Request() request: AuthenticatedRequest) { + return this.service.getUserPenalties(request.user.id); } - @Roles([]) - @UseGuards(AuthGuard('jwt'), RolesGuard) - @Get(':id') - getTeams(@Param('id') teamId: string) { - return this.service.getTeamPenalties(teamId); + @Get(':teamId') + getTeamPenalties( + @Request() request: AuthenticatedRequest, + @Param('teamId', ParseIntPipe) teamId: number, + ) { + return this.service.getTeamPenalties(request.user.id, teamId); } - @UseGuards(AuthGuard('jwt'), RolesGuard) @Post() - createPenalty(@Req() req: any, @Body() createPenaltyDto: CreatePenaltyDTO) { - const userId = req.user?.id; - return this.service.createPenalty(createPenaltyDto, userId); + createPenalty( + @Request() request: AuthenticatedRequest, + @Body() dto: CreatePenaltyDTO, + ) { + return this.service.createPenalty(dto, request.user.id); + } + + @Patch(':penaltyId') + updatePenalty( + @Request() request: AuthenticatedRequest, + @Param('penaltyId', ParseIntPipe) penaltyId: number, + @Body() dto: UpdatePenaltyDTO, + ) { + return this.service.updatePenalty(penaltyId, dto, request.user.id); + } + + @Delete(':penaltyId') + @HttpCode(HttpStatus.NO_CONTENT) + async deletePenalty( + @Request() request: AuthenticatedRequest, + @Param('penaltyId', ParseIntPipe) penaltyId: number, + ): Promise { + await this.service.deletePenalty(penaltyId, request.user.id); } } diff --git a/myteamwallet_backend/src/penalty/penalty.http.spec.ts b/myteamwallet_backend/src/penalty/penalty.http.spec.ts new file mode 100644 index 0000000..17e8f7d --- /dev/null +++ b/myteamwallet_backend/src/penalty/penalty.http.spec.ts @@ -0,0 +1,126 @@ +import { + INestApplication, + UnauthorizedException, + ValidationPipe, + VersioningType, +} from '@nestjs/common'; +import { AuthGuard } from '@nestjs/passport'; +import { Test } from '@nestjs/testing'; +import * as request from 'supertest'; +import validationOptions from '../utils/validation-options'; +import { PenaltyController } from './penalty.controller'; +import { PenaltyService } from './penalty.service'; + +describe('penalty catalog HTTP boundary', () => { + let app: INestApplication; + const penalty = { + id: 8, + description: 'Zu spät', + amount: 5, + createdAt: '2026-01-02T00:00:00.000Z', + }; + const service = { + getUserPenalties: jest.fn(() => [penalty]), + getTeamPenalties: jest.fn(() => [penalty]), + createPenalty: jest.fn(() => penalty), + updatePenalty: jest.fn(() => penalty), + deletePenalty: jest.fn(), + }; + + beforeAll(async () => { + const module = await Test.createTestingModule({ + controllers: [PenaltyController], + providers: [{ provide: PenaltyService, useValue: service }], + }) + .overrideGuard(AuthGuard('jwt')) + .useValue({ + canActivate(context) { + const httpRequest = context.switchToHttp().getRequest(); + if (httpRequest.headers.authorization !== 'Bearer user') { + throw new UnauthorizedException(); + } + httpRequest.user = { id: 42, role: { id: 2 } }; + return true; + }, + }) + .compile(); + app = module.createNestApplication(); + app.setGlobalPrefix('api'); + app.enableVersioning({ type: VersioningType.URI }); + app.useGlobalPipes(new ValidationPipe(validationOptions)); + await app.init(); + }); + + afterAll(() => app.close()); + beforeEach(() => jest.clearAllMocks()); + + it('requires authentication and passes the actor to a team read', async () => { + await request(app.getHttpServer()).get('/api/v1/penalty/5').expect(401); + await request(app.getHttpServer()) + .get('/api/v1/penalty/5') + .set('Authorization', 'Bearer user') + .expect(200, [penalty]); + expect(service.getTeamPenalties).toHaveBeenCalledWith(42, 5); + }); + + it('keeps the authenticated cross-team catalog route available', async () => { + await request(app.getHttpServer()).get('/api/v1/penalty').expect(401); + await request(app.getHttpServer()) + .get('/api/v1/penalty') + .set('Authorization', 'Bearer user') + .expect(200, [penalty]); + expect(service.getUserPenalties).toHaveBeenCalledWith(42); + }); + + it('validates and strips create fields before invoking the service', async () => { + await request(app.getHttpServer()) + .post('/api/v1/penalty') + .set('Authorization', 'Bearer user') + .send({ + teamId: 5, + description: ' Zu spät ', + amount: 5.25, + team: { id: 99 }, + }) + .expect(201, penalty); + expect(service.createPenalty).toHaveBeenCalledWith( + { teamId: 5, description: 'Zu spät', amount: 5.25 }, + 42, + ); + }); + + it.each([ + [{ teamId: 5, description: '', amount: 5 }], + [{ teamId: 5, description: 'x'.repeat(121), amount: 5 }], + [{ teamId: 5, description: 'Test', amount: 0 }], + [{ teamId: 5, description: 'Test', amount: 10000.01 }], + [{ teamId: 5, description: 'Test', amount: 1.001 }], + [{ teamId: true, description: 'Test', amount: 1 }], + ])('rejects invalid create input: %p', async (body) => { + await request(app.getHttpServer()) + .post('/api/v1/penalty') + .set('Authorization', 'Bearer user') + .send(body) + .expect(422); + expect(service.createPenalty).not.toHaveBeenCalled(); + }); + + it('exposes update and delete with numeric IDs and safe payloads', async () => { + await request(app.getHttpServer()) + .patch('/api/v1/penalty/8') + .set('Authorization', 'Bearer user') + .send({ description: ' Neu ', amount: 2 }) + .expect(200, penalty); + expect(service.updatePenalty).toHaveBeenCalledWith( + 8, + { description: 'Neu', amount: 2 }, + 42, + ); + + await request(app.getHttpServer()) + .delete('/api/v1/penalty/8') + .set('Authorization', 'Bearer user') + .expect(204); + expect(service.deletePenalty).toHaveBeenCalledWith(8, 42); + }); +}); diff --git a/myteamwallet_backend/src/penalty/penalty.module.ts b/myteamwallet_backend/src/penalty/penalty.module.ts index a3208f9..847720d 100644 --- a/myteamwallet_backend/src/penalty/penalty.module.ts +++ b/myteamwallet_backend/src/penalty/penalty.module.ts @@ -1,29 +1,18 @@ import { Module } from '@nestjs/common'; -import { PenaltyController } from './penalty.controller'; -import { PenaltyService } from './penalty.service'; -import { LoggingModule } from 'src/database/logging/logging.module'; import { TypeOrmModule } from '@nestjs/typeorm'; -import { TeamWalletTransactionType } from 'src/team-wallet-transactions/entities/team-wallet-transaction-type.entity'; -import { TeamWalletTransaction } from 'src/team-wallet-transactions/entities/team-wallet-transaction.entity'; +import { LoggingModule } from 'src/database/logging/logging.module'; import { Team } from 'src/teams/entities/team.entity'; -import { User } from 'src/users/entities/user.entity'; -import { Role } from 'src/roles/entities/role.entity'; +import { TeamsModule } from 'src/teams/teams.module'; +import { PenaltyController } from './penalty.controller'; import { PenaltyEntity } from './entities/penalty.entity'; -import { Player } from 'src/players/entities/player.entity'; +import { PenaltyService } from './penalty.service'; @Module({ controllers: [PenaltyController], providers: [PenaltyService], imports: [ - TypeOrmModule.forFeature([ - User, - Role, - TeamWalletTransaction, - TeamWalletTransactionType, - Team, - PenaltyEntity, - Player, - ]), + TypeOrmModule.forFeature([Team, PenaltyEntity]), + TeamsModule, LoggingModule, ], }) diff --git a/myteamwallet_backend/src/penalty/penalty.service.spec.ts b/myteamwallet_backend/src/penalty/penalty.service.spec.ts new file mode 100644 index 0000000..65995ac --- /dev/null +++ b/myteamwallet_backend/src/penalty/penalty.service.spec.ts @@ -0,0 +1,223 @@ +import { ConflictException, NotFoundException } from '@nestjs/common'; +import { PenaltyService } from './penalty.service'; + +describe('PenaltyService catalog management', () => { + const readRepository = { find: jest.fn(), findOne: jest.fn() }; + const teamReadRepository = { findOne: jest.fn() }; + const teamQuery = { + where: jest.fn().mockReturnThis(), + setLock: jest.fn().mockReturnThis(), + getOne: jest.fn(), + }; + const duplicateQuery = { + where: jest.fn().mockReturnThis(), + andWhere: jest.fn().mockReturnThis(), + getCount: jest.fn(), + }; + const teamRepository = { createQueryBuilder: jest.fn(() => teamQuery) }; + const writeRepository = { + createQueryBuilder: jest.fn(() => duplicateQuery), + create: jest.fn((value) => value), + save: jest.fn(), + findOne: jest.fn(), + remove: jest.fn(), + }; + const manager = { + getRepository: jest.fn((entity) => + entity.name === 'Team' ? teamRepository : writeRepository, + ), + }; + const dataSource = { transaction: jest.fn((work) => work(manager)) }; + const access = { assertMember: jest.fn(), assertManager: jest.fn() }; + const logger = { info: jest.fn() }; + let service: PenaltyService; + + beforeEach(() => { + jest.clearAllMocks(); + teamQuery.getOne.mockResolvedValue({ id: 5 }); + duplicateQuery.getCount.mockResolvedValue(0); + teamReadRepository.findOne.mockResolvedValue({ id: 5 }); + service = new PenaltyService( + readRepository as any, + teamReadRepository as any, + dataSource as any, + access as any, + logger as any, + ); + }); + + it('returns not found instead of leaking an unknown team as an empty catalog', async () => { + teamReadRepository.findOne.mockResolvedValue(null); + + await expect(service.getTeamPenalties(42, 999)).rejects.toBeInstanceOf( + NotFoundException, + ); + expect(access.assertMember).not.toHaveBeenCalled(); + }); + + it('keeps the authenticated cross-team catalog route compatible and safely mapped', async () => { + readRepository.find.mockResolvedValue([ + { + id: 8, + description: 'Zu spät', + amount: '5.50', + createdAt: new Date('2026-01-02T00:00:00.000Z'), + team: { id: 5, secret: 'hidden' }, + }, + ]); + + await expect(service.getUserPenalties(42)).resolves.toEqual([ + { + id: 8, + description: 'Zu spät', + amount: 5.5, + createdAt: new Date('2026-01-02T00:00:00.000Z'), + }, + ]); + expect(readRepository.find).toHaveBeenCalledWith({ + where: { + team: { players: { user: { id: 42 }, active: true } }, + }, + order: { description: 'ASC' }, + }); + }); + + it('authorizes team reads, sorts them, and maps only safe fields', async () => { + readRepository.find.mockResolvedValue([ + { + id: 8, + description: 'Zu spät', + amount: '5.50', + createdAt: new Date('2026-01-02T00:00:00.000Z'), + team: { id: 5, secret: 'hidden' }, + }, + ]); + + await expect(service.getTeamPenalties(42, 5)).resolves.toEqual([ + { + id: 8, + description: 'Zu spät', + amount: 5.5, + createdAt: new Date('2026-01-02T00:00:00.000Z'), + }, + ]); + expect(access.assertMember).toHaveBeenCalledWith(42, 5); + expect(readRepository.find).toHaveBeenCalledWith({ + where: { team: { id: 5 } }, + order: { description: 'ASC' }, + }); + }); + + it('creates a normalized entry under a team lock and audits in the transaction', async () => { + writeRepository.save.mockImplementation(async (value) => ({ + id: 9, + createdAt: new Date('2026-01-03T00:00:00.000Z'), + ...value, + })); + + await expect( + service.createPenalty( + { teamId: 5, description: ' Handy in der Kabine ', amount: 3.25 }, + 42, + ), + ).resolves.toMatchObject({ + id: 9, + description: 'Handy in der Kabine', + amount: 3.25, + }); + + expect(teamQuery.setLock).toHaveBeenCalledWith('pessimistic_write'); + expect(access.assertManager).toHaveBeenCalledWith(42, 5, manager); + expect(logger.info).toHaveBeenCalledWith( + { + event: 'penalty_catalog_create', + details: 'teamId=5 penaltyId=9 action=create', + userId: 42, + }, + manager, + ); + }); + + it('rejects a normalized duplicate before writing or auditing', async () => { + duplicateQuery.getCount.mockResolvedValue(1); + + await expect( + service.createPenalty( + { teamId: 5, description: ' ZU SPÄT ', amount: 5 }, + 42, + ), + ).rejects.toBeInstanceOf(ConflictException); + expect(writeRepository.save).not.toHaveBeenCalled(); + expect(logger.info).not.toHaveBeenCalled(); + }); + + it('updates an entry in its owning team and excludes itself from duplicate detection', async () => { + readRepository.findOne.mockResolvedValue({ id: 8, team: { id: 5 } }); + writeRepository.findOne.mockResolvedValue({ + id: 8, + team: { id: 5 }, + description: 'Alt', + amount: 1, + createdAt: new Date('2026-01-02T00:00:00.000Z'), + }); + writeRepository.save.mockImplementation(async (value) => value); + + await expect( + service.updatePenalty(8, { description: ' Neu ', amount: 2.5 }, 42), + ).resolves.toMatchObject({ id: 8, description: 'Neu', amount: 2.5 }); + expect(duplicateQuery.andWhere).toHaveBeenCalledWith( + 'penalty.id != :penaltyId', + { penaltyId: 8 }, + ); + expect(logger.info).toHaveBeenCalledWith( + { + event: 'penalty_catalog_update', + details: 'teamId=5 penaltyId=8 action=update', + userId: 42, + }, + manager, + ); + }); + + it('deletes an entry permanently after manager authorization and audits it', async () => { + readRepository.findOne.mockResolvedValue({ id: 8, team: { id: 5 } }); + const penalty = { id: 8, team: { id: 5 } }; + writeRepository.findOne.mockResolvedValue(penalty); + + await service.deletePenalty(8, 42); + + expect(writeRepository.remove).toHaveBeenCalledWith(penalty); + expect(logger.info).toHaveBeenCalledWith( + { + event: 'penalty_catalog_delete', + details: 'teamId=5 penaltyId=8 action=delete', + userId: 42, + }, + manager, + ); + }); + + it('returns not found when a mutation target does not exist', async () => { + readRepository.findOne.mockResolvedValue(null); + + await expect(service.deletePenalty(999, 42)).rejects.toBeInstanceOf( + NotFoundException, + ); + expect(dataSource.transaction).not.toHaveBeenCalled(); + }); + + it('rejects the mutation when transactional audit persistence fails', async () => { + writeRepository.save.mockResolvedValue({ + id: 9, + team: { id: 5 }, + description: 'Neu', + amount: 2, + createdAt: new Date(), + }); + logger.info.mockRejectedValue(new Error('audit unavailable')); + + await expect( + service.createPenalty({ teamId: 5, description: 'Neu', amount: 2 }, 42), + ).rejects.toThrow('audit unavailable'); + }); +}); diff --git a/myteamwallet_backend/src/penalty/penalty.service.ts b/myteamwallet_backend/src/penalty/penalty.service.ts index 1e5ece3..205e2dc 100644 --- a/myteamwallet_backend/src/penalty/penalty.service.ts +++ b/myteamwallet_backend/src/penalty/penalty.service.ts @@ -1,61 +1,203 @@ -import { Injectable } from '@nestjs/common'; -import { CreatePenaltyDTO } from './dto/create-penalty.dto'; +import { + ConflictException, + Injectable, + NotFoundException, +} from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; +import { LoggingService } from 'src/database/logging/logging.service'; import { Team } from 'src/teams/entities/team.entity'; -import { Repository } from 'typeorm'; +import { TeamAccessService } from 'src/teams/team-access.service'; +import { DataSource, EntityManager, Repository } from 'typeorm'; +import { CreatePenaltyDTO } from './dto/create-penalty.dto'; +import { PenaltyResponseDTO } from './dto/penalty-response.dto'; +import { UpdatePenaltyDTO } from './dto/update-penalty.dto'; import { PenaltyEntity } from './entities/penalty.entity'; -import { Player } from 'src/players/entities/player.entity'; @Injectable() export class PenaltyService { constructor( - @InjectRepository(Team) - private teamRepository: Repository, - @InjectRepository(Player) - private playerRepository: Repository, @InjectRepository(PenaltyEntity) - private repository: Repository, + private readonly repository: Repository, + @InjectRepository(Team) + private readonly teamRepository: Repository, + private readonly dataSource: DataSource, + private readonly access: TeamAccessService, + private readonly logger: LoggingService, ) {} - async createPenalty(dto: CreatePenaltyDTO, userId: string) { - const player = await this.playerRepository.findOne({ - where: { user: { id: Number(userId) }, team: { id: dto.teamId } }, + async getUserPenalties(userId: number): Promise { + const penalties = await this.repository.find({ + where: { + team: { players: { user: { id: userId }, active: true } }, + }, + order: { description: 'ASC' }, + }); + return penalties.map((penalty) => this.toResponse(penalty)); + } + + async getTeamPenalties( + userId: number, + teamId: number, + ): Promise { + const team = await this.teamRepository.findOne({ where: { id: teamId } }); + if (!team) throw new NotFoundException('Team nicht gefunden.'); + await this.access.assertMember(userId, teamId); + const penalties = await this.repository.find({ + where: { team: { id: teamId } }, + order: { description: 'ASC' }, + }); + return penalties.map((penalty) => this.toResponse(penalty)); + } + + createPenalty( + dto: CreatePenaltyDTO, + userId: number, + ): Promise { + return this.dataSource.transaction(async (manager) => { + const team = await this.lockTeam(manager, dto.teamId); + await this.access.assertManager(userId, team.id, manager); + const repository = manager.getRepository(PenaltyEntity); + const description = dto.description.trim(); + await this.assertUniqueDescription(repository, team.id, description); + const saved = await repository.save( + repository.create({ team, description, amount: dto.amount }), + ); + await this.logger.info( + { + event: 'penalty_catalog_create', + details: `teamId=${team.id} penaltyId=${saved.id} action=create`, + userId, + }, + manager, + ); + return this.toResponse(saved); + }); + } + + async updatePenalty( + penaltyId: number, + dto: UpdatePenaltyDTO, + userId: number, + ): Promise { + const owner = await this.findOwner(penaltyId); + return this.dataSource.transaction(async (manager) => { + const team = await this.lockTeam(manager, owner.team.id); + await this.access.assertManager(userId, team.id, manager); + const repository = manager.getRepository(PenaltyEntity); + const penalty = await this.findTransactionalPenalty( + repository, + penaltyId, + team.id, + ); + const description = dto.description.trim(); + await this.assertUniqueDescription( + repository, + team.id, + description, + penaltyId, + ); + penalty.description = description; + penalty.amount = dto.amount; + const saved = await repository.save(penalty); + await this.logger.info( + { + event: 'penalty_catalog_update', + details: `teamId=${team.id} penaltyId=${penaltyId} action=update`, + userId, + }, + manager, + ); + return this.toResponse(saved); + }); + } + + async deletePenalty(penaltyId: number, userId: number): Promise { + const owner = await this.findOwner(penaltyId); + await this.dataSource.transaction(async (manager) => { + const team = await this.lockTeam(manager, owner.team.id); + await this.access.assertManager(userId, team.id, manager); + const repository = manager.getRepository(PenaltyEntity); + const penalty = await this.findTransactionalPenalty( + repository, + penaltyId, + team.id, + ); + await repository.remove(penalty); + await this.logger.info( + { + event: 'penalty_catalog_delete', + details: `teamId=${team.id} penaltyId=${penaltyId} action=delete`, + userId, + }, + manager, + ); + }); + } + + private async findOwner(penaltyId: number): Promise { + const penalty = await this.repository.findOne({ + where: { id: penaltyId }, relations: ['team'], }); + if (!penalty?.team) throw new NotFoundException('Strafe nicht gefunden.'); + return penalty; + } - if (!player || !player.team) { - return; + private async lockTeam( + manager: EntityManager, + teamId: number, + ): Promise { + const team = await manager + .getRepository(Team) + .createQueryBuilder('team') + .where('team.id = :teamId', { teamId }) + .setLock('pessimistic_write') + .getOne(); + if (!team) throw new NotFoundException('Team nicht gefunden.'); + return team; + } + + private async findTransactionalPenalty( + repository: Repository, + penaltyId: number, + teamId: number, + ): Promise { + const penalty = await repository.findOne({ + where: { id: penaltyId, team: { id: teamId } }, + relations: ['team'], + }); + if (!penalty) throw new NotFoundException('Strafe nicht gefunden.'); + return penalty; + } + + private async assertUniqueDescription( + repository: Repository, + teamId: number, + description: string, + penaltyId?: number, + ): Promise { + const query = repository + .createQueryBuilder('penalty') + .where('penalty.teamId = :teamId', { teamId }) + .andWhere('LOWER(TRIM(penalty.description)) = :description', { + description: description.toLocaleLowerCase('de'), + }); + if (penaltyId !== undefined) { + query.andWhere('penalty.id != :penaltyId', { penaltyId }); + } + if ((await query.getCount()) > 0) { + throw new ConflictException( + 'Ein Eintrag mit dieser Beschreibung existiert bereits.', + ); } - - dto.team = player.team; - - const e = this.repository.create(dto); - - return this.repository.save(e); } - getAll(userId: string) { - const id = Number(userId); - - return this.repository.find({ - where: { - team: { players: { user: { id } } }, - }, - relations: ['team'], - }); - } - - async getTeamPenalties(teamId: string | number) { - teamId = Number(teamId); - - const res = await this.repository.find({ - where: { - team: { id: teamId }, - }, - }); - return res.map((r) => { - r.amount = Number(r.amount); - return r; - }); + private toResponse(penalty: PenaltyEntity): PenaltyResponseDTO { + return { + id: penalty.id, + description: penalty.description, + amount: Number(penalty.amount), + createdAt: penalty.createdAt, + }; } } diff --git a/myteamwallet_backend/src/teams/team-access.service.spec.ts b/myteamwallet_backend/src/teams/team-access.service.spec.ts index 6e39348..b28b422 100644 --- a/myteamwallet_backend/src/teams/team-access.service.spec.ts +++ b/myteamwallet_backend/src/teams/team-access.service.spec.ts @@ -65,6 +65,30 @@ describe('TeamAccessService', () => { await expect(service.assertManager(2, 9)).resolves.toBeUndefined(); }); + it('uses repositories from the supplied transaction manager', async () => { + const transactionalUserRepository = { + findOne: jest.fn().mockResolvedValue({ + id: 2, + role: { id: RoleEnum.user }, + }), + }; + const transactionalPlayerRepository = { + find: jest + .fn() + .mockResolvedValue([{ active: true, teamRole: { id: 3 } }]), + }; + const manager = { + getRepository: jest + .fn() + .mockReturnValueOnce(transactionalUserRepository) + .mockReturnValueOnce(transactionalPlayerRepository), + }; + + await expect(service.assertManager(2, 9, manager as any)).resolves.toBeUndefined(); + expect(userRepository.findOne).not.toHaveBeenCalled(); + expect(playerRepository.find).not.toHaveBeenCalled(); + }); + it.each([ ['a player', [{ active: true, teamRole: { id: 1 } }]], ['a second treasurer', [{ active: true, teamRole: { id: 2 } }]], diff --git a/myteamwallet_backend/src/teams/team-access.service.ts b/myteamwallet_backend/src/teams/team-access.service.ts index b5bcd38..01fa092 100644 --- a/myteamwallet_backend/src/teams/team-access.service.ts +++ b/myteamwallet_backend/src/teams/team-access.service.ts @@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { Player } from '../players/entities/player.entity'; import { RoleEnum } from '../roles/roles.enum'; import { User } from '../users/entities/user.entity'; -import { Repository } from 'typeorm'; +import { EntityManager, Repository } from 'typeorm'; @Injectable() export class TeamAccessService { @@ -14,23 +14,35 @@ export class TeamAccessService { private readonly playerRepository: Repository, ) {} - async assertMember(userId: number, teamId: number): Promise { - await this.assertMinimumRole(userId, teamId, 1); + async assertMember( + userId: number, + teamId: number, + manager?: EntityManager, + ): Promise { + await this.assertMinimumRole(userId, teamId, 1, manager); } - async assertManager(userId: number, teamId: number): Promise { - await this.assertMinimumRole(userId, teamId, 3); + async assertManager( + userId: number, + teamId: number, + manager?: EntityManager, + ): Promise { + await this.assertMinimumRole(userId, teamId, 3, manager); } private async assertMinimumRole( userId: number, teamId: number, minimumRole: number, + manager?: EntityManager, ): Promise { - const user = await this.userRepository.findOne({ where: { id: userId } }); + const userRepository = manager?.getRepository(User) ?? this.userRepository; + const playerRepository = + manager?.getRepository(Player) ?? this.playerRepository; + const user = await userRepository.findOne({ where: { id: userId } }); if (user?.role?.id === RoleEnum.admin) return; - const players = await this.playerRepository.find({ + const players = await playerRepository.find({ where: { user: { id: userId }, team: { id: teamId } }, }); const highestActiveRole = players diff --git a/myteamwallet_backend/src/teams/teams.module.ts b/myteamwallet_backend/src/teams/teams.module.ts index fb5e832..3f02e5a 100644 --- a/myteamwallet_backend/src/teams/teams.module.ts +++ b/myteamwallet_backend/src/teams/teams.module.ts @@ -31,5 +31,6 @@ import { PenaltyEntity } from '../penalty/entities/penalty.entity'; ], controllers: [TeamsController, PublicTeamsController], providers: [TeamsService, TeamAccessService, PublicTeamAccessService], + exports: [TeamAccessService], }) export class TeamsModule {} diff --git a/myteamwallet_frontend_modern/src/app/core/team/penalty-api.spec.ts b/myteamwallet_frontend_modern/src/app/core/team/penalty-api.spec.ts index 65dbbe1..6796306 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/penalty-api.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/penalty-api.spec.ts @@ -33,4 +33,20 @@ describe('PenaltyApi', () => { expect(request.request.body).toEqual(penalty); request.flush({ id: 1, ...penalty }); }); + + it('updates a penalty catalog entry', () => { + const update = { description: 'Zu spät', amount: 7.5 }; + api.updatePenalty(8, update).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}penalty/8`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual(update); + request.flush({ id: 8, ...update }); + }); + + it('deletes a penalty catalog entry', () => { + api.deletePenalty(8).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}penalty/8`); + expect(request.request.method).toBe('DELETE'); + request.flush(null); + }); }); diff --git a/myteamwallet_frontend_modern/src/app/core/team/penalty-api.ts b/myteamwallet_frontend_modern/src/app/core/team/penalty-api.ts index eadc2c3..dfe5873 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/penalty-api.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/penalty-api.ts @@ -2,7 +2,7 @@ import { HttpClient } from '@angular/common/http'; import { Injectable, inject } from '@angular/core'; import { Observable } from 'rxjs'; import { environment } from '../../../environments/environment'; -import { CreatePenalty, Penalty } from '../../models/penalty.model'; +import { CreatePenalty, Penalty, UpdatePenalty } from '../../models/penalty.model'; @Injectable({ providedIn: 'root' }) export class PenaltyApi { @@ -16,4 +16,12 @@ export class PenaltyApi { createPenalty(penalty: CreatePenalty): Observable { return this.http.post(this.baseUrl, penalty); } + + updatePenalty(penaltyId: number, penalty: UpdatePenalty): Observable { + return this.http.patch(`${this.baseUrl}/${penaltyId}`, penalty); + } + + deletePenalty(penaltyId: number): Observable { + return this.http.delete(`${this.baseUrl}/${penaltyId}`); + } } diff --git a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.html b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.html index 8f07b67..f868461 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.html +++ b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.html @@ -4,40 +4,148 @@

Strafenkatalog

Klare Regeln, transparent für das ganze Team.

+ @if (canManage()) { -
- Beschreibung - Betrag - -
+ + } -Strafen durchsuchensearch + +@if (mutationError()) { + +} + + + Strafen durchsuchen + search + + + @if (loading()) { -
+
+ + Katalog wird geladen … +
+} @else if (loadError()) { + +} @else if (penalties().length === 0) { +
+ gavel + Noch keine Einträge + Der Strafenkatalog dieses Teams ist leer. +
} @else if (filteredPenalties().length === 0) { -
gavelKeine Einträge gefunden.
+
+ search_off + Keine passenden Einträge + Versuche einen anderen Suchbegriff. +
} @else {
@for (penalty of filteredPenalties(); track penalty.id) { - {{ penalty.description }}{{ penalty.amount | currency: 'EUR' }} + + @if (editingPenaltyId() === penalty.id) { +
+ + Beschreibung + + + + Betrag + + + +
+ + +
+
+ } @else { +
+ {{ penalty.description }} + {{ penalty.amount | currency: 'EUR' }} +
+ @if (canManage()) { +
+ + +
+ } + } +
}
} diff --git a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.scss b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.scss index 407924f..081a25b 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.scss +++ b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.scss @@ -4,18 +4,22 @@ max-width: 900px; margin: 0 auto; } + header { margin: 20px 0 26px; } + h1, p { margin-top: 0; } + h1 { font-size: clamp(2rem, 4vw, 3rem); line-height: clamp(2rem, 4vw, 3rem); margin-bottom: 8px; } + .eyebrow { color: var(--mat-sys-primary); font-size: 0.75rem; @@ -24,48 +28,118 @@ h1 { text-transform: uppercase; margin-bottom: 6px; } + .create-card { padding: 20px; border-radius: 18px; margin-bottom: 22px; } -form { + +.create-card form, +.edit-form { display: grid; - grid-template-columns: 1fr 160px auto; + grid-template-columns: minmax(0, 1fr) 160px auto; gap: 12px; align-items: start; } + +.create-card button, +.edit-actions button { + min-height: 48px; +} + +.create-card button mat-spinner, +.edit-actions button mat-spinner { + display: inline-block; + margin-right: 8px; +} + .search { width: 100%; } + +.error-message { + padding: 12px 16px; + border-radius: 12px; + color: var(--mat-sys-error); + background: var(--mat-sys-error-container); +} + .catalog { display: grid; gap: 10px; } -.catalog mat-card { - display: flex; - flex-direction: row; - justify-content: space-between; - gap: 16px; - padding: 18px; + +.penalty-card { + padding: 16px 18px; border-radius: 16px; } + +.penalty-content { + display: flex; + justify-content: space-between; + gap: 16px; + align-items: baseline; +} + +.penalty-actions { + display: flex; + justify-content: flex-end; + gap: 4px; + margin-top: 8px; +} + +.edit-form { + grid-template-columns: minmax(0, 1fr) 150px; +} + +.edit-actions { + grid-column: 1 / -1; + display: flex; + justify-content: flex-end; + gap: 8px; +} + .state { min-height: 180px; display: grid; place-content: center; justify-items: center; + text-align: center; gap: 10px; color: var(--mat-sys-on-surface-variant); } + @media (max-width: 700px) { :host { padding: 20px 16px; } - form { + + .create-card form, + .edit-form { grid-template-columns: 1fr; } - form button { - justify-self: stretch; + + .create-card form button, + .edit-actions, + .edit-actions button { + width: 100%; + } + + .edit-actions { + grid-column: auto; + flex-direction: column-reverse; + } + + .penalty-content { + align-items: flex-start; + } + + .penalty-actions { + justify-content: stretch; + } + + .penalty-actions button { + flex: 1; } } diff --git a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.spec.ts b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.spec.ts index 388a3ee..3e591c2 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.spec.ts @@ -1,54 +1,273 @@ +import { HttpErrorResponse } from '@angular/common/http'; import { signal } from '@angular/core'; -import { TestBed } from '@angular/core/testing'; -import { of } from 'rxjs'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { MatDialog } from '@angular/material/dialog'; import { provideRouter } from '@angular/router'; +import { Subject, of, throwError } from 'rxjs'; import { AuthStore } from '../../../../core/auth/auth-store'; import { PenaltyApi } from '../../../../core/team/penalty-api'; import { TeamStore } from '../../../../core/team/team-store'; +import { Penalty } from '../../../../models/penalty.model'; import { Penalties } from './penalties'; -describe('Penalties', () => { - it('renders the catalog and lets a captain add an entry', async () => { - const createPenalty = vi.fn(() => of({ id: 2, description: 'Handy in der Kabine', amount: 3 })); - const loadPenalties = vi.fn(() => of([{ id: 1, description: 'Zu spät', amount: 5 }])); - const team = { - id: 5, - name: 'Team A', - alias: 'a', +const first: Penalty = { + id: 1, + description: 'Zu spät', + amount: 5, + createdAt: '2026-01-01T00:00:00.000Z', +}; +const team = { + id: 5, + name: 'Team A', + alias: 'a', + balance: 0, + players: [ + { + id: 1, + firstName: 'Alex', + lastName: 'Muster', balance: 0, - players: [ - { - id: 1, - firstName: 'Alex', - lastName: 'Muster', - balance: 0, - active: true, - teamRole: { id: 3 }, - user: { id: 42 }, - }, - ], - }; + active: true, + teamRole: { id: 3 }, + user: { id: 42 }, + }, + ], +}; + +describe('Penalties', () => { + let fixture: ComponentFixture; + let currentUser: ReturnType>; + let loadPenalties: ReturnType; + let createPenalty: ReturnType; + let updatePenalty: ReturnType; + let deletePenalty: ReturnType; + let dialogClosed: Subject; + let dialog: { open: ReturnType }; + + beforeEach(async () => { + currentUser = signal({ id: 42, role: { id: 2 } }); + loadPenalties = vi.fn(() => of([first])); + createPenalty = vi.fn(() => of({ id: 2, description: 'Handy', amount: 3 })); + updatePenalty = vi.fn(() => of({ ...first, description: 'Neu', amount: 6 })); + deletePenalty = vi.fn(() => of(undefined)); + dialogClosed = new Subject(); + dialog = { open: vi.fn(() => ({ afterClosed: () => dialogClosed.asObservable() })) }; + await TestBed.configureTestingModule({ imports: [Penalties], providers: [ provideRouter([]), { provide: TeamStore, useValue: { team: signal(team) } }, - { provide: AuthStore, useValue: { currentUser: signal({ id: 42, role: { id: 2 } }) } }, - { provide: PenaltyApi, useValue: { loadPenalties, createPenalty } }, + { provide: AuthStore, useValue: { currentUser } }, + { + provide: PenaltyApi, + useValue: { loadPenalties, createPenalty, updatePenalty, deletePenalty }, + }, + { provide: MatDialog, useValue: dialog }, ], }).compileComponents(); - const fixture = TestBed.createComponent(Penalties); + }); + + function create(): void { + fixture = TestBed.createComponent(Penalties); fixture.detectChanges(); - expect(fixture.nativeElement.textContent).toContain('Zu spät'); + } + + function text(): string { + return (fixture.nativeElement as HTMLElement).textContent ?? ''; + } + + function button(label: string): HTMLButtonElement { + const match = [...(fixture.nativeElement as HTMLElement).querySelectorAll('button')].find( + (element) => element.textContent?.includes(label), + ); + if (!match) throw new Error(`Missing button: ${label}`); + return match as HTMLButtonElement; + } + + it('shows the catalog without mutation controls to a reader', () => { + currentUser.set({ id: 7, role: { id: 2 } }); + create(); + + expect(text()).toContain('Zu spät'); + expect((fixture.nativeElement as HTMLElement).querySelector('.create-card')).toBeNull(); + expect(text()).not.toContain('Bearbeiten'); + expect(text()).not.toContain('Löschen'); + }); + + it('lets a captain create an entry and reloads authoritative data', () => { + loadPenalties + .mockReturnValueOnce(of([first])) + .mockReturnValueOnce(of([first, { id: 2, description: 'Handy', amount: 3 }])); + create(); + fixture.componentInstance['form'].setValue({ description: 'Handy', amount: 3 }); - fixture.componentInstance['form'].setValue({ description: 'Handy in der Kabine', amount: 3 }); fixture.componentInstance['createPenalty'](); + fixture.detectChanges(); - expect(createPenalty).toHaveBeenCalledWith({ - teamId: 5, - description: 'Handy in der Kabine', - amount: 3, + expect(createPenalty).toHaveBeenCalledWith({ teamId: 5, description: 'Handy', amount: 3 }); + expect(loadPenalties).toHaveBeenCalledTimes(2); + expect(text()).toContain('Handy'); + }); + + it('rejects blank descriptions and amounts with more than two decimals', () => { + create(); + + fixture.componentInstance['form'].setValue({ description: ' ', amount: 1.234 }); + + expect(fixture.componentInstance['form'].invalid).toBe(true); + fixture.componentInstance['createPenalty'](); + expect(createPenalty).not.toHaveBeenCalled(); + }); + + it('opens one inline editor, supports cancel, and saves pessimistically', () => { + const updateResult = new Subject(); + updatePenalty.mockReturnValue(updateResult); + loadPenalties + .mockReturnValueOnce(of([first])) + .mockReturnValueOnce(of([{ ...first, description: 'Neu', amount: 6 }])); + create(); + + button('Bearbeiten').click(); + fixture.detectChanges(); + expect((fixture.nativeElement as HTMLElement).querySelector('.edit-form')).not.toBeNull(); + fixture.componentInstance['cancelEdit'](); + fixture.detectChanges(); + expect((fixture.nativeElement as HTMLElement).querySelector('.edit-form')).toBeNull(); + + button('Bearbeiten').click(); + fixture.componentInstance['editForm'].setValue({ description: 'Neu', amount: 6 }); + fixture.componentInstance['savePenalty'](first); + fixture.detectChanges(); + expect(fixture.componentInstance['editingPenaltyId']()).toBe(first.id); + expect(fixture.componentInstance['penalties']()).toEqual([first]); + expect(loadPenalties).toHaveBeenCalledTimes(1); + + updateResult.next({ ...first, description: 'Neu', amount: 6 }); + fixture.detectChanges(); + expect(updatePenalty).toHaveBeenCalledWith(1, { description: 'Neu', amount: 6 }); + expect(loadPenalties).toHaveBeenCalledTimes(2); + expect(text()).toContain('Neu'); + }); + + it('confirms deletion and reloads only after server success', () => { + const deletion = new Subject(); + deletePenalty.mockReturnValue(deletion); + loadPenalties.mockReturnValueOnce(of([first])).mockReturnValueOnce(of([])); + create(); + + button('Löschen').click(); + expect(dialog.open).toHaveBeenCalled(); + dialogClosed.next(true); + expect(deletePenalty).toHaveBeenCalledWith(1); + expect(loadPenalties).toHaveBeenCalledTimes(1); + expect(text()).toContain('Zu spät'); + + deletion.next(); + fixture.detectChanges(); + expect(loadPenalties).toHaveBeenCalledTimes(2); + expect(text()).toContain('Noch keine Einträge'); + }); + + it('keeps the inline editor and explains a duplicate conflict', () => { + updatePenalty.mockReturnValue( + throwError( + () => + new HttpErrorResponse({ + status: 409, + error: { message: 'Ein Eintrag mit dieser Beschreibung existiert bereits.' }, + }), + ), + ); + create(); + button('Bearbeiten').click(); + fixture.componentInstance['editForm'].setValue({ description: 'Zu spät', amount: 6 }); + + fixture.componentInstance['savePenalty'](first); + fixture.detectChanges(); + + expect((fixture.nativeElement as HTMLElement).querySelector('.edit-form')).not.toBeNull(); + expect(text()).toContain('existiert bereits'); + }); + + it('distinguishes a successful update from a failed authoritative reload', () => { + loadPenalties + .mockReturnValueOnce(of([first])) + .mockReturnValueOnce(throwError(() => new HttpErrorResponse({ status: 500 }))); + create(); + button('Bearbeiten').click(); + fixture.componentInstance['editForm'].setValue({ description: 'Neu', amount: 6 }); + + fixture.componentInstance['savePenalty'](first); + fixture.detectChanges(); + + expect(fixture.componentInstance['editingPenaltyId']()).toBeNull(); + expect(fixture.componentInstance['mutationError']()).toBeNull(); + expect(text()).toContain('Änderung wurde gespeichert'); + expect(text()).toContain('Erneut versuchen'); + }); + + it('clears the create form after success even when the reload fails', () => { + loadPenalties + .mockReturnValueOnce(of([first])) + .mockReturnValueOnce(throwError(() => new HttpErrorResponse({ status: 500 }))); + create(); + fixture.componentInstance['form'].setValue({ description: 'Handy', amount: 3 }); + + fixture.componentInstance['createPenalty'](); + fixture.detectChanges(); + + expect(fixture.componentInstance['form'].getRawValue()).toEqual({ + description: '', + amount: 0, }); - expect(fixture.componentInstance['penalties']().length).toBe(2); + expect(fixture.componentInstance['mutationError']()).toBeNull(); + expect(text()).toContain('Änderung wurde gespeichert'); + }); + + it('reports a reload problem instead of a delete failure after successful deletion', () => { + loadPenalties + .mockReturnValueOnce(of([first])) + .mockReturnValueOnce(throwError(() => new HttpErrorResponse({ status: 500 }))); + create(); + + button('Löschen').click(); + dialogClosed.next(true); + fixture.detectChanges(); + + expect(deletePenalty).toHaveBeenCalledWith(first.id); + expect(fixture.componentInstance['mutationError']()).toBeNull(); + expect(text()).toContain('Änderung wurde gespeichert'); + }); + + it('prevents overlapping catalog mutations', () => { + const creation = new Subject(); + createPenalty.mockReturnValue(creation); + create(); + fixture.componentInstance['form'].setValue({ description: 'Handy', amount: 3 }); + + fixture.componentInstance['createPenalty'](); + fixture.componentInstance['startEdit'](first); + fixture.componentInstance['confirmDelete'](first); + + expect(fixture.componentInstance['editingPenaltyId']()).toBeNull(); + expect(dialog.open).not.toHaveBeenCalled(); + }); + + it('shows a load error, retries, and distinguishes an empty search result', () => { + loadPenalties + .mockReturnValueOnce(throwError(() => new HttpErrorResponse({ status: 500 }))) + .mockReturnValueOnce(of([first])); + create(); + fixture.detectChanges(); + expect(text()).toContain('Katalog konnte nicht geladen werden'); + + button('Erneut versuchen').click(); + fixture.detectChanges(); + expect(text()).toContain('Zu spät'); + + fixture.componentInstance['search'].set('Nicht vorhanden'); + fixture.detectChanges(); + expect(text()).toContain('Keine passenden Einträge'); }); }); diff --git a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.ts b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.ts index 9481803..08d3c30 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.ts +++ b/myteamwallet_frontend_modern/src/app/features/team/more/penalties/penalties.ts @@ -1,18 +1,23 @@ import { CurrencyPipe, registerLocaleData } from '@angular/common'; import localeDe from '@angular/common/locales/de'; -import { Component, LOCALE_ID, computed, effect, inject, signal } from '@angular/core'; +import { HttpErrorResponse } from '@angular/common/http'; +import { Component, DestroyRef, LOCALE_ID, computed, effect, inject, signal } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { FormBuilder, ReactiveFormsModule, Validators } from '@angular/forms'; -import { 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 { MatInputModule } from '@angular/material/input'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { RouterLink } from '@angular/router'; +import { EMPTY, Observable, catchError, finalize, switchMap, take, tap } from 'rxjs'; import { AuthStore } from '../../../../core/auth/auth-store'; import { PenaltyApi } from '../../../../core/team/penalty-api'; import { TeamStore } from '../../../../core/team/team-store'; import { Penalty } from '../../../../models/penalty.model'; +import { ConfirmDialog } from '../../../../shared/confirm-dialog/confirm-dialog'; registerLocaleData(localeDe); @@ -35,25 +40,56 @@ registerLocaleData(localeDe); }) export class Penalties { private readonly authStore = inject(AuthStore); + private readonly destroyRef = inject(DestroyRef); + private readonly dialog = inject(MatDialog); private readonly formBuilder = inject(FormBuilder); private readonly penaltyApi = inject(PenaltyApi); private readonly teamStore = inject(TeamStore); private loadedTeamId: number | null = null; + protected readonly team = this.teamStore.team; protected readonly penalties = signal([]); protected readonly loading = signal(false); protected readonly saving = signal(false); + protected readonly pendingPenaltyId = signal(null); + protected readonly editingPenaltyId = signal(null); + protected readonly loadError = signal(null); + protected readonly mutationError = signal(null); + protected readonly mutationPending = computed( + () => this.saving() || this.pendingPenaltyId() !== null, + ); protected readonly search = signal(''); protected readonly form = this.formBuilder.nonNullable.group({ - description: ['', Validators.required], - amount: [0, [Validators.required, Validators.min(0.01), Validators.max(10000)]], + description: ['', [Validators.required, Validators.maxLength(120), Validators.pattern(/\S/)]], + amount: [ + 0, + [ + Validators.required, + Validators.min(0.01), + Validators.max(10000), + Validators.pattern(/^\d+(\.\d{1,2})?$/), + ], + ], + }); + protected readonly editForm = this.formBuilder.nonNullable.group({ + description: ['', [Validators.required, Validators.maxLength(120), Validators.pattern(/\S/)]], + amount: [ + 0, + [ + Validators.required, + Validators.min(0.01), + Validators.max(10000), + Validators.pattern(/^\d+(\.\d{1,2})?$/), + ], + ], }); 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) > 2, + (player) => + player.active && player.user?.id === user?.id && (player.teamRole?.id ?? 0) >= 3, ) ?? false ); }); @@ -76,26 +112,152 @@ export class Penalties { protected createPenalty(): void { const team = this.team(); - if (!this.canManage() || !team || this.form.invalid || this.saving()) return; + if (!this.canManage() || !team || this.form.invalid || this.mutationPending()) return; this.saving.set(true); - this.penaltyApi.createPenalty({ teamId: team.id, ...this.form.getRawValue() }).subscribe({ - next: (penalty) => { - this.penalties.update((items) => [...items, penalty]); - this.form.reset({ description: '', amount: 0 }); - this.saving.set(false); - }, - error: () => this.saving.set(false), - }); + this.mutationError.set(null); + this.penaltyApi + .createPenalty({ teamId: team.id, ...this.form.getRawValue() }) + .pipe( + tap(() => this.form.reset({ description: '', amount: 0 })), + switchMap(() => this.reloadAfterMutation(team.id)), + finalize(() => this.saving.set(false)), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe({ + next: (penalties) => { + this.penalties.set(penalties); + }, + error: (error: HttpErrorResponse) => + this.mutationError.set(this.errorMessage(error, 'Eintrag konnte nicht angelegt werden.')), + }); + } + + protected startEdit(penalty: Penalty): void { + if (!this.canManage() || this.mutationPending()) return; + this.editingPenaltyId.set(penalty.id); + this.editForm.setValue({ description: penalty.description, amount: penalty.amount }); + this.mutationError.set(null); + } + + protected cancelEdit(): void { + if (this.pendingPenaltyId() !== null) return; + this.editingPenaltyId.set(null); + this.mutationError.set(null); + } + + protected savePenalty(penalty: Penalty): void { + const teamId = this.team()?.id; + if ( + !this.canManage() || + !teamId || + this.editingPenaltyId() !== penalty.id || + this.editForm.invalid || + this.mutationPending() + ) { + return; + } + this.pendingPenaltyId.set(penalty.id); + this.mutationError.set(null); + this.penaltyApi + .updatePenalty(penalty.id, this.editForm.getRawValue()) + .pipe( + tap(() => this.editingPenaltyId.set(null)), + switchMap(() => this.reloadAfterMutation(teamId)), + finalize(() => this.pendingPenaltyId.set(null)), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe({ + next: (penalties) => { + this.penalties.set(penalties); + }, + error: (error: HttpErrorResponse) => + this.mutationError.set( + this.errorMessage(error, 'Eintrag konnte nicht gespeichert werden.'), + ), + }); + } + + protected confirmDelete(penalty: Penalty): void { + if (!this.canManage() || this.mutationPending()) return; + this.dialog + .open(ConfirmDialog, { + data: { + title: 'Eintrag löschen?', + message: `„${penalty.description}“ wird endgültig aus dem Strafenkatalog gelöscht.`, + confirmLabel: 'Löschen', + }, + restoreFocus: true, + }) + .afterClosed() + .pipe(take(1), takeUntilDestroyed(this.destroyRef)) + .subscribe((confirmed) => { + if (confirmed) this.deletePenalty(penalty); + }); + } + + protected retryLoad(): void { + const teamId = this.team()?.id; + if (teamId) this.load(teamId); + } + + private deletePenalty(penalty: Penalty): void { + const teamId = this.team()?.id; + if (!teamId) return; + this.pendingPenaltyId.set(penalty.id); + this.mutationError.set(null); + this.penaltyApi + .deletePenalty(penalty.id) + .pipe( + switchMap(() => this.reloadAfterMutation(teamId)), + finalize(() => this.pendingPenaltyId.set(null)), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe({ + next: (penalties) => { + this.penalties.set(penalties); + if (this.editingPenaltyId() === penalty.id) this.editingPenaltyId.set(null); + }, + error: (error: HttpErrorResponse) => + this.mutationError.set(this.errorMessage(error, 'Eintrag konnte nicht gelöscht werden.')), + }); + } + + private reloadAfterMutation(teamId: number): Observable { + return this.penaltyApi.loadPenalties(teamId).pipe( + catchError(() => { + this.loadError.set( + 'Änderung wurde gespeichert, aber der Katalog konnte nicht aktualisiert werden.', + ); + return EMPTY; + }), + ); } private load(teamId: number): void { this.loading.set(true); - this.penaltyApi.loadPenalties(teamId).subscribe({ - next: (penalties) => { - this.penalties.set(penalties); - this.loading.set(false); - }, - error: () => this.loading.set(false), - }); + this.loadError.set(null); + this.penaltyApi + .loadPenalties(teamId) + .pipe( + finalize(() => this.loading.set(false)), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe({ + next: (penalties) => this.penalties.set(penalties), + error: () => { + this.penalties.set([]); + this.loadError.set('Katalog konnte nicht geladen 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 für diese Änderung.'; + if (error.status === 404) + return 'Der Eintrag wurde nicht gefunden. Bitte lade den Katalog neu.'; + if (error.status === 409) return detail || 'Diese Beschreibung existiert bereits.'; + if (error.status === 422) return 'Bitte prüfe Beschreibung und Betrag.'; + return detail || fallback; } } diff --git a/myteamwallet_frontend_modern/src/app/models/penalty.model.ts b/myteamwallet_frontend_modern/src/app/models/penalty.model.ts index 01008b9..f6a1704 100644 --- a/myteamwallet_frontend_modern/src/app/models/penalty.model.ts +++ b/myteamwallet_frontend_modern/src/app/models/penalty.model.ts @@ -10,3 +10,8 @@ export interface CreatePenalty { description: string; amount: number; } + +export interface UpdatePenalty { + description: string; + amount: number; +}