From b9eb7ecf7232b083c3781eb2cd4d378ce4c60ec2 Mon Sep 17 00:00:00 2001 From: Joeloo1 Date: Mon, 28 Sep 2026 11:56:20 +0100 Subject: [PATCH] fix: add @IsNotEmpty, ApiProperty, UUID pipe, and server-side identity derivation (#354 #355 #356 #357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #354 — @IsNotEmpty() missing from required free-text fields - CreateMilestoneDto.title: add @Transform(trim) + @IsNotEmpty() - CreatePoolDto.name: add @Transform(trim) + @IsNotEmpty() - CreateTeamDto.name: add @Transform(trim) + @IsNotEmpty() + @MaxLength(100) - MilestonesService.create: defensive .trim() on title at persistence - MaintenancePoolService.create: defensive .trim() on name at persistence - TeamsService.create: defensive .trim() on name at persistence - New unit specs: create-milestone.dto.spec.ts, create-pool.dto.spec.ts, create-team.dto.spec.ts covering blank/whitespace-only rejection Issue #355 — inline controller DTOs missing @ApiProperty - DepositDto (amount, funderAddress) in maintenance-pool.controller.ts - AssignRewardDto (issueId, amount, recipientAddress, recipientId) same file - FundBountyDto (funderAddress) in bounties.controller.ts - SetStellarAddressDto (stellarAddress) in users.controller.ts - FundMilestoneDto (funderAddress) in milestones.controller.ts - ResolveIssueDto (issueId) same file Issue #356 — repositoryId query param on GET /bounties not validated as UUID - Add ParseUUIDPipe({ version: '4', optional: true }) to repositoryId - New regression spec bounties.query.spec.ts: malformed UUID -> 400, valid UUID and omitted param pass through Issue #357 — sponsorId/createdById null when MAINTAINER omits the field - MilestonesService.create: sponsorId = dto.sponsorId ?? callerUserId - MaintenancePoolService.create: createdById = dto.createdById ?? callerUserId - TeamsService.create: createdById = dto.createdById ?? callerUserId - MaintenancePoolController.create: extract userId from JWT req.user, pass through - TeamsController.create: use existing @CurrentUser() decorator, pass through - Update service specs to assert fallback-to-callerUserId behaviour --- src/bounties/bounties.controller.spec.ts | 57 +++++++++++++++-- src/bounties/bounties.controller.ts | 18 +++--- src/bounties/bounties.query.spec.ts | 29 +++++++++ .../dto/create-pool.dto.spec.ts | 57 +++++++++++++++++ src/maintenance-pool/dto/create-pool.dto.ts | 13 +++- .../maintenance-pool.controller.ts | 27 ++++++-- .../maintenance-pool.service.spec.ts | 15 +++-- .../maintenance-pool.service.ts | 10 ++- .../dto/create-milestone.dto.spec.ts | 59 ++++++++++++++++++ src/milestones/dto/create-milestone.dto.ts | 6 ++ src/milestones/milestones.controller.ts | 17 +++++- src/milestones/milestones.service.ts | 22 +++++-- src/teams/dto/create-team.dto.spec.ts | 61 +++++++++++++++++++ src/teams/dto/create-team.dto.ts | 7 ++- src/teams/teams.controller.ts | 4 +- src/teams/teams.service.spec.ts | 39 ++++++------ src/teams/teams.service.ts | 15 +++-- src/users/users.controller.ts | 5 +- 18 files changed, 395 insertions(+), 66 deletions(-) create mode 100644 src/bounties/bounties.query.spec.ts create mode 100644 src/maintenance-pool/dto/create-pool.dto.spec.ts create mode 100644 src/milestones/dto/create-milestone.dto.spec.ts create mode 100644 src/teams/dto/create-team.dto.spec.ts diff --git a/src/bounties/bounties.controller.spec.ts b/src/bounties/bounties.controller.spec.ts index 0b5d44e1..9fa19668 100644 --- a/src/bounties/bounties.controller.spec.ts +++ b/src/bounties/bounties.controller.spec.ts @@ -1,3 +1,4 @@ +import { ArgumentMetadata, ParseUUIDPipe } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { plainToInstance } from 'class-transformer'; import { validate } from 'class-validator'; @@ -24,13 +25,23 @@ describe('BountiesController', () => { beforeEach(async () => { bountiesService = { - create: jest.fn().mockResolvedValue({ id: 'b1', status: BountyStatus.OPEN }), - findOne: jest.fn().mockResolvedValue({ id: 'b1', status: BountyStatus.OPEN }), - fund: jest.fn().mockResolvedValue({ id: 'b1', status: BountyStatus.FUNDED }), - claim: jest.fn().mockResolvedValue({ id: 'b1', status: BountyStatus.CLAIMED }), + create: jest + .fn() + .mockResolvedValue({ id: 'b1', status: BountyStatus.OPEN }), + findOne: jest + .fn() + .mockResolvedValue({ id: 'b1', status: BountyStatus.OPEN }), + fund: jest + .fn() + .mockResolvedValue({ id: 'b1', status: BountyStatus.FUNDED }), + claim: jest + .fn() + .mockResolvedValue({ id: 'b1', status: BountyStatus.CLAIMED }), approve: jest.fn().mockResolvedValue({ id: 'b1', status: 'approved' }), reject: jest.fn().mockResolvedValue({ id: 'b1', status: 'rejected' }), - refund: jest.fn().mockResolvedValue({ id: 'b1', status: BountyStatus.REFUNDED }), + refund: jest + .fn() + .mockResolvedValue({ id: 'b1', status: BountyStatus.REFUNDED }), list: jest.fn().mockResolvedValue([]), }; @@ -113,7 +124,13 @@ describe('BountiesController', () => { describe('list', () => { it('calls bountiesService.list with query params', async () => { - await controller.list(BountyStatus.OPEN, BountyDifficulty.BEGINNER, AssetType.USDC, 'repo-1', 'TypeScript'); + await controller.list( + BountyStatus.OPEN, + BountyDifficulty.BEGINNER, + AssetType.USDC, + 'repo-1', + 'TypeScript', + ); expect(bountiesService.list).toHaveBeenCalledWith({ status: BountyStatus.OPEN, @@ -125,6 +142,34 @@ describe('BountiesController', () => { }); }); + describe('repositoryId query pipe behavior', () => { + const repositoryIdMetadata: ArgumentMetadata = { + type: 'query', + metatype: String, + data: 'repositoryId', + }; + const pipe = new ParseUUIDPipe({ version: '4', optional: true }); + + it('accepts undefined when the query param is absent', async () => { + await expect( + pipe.transform(undefined, repositoryIdMetadata), + ).resolves.toBeUndefined(); + }); + + it('accepts a valid v4 UUID', async () => { + const validUuid = 'c73bcdcc-2669-4bf6-81d3-e4ae73fb11fd'; + await expect( + pipe.transform(validUuid, repositoryIdMetadata), + ).resolves.toBe(validUuid); + }); + + it('rejects a non-UUID string with BadRequestException', async () => { + await expect( + pipe.transform('not-a-uuid', repositoryIdMetadata), + ).rejects.toThrow(); + }); + }); + describe('CreateBountyDto validation', () => { it('rejects a body with no issueId', async () => { const dto = plainToInstance(CreateBountyDto, { diff --git a/src/bounties/bounties.controller.ts b/src/bounties/bounties.controller.ts index 752f28df..38ad22e1 100644 --- a/src/bounties/bounties.controller.ts +++ b/src/bounties/bounties.controller.ts @@ -10,7 +10,12 @@ import { Req, UseGuards, } from '@nestjs/common'; -import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; +import { + ApiBearerAuth, + ApiOperation, + ApiProperty, + ApiTags, +} from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; // Import the Throttle decorator import { BountiesService } from './bounties.service'; import { CreateBountyDto } from './dto/create-bounty.dto'; @@ -28,7 +33,8 @@ import { ApiStandardErrorResponses, } from '../common/swagger/api-common-responses.decorator'; -class FundBountyDto { +export class FundBountyDto { + @ApiProperty({ description: 'Stellar public key of the funder' }) @IsStellarAddress() funderAddress: string; } @@ -68,7 +74,8 @@ export class BountiesController { difficulty?: BountyDifficulty, @Query('asset', new ParseEnumPipe(AssetType, { optional: true })) asset?: AssetType, - @Query('repositoryId') repositoryId?: string, + @Query('repositoryId', new ParseUUIDPipe({ version: '4', optional: true })) + repositoryId?: string, @Query('primaryLanguage') primaryLanguage?: string, ) { return this.bountiesService.list({ @@ -120,10 +127,7 @@ export class BountiesController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.CONTRIBUTOR) @Post(':id/claim') - claim( - @Param('id', new ParseUUIDPipe()) id: string, - @Req() req: Request, - ) { + claim(@Param('id', new ParseUUIDPipe()) id: string, @Req() req: Request) { const userId = (req.user as any).userId; return this.bountiesService.claim(id, userId); } diff --git a/src/bounties/bounties.query.spec.ts b/src/bounties/bounties.query.spec.ts new file mode 100644 index 00000000..ec5d2053 --- /dev/null +++ b/src/bounties/bounties.query.spec.ts @@ -0,0 +1,29 @@ +import { ArgumentMetadata, ParseUUIDPipe } from '@nestjs/common'; + +describe('BountiesController repositoryId query pipe', () => { + const repositoryIdMetadata: ArgumentMetadata = { + type: 'query', + metatype: String, + data: 'repositoryId', + }; + const pipe = new ParseUUIDPipe({ version: '4', optional: true }); + + it('accepts undefined when repositoryId is omitted', async () => { + await expect( + pipe.transform(undefined, repositoryIdMetadata), + ).resolves.toBeUndefined(); + }); + + it('accepts a valid v4 UUID string', async () => { + const validUuid = 'c73bcdcc-2669-4bf6-81d3-e4ae73fb11fd'; + await expect(pipe.transform(validUuid, repositoryIdMetadata)).resolves.toBe( + validUuid, + ); + }); + + it('rejects a non-UUID string with BadRequestException', async () => { + await expect( + pipe.transform('not-a-uuid', repositoryIdMetadata), + ).rejects.toThrow(); + }); +}); diff --git a/src/maintenance-pool/dto/create-pool.dto.spec.ts b/src/maintenance-pool/dto/create-pool.dto.spec.ts new file mode 100644 index 00000000..f3b9adf8 --- /dev/null +++ b/src/maintenance-pool/dto/create-pool.dto.spec.ts @@ -0,0 +1,57 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreatePoolDto } from './create-pool.dto'; +import { AssetType } from '../../common/enums'; + +describe('CreatePoolDto validation', () => { + const baseValidPayload = { + name: 'Valid Pool Name', + asset: AssetType.USDC, + }; + + it('accepts a valid payload and trims name whitespace', async () => { + const dto = plainToInstance(CreatePoolDto, { + ...baseValidPayload, + name: ' Trimmed Pool Name ', + }); + const errors = await validate(dto); + expect(errors.length).toBe(0); + expect(dto.name).toBe('Trimmed Pool Name'); + }); + + it('rejects an empty name', async () => { + const dto = plainToInstance(CreatePoolDto, { + ...baseValidPayload, + name: '', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const nameError = errors.find((e) => e.property === 'name'); + expect(nameError).toBeDefined(); + expect(nameError?.constraints?.isNotEmpty).toBeDefined(); + }); + + it('rejects a whitespace-only name', async () => { + const dto = plainToInstance(CreatePoolDto, { + ...baseValidPayload, + name: ' ', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const nameError = errors.find((e) => e.property === 'name'); + expect(nameError).toBeDefined(); + expect(nameError?.constraints?.isNotEmpty).toBeDefined(); + }); + + it('rejects a non-string name', async () => { + const dto = plainToInstance(CreatePoolDto, { + ...baseValidPayload, + name: 999, + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const nameError = errors.find((e) => e.property === 'name'); + expect(nameError).toBeDefined(); + expect(nameError?.constraints?.isString).toBeDefined(); + }); +}); diff --git a/src/maintenance-pool/dto/create-pool.dto.ts b/src/maintenance-pool/dto/create-pool.dto.ts index 5f428f5d..35092577 100644 --- a/src/maintenance-pool/dto/create-pool.dto.ts +++ b/src/maintenance-pool/dto/create-pool.dto.ts @@ -1,5 +1,12 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsOptional, IsString, IsUUID, MaxLength } from 'class-validator'; +import { Transform } from 'class-transformer'; +import { + IsNotEmpty, + IsOptional, + IsString, + IsUUID, + MaxLength, +} from 'class-validator'; import { AssetType } from '../../common/enums'; import { IsMoneyAmount, @@ -8,7 +15,11 @@ import { export class CreatePoolDto { @ApiProperty({ maxLength: 100 }) + @Transform(({ value }: { value: unknown }) => + typeof value === 'string' ? value.trim() : value, + ) @IsString() + @IsNotEmpty() @MaxLength(100) name: string; diff --git a/src/maintenance-pool/maintenance-pool.controller.ts b/src/maintenance-pool/maintenance-pool.controller.ts index b7fbe999..0b370c4c 100644 --- a/src/maintenance-pool/maintenance-pool.controller.ts +++ b/src/maintenance-pool/maintenance-pool.controller.ts @@ -5,9 +5,16 @@ import { Param, ParseUUIDPipe, Post, + Req, UseGuards, } from '@nestjs/common'; -import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; +import { Request } from 'express'; +import { + ApiBearerAuth, + ApiOperation, + ApiProperty, + ApiTags, +} from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; import { IsOptional, IsUUID } from 'class-validator'; import { MaintenancePoolService } from './maintenance-pool.service'; @@ -24,24 +31,33 @@ import { ApiStandardErrorResponses, } from '../common/swagger/api-common-responses.decorator'; -class DepositDto { +export class DepositDto { + @ApiProperty({ description: 'Amount to deposit into the maintenance pool' }) @IsMoneyAmount() amount: string; + @ApiProperty({ description: 'Stellar public key of the funder' }) @IsStellarAddress() funderAddress: string; } -class AssignRewardDto { +export class AssignRewardDto { + @ApiProperty({ description: 'UUID of the issue being rewarded' }) @IsUUID() issueId: string; + @ApiProperty({ description: 'Amount to reward from the pool' }) @IsMoneyAmount() amount: string; + @ApiProperty({ description: 'Stellar public key of the recipient' }) @IsStellarAddress() recipientAddress: string; + @ApiProperty({ + required: false, + description: 'Optional user UUID of the recipient', + }) @IsOptional() @IsUUID() recipientId?: string; @@ -59,8 +75,9 @@ export class MaintenancePoolController { @Post() @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.SPONSOR, UserRole.MAINTAINER) - create(@Body() dto: CreatePoolDto) { - return this.poolService.create(dto); + create(@Body() dto: CreatePoolDto, @Req() req: Request) { + const callerUserId = (req.user as any).userId as string; + return this.poolService.create(dto, callerUserId); } @ApiOperation({ summary: 'List maintenance pools' }) diff --git a/src/maintenance-pool/maintenance-pool.service.spec.ts b/src/maintenance-pool/maintenance-pool.service.spec.ts index 5b761ef4..88ff0c4a 100644 --- a/src/maintenance-pool/maintenance-pool.service.spec.ts +++ b/src/maintenance-pool/maintenance-pool.service.spec.ts @@ -69,11 +69,10 @@ describe('MaintenancePoolService', () => { describe('create', () => { it('saves a new pool with ACTIVE status', async () => { - const pool = await service.create({ - name: 'Docs pool', - asset: AssetType.USDC, - createdById: 'creator-1', - }); + const pool = await service.create( + { name: 'Docs pool', asset: AssetType.USDC, createdById: 'creator-1' }, + 'caller-99', + ); expect(poolRepo.save).toHaveBeenCalledWith( expect.objectContaining({ @@ -86,11 +85,11 @@ describe('MaintenancePoolService', () => { expect(pool.status).toBe(MaintenancePoolStatus.ACTIVE); }); - it('defaults repositoryId/createdById to null when not provided', async () => { - await service.create({ name: 'Pool', asset: AssetType.USDC }); + it('falls back to callerUserId for createdById when client omits it', async () => { + await service.create({ name: 'Pool', asset: AssetType.USDC }, 'caller-99'); expect(poolRepo.save).toHaveBeenCalledWith( - expect.objectContaining({ repositoryId: null, createdById: null }), + expect.objectContaining({ repositoryId: null, createdById: 'caller-99' }), ); }); }); diff --git a/src/maintenance-pool/maintenance-pool.service.ts b/src/maintenance-pool/maintenance-pool.service.ts index 69d83cf0..13415a62 100644 --- a/src/maintenance-pool/maintenance-pool.service.ts +++ b/src/maintenance-pool/maintenance-pool.service.ts @@ -29,11 +29,15 @@ export class MaintenancePoolService { private readonly escrowService: EscrowService, ) {} - async create(dto: CreatePoolDto): Promise { + async create( + dto: CreatePoolDto, + callerUserId: string, + ): Promise { const pool = this.poolRepo.create({ - name: dto.name, + name: dto.name?.trim() ?? dto.name, repositoryId: dto.repositoryId ?? null, - createdById: dto.createdById ?? null, + // Fall back to the authenticated caller's id when the client omits createdById. + createdById: dto.createdById ?? callerUserId, monthlyDeposit: dto.monthlyDeposit, asset: dto.asset, status: MaintenancePoolStatus.ACTIVE, diff --git a/src/milestones/dto/create-milestone.dto.spec.ts b/src/milestones/dto/create-milestone.dto.spec.ts new file mode 100644 index 00000000..d1cf8201 --- /dev/null +++ b/src/milestones/dto/create-milestone.dto.spec.ts @@ -0,0 +1,59 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreateMilestoneDto } from './create-milestone.dto'; +import { AssetType } from '../../common/enums'; + +describe('CreateMilestoneDto validation', () => { + const baseValidPayload = { + repositoryId: 'c73bcdcc-2669-4bf6-81d3-e4ae73fb11fd', + title: 'Valid Milestone Title', + budget: '500', + asset: AssetType.USDC, + }; + + it('accepts a valid payload and trims title whitespace', async () => { + const dto = plainToInstance(CreateMilestoneDto, { + ...baseValidPayload, + title: ' Trimmed Milestone Title ', + }); + const errors = await validate(dto); + expect(errors.length).toBe(0); + expect(dto.title).toBe('Trimmed Milestone Title'); + }); + + it('rejects an empty title', async () => { + const dto = plainToInstance(CreateMilestoneDto, { + ...baseValidPayload, + title: '', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const titleError = errors.find((e) => e.property === 'title'); + expect(titleError).toBeDefined(); + expect(titleError?.constraints?.isNotEmpty).toBeDefined(); + }); + + it('rejects a whitespace-only title', async () => { + const dto = plainToInstance(CreateMilestoneDto, { + ...baseValidPayload, + title: ' ', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const titleError = errors.find((e) => e.property === 'title'); + expect(titleError).toBeDefined(); + expect(titleError?.constraints?.isNotEmpty).toBeDefined(); + }); + + it('rejects a non-string title', async () => { + const dto = plainToInstance(CreateMilestoneDto, { + ...baseValidPayload, + title: 12345, + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const titleError = errors.find((e) => e.property === 'title'); + expect(titleError).toBeDefined(); + expect(titleError?.constraints?.isString).toBeDefined(); + }); +}); diff --git a/src/milestones/dto/create-milestone.dto.ts b/src/milestones/dto/create-milestone.dto.ts index 11bf5993..390ae263 100644 --- a/src/milestones/dto/create-milestone.dto.ts +++ b/src/milestones/dto/create-milestone.dto.ts @@ -1,6 +1,8 @@ import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; import { IsISO8601, + IsNotEmpty, IsOptional, IsString, IsUUID, @@ -24,7 +26,11 @@ export class CreateMilestoneDto { sponsorId?: string; @ApiProperty({ maxLength: 200 }) + @Transform(({ value }: { value: unknown }) => + typeof value === 'string' ? value.trim() : value, + ) @IsString() + @IsNotEmpty() @MaxLength(200) title: string; diff --git a/src/milestones/milestones.controller.ts b/src/milestones/milestones.controller.ts index fdf33077..0aaf9d99 100644 --- a/src/milestones/milestones.controller.ts +++ b/src/milestones/milestones.controller.ts @@ -8,7 +8,12 @@ import { Req, UseGuards, } from '@nestjs/common'; -import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; +import { + ApiBearerAuth, + ApiOperation, + ApiProperty, + ApiTags, +} from '@nestjs/swagger'; import { IsOptional, IsUUID } from 'class-validator'; import { Throttle } from '@nestjs/throttler'; import { MilestonesService } from './milestones.service'; @@ -25,15 +30,21 @@ import { ApiStandardErrorResponses, } from '../common/swagger/api-common-responses.decorator'; -class FundMilestoneDto { +export class FundMilestoneDto { + @ApiProperty({ description: 'Stellar public key of the funder' }) @IsStellarAddress() funderAddress!: string; } -class ResolveIssueDto { +export class ResolveIssueDto { + @ApiProperty({ description: 'Stellar public key of the payout recipient' }) @IsStellarAddress() recipientAddress!: string; + @ApiProperty({ + required: false, + description: 'Optional user UUID of the recipient', + }) @IsOptional() @IsUUID() recipientId?: string; diff --git a/src/milestones/milestones.service.ts b/src/milestones/milestones.service.ts index d03cce43..c2b7ee8b 100644 --- a/src/milestones/milestones.service.ts +++ b/src/milestones/milestones.service.ts @@ -22,7 +22,10 @@ export class MilestonesService { private readonly escrowService: EscrowService, ) {} - async create(dto: CreateMilestoneDto, callerUserId: string): Promise { + async create( + dto: CreateMilestoneDto, + callerUserId: string, + ): Promise { // Verify repositoryId exists const repoExists = await this.dataSource.query( 'SELECT 1 FROM repositories WHERE id = $1', @@ -32,10 +35,13 @@ export class MilestonesService { throw new NotFoundException(`Repository ${dto.repositoryId} not found`); } + // Fall back to the authenticated caller's id when the client omits sponsorId. + // This prevents milestones with sponsorId = null that become permanently + // invisible to every sponsor dashboard filter. const milestone = this.milestoneRepo.create({ repositoryId: dto.repositoryId, - sponsorId: dto.sponsorId ?? null, - title: dto.title, + sponsorId: dto.sponsorId ?? callerUserId, + title: dto.title?.trim() ?? dto.title, description: dto.description ?? null, budget: dto.budget, asset: dto.asset, @@ -55,12 +61,18 @@ export class MilestonesService { } /** Sponsor funds the full milestone budget up front; distributed incrementally per issue. */ - async fund(id: string, funderAddress: string, callerUserId: string): Promise { + async fund( + id: string, + funderAddress: string, + callerUserId: string, + ): Promise { const milestone = await this.findOne(id); // Verify caller is the sponsor if (milestone.sponsorId && milestone.sponsorId !== callerUserId) { - throw new ForbiddenException('Only the milestone sponsor can fund this milestone'); + throw new ForbiddenException( + 'Only the milestone sponsor can fund this milestone', + ); } if (milestone.status !== MilestoneStatus.OPEN) { diff --git a/src/teams/dto/create-team.dto.spec.ts b/src/teams/dto/create-team.dto.spec.ts new file mode 100644 index 00000000..05b7bd07 --- /dev/null +++ b/src/teams/dto/create-team.dto.spec.ts @@ -0,0 +1,61 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreateTeamDto } from './create-team.dto'; + +describe('CreateTeamDto validation', () => { + const baseValidPayload = { + name: 'Valid Team Name', + members: [ + { + userId: 'c73bcdcc-2669-4bf6-81d3-e4ae73fb11fd', + percentage: 100, + }, + ], + }; + + it('accepts a valid payload and trims name whitespace', async () => { + const dto = plainToInstance(CreateTeamDto, { + ...baseValidPayload, + name: ' Trimmed Team Name ', + }); + const errors = await validate(dto); + expect(errors.length).toBe(0); + expect(dto.name).toBe('Trimmed Team Name'); + }); + + it('rejects an empty name', async () => { + const dto = plainToInstance(CreateTeamDto, { + ...baseValidPayload, + name: '', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const nameError = errors.find((e) => e.property === 'name'); + expect(nameError).toBeDefined(); + expect(nameError?.constraints?.isNotEmpty).toBeDefined(); + }); + + it('rejects a whitespace-only name', async () => { + const dto = plainToInstance(CreateTeamDto, { + ...baseValidPayload, + name: ' ', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const nameError = errors.find((e) => e.property === 'name'); + expect(nameError).toBeDefined(); + expect(nameError?.constraints?.isNotEmpty).toBeDefined(); + }); + + it('rejects a non-string name', async () => { + const dto = plainToInstance(CreateTeamDto, { + ...baseValidPayload, + name: 12345, + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const nameError = errors.find((e) => e.property === 'name'); + expect(nameError).toBeDefined(); + expect(nameError?.constraints?.isString).toBeDefined(); + }); +}); diff --git a/src/teams/dto/create-team.dto.ts b/src/teams/dto/create-team.dto.ts index 75acea82..e5487c21 100644 --- a/src/teams/dto/create-team.dto.ts +++ b/src/teams/dto/create-team.dto.ts @@ -1,7 +1,8 @@ import { ApiProperty } from '@nestjs/swagger'; -import { Type } from 'class-transformer'; +import { Transform, Type } from 'class-transformer'; import { ArrayMinSize, + IsNotEmpty, IsNumber, IsOptional, IsString, @@ -40,7 +41,11 @@ export class UpdateTeamSplitsDto { export class CreateTeamDto { @ApiProperty() + @Transform(({ value }: { value: unknown }) => + typeof value === 'string' ? value.trim() : value, + ) @IsString() + @IsNotEmpty() @MaxLength(100) name: string; diff --git a/src/teams/teams.controller.ts b/src/teams/teams.controller.ts index a2e45671..f2ea54db 100644 --- a/src/teams/teams.controller.ts +++ b/src/teams/teams.controller.ts @@ -42,8 +42,8 @@ export class TeamsController { @Post() @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.MAINTAINER, UserRole.SPONSOR) - create(@Body() dto: CreateTeamDto) { - return this.teamsService.create(dto); + create(@Body() dto: CreateTeamDto, @CurrentUser() user: AuthenticatedUser) { + return this.teamsService.create(dto, user.userId); } @ApiOperation({ summary: 'Get a team and its current member splits' }) diff --git a/src/teams/teams.service.spec.ts b/src/teams/teams.service.spec.ts index b66b0a90..6bdd622d 100644 --- a/src/teams/teams.service.spec.ts +++ b/src/teams/teams.service.spec.ts @@ -54,10 +54,10 @@ describe('TeamsService', () => { describe('create', () => { it('rejects via validateSplitPercentages when splits do not sum to 100', async () => { await expect( - service.create({ - name: 'Team A', - members: [{ userId: 'u1', percentage: 60 }], - }), + service.create( + { name: 'Team A', members: [{ userId: 'u1', percentage: 60 }] }, + 'caller-99', + ), ).rejects.toThrow( 'team member split percentages must sum to 100, got 60.00', ); @@ -66,14 +66,17 @@ describe('TeamsService', () => { }); it('saves the team and one split per member when percentages sum to 100', async () => { - const team = await service.create({ - name: 'Team A', - createdById: 'creator-1', - members: [ - { userId: 'u1', role: 'frontend', percentage: 60 }, - { userId: 'u2', percentage: 40 }, - ], - }); + const team = await service.create( + { + name: 'Team A', + createdById: 'creator-1', + members: [ + { userId: 'u1', role: 'frontend', percentage: 60 }, + { userId: 'u2', percentage: 40 }, + ], + }, + 'caller-99', + ); expect(teamRepo.save).toHaveBeenCalledWith( expect.objectContaining({ name: 'Team A', createdById: 'creator-1' }), @@ -99,14 +102,14 @@ describe('TeamsService', () => { expect(team.splits).toHaveLength(2); }); - it('defaults createdById to null when not provided', async () => { - await service.create({ - name: 'Team B', - members: [{ userId: 'u1', percentage: 100 }], - }); + it('falls back to callerUserId for createdById when client omits it', async () => { + await service.create( + { name: 'Team B', members: [{ userId: 'u1', percentage: 100 }] }, + 'caller-99', + ); expect(teamRepo.save).toHaveBeenCalledWith( - expect.objectContaining({ createdById: null }), + expect.objectContaining({ createdById: 'caller-99' }), ); }); }); diff --git a/src/teams/teams.service.ts b/src/teams/teams.service.ts index 7e0a656b..fef4a7df 100644 --- a/src/teams/teams.service.ts +++ b/src/teams/teams.service.ts @@ -21,12 +21,14 @@ export class TeamsService { @InjectRepository(User) private readonly userRepo: Repository, ) {} - async create(dto: CreateTeamDto): Promise { + async create(dto: CreateTeamDto, callerUserId: string): Promise { validateSplitPercentages(dto.members); // Verify all member userIds exist for (const member of dto.members) { - const userExists = await this.userRepo.findOne({ where: { id: member.userId } }); + const userExists = await this.userRepo.findOne({ + where: { id: member.userId }, + }); if (!userExists) { throw new NotFoundException(`User ${member.userId} not found`); } @@ -34,8 +36,9 @@ export class TeamsService { const team = await this.teamRepo.save( this.teamRepo.create({ - name: dto.name, - createdById: dto.createdById ?? null, + name: dto.name?.trim() ?? dto.name, + // Fall back to the authenticated caller's id when the client omits createdById. + createdById: dto.createdById ?? callerUserId, }), ); @@ -75,7 +78,9 @@ export class TeamsService { // Verify all member userIds exist for (const member of members) { - const userExists = await this.userRepo.findOne({ where: { id: member.userId } }); + const userExists = await this.userRepo.findOne({ + where: { id: member.userId }, + }); if (!userExists) { throw new NotFoundException(`User ${member.userId} not found`); } diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index a62689fd..21559de8 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -7,7 +7,7 @@ import { Patch, UseGuards, } from '@nestjs/common'; -import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { ApiBearerAuth, ApiProperty, ApiTags } from '@nestjs/swagger'; import { UsersService } from './users.service'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; import { IsStellarAddress } from '../common/validators/stellar-address.validator'; @@ -16,7 +16,8 @@ import { ApiInternalErrorResponse } from '../common/swagger/api-common-responses // Checksum-validated (StrKey), like every other Stellar-address-accepting DTO. // This value later becomes the on-chain recipient of escrow releases, so a // malformed address must be rejected here rather than at the Soroban call (#292). -class SetStellarAddressDto { +export class SetStellarAddressDto { + @ApiProperty({ description: 'Stellar public address of the user' }) @IsStellarAddress() stellarAddress: string; }