162 lines
6.8 KiB
Markdown
162 lines
6.8 KiB
Markdown
# 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 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.
|
|
|
|
## Fix Round 1
|
|
|
|
### Files changed
|
|
|
|
- `myteamwallet_backend/src/users/users.service.ts` — replaces whole-entity loading with database-side raw projections for visibility, search, distinct count, deterministic ordering, pagination, and assignment filtering.
|
|
- `myteamwallet_backend/src/users/users.service.spec.ts` — adds the inactive-requester regression and runs the directory contract against query-builder doubles that reject entity hydration and unsafe projected authentication fields.
|
|
|
|
### RED evidence
|
|
|
|
Test file: `myteamwallet_backend/src/users/users.service.spec.ts`
|
|
|
|
Command:
|
|
|
|
```powershell
|
|
npm test -- users/users.service.spec.ts --runInBand
|
|
```
|
|
|
|
Result: failed as expected with 2 failures. `treats an inactive requester assignment as a shared team membership` received `[]` instead of `[1, 2]`; `does not hydrate whole user entities for the directory` rejected with `directory queries must use a safe database projection` because the old code called `usersRepository.find`.
|
|
|
|
### GREEN verification
|
|
|
|
```powershell
|
|
npm test -- users/users.service.spec.ts --runInBand
|
|
```
|
|
|
|
Passed: 1 suite, 9 tests.
|
|
|
|
```powershell
|
|
.\node_modules\.bin\eslint.cmd src\users\users.service.ts src\users\users.service.spec.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.
|
|
|
|
### Implementation notes
|
|
|
|
- Shared-team membership now uses every requester `Player` row, including inactive ones, exactly as required by the directory plan.
|
|
- The user query joins only `status` and `role`, projects safe raw columns, applies shared-team visibility/search in SQL, counts `DISTINCT user.id`, orders by `user.id`, and applies offset/limit before mapping.
|
|
- Assignment rows are fetched only for the selected page of user IDs and are scoped with the same shared-team subquery for non-admins. No directory query selects or hydrates `User` authentication columns.
|
|
|
|
## Fix Round 2
|
|
|
|
### Files changed
|
|
|
|
- `myteamwallet_backend/src/users/users.service.spec.ts` — strengthens the inactive-requester regression with a QueryBuilder boundary that rejects `requesterPlayer.active` in the shared-team predicate.
|
|
|
|
### RED evidence
|
|
|
|
Test file: `myteamwallet_backend/src/users/users.service.spec.ts`
|
|
|
|
After installing the boundary guard, the shared-team query was deliberately mutated to add `requesterPlayer.active = :active`.
|
|
|
|
```powershell
|
|
npm test -- users/users.service.spec.ts --runInBand
|
|
```
|
|
|
|
Result: failed as expected, 1/9 tests failed. `treats an inactive requester assignment as a shared team membership` failed with `shared-team membership must not filter inactive requester assignments`. The mutation was then removed; the production query remains user-ID-only.
|
|
|
|
### GREEN verification
|
|
|
|
```powershell
|
|
npm test -- users/users.service.spec.ts --runInBand
|
|
```
|
|
|
|
Passed: 1 suite, 9 tests.
|
|
|
|
```powershell
|
|
.\node_modules\.bin\eslint.cmd src\users\users.service.spec.ts --max-warnings=0
|
|
```
|
|
|
|
Passed with no warnings or errors.
|
|
|
|
```powershell
|
|
git diff --check
|
|
```
|
|
|
|
Passed with no whitespace errors.
|
|
|
|
### Implementation notes
|
|
|
|
- The test double checks the actual shared-team predicate supplied by the service, rather than returning fixed rows alone. It rejects only for the inactive-requester regression if a predicate references `requesterPlayer.active`, so the test now fails for the realistic authorization regression while preserving the existing output-contract assertions.
|