diff --git a/.superpowers/sdd/admin-user-management/task-1-report.md b/.superpowers/sdd/admin-user-management/task-1-report.md new file mode 100644 index 0000000..15233d2 --- /dev/null +++ b/.superpowers/sdd/admin-user-management/task-1-report.md @@ -0,0 +1,69 @@ +# Task 1 implementation report: backend directory contract and query + +## Files changed + +- `myteamwallet_backend/src/users/dto/user-directory-query.dto.ts` — page, limit, and optional search input validation. +- `myteamwallet_backend/src/users/dto/user-directory-response.dto.ts` — explicit safe directory, admin, assignment, team, and reference response DTOs. +- `myteamwallet_backend/src/users/users.service.ts` — scoped directory query, search, pagination, deduplication, and explicit entity-to-DTO mapping. +- `myteamwallet_backend/src/users/users.controller.ts` — authenticated `GET /api/v1/users/directory` endpoint, declared before `:id`. +- `myteamwallet_backend/src/users/users.service.spec.ts` — focused contract coverage. + +## RED test evidence + +Command: + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Result: failed as expected, 7/7 tests failed with `TypeError: service.findDirectory is not a function`. This proved the missing directory-query behavior before implementation. + +## GREEN verification + +Commands and results: + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Passed: 1 suite, 7 tests. Covers cross-team isolation, non-admin email/secret redaction, inactive visibility, admin visibility, deduplication before pagination, search, and pagination metadata. + +```powershell +.\node_modules\.bin\eslint.cmd src\users\users.service.ts src\users\users.controller.ts src\users\users.service.spec.ts src\users\dto\user-directory-query.dto.ts src\users\dto\user-directory-response.dto.ts --max-warnings=0 +``` + +Passed with no warnings or errors. + +```powershell +npm run build +``` + +Passed: Nest build completed successfully. + +```powershell +git diff --check +``` + +Passed with no whitespace errors. + +## Design notes + +- `findDirectory(requester, query)` returns `{ data, page, limit, total, hasNextPage }`. +- A non-admin's shared-team set is derived from their active player assignments. Only users with an assignment in that set are included, and each returned assignment is filtered to that same set. +- Inactive target users and inactive assignments remain visible when their team is shared. +- Admins receive all non-deleted users and every linked player assignment. Their records extend the safe base summary with `email` and the existing `{ id, name }` role shape. +- The query maps selected DTO fields explicitly. It never serializes a `User` or `Player` entity, so passwords, hashes, social IDs, providers, and other authentication fields cannot leak through this endpoint. +- User IDs are ordered before search/pagination for deterministic pages. Users are the primary result set, which guarantees deduplication before pagination even when they have multiple player assignments. + +## Self-review + +- Confirmed `GET directory` is registered before `GET :id`. +- Confirmed non-admin searches only operate after visibility filtering and do not include email. +- Confirmed admin search may include email and admin mapping includes role/status using the backend's existing `{ id, name }` shapes. +- Confirmed an admin with no player assignment is included and an unassigned non-admin is not exposed to other non-admins. +- Confirmed assignment mapping includes team/team-role summary fields only, never its linked user entity. + +## Concerns + +- The service intentionally fetches the user and player directory sets and applies the authorization filter in memory. Task 2's planned foreign-key index work can support a future query-builder optimization without changing this safe response contract. +- The repository-wide Jest suite has documented pre-existing placeholder dependency failures in the SDD ledger; this task verified its focused suite, lint, build, and whitespace check. diff --git a/myteamwallet_backend/src/users/dto/user-directory-query.dto.ts b/myteamwallet_backend/src/users/dto/user-directory-query.dto.ts new file mode 100644 index 0000000..582fda3 --- /dev/null +++ b/myteamwallet_backend/src/users/dto/user-directory-query.dto.ts @@ -0,0 +1,21 @@ +import { Type } from 'class-transformer'; +import { IsInt, IsOptional, IsString, Max, Min } from 'class-validator'; + +export class UserDirectoryQueryDto { + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + page = 1; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + @Max(50) + limit = 20; + + @IsOptional() + @IsString() + search?: string; +} diff --git a/myteamwallet_backend/src/users/dto/user-directory-response.dto.ts b/myteamwallet_backend/src/users/dto/user-directory-response.dto.ts new file mode 100644 index 0000000..6f1f107 --- /dev/null +++ b/myteamwallet_backend/src/users/dto/user-directory-response.dto.ts @@ -0,0 +1,40 @@ +export class UserDirectoryReferenceDto { + id: number; + name?: string; +} + +export class UserDirectoryTeamDto { + id: number; + name: string; + alias: string; +} + +export class UserDirectoryAssignmentDto { + id: number; + firstName: string; + lastName: string; + active: boolean; + team: UserDirectoryTeamDto; + teamRole: UserDirectoryReferenceDto | null; +} + +export class UserDirectorySummaryDto { + id: number; + firstName: string | null; + lastName: string | null; + status: UserDirectoryReferenceDto | null; + assignments: UserDirectoryAssignmentDto[]; +} + +export class AdminUserDirectorySummaryDto extends UserDirectorySummaryDto { + email: string | null; + role: UserDirectoryReferenceDto | null; +} + +export class UserDirectoryPageDto { + data: Array; + page: number; + limit: number; + total: number; + hasNextPage: boolean; +} diff --git a/myteamwallet_backend/src/users/users.controller.ts b/myteamwallet_backend/src/users/users.controller.ts index 57f4f54..2a40f30 100644 --- a/myteamwallet_backend/src/users/users.controller.ts +++ b/myteamwallet_backend/src/users/users.controller.ts @@ -12,6 +12,7 @@ import { ParseIntPipe, HttpStatus, HttpCode, + Request, } from '@nestjs/common'; import { UsersService } from './users.service'; import { CreateUserDto } from './dto/create-user.dto'; @@ -22,6 +23,8 @@ import { RoleEnum } from 'src/roles/roles.enum'; import { AuthGuard } from '@nestjs/passport'; import { RolesGuard } from 'src/roles/roles.guard'; import { infinityPagination } from 'src/utils/infinity-pagination'; +import { UserDirectoryQueryDto } from './dto/user-directory-query.dto'; +import { User } from './entities/user.entity'; @ApiBearerAuth() @UseGuards(AuthGuard('jwt'), RolesGuard) @@ -60,6 +63,16 @@ export class UsersController { ); } + @Roles([RoleEnum.user, RoleEnum.admin]) + @Get('directory') + @HttpCode(HttpStatus.OK) + findDirectory( + @Request() request: { user: Pick }, + @Query() query: UserDirectoryQueryDto, + ) { + return this.usersService.findDirectory(request.user, query); + } + @Roles([RoleEnum.admin]) @Get(':id') @HttpCode(HttpStatus.OK) diff --git a/myteamwallet_backend/src/users/users.service.spec.ts b/myteamwallet_backend/src/users/users.service.spec.ts new file mode 100644 index 0000000..b039258 --- /dev/null +++ b/myteamwallet_backend/src/users/users.service.spec.ts @@ -0,0 +1,208 @@ +import { RoleEnum } from '../roles/roles.enum'; +import { StatusEnum } from '../statuses/statuses.enum'; +import { UsersService } from './users.service'; + +describe('UsersService directory', () => { + const teamA = { id: 10, name: 'Alpha', alias: 'alpha' }; + const teamB = { id: 20, name: 'Bravo', alias: 'bravo' }; + const playerRole = { id: 1, name: 'Player' }; + + const users = [ + user(1, 'Riley', 'Reader', 'reader@example.com'), + user(2, 'Emma', 'Shared', 'emma@example.com'), + user(3, 'Iva', 'Inactive', 'iva@example.com', StatusEnum.inactive), + user(4, 'Otis', 'Outside', 'otis@example.com'), + user(5, 'Morgan', 'Multiple', 'morgan@example.com'), + user( + 6, + 'Ada', + 'Admin', + 'admin@example.com', + StatusEnum.active, + RoleEnum.admin, + ), + ]; + + const players = [ + assignment(101, users[0], teamA), + assignment(201, users[1], teamA), + assignment(301, users[2], teamA, false), + assignment(401, users[3], teamB), + assignment(501, users[4], teamA), + assignment(502, users[4], teamB), + ]; + + const usersRepository = { find: jest.fn() }; + const playersRepository = { find: jest.fn() }; + let service: UsersService; + + beforeEach(() => { + jest.resetAllMocks(); + usersRepository.find.mockResolvedValue(users); + playersRepository.find.mockResolvedValue(players); + service = new UsersService( + usersRepository as any, + playersRepository as any, + ); + }); + + it('hides users and assignments from teams that the requester does not share', async () => { + const result = await directoryFor(users[0]); + + expect(result.data.map((entry) => entry.id)).toEqual([1, 2, 3, 5]); + expect(result.data.find((entry) => entry.id === 4)).toBeUndefined(); + expect(result.data.find((entry) => entry.id === 5).assignments).toEqual([ + assignmentSummary(501, 'Morgan', 'Multiple', true, teamA), + ]); + }); + + it('redacts email and authentication secrets for a non-admin requester', async () => { + const result = await directoryFor(users[0]); + const entry = result.data.find((candidate) => candidate.id === 2); + + expect(entry).toEqual({ + id: 2, + firstName: 'Emma', + lastName: 'Shared', + status: { id: StatusEnum.active, name: 'Active' }, + assignments: [assignmentSummary(201, 'Emma', 'Shared', true, teamA)], + }); + expect(entry).not.toHaveProperty('email'); + expect(entry).not.toHaveProperty('password'); + expect(entry).not.toHaveProperty('hash'); + expect(entry).not.toHaveProperty('socialId'); + }); + + it('keeps inactive users and inactive assignments visible in shared teams', async () => { + const result = await directoryFor(users[0]); + + expect(result.data.find((entry) => entry.id === 3)).toEqual({ + id: 3, + firstName: 'Iva', + lastName: 'Inactive', + status: { id: StatusEnum.inactive, name: 'Inactive' }, + assignments: [assignmentSummary(301, 'Iva', 'Inactive', false, teamA)], + }); + }); + + it('returns every user and assignment with email and role for an admin requester', async () => { + const result = await directoryFor(users[5]); + const multiple = result.data.find((entry) => entry.id === 5); + const outsider = result.data.find((entry) => entry.id === 4); + + expect(result.data).toHaveLength(6); + expect(outsider).toMatchObject({ + email: 'otis@example.com', + role: { id: RoleEnum.user, name: 'User' }, + }); + expect(multiple.assignments).toEqual([ + assignmentSummary(501, 'Morgan', 'Multiple', true, teamA), + assignmentSummary(502, 'Morgan', 'Multiple', true, teamB), + ]); + expect(multiple).not.toHaveProperty('password'); + expect(multiple).not.toHaveProperty('hash'); + expect(multiple).not.toHaveProperty('socialId'); + }); + + it('deduplicates a user with assignments in more than one shared team before pagination', async () => { + const result = await directoryFor(users[0], { page: 2, limit: 2 }); + + expect(result.data.map((entry) => entry.id)).toEqual([3, 5]); + expect(result.total).toBe(4); + expect(result.hasNextPage).toBe(false); + }); + + it('searches visible names case-insensitively without exposing outside-team users', async () => { + const matched = await directoryFor(users[0], { search: 'mOrGaN' }); + const hidden = await directoryFor(users[0], { search: 'outside' }); + + expect(matched.data.map((entry) => entry.id)).toEqual([5]); + expect(hidden.data).toEqual([]); + }); + + it('paginates the deduplicated, filtered directory and reports the next page', async () => { + const result = await directoryFor(users[0], { page: 1, limit: 2 }); + + expect(result).toMatchObject({ + page: 1, + limit: 2, + total: 4, + hasNextPage: true, + }); + expect(result.data.map((entry) => entry.id)).toEqual([1, 2]); + }); + + function directoryFor( + requester: typeof users[number], + query: { page?: number; limit?: number; search?: string } = {}, + ) { + return (service as any).findDirectory(requester, { + page: 1, + limit: 20, + ...query, + }); + } + + function user( + id: number, + firstName: string, + lastName: string, + email: string, + statusId = StatusEnum.active, + roleId = RoleEnum.user, + ) { + return { + id, + firstName, + lastName, + email, + password: `password-${id}`, + hash: `hash-${id}`, + socialId: `social-${id}`, + provider: 'email', + previousPassword: `previous-password-${id}`, + status: { + id: statusId, + name: statusId === StatusEnum.active ? 'Active' : 'Inactive', + }, + role: { + id: roleId, + name: roleId === RoleEnum.admin ? 'Admin' : 'User', + }, + }; + } + + function assignment( + id: number, + playerUser: typeof users[number], + team: typeof teamA, + active = true, + ) { + return { + id, + firstName: playerUser.firstName, + lastName: playerUser.lastName, + active, + user: playerUser, + team, + teamRole: playerRole, + }; + } + + function assignmentSummary( + id: number, + firstName: string, + lastName: string, + active: boolean, + team: typeof teamA, + ) { + return { + id, + firstName, + lastName, + active, + team, + teamRole: playerRole, + }; + } +}); diff --git a/myteamwallet_backend/src/users/users.service.ts b/myteamwallet_backend/src/users/users.service.ts index e7ddc52..2060398 100644 --- a/myteamwallet_backend/src/users/users.service.ts +++ b/myteamwallet_backend/src/users/users.service.ts @@ -4,8 +4,18 @@ import { Player } from 'src/players/entities/player.entity'; import { EntityCondition } from 'src/utils/types/entity-condition.type'; import { IPaginationOptions } from 'src/utils/types/pagination-options'; import { Repository } from 'typeorm'; +import { RoleEnum } from '../roles/roles.enum'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; +import { UserDirectoryQueryDto } from './dto/user-directory-query.dto'; +import { + AdminUserDirectorySummaryDto, + UserDirectoryAssignmentDto, + UserDirectoryPageDto, + UserDirectoryReferenceDto, + UserDirectorySummaryDto, + UserDirectoryTeamDto, +} from './dto/user-directory-response.dto'; import { User } from './entities/user.entity'; @Injectable() @@ -30,6 +40,66 @@ export class UsersService { }); } + async findDirectory( + requester: Pick, + query: UserDirectoryQueryDto, + ): Promise { + const [users, players] = await Promise.all([ + this.usersRepository.find({ order: { id: 'ASC' } }), + this.playersRepository.find({ + relations: ['user', 'team', 'teamRole'], + order: { id: 'ASC' }, + }), + ]); + const isAdmin = requester.role?.id === RoleEnum.admin; + const sharedTeamIds = new Set( + players + .filter((player) => player.user?.id === requester.id && player.active) + .map((player) => player.team.id), + ); + const assignmentsByUserId = new Map(); + + for (const player of players) { + if (!player.user || (!isAdmin && !sharedTeamIds.has(player.team.id))) { + continue; + } + + const assignments = assignmentsByUserId.get(player.user.id) ?? []; + assignments.push(player); + assignmentsByUserId.set(player.user.id, assignments); + } + + const visibleUsers = users.filter( + (user) => isAdmin || assignmentsByUserId.has(user.id), + ); + const searchedUsers = this.filterDirectorySearch( + visibleUsers, + assignmentsByUserId, + query.search, + isAdmin, + ); + const total = searchedUsers.length; + const page = query.page ?? 1; + const limit = query.limit ?? 20; + const data = searchedUsers + .slice((page - 1) * limit, page * limit) + .map((user) => + this.mapDirectoryUser( + user, + assignmentsByUserId.get(user.id) ?? [], + isAdmin, + ), + ); + + return { + data, + page, + limit, + total, + hasNextPage: page * limit < total, + }; + } + findOne(fields: EntityCondition) { return this.usersRepository.findOne({ where: fields, @@ -77,4 +147,89 @@ export class UsersService { return resolve(true); }); } + + private filterDirectorySearch( + users: User[], + assignmentsByUserId: Map, + search: string | undefined, + includeEmail: boolean, + ): User[] { + const term = search?.trim().toLocaleLowerCase(); + if (!term) { + return users; + } + + return users.filter((user) => { + const assignments = assignmentsByUserId.get(user.id) ?? []; + const values = [ + user.firstName, + user.lastName, + ...(includeEmail ? [user.email] : []), + ...assignments.flatMap((assignment) => [ + assignment.firstName, + assignment.lastName, + ]), + ]; + + return values.some((value) => value?.toLocaleLowerCase().includes(term)); + }); + } + + private mapDirectoryUser( + user: User, + assignments: Player[], + includeAdminFields: boolean, + ): UserDirectorySummaryDto | AdminUserDirectorySummaryDto { + const summary: UserDirectorySummaryDto = { + id: user.id, + firstName: user.firstName, + lastName: user.lastName, + status: this.mapDirectoryReference(user.status), + assignments: assignments + .sort((left, right) => left.id - right.id) + .map((assignment) => this.mapDirectoryAssignment(assignment)), + }; + + if (!includeAdminFields) { + return summary; + } + + return { + ...summary, + email: user.email, + role: this.mapDirectoryReference(user.role), + }; + } + + private mapDirectoryAssignment(player: Player): UserDirectoryAssignmentDto { + return { + id: player.id, + firstName: player.firstName, + lastName: player.lastName, + active: player.active, + team: this.mapDirectoryTeam(player.team), + teamRole: this.mapDirectoryReference(player.teamRole), + }; + } + + private mapDirectoryTeam(team: Player['team']): UserDirectoryTeamDto { + return { + id: team.id, + name: team.name, + alias: team.alias, + }; + } + + private mapDirectoryReference( + reference: { id: number; name?: string } | null | undefined, + ): UserDirectoryReferenceDto | null { + if (!reference) { + return null; + } + + return { + id: reference.id, + name: reference.name, + }; + } }