From bf739c109be711999db10287642e767e8755bab1 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 12 Mar 2026 09:54:51 -0400 Subject: [PATCH 01/20] [TM-3071] add users index --- apps/user-service/src/app.module.ts | 2 + .../src/users/dto/user-query.dto.ts | 15 ++++ .../src/users/users.controller.ts | 28 +++++++- apps/user-service/src/users/users.service.ts | 70 +++++++++++++++++++ libs/common/src/lib/dto/user.dto.ts | 10 +++ 5 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 apps/user-service/src/users/dto/user-query.dto.ts create mode 100644 apps/user-service/src/users/users.service.ts diff --git a/apps/user-service/src/app.module.ts b/apps/user-service/src/app.module.ts index b1d786f3b..37620b248 100644 --- a/apps/user-service/src/app.module.ts +++ b/apps/user-service/src/app.module.ts @@ -10,6 +10,7 @@ import { ResetPasswordService } from "./auth/reset-password.service"; import { VerificationUserController } from "./auth/verification-user.controller"; import { VerificationUserService } from "./auth/verification-user.service"; import { UserCreationService } from "./users/user-creation.service"; +import { UsersService } from "./users/users.service"; import { HealthModule } from "@terramatch-microservices/common/health/health.module"; import { OrganisationsController } from "./organisations/organisations.controller"; import { OrganisationsService } from "./organisations/organisations.service"; @@ -44,6 +45,7 @@ import { UserAssociationService } from "./user-association/user-association.serv ResetPasswordService, VerificationUserService, UserCreationService, + UsersService, OrganisationsService, OrganisationCreationService, ActionsService, diff --git a/apps/user-service/src/users/dto/user-query.dto.ts b/apps/user-service/src/users/dto/user-query.dto.ts new file mode 100644 index 000000000..1f90fea99 --- /dev/null +++ b/apps/user-service/src/users/dto/user-query.dto.ts @@ -0,0 +1,15 @@ +import { IsOptional } from "class-validator"; +import { ApiProperty } from "@nestjs/swagger"; +import { IndexQueryDto } from "@terramatch-microservices/common/dto/index-query.dto"; +import { TransformBooleanString } from "@terramatch-microservices/common/decorators/transform-boolean-string.decorator"; + +export class UserQueryDto extends IndexQueryDto { + @ApiProperty({ required: false }) + @IsOptional() + search?: string; + + @ApiProperty({ required: false, description: "Filter users by email address verification status" }) + @IsOptional() + @TransformBooleanString() + isVerified?: boolean; +} diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index 28e7e5c22..18be9cb51 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -7,6 +7,7 @@ import { Param, Patch, Post, + Query, UnauthorizedException } from "@nestjs/common"; import { User } from "@terramatch-microservices/database/entities"; @@ -14,11 +15,13 @@ import { PolicyService } from "@terramatch-microservices/common"; import { ApiOperation, ApiParam } from "@nestjs/swagger"; import { OrganisationLightDto, UserDto } from "@terramatch-microservices/common/dto"; import { ExceptionResponse, JsonApiResponse } from "@terramatch-microservices/common/decorators"; -import { buildJsonApi, DocumentBuilder } from "@terramatch-microservices/common/util"; +import { buildJsonApi, DocumentBuilder, getStableRequestQuery } from "@terramatch-microservices/common/util"; import { UserUpdateBody } from "./dto/user-update.dto"; import { NoBearerAuth } from "@terramatch-microservices/common/guards"; import { UserCreateBody } from "./dto/user-create.dto"; import { UserCreationService } from "./user-creation.service"; +import { UserQueryDto } from "./dto/user-query.dto"; +import { UsersService } from "./users.service"; import { authenticatedUserId } from "@terramatch-microservices/common/guards/auth.guard"; export const USER_ORG_RELATIONSHIP = { @@ -43,9 +46,30 @@ const USER_RESPONSE_SHAPE = { export class UsersController { constructor( private readonly policyService: PolicyService, - private readonly userCreationService: UserCreationService + private readonly userCreationService: UserCreationService, + private readonly usersService: UsersService ) {} + @Get() + @ApiOperation({ operationId: "userIndex", description: "Fetch a paginated list of users" }) + @JsonApiResponse([{ data: UserDto, pagination: "number" }]) + @ExceptionResponse(UnauthorizedException, { description: "Authorization failed" }) + async userIndex(@Query() query: UserQueryDto) { + const { users, paginationTotal } = await this.usersService.findMany(query); + if (users.length > 0) { + await this.policyService.authorize("read", users); + } + + const document = buildJsonApi(UserDto, { forceDataArray: true }).addIndex({ + requestPath: `/users/v3/users${getStableRequestQuery(query)}`, + total: paginationTotal, + pageNumber: query.page?.number ?? 1 + }); + + await this.usersService.addUsersToDocument(document, users); + return document; + } + @Get(":uuid") @ApiOperation({ operationId: "usersFind", description: "Fetch a user by UUID, or with the 'me' identifier" }) @ApiParam({ name: "uuid", example: "me", description: 'A valid user UUID or "me"' }) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts new file mode 100644 index 000000000..024dab0de --- /dev/null +++ b/apps/user-service/src/users/users.service.ts @@ -0,0 +1,70 @@ +import { BadRequestException, Injectable } from "@nestjs/common"; +import { Op } from "sequelize"; +import { User } from "@terramatch-microservices/database/entities"; +import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; +import { UserQueryDto } from "./dto/user-query.dto"; +import { UserDto } from "@terramatch-microservices/common/dto"; +import { DocumentBuilder } from "@terramatch-microservices/common/util"; + +@Injectable() +export class UsersService { + async findMany(query: UserQueryDto) { + const includes = [ + { + association: "organisation", + attributes: ["id", "uuid", "name"] + } + ]; + + const builder = PaginatedQueryBuilder.forNumberPage(User, query.page, includes); + + if (query.search != null && query.search.trim() !== "") { + const search = `%${query.search.trim()}%`; + builder.where({ + [Op.or]: [ + { emailAddress: { [Op.like]: search } }, + { firstName: { [Op.like]: search } }, + { lastName: { [Op.like]: search } }, + { "$organisation.name$": { [Op.like]: search } } + ] + }); + } + + if (query.isVerified != null) { + builder.where({ + emailAddressVerifiedAt: { + [query.isVerified ? Op.ne : Op.eq]: null + } + }); + } + + if (query.sort?.field != null) { + const sortField = query.sort.field; + const direction = query.sort.direction ?? "DESC"; + + const directFields = ["createdAt", "firstName", "lastName", "emailAddress", "lastLoggedInAt"]; + + if (directFields.includes(sortField)) { + builder.order([sortField, direction]); + } else if (sortField === "organisationName") { + builder.order(["organisation", "name", direction]); + } else if (sortField !== "id") { + throw new BadRequestException(`Invalid sort field: ${query.sort.field}`); + } + } else { + builder.order(["createdAt", "DESC"]); + } + + return { + users: await builder.execute(), + paginationTotal: await builder.paginationTotal() + }; + } + + async addUsersToDocument(document: DocumentBuilder, users: User[]) { + for (const user of users) { + document.addData(user.uuid ?? "no-uuid", new UserDto(user, [])); + } + return document; + } +} diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index 0efc33342..9cf45fc05 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -17,6 +17,7 @@ export class UserDto { constructor(user: User, frameworks: Framework[]) { populateDto>(this, user, { uuid: user.uuid ?? "", + organisationName: user.organisation?.name ?? null, frameworks: frameworks .filter(({ slug }) => slug != null) .map(({ name, slug }) => ({ name, slug })) as UserFramework[] @@ -48,6 +49,15 @@ export class UserDto { @ApiProperty({ nullable: true, type: Date }) emailAddressVerifiedAt: Date | null; + @ApiProperty({ nullable: true, type: String, description: "Name of the user's primary organisation, if any." }) + organisationName: string | null; + + @ApiProperty({ type: Date }) + createdAt: Date; + + @ApiProperty({ nullable: true, type: Date }) + lastLoggedInAt: Date | null; + @ApiProperty({ nullable: true, type: String }) locale: string | null; From 27c35e1a66128972b5222be4a15c071c1b50f70b Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 12 Mar 2026 23:29:32 -0400 Subject: [PATCH 02/20] [TM-3071] add missing fields on response --- apps/user-service/src/users/users.service.ts | 4 ++++ libs/common/src/lib/dto/user.dto.ts | 3 +++ 2 files changed, 7 insertions(+) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 024dab0de..d3c3de8c5 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -13,6 +13,10 @@ export class UsersService { { association: "organisation", attributes: ["id", "uuid", "name"] + }, + { + association: "roles", + attributes: ["name"] } ]; diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index 9cf45fc05..81ac01b15 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -58,6 +58,9 @@ export class UserDto { @ApiProperty({ nullable: true, type: Date }) lastLoggedInAt: Date | null; + @ApiProperty({ nullable: true, type: String }) + jobRole: string | null; + @ApiProperty({ nullable: true, type: String }) locale: string | null; From 67c060712338611ea2f750d7d0e7b7d5d05dd1f9 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Sun, 15 Mar 2026 23:14:21 -0400 Subject: [PATCH 03/20] [TM-3071] add conditional login create user --- .../src/users/dto/admin-user-create.dto.ts | 18 +++++ .../src/users/dto/user-create.dto.ts | 26 ++++--- .../src/users/user-creation.service.ts | 70 +++++++++++++++++-- .../src/users/users.controller.ts | 14 ++-- libs/common/src/lib/guards/auth.guard.ts | 18 ++++- libs/common/src/lib/guards/index.ts | 2 +- libs/common/src/lib/policies/user.policy.ts | 3 +- 7 files changed, 124 insertions(+), 27 deletions(-) create mode 100644 apps/user-service/src/users/dto/admin-user-create.dto.ts diff --git a/apps/user-service/src/users/dto/admin-user-create.dto.ts b/apps/user-service/src/users/dto/admin-user-create.dto.ts new file mode 100644 index 000000000..873b312a3 --- /dev/null +++ b/apps/user-service/src/users/dto/admin-user-create.dto.ts @@ -0,0 +1,18 @@ +import { CreateDataDto, JsonApiBodyDto } from "@terramatch-microservices/common/util/json-api-update-dto"; +import { IsNotEmpty } from "class-validator"; +import { ApiProperty } from "@nestjs/swagger"; +import { UserCreateBaseAttributes } from "./user-create.dto"; + +export class AdminUserCreateAttributes extends UserCreateBaseAttributes { + @IsNotEmpty() + @ApiProperty() + role: string; + + @IsNotEmpty() + @ApiProperty() + organisationUuid: string; +} + +export class AdminUserCreateBody extends JsonApiBodyDto( + class AdminUserCreateData extends CreateDataDto("users", AdminUserCreateAttributes) {} +) {} diff --git a/apps/user-service/src/users/dto/user-create.dto.ts b/apps/user-service/src/users/dto/user-create.dto.ts index d8140cc00..dc967ac38 100644 --- a/apps/user-service/src/users/dto/user-create.dto.ts +++ b/apps/user-service/src/users/dto/user-create.dto.ts @@ -2,7 +2,7 @@ import { IsEmail, IsIn, IsNotEmpty, IsOptional } from "class-validator"; import { ApiProperty } from "@nestjs/swagger"; import { CreateDataDto, JsonApiBodyDto } from "@terramatch-microservices/common/util/json-api-update-dto"; -export class UserCreateAttributes { +export class UserCreateBaseAttributes { @IsNotEmpty() @ApiProperty() firstName: string; @@ -11,10 +11,6 @@ export class UserCreateAttributes { @ApiProperty() lastName: string; - @IsNotEmpty() - @ApiProperty() - password: string; - @IsEmail() @ApiProperty() emailAddress: string; @@ -27,11 +23,6 @@ export class UserCreateAttributes { @ApiProperty() jobRole: string; - @IsNotEmpty() - @IsIn(["project-developer", "funder", "government"]) - @ApiProperty() - role: string; - @IsOptional() @ApiProperty() country: string; @@ -39,6 +30,21 @@ export class UserCreateAttributes { @IsOptional() @ApiProperty() program: string; +} + +export class UserCreateBaseBody extends JsonApiBodyDto( + class UserCreateBaseData extends CreateDataDto("users", UserCreateBaseAttributes) {} +) {} + +export class UserCreateAttributes extends UserCreateBaseAttributes { + @IsNotEmpty() + @ApiProperty() + password: string; + + @IsNotEmpty() + @IsIn(["project-developer", "funder", "government"]) + @ApiProperty() + role: string; @IsNotEmpty() @ApiProperty() diff --git a/apps/user-service/src/users/user-creation.service.ts b/apps/user-service/src/users/user-creation.service.ts index 8c5f1d473..7c829c50f 100644 --- a/apps/user-service/src/users/user-creation.service.ts +++ b/apps/user-service/src/users/user-creation.service.ts @@ -1,16 +1,20 @@ import { + BadRequestException, Injectable, InternalServerErrorException, NotFoundException, UnprocessableEntityException } from "@nestjs/common"; -import { ModelHasRole, Role, User, Verification } from "@terramatch-microservices/database/entities"; +import { ModelHasRole, Organisation, Role, User, Verification } from "@terramatch-microservices/database/entities"; import { EmailService } from "@terramatch-microservices/common/email/email.service"; -import { UserCreateAttributes } from "./dto/user-create.dto"; +import { UserCreateAttributes, UserCreateBaseAttributes } from "./dto/user-create.dto"; import crypto from "node:crypto"; import { omit } from "lodash"; import bcrypt from "bcryptjs"; +import { validate } from "class-validator"; import { TMLogger } from "@terramatch-microservices/common/util/tm-logger"; +import { AdminUserCreateAttributes } from "./dto/admin-user-create.dto"; +import { plainToInstance } from "class-transformer"; const EMAIL_KEYS = { body: "user-verification.body", @@ -26,8 +30,21 @@ export class UserCreationService { constructor(private readonly emailService: EmailService) {} - async createNewUser(request: UserCreateAttributes): Promise { - const role = request.role; + async createNewUser(isAuthenticated: boolean, request: UserCreateBaseAttributes): Promise { + if (isAuthenticated) { + return await this.adminUserCreateProcess(request as AdminUserCreateAttributes); + } + return await this.signUpProcess(request as UserCreateAttributes); + } + + private async signUpProcess(request: UserCreateAttributes): Promise { + const dto = plainToInstance(UserCreateAttributes, request); + const errors = await validate(dto); + if (errors.length > 0) { + throw new BadRequestException(errors); + } + const userCreateRequest = request as UserCreateAttributes; + const role = userCreateRequest.role; if (!this.roles.includes(role)) { throw new UnprocessableEntityException("Role not valid"); } @@ -43,9 +60,9 @@ export class UserCreationService { } try { - const hashPassword = await bcrypt.hash(request.password, 10); - const callbackUrl = request.callbackUrl; - const newUser = omit(request, ["callbackUrl", "role", "password"]); + const hashPassword = await bcrypt.hash(userCreateRequest.password, 10); + const callbackUrl = userCreateRequest.callbackUrl; + const newUser = omit(userCreateRequest, ["callbackUrl", "role", "password"]); const user = await User.create({ ...newUser, password: hashPassword } as User); @@ -66,6 +83,45 @@ export class UserCreationService { } } + private async adminUserCreateProcess(request: AdminUserCreateAttributes): Promise { + const dto = plainToInstance(AdminUserCreateAttributes, request); + const errors = await validate(dto); + if (errors.length > 0) { + throw new BadRequestException(errors); + } + const adminUserCreateRequest = request as AdminUserCreateAttributes; + const organisation = await Organisation.findOne({ + where: { uuid: adminUserCreateRequest.organisationUuid }, + attributes: ["id"] + }); + if (organisation == null) { + throw new NotFoundException("Organisation not found"); + } + const userExists = (await User.count({ where: { emailAddress: request.emailAddress } })) !== 0; + if (userExists) { + throw new UnprocessableEntityException("User already exists"); + } + try { + const newUser = omit(adminUserCreateRequest, ["role", "organisationUuid"]); + const user = await User.create({ ...newUser, organisationId: organisation.id } as User); + const role = adminUserCreateRequest.role; + const roleEntity = await Role.findOne({ where: { name: role } }); + if (roleEntity == null) { + throw new NotFoundException("Role not found"); + } + + await ModelHasRole.findOrCreate({ + where: { modelId: user.id, roleId: roleEntity.id }, + defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole + }); + + return user; + } catch (error) { + this.logger.error(error); + throw new InternalServerErrorException("User creation failed"); + } + } + private async sendEmailVerification({ emailAddress, locale }: User, token: string, callbackUrl: string) { await this.emailService.sendI18nTemplateEmail(emailAddress, locale, EMAIL_KEYS, { additionalValues: { link: `${callbackUrl}/${token}`, monitoring: "monitoring" } diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index 18be9cb51..2433e6bde 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -17,8 +17,8 @@ import { OrganisationLightDto, UserDto } from "@terramatch-microservices/common/ import { ExceptionResponse, JsonApiResponse } from "@terramatch-microservices/common/decorators"; import { buildJsonApi, DocumentBuilder, getStableRequestQuery } from "@terramatch-microservices/common/util"; import { UserUpdateBody } from "./dto/user-update.dto"; -import { NoBearerAuth } from "@terramatch-microservices/common/guards"; -import { UserCreateBody } from "./dto/user-create.dto"; +import { OptionalBearerAuth } from "@terramatch-microservices/common/guards"; +import { UserCreateBaseBody } from "./dto/user-create.dto"; import { UserCreationService } from "./user-creation.service"; import { UserQueryDto } from "./dto/user-query.dto"; import { UsersService } from "./users.service"; @@ -120,15 +120,19 @@ export class UsersController { } @Post() - @NoBearerAuth + @OptionalBearerAuth @ApiOperation({ operationId: "userCreation", description: "Create a new user" }) @JsonApiResponse(USER_RESPONSE_SHAPE) @ExceptionResponse(UnauthorizedException, { description: "user creation failed." }) - async create(@Body() payload: UserCreateBody) { - const user = await this.userCreationService.createNewUser(payload.data.attributes); + async create(@Body() payload: UserCreateBaseBody) { + const isAuthenticated = authenticatedUserId() != null; + if (isAuthenticated) { + await this.policyService.authorize("create", User); + } + const user = await this.userCreationService.createNewUser(isAuthenticated, payload.data.attributes); return await this.addUserResource(buildJsonApi(UserDto), user); } diff --git a/libs/common/src/lib/guards/auth.guard.ts b/libs/common/src/lib/guards/auth.guard.ts index 9f0e9f677..b8505732d 100644 --- a/libs/common/src/lib/guards/auth.guard.ts +++ b/libs/common/src/lib/guards/auth.guard.ts @@ -7,6 +7,9 @@ import { RequestContext } from "nestjs-request-context"; const NO_BEARER_AUTH = "noBearerAuth"; export const NoBearerAuth = SetMetadata(NO_BEARER_AUTH, true); +const OPTIONAL_BEARER_AUTH = "optionalBearerAuth"; +export const OptionalBearerAuth = SetMetadata(OPTIONAL_BEARER_AUTH, true); + export const authenticatedUserId = () => RequestContext.currentContext?.req?.authenticatedUserId as number | undefined; @Injectable() @@ -20,12 +23,23 @@ export class AuthGuard implements CanActivate { ]); if (skipAuth) return true; + const optionalAuth = this.reflector.getAllAndOverride(OPTIONAL_BEARER_AUTH, [ + context.getHandler(), + context.getClass() + ]); + const request = context.switchToHttp().getRequest(); const [type, token] = request.headers.authorization?.split(" ") ?? []; - if (type !== "Bearer" || token == null) throw new UnauthorizedException(); + if (type !== "Bearer" || token == null) { + if (optionalAuth) return true; + throw new UnauthorizedException(); + } const userId = this.isJwtToken(token) ? await this.getJwtUserId(token) : await this.getApiKeyUserId(token); - if (userId == null) throw new UnauthorizedException(); + if (userId == null) { + if (optionalAuth) return true; + throw new UnauthorizedException(); + } // Most requests won't need the actual user object; instead, the roles and permissions // are fetched from other (smaller) tables, and only the user id is needed. diff --git a/libs/common/src/lib/guards/index.ts b/libs/common/src/lib/guards/index.ts index c3b34854c..c4bb23da4 100644 --- a/libs/common/src/lib/guards/index.ts +++ b/libs/common/src/lib/guards/index.ts @@ -1 +1 @@ -export { AuthGuard, NoBearerAuth } from "./auth.guard"; +export { AuthGuard, NoBearerAuth, OptionalBearerAuth } from "./auth.guard"; diff --git a/libs/common/src/lib/policies/user.policy.ts b/libs/common/src/lib/policies/user.policy.ts index 684647a27..fde88b524 100644 --- a/libs/common/src/lib/policies/user.policy.ts +++ b/libs/common/src/lib/policies/user.policy.ts @@ -4,8 +4,7 @@ import { UserPermissionsPolicy } from "./user-permissions.policy"; export class UserPolicy extends UserPermissionsPolicy { async addRules() { if (this.permissions.includes("users-manage")) { - this.builder.can("read", User); - this.builder.can("update", User); + this.builder.can(["read", "create", "update"], User); } else { this.builder.can("read", User, { id: this.userId }); this.builder.can("update", User, { id: this.userId }); From 7c2af91aa93a380b3526fa5ff5b9dcac12773540 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Mon, 16 Mar 2026 16:28:32 -0400 Subject: [PATCH 04/20] [TM-3071] add missing fields --- libs/common/src/lib/dto/user.dto.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index 81ac01b15..01916f996 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -18,6 +18,7 @@ export class UserDto { populateDto>(this, user, { uuid: user.uuid ?? "", organisationName: user.organisation?.name ?? null, + organisationUuid: user.organisation?.uuid ?? null, frameworks: frameworks .filter(({ slug }) => slug != null) .map(({ name, slug }) => ({ name, slug })) as UserFramework[] @@ -49,9 +50,15 @@ export class UserDto { @ApiProperty({ nullable: true, type: Date }) emailAddressVerifiedAt: Date | null; + @ApiProperty({ nullable: true, type: String }) + phoneNumber: string | null; + @ApiProperty({ nullable: true, type: String, description: "Name of the user's primary organisation, if any." }) organisationName: string | null; + @ApiProperty({ nullable: true, type: String, description: "UUID of the user's primary organisation, if any." }) + organisationUuid: string | null; + @ApiProperty({ type: Date }) createdAt: Date; @@ -61,6 +68,12 @@ export class UserDto { @ApiProperty({ nullable: true, type: String }) jobRole: string | null; + @ApiProperty({ nullable: true, type: String }) + country: string | null; + + @ApiProperty({ nullable: true, type: String }) + program: string | null; + @ApiProperty({ nullable: true, type: String }) locale: string | null; From 83ff0c13a7982c720a7d2897f48daf0cfbc739b0 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Fri, 20 Mar 2026 11:42:26 -0400 Subject: [PATCH 05/20] [TM-3071] update unit tests --- .../src/users/user-creation.service.spec.ts | 125 +++++++++++- .../src/users/users.controller.spec.ts | 50 +++++ .../src/users/users.service.spec.ts | 192 ++++++++++++++++++ 3 files changed, 361 insertions(+), 6 deletions(-) create mode 100644 apps/user-service/src/users/users.service.spec.ts diff --git a/apps/user-service/src/users/user-creation.service.spec.ts b/apps/user-service/src/users/user-creation.service.spec.ts index 48a68ff53..b6f234ab8 100644 --- a/apps/user-service/src/users/user-creation.service.spec.ts +++ b/apps/user-service/src/users/user-creation.service.spec.ts @@ -3,22 +3,29 @@ import { createMock, DeepMocked } from "@golevelup/ts-jest"; import { LocalizationKey, ModelHasRole, - Role, - User, - Verification, - PasswordReset, + Organisation, OrganisationInvite, + PasswordReset, ProjectInvite, - ProjectUser + ProjectUser, + Role, + User, + Verification } from "@terramatch-microservices/database/entities"; import { EmailService } from "@terramatch-microservices/common/email/email.service"; import { LocalizationService } from "@terramatch-microservices/common/localization/localization.service"; import { UserCreationService } from "./user-creation.service"; import { UserCreateAttributes } from "./dto/user-create.dto"; -import { InternalServerErrorException, NotFoundException, UnprocessableEntityException } from "@nestjs/common"; +import { + BadRequestException, + InternalServerErrorException, + NotFoundException, + UnprocessableEntityException +} from "@nestjs/common"; import { RoleFactory, UserFactory, ProjectFactory } from "@terramatch-microservices/database/factories"; import { LocalizationKeyFactory } from "@terramatch-microservices/database/factories/localization-key.factory"; import { TemplateService } from "@terramatch-microservices/common/templates/template.service"; +import { AdminUserCreateAttributes } from "./dto/admin-user-create.dto"; describe("UserCreationService", () => { let service: UserCreationService; @@ -412,5 +419,111 @@ describe("UserCreationService", () => { expect(Role.findOne).toHaveBeenCalledWith({ where: { name: "project-developer" } }); }); + + it("should update existing project-user relation if already present", async () => { + const user = await UserFactory.create(); + const project = await ProjectFactory.create(); + const request = getInviteRequest("new@example.com", "valid-token"); + const passwordReset = { + user, + destroy: jest.fn().mockResolvedValue(undefined) + } as unknown as PasswordReset; + const projectInvite = { + emailAddress: user.emailAddress, + project, + acceptedAt: null, + save: jest.fn().mockResolvedValue(undefined) + } as unknown as ProjectInvite; + const role = RoleFactory.create({ name: "project-developer" }); + const projectUser = { + isMonitoring: false, + status: "inactive", + save: jest.fn().mockResolvedValue(undefined) + } as unknown as ProjectUser; + + jest.spyOn(PasswordReset, "findOne").mockResolvedValue(passwordReset); + jest.spyOn(OrganisationInvite, "findAll").mockResolvedValue([]); + jest.spyOn(ProjectInvite, "findAll").mockResolvedValue([projectInvite]); + jest.spyOn(ProjectUser, "findOrCreate").mockResolvedValue([projectUser, false]); + jest.spyOn(Role, "findOne").mockResolvedValue(role); + jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); + jest.spyOn(user, "save").mockResolvedValue(user); + + await service.createNewUser(false, request); + + expect(projectUser.isMonitoring).toBe(true); + expect(projectUser.status).toBe("active"); + expect(projectUser.save).toHaveBeenCalled(); + }); + }); + + describe("authenticated/admin user creation", () => { + const getAdminRequest = (email: string, role: string) => { + const request = new AdminUserCreateAttributes(); + request.emailAddress = email; + request.firstName = "firstName"; + request.lastName = "lastName"; + request.jobRole = "developer"; + request.phoneNumber = "1234567890"; + request.role = role; + request.organisationUuid = "org-uuid"; + return request; + }; + + it("should create user for authenticated request", async () => { + const request = getAdminRequest("admin-created@example.com", "project-manager"); + const role = RoleFactory.create({ name: request.role }); + const createdUser = await UserFactory.create(); + const organisation = { id: 999 } as Organisation; + + jest.spyOn(Organisation, "findOne").mockResolvedValue(organisation); + jest.spyOn(User, "count").mockResolvedValue(0); + jest.spyOn(User, "create").mockResolvedValue(createdUser); + jest.spyOn(Role, "findOne").mockResolvedValue(role); + jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); + + const result = await service.createNewUser(true, request); + + expect(Organisation.findOne).toHaveBeenCalledWith({ + where: { uuid: request.organisationUuid }, + attributes: ["id"] + }); + expect(User.create).toHaveBeenCalledWith(expect.objectContaining({ organisationId: organisation.id })); + expect(result).toBe(createdUser); + }); + + it("should fail when organisation does not exist", async () => { + const request = getAdminRequest("admin-created@example.com", "project-manager"); + + jest.spyOn(Organisation, "findOne").mockResolvedValue(null); + + await expect(service.createNewUser(true, request)).rejects.toThrow( + new NotFoundException("Organisation not found") + ); + }); + + it("should fail when admin role does not exist", async () => { + const request = getAdminRequest("admin-created@example.com", "project-manager"); + const createdUser = await UserFactory.create(); + const organisation = { id: 888 } as Organisation; + + jest.spyOn(Organisation, "findOne").mockResolvedValue(organisation); + jest.spyOn(User, "count").mockResolvedValue(0); + jest.spyOn(User, "create").mockResolvedValue(createdUser); + jest.spyOn(Role, "findOne").mockResolvedValue(null); + + await expect(service.createNewUser(true, request)).rejects.toThrow( + new InternalServerErrorException("User creation failed") + ); + }); + + it("should fail validation for malformed admin payload", async () => { + const request = getAdminRequest("invalid-email", "project-manager"); + request.organisationUuid = ""; + // @ts-expect-error test invalid payload + request.phoneNumber = null; + + await expect(service.createNewUser(true, request)).rejects.toThrow(BadRequestException); + }); }); }); diff --git a/apps/user-service/src/users/users.controller.spec.ts b/apps/user-service/src/users/users.controller.spec.ts index a34895283..088dd09ab 100644 --- a/apps/user-service/src/users/users.controller.spec.ts +++ b/apps/user-service/src/users/users.controller.spec.ts @@ -21,6 +21,7 @@ describe("UsersController", () => { let controller: UsersController; let policyService: DeepMocked; let userCreationService: DeepMocked; + // eslint-disable-next-line @typescript-eslint/no-unused-vars let usersService: DeepMocked; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -139,6 +140,31 @@ describe("UsersController", () => { }); }); + describe("userIndex", () => { + it("authorizes and enriches the document when users are returned", async () => { + const user = await UserFactory.create(); + const query = { page: { number: 2 } }; + usersService.findMany.mockResolvedValue({ users: [user], paginationTotal: 1 }); + usersService.addUsersToDocument.mockImplementation(async document => document); + + await controller.userIndex(query); + + expect(usersService.findMany).toHaveBeenCalledWith(query); + expect(policyService.authorize).toHaveBeenCalledWith("read", [user]); + expect(usersService.addUsersToDocument).toHaveBeenCalledWith(expect.anything(), [user]); + }); + + it("does not authorize when no users are returned", async () => { + usersService.findMany.mockResolvedValue({ users: [], paginationTotal: 0 }); + usersService.addUsersToDocument.mockImplementation(async document => document); + + await controller.userIndex({}); + + expect(policyService.authorize).not.toHaveBeenCalled(); + expect(usersService.addUsersToDocument).toHaveBeenCalledWith(expect.anything(), []); + }); + }); + describe("update", () => { const makeValidBody = (uuid: string, locale?: ValidLocale) => ({ data: { @@ -178,6 +204,30 @@ describe("UsersController", () => { }); describe("create", () => { + it("authorizes creation when request is authenticated", async () => { + const user = await UserFactory.create(); + const attributes = new UserCreateAttributes(); + mockUserId(1); + userCreationService.createNewUser.mockResolvedValue(user); + + await controller.create(createRequest(attributes)); + + expect(policyService.authorize).toHaveBeenCalledWith("create", User); + expect(userCreationService.createNewUser).toHaveBeenCalledWith(true, attributes); + }); + + it("does not authorize creation when request is unauthenticated", async () => { + const user = await UserFactory.create(); + const attributes = new UserCreateAttributes(); + mockUserId(); + userCreationService.createNewUser.mockResolvedValue(user); + + await controller.create(createRequest(attributes)); + + expect(policyService.authorize).not.toHaveBeenCalled(); + expect(userCreationService.createNewUser).toHaveBeenCalledWith(false, attributes); + }); + it("should create a new user", async () => { const user = await UserFactory.create(); const attributes = new UserCreateAttributes(); diff --git a/apps/user-service/src/users/users.service.spec.ts b/apps/user-service/src/users/users.service.spec.ts new file mode 100644 index 000000000..db5a1069d --- /dev/null +++ b/apps/user-service/src/users/users.service.spec.ts @@ -0,0 +1,192 @@ +import { BadRequestException } from "@nestjs/common"; +import { Op } from "sequelize"; +import { UsersService } from "./users.service"; +import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; +import { User } from "@terramatch-microservices/database/entities"; +import { UserQueryDto } from "./dto/user-query.dto"; +import { DocumentBuilder } from "@terramatch-microservices/common/util"; + +describe("UsersService", () => { + let service: UsersService; + + const createBuilderMock = () => ({ + where: jest.fn(), + order: jest.fn(), + execute: jest.fn().mockResolvedValue([{ id: 1 }]), + paginationTotal: jest.fn().mockResolvedValue(42) + }); + + beforeEach(() => { + service = new UsersService(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe("findMany", () => { + it("should return users and pagination total", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + const query = { page: 1 } as UserQueryDto; + const result = await service.findMany(query); + + expect(PaginatedQueryBuilder.forNumberPage).toHaveBeenCalledWith( + User, + query.page, + expect.arrayContaining([ + expect.objectContaining({ association: "organisation" }), + expect.objectContaining({ association: "roles" }) + ]) + ); + expect(builder.order).toHaveBeenCalledWith(["createdAt", "DESC"]); + expect(result).toEqual({ + users: [{ id: 1 }], + paginationTotal: 42 + }); + }); + + it("should apply search filter when search is provided", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, search: " john " } as UserQueryDto); + + expect(builder.where).toHaveBeenCalledWith({ + [Op.or]: [ + { emailAddress: { [Op.like]: "%john%" } }, + { firstName: { [Op.like]: "%john%" } }, + { lastName: { [Op.like]: "%john%" } }, + { "$organisation.name$": { [Op.like]: "%john%" } } + ] + }); + }); + + it("should ignore blank search values", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, search: " " } as UserQueryDto); + + expect(builder.where).not.toHaveBeenCalledWith( + expect.objectContaining({ + [Op.or]: expect.anything() + }) + ); + }); + + it("should filter verified users when isVerified is true", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, isVerified: true } as UserQueryDto); + + expect(builder.where).toHaveBeenCalledWith({ + emailAddressVerifiedAt: { + [Op.ne]: null + } + }); + }); + + it("should filter unverified users when isVerified is false", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ page: 1, isVerified: false } as UserQueryDto); + + expect(builder.where).toHaveBeenCalledWith({ + emailAddressVerifiedAt: { + [Op.eq]: null + } + }); + }); + + it("should sort by direct fields", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "firstName", direction: "ASC" } + } as UserQueryDto); + + expect(builder.order).toHaveBeenCalledWith(["firstName", "ASC"]); + }); + + it("should sort by organisation name", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "organisationName", direction: "DESC" } + } as UserQueryDto); + + expect(builder.order).toHaveBeenCalledWith(["organisation", "name", "DESC"]); + }); + + it("should default sort direction to DESC when direction is missing", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "emailAddress" } + } as UserQueryDto); + + expect(builder.order).toHaveBeenCalledWith(["emailAddress", "DESC"]); + }); + + it("should allow id sort field without adding explicit order", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await service.findMany({ + page: 1, + sort: { field: "id", direction: "ASC" } + } as UserQueryDto); + + expect(builder.order).not.toHaveBeenCalled(); + }); + + it("should throw for invalid sort field", async () => { + const builder = createBuilderMock(); + jest.spyOn(PaginatedQueryBuilder, "forNumberPage").mockReturnValue(builder as never); + + await expect( + service.findMany({ + page: 1, + sort: { field: "invalidField", direction: "ASC" } + } as UserQueryDto) + ).rejects.toThrow(new BadRequestException("Invalid sort field: invalidField")); + }); + }); + + describe("addUsersToDocument", () => { + it("should add each user with uuid key", async () => { + const document = { + addData: jest.fn() + } as unknown as DocumentBuilder; + + const users = [{ uuid: "user-1" } as User, { uuid: "user-2" } as User]; + + const result = await service.addUsersToDocument(document, users); + + expect(document.addData).toHaveBeenNthCalledWith(1, "user-1", expect.anything()); + expect(document.addData).toHaveBeenNthCalledWith(2, "user-2", expect.anything()); + expect(result).toBe(document); + }); + + it('should use "no-uuid" when user uuid is missing', async () => { + const document = { + addData: jest.fn() + } as unknown as DocumentBuilder; + + const users = [{ uuid: null } as unknown as User]; + await service.addUsersToDocument(document, users); + + expect(document.addData).toHaveBeenCalledWith("no-uuid", expect.anything()); + }); + }); +}); From 1a12cb7e337dd64a859396644f00ffa30b23ca66 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Sun, 22 Mar 2026 15:59:41 -0400 Subject: [PATCH 06/20] [TM-3071] add delete user method --- .../src/users/dto/user-update.dto.ts | 26 ++++++++++++- .../src/users/users.controller.spec.ts | 33 ++++++++++++++++ .../src/users/users.controller.ts | 36 ++++++++++++++---- apps/user-service/src/users/users.service.ts | 38 +++++++++++++++++++ .../src/lib/policies/user.policy.spec.ts | 23 +++++++++++ libs/common/src/lib/policies/user.policy.ts | 9 ++--- 6 files changed, 150 insertions(+), 15 deletions(-) diff --git a/apps/user-service/src/users/dto/user-update.dto.ts b/apps/user-service/src/users/dto/user-update.dto.ts index 78ebcf8e0..de47a6842 100644 --- a/apps/user-service/src/users/dto/user-update.dto.ts +++ b/apps/user-service/src/users/dto/user-update.dto.ts @@ -3,7 +3,31 @@ import { ApiProperty } from "@nestjs/swagger"; import { JsonApiBodyDto, JsonApiDataDto } from "@terramatch-microservices/common/util/json-api-update-dto"; import { VALID_LOCALES, ValidLocale } from "@terramatch-microservices/database/constants/locale"; -class UserUpdateAttributes { +export class UserUpdateAttributes { + @ApiProperty({ description: "Organisation UUID", nullable: true, required: false, format: "uuid" }) + organisationUuid?: string | null; + + @ApiProperty({ description: "First name", nullable: true, required: false }) + firstName?: string | null; + + @ApiProperty({ description: "Last name", nullable: true, required: false }) + lastName?: string | null; + + @ApiProperty({ description: "Email address", nullable: true, required: false, format: "email" }) + emailAddress?: string | null; + + @ApiProperty({ description: "Job role", nullable: true, required: false }) + jobRole?: string | null; + + @ApiProperty({ description: "Phone number", nullable: true, required: false }) + phoneNumber?: string | null; + + @ApiProperty({ description: "Country", nullable: true, required: false }) + country?: string | null; + + @ApiProperty({ description: "Program", nullable: true, required: false }) + program?: string | null; + @IsEnum(VALID_LOCALES) @ApiProperty({ description: "New default locale for the given user", nullable: true, enum: VALID_LOCALES }) locale?: ValidLocale | null; diff --git a/apps/user-service/src/users/users.controller.spec.ts b/apps/user-service/src/users/users.controller.spec.ts index 088dd09ab..c51bc2d71 100644 --- a/apps/user-service/src/users/users.controller.spec.ts +++ b/apps/user-service/src/users/users.controller.spec.ts @@ -140,6 +140,39 @@ describe("UsersController", () => { }); }); + describe("delete", () => { + it("should throw not found if the user is not found", async () => { + await expect(controller.delete({ uuid: "00000000-0000-4000-8000-000000000000" })).rejects.toThrow( + NotFoundException + ); + }); + + it("should throw unauthorized if policy does not authorize", async () => { + const { uuid } = await UserFactory.create(); + policyService.authorize.mockRejectedValue(new UnauthorizedException()); + + await expect(controller.delete({ uuid: uuid! })).rejects.toThrow(UnauthorizedException); + }); + + it("should soft-delete the user and return a JSON:API deleted document", async () => { + const user = await UserFactory.create(); + policyService.authorize.mockResolvedValue(undefined); + usersService.delete.mockImplementation(async u => { + await u.destroy(); + }); + + const result = serialize(await controller.delete({ uuid: user.uuid! })); + + expect(policyService.authorize).toHaveBeenCalledWith("delete", expect.objectContaining({ id: user.id })); + expect(usersService.delete).toHaveBeenCalledWith(expect.objectContaining({ id: user.id })); + expect(result.meta.resourceType).toBe("users"); + expect(result.meta.resourceIds).toEqual([user.uuid]); + + const reloaded = await User.findOne({ where: { id: user.id }, paranoid: false }); + expect(reloaded?.deletedAt).not.toBeNull(); + }); + }); + describe("userIndex", () => { it("authorizes and enriches the document when users are returned", async () => { const user = await UserFactory.create(); diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index cb291de40..72871edf2 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -2,6 +2,7 @@ import { BadRequestException, Body, Controller, + Delete, Get, HttpCode, HttpStatus, @@ -16,8 +17,16 @@ import { User } from "@terramatch-microservices/database/entities"; import { PolicyService } from "@terramatch-microservices/common"; import { ApiOperation, ApiParam } from "@nestjs/swagger"; import { OrganisationLightDto, UserDto } from "@terramatch-microservices/common/dto"; +import { SingleResourceDto } from "@terramatch-microservices/common/dto/single-resource.dto"; import { ExceptionResponse, JsonApiResponse } from "@terramatch-microservices/common/decorators"; -import { buildJsonApi, DocumentBuilder, getStableRequestQuery } from "@terramatch-microservices/common/util"; +import { JsonApiDeletedResponse } from "@terramatch-microservices/common/decorators/json-api-response.decorator"; +import { + buildDeletedResponse, + buildJsonApi, + DocumentBuilder, + getDtoType, + getStableRequestQuery +} from "@terramatch-microservices/common/util"; import { UserUpdateBody } from "./dto/user-update.dto"; import { OptionalBearerAuth } from "@terramatch-microservices/common/guards"; import { UserCreateBaseBody } from "./dto/user-create.dto"; @@ -111,14 +120,25 @@ export class UsersController { await this.policyService.authorize("update", user); - // The only thing allowed to update for now is the locale - const { locale } = updatePayload.data.attributes; - if (locale != null) { - user.locale = locale; - await user.save(); - } + const updatedUser = await this.usersService.update(user, updatePayload.data.attributes); - return await this.addUserResource(buildJsonApi(UserDto), user); + return await this.addUserResource(buildJsonApi(UserDto), updatedUser as User); + } + + @Delete(":uuid") + @ApiOperation({ operationId: "userDelete", summary: "Delete a user by UUID" }) + @JsonApiDeletedResponse(getDtoType(UserDto), { description: "User was deleted" }) + @ExceptionResponse(NotFoundException, { description: "User with that UUID not found" }) + @ExceptionResponse(UnauthorizedException, { description: "User is not authorized to delete this user" }) + async delete(@Param() { uuid }: SingleResourceDto) { + const user = await User.findOne({ where: { uuid }, attributes: ["id", "uuid"] }); + if (user == null) throw new NotFoundException(); + + await this.policyService.authorize("delete", user); + + await this.usersService.delete(user); + + return buildDeletedResponse(getDtoType(UserDto), uuid); } @Post() diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index d3c3de8c5..7fda4714d 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -5,6 +5,9 @@ import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/pag import { UserQueryDto } from "./dto/user-query.dto"; import { UserDto } from "@terramatch-microservices/common/dto"; import { DocumentBuilder } from "@terramatch-microservices/common/util"; +import { UserUpdateAttributes } from "./dto/user-update.dto"; +import { ValidLocale } from "@terramatch-microservices/database/constants/locale"; +import { Organisation } from "@terramatch-microservices/database/entities/organisation.entity"; @Injectable() export class UsersService { @@ -71,4 +74,39 @@ export class UsersService { } return document; } + + async update(user: User, update: UserUpdateAttributes) { + user.organisationId = update.organisationUuid + ? (await Organisation.findOne({ where: { uuid: update.organisationUuid } }))?.id ?? null + : null; + if (update.firstName != null) { + user.firstName = update.firstName ?? null; + } + if (update.lastName != null) { + user.lastName = update.lastName ?? null; + } + if (update.emailAddress != null) { + user.emailAddress = update.emailAddress ?? null; + } + if (update.jobRole != null) { + user.jobRole = update.jobRole ?? null; + } + if (update.phoneNumber != null) { + user.phoneNumber = update.phoneNumber ?? null; + } + if (update.country != null) { + user.country = update.country ?? null; + } + if (update.program != null) { + user.program = update.program ?? null; + } + if (update.locale != null) { + user.locale = update.locale as ValidLocale; + } + return await user.save(); + } + + async delete(user: User): Promise { + await user.destroy(); + } } diff --git a/libs/common/src/lib/policies/user.policy.spec.ts b/libs/common/src/lib/policies/user.policy.spec.ts index 890dc5c4a..8e787c942 100644 --- a/libs/common/src/lib/policies/user.policy.spec.ts +++ b/libs/common/src/lib/policies/user.policy.spec.ts @@ -78,4 +78,27 @@ describe("UserPolicy", () => { await expectCan(service, "verify", new User()); }); + + it("allows deleting any user as users-manage", async () => { + mockPermissions("users-manage"); + await expectCan(service, "delete", new User()); + }); + + it("disallows deleting users as non-admin", async () => { + mockPermissions(); + await expectCannot(service, "delete", new User()); + await expectCannot(service, "delete", await UserFactory.build({ id: 123 })); + }); + + it("allows deleting any user for verified admin without users-manage", async () => { + mockPermissions(); + + const verifiedAdminUser = new User(); + (verifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedAdminUser); + + await expectCan(service, "delete", new User()); + }); }); diff --git a/libs/common/src/lib/policies/user.policy.ts b/libs/common/src/lib/policies/user.policy.ts index df68d7f59..c233f01c8 100644 --- a/libs/common/src/lib/policies/user.policy.ts +++ b/libs/common/src/lib/policies/user.policy.ts @@ -4,17 +4,14 @@ import { UserPermissionsPolicy } from "./user-permissions.policy"; export class UserPolicy extends UserPermissionsPolicy { async addRules() { if (this.permissions.includes("users-manage")) { - this.builder.can(["read", "create", "update", "verify"], User); - } else { - this.builder.can("read", User, { id: this.userId }); - this.builder.can("update", User, { id: this.userId }); + this.builder.can(["read", "readAll", "create", "update", "verify", "delete"], User); } if (await this.isVerifiedAdmin()) { - this.builder.can("verify", User); + this.builder.can(["readAll", "update", "verify", "delete"], User); } - this.builder.can("verify", User, { id: this.userId }); + this.builder.can(["read", "update", "verify"], User, { id: this.userId }); } protected _isVerifiedAdmin?: boolean; From 14b162a38c80a0c03d004df0512e5cbcec5105e5 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Sun, 22 Mar 2026 18:09:28 -0400 Subject: [PATCH 07/20] [TM-3071] update role and organisation update --- .../src/users/dto/user-update.dto.ts | 3 ++ apps/user-service/src/users/users.service.ts | 31 +++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/apps/user-service/src/users/dto/user-update.dto.ts b/apps/user-service/src/users/dto/user-update.dto.ts index de47a6842..ca9b1ed33 100644 --- a/apps/user-service/src/users/dto/user-update.dto.ts +++ b/apps/user-service/src/users/dto/user-update.dto.ts @@ -31,6 +31,9 @@ export class UserUpdateAttributes { @IsEnum(VALID_LOCALES) @ApiProperty({ description: "New default locale for the given user", nullable: true, enum: VALID_LOCALES }) locale?: ValidLocale | null; + + @ApiProperty() + primaryRole: string; } export class UserUpdateBody extends JsonApiBodyDto( diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 7fda4714d..a5c263466 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -1,6 +1,6 @@ -import { BadRequestException, Injectable } from "@nestjs/common"; +import { BadRequestException, Injectable, NotFoundException } from "@nestjs/common"; import { Op } from "sequelize"; -import { User } from "@terramatch-microservices/database/entities"; +import { ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; import { UserQueryDto } from "./dto/user-query.dto"; import { UserDto } from "@terramatch-microservices/common/dto"; @@ -76,9 +76,15 @@ export class UsersService { } async update(user: User, update: UserUpdateAttributes) { - user.organisationId = update.organisationUuid - ? (await Organisation.findOne({ where: { uuid: update.organisationUuid } }))?.id ?? null - : null; + if (update.organisationUuid != null) { + const organisationUuid = update.organisationUuid; + const organisation = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); + if (organisation == null) { + throw new NotFoundException("Organisation not found"); + } + user.organisationId = organisation.id; + console.log("organisation", organisation); + } if (update.firstName != null) { user.firstName = update.firstName ?? null; } @@ -103,7 +109,20 @@ export class UsersService { if (update.locale != null) { user.locale = update.locale as ValidLocale; } - return await user.save(); + user = await user.save(); + if (update.primaryRole != null) { + await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); + const roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); + if (roleEntity == null) { + throw new NotFoundException("Role not found"); + } + await ModelHasRole.findOrCreate({ + where: { modelId: user.id, roleId: roleEntity.id }, + defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole + }); + await user.reload(); + } + return user; } async delete(user: User): Promise { From 2026a573efbb60e1850e051d72a3257711edf26f Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Sun, 22 Mar 2026 22:50:41 -0400 Subject: [PATCH 08/20] [TM-3071] add missing update of direct frameworks --- .../organisations/organisations.controller.ts | 5 ++++- .../src/users/dto/admin-user-create.dto.ts | 7 +++++- .../src/users/dto/user-update.dto.ts | 7 +++++- .../src/users/user-creation.service.ts | 22 ++++++++++++++++--- .../src/users/users.controller.ts | 5 ++++- apps/user-service/src/users/users.service.ts | 16 ++++++++++++-- libs/common/src/lib/dto/user.dto.ts | 8 ++++++- 7 files changed, 60 insertions(+), 10 deletions(-) diff --git a/apps/user-service/src/organisations/organisations.controller.ts b/apps/user-service/src/organisations/organisations.controller.ts index d0e6c01a5..e85add1ef 100644 --- a/apps/user-service/src/organisations/organisations.controller.ts +++ b/apps/user-service/src/organisations/organisations.controller.ts @@ -215,7 +215,10 @@ export class OrganisationsController { const document = buildJsonApi(OrganisationLightDto); const orgResource = document.addData(organisation.uuid, new OrganisationLightDto(organisation)); if (user != null) { - const userResource = document.addData(user.uuid ?? "no-uuid", new UserDto(user, await user.myFrameworks())); + const userResource = document.addData( + user.uuid ?? "no-uuid", + new UserDto(user, user.frameworks, await user.myFrameworks()) + ); userResource.relateTo("org", orgResource, { meta: { userStatus: "na" } }); } return document; diff --git a/apps/user-service/src/users/dto/admin-user-create.dto.ts b/apps/user-service/src/users/dto/admin-user-create.dto.ts index 873b312a3..19bf610a5 100644 --- a/apps/user-service/src/users/dto/admin-user-create.dto.ts +++ b/apps/user-service/src/users/dto/admin-user-create.dto.ts @@ -1,5 +1,5 @@ import { CreateDataDto, JsonApiBodyDto } from "@terramatch-microservices/common/util/json-api-update-dto"; -import { IsNotEmpty } from "class-validator"; +import { IsArray, IsNotEmpty, IsString } from "class-validator"; import { ApiProperty } from "@nestjs/swagger"; import { UserCreateBaseAttributes } from "./user-create.dto"; @@ -11,6 +11,11 @@ export class AdminUserCreateAttributes extends UserCreateBaseAttributes { @IsNotEmpty() @ApiProperty() organisationUuid: string; + + @ApiProperty({ isArray: true, type: String }) + @IsArray() + @IsString({ each: true }) + directFrameworks: string[]; } export class AdminUserCreateBody extends JsonApiBodyDto( diff --git a/apps/user-service/src/users/dto/user-update.dto.ts b/apps/user-service/src/users/dto/user-update.dto.ts index ca9b1ed33..b0306743b 100644 --- a/apps/user-service/src/users/dto/user-update.dto.ts +++ b/apps/user-service/src/users/dto/user-update.dto.ts @@ -1,4 +1,4 @@ -import { IsEnum } from "class-validator"; +import { IsArray, IsEnum, IsString } from "class-validator"; import { ApiProperty } from "@nestjs/swagger"; import { JsonApiBodyDto, JsonApiDataDto } from "@terramatch-microservices/common/util/json-api-update-dto"; import { VALID_LOCALES, ValidLocale } from "@terramatch-microservices/database/constants/locale"; @@ -34,6 +34,11 @@ export class UserUpdateAttributes { @ApiProperty() primaryRole: string; + + @ApiProperty({ isArray: true, type: String }) + @IsArray() + @IsString({ each: true }) + directFrameworks: string[]; } export class UserUpdateBody extends JsonApiBodyDto( diff --git a/apps/user-service/src/users/user-creation.service.ts b/apps/user-service/src/users/user-creation.service.ts index dd483d8a0..d65beda98 100644 --- a/apps/user-service/src/users/user-creation.service.ts +++ b/apps/user-service/src/users/user-creation.service.ts @@ -14,7 +14,9 @@ import { Organisation, OrganisationInvite, ProjectInvite, - ProjectUser + ProjectUser, + FrameworkUser, + Framework } from "@terramatch-microservices/database/entities"; import { EmailService } from "@terramatch-microservices/common/email/email.service"; import { UserCreateAttributes, UserCreateBaseAttributes } from "./dto/user-create.dto"; @@ -119,7 +121,7 @@ export class UserCreationService { throw new UnprocessableEntityException("User already exists"); } try { - const newUser = omit(adminUserCreateRequest, ["role", "organisationUuid"]); + const newUser = omit(adminUserCreateRequest, ["role", "organisationUuid", "directFrameworks"]); const user = await User.create({ ...newUser, organisationId: organisation.id } as User); const role = adminUserCreateRequest.role; const roleEntity = await Role.findOne({ where: { name: role } }); @@ -132,7 +134,21 @@ export class UserCreationService { defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole }); - return user; + for (const framework of adminUserCreateRequest.directFrameworks ?? []) { + const frameworkEntity = await Framework.findOne({ where: { slug: framework } }); + if (frameworkEntity == null) { + throw new NotFoundException("Framework not found"); + } + await FrameworkUser.findOrCreate({ + where: { frameworkId: frameworkEntity.id, userId: user.id } + }); + } + + const reloadedUser = await user.reload({ + include: [{ association: "organisation", attributes: ["id", "uuid", "name"] }, { association: "frameworks" }] + }); + + return reloadedUser; } catch (error) { this.logger.error(error); throw new InternalServerErrorException("User creation failed"); diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index 72871edf2..a82985504 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -185,7 +185,10 @@ export class UsersController { } private async addUserResource(document: DocumentBuilder, user: User) { - const userResource = document.addData(user.uuid ?? "no-uuid", new UserDto(user, await user.myFrameworks())); + const userResource = document.addData( + user.uuid ?? "no-uuid", + new UserDto(user, user.frameworks ?? [], (await user.myFrameworks()) ?? []) + ); const org = await user.primaryOrganisation(); if (org != null) { diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index a5c263466..f91fdef58 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -1,6 +1,6 @@ import { BadRequestException, Injectable, NotFoundException } from "@nestjs/common"; import { Op } from "sequelize"; -import { ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; +import { Framework, FrameworkUser, ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; import { UserQueryDto } from "./dto/user-query.dto"; import { UserDto } from "@terramatch-microservices/common/dto"; @@ -70,7 +70,7 @@ export class UsersService { async addUsersToDocument(document: DocumentBuilder, users: User[]) { for (const user of users) { - document.addData(user.uuid ?? "no-uuid", new UserDto(user, [])); + document.addData(user.uuid ?? "no-uuid", new UserDto(user, [], [])); } return document; } @@ -109,6 +109,18 @@ export class UsersService { if (update.locale != null) { user.locale = update.locale as ValidLocale; } + if (update.directFrameworks != null) { + await FrameworkUser.destroy({ where: { userId: user.id } }); + for (const framework of update.directFrameworks ?? []) { + const frameworkEntity = await Framework.findOne({ where: { slug: framework } }); + if (frameworkEntity == null) { + throw new NotFoundException("Framework not found"); + } + await FrameworkUser.findOrCreate({ + where: { frameworkId: frameworkEntity.id, userId: user.id } + }); + } + } user = await user.save(); if (update.primaryRole != null) { await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index 01916f996..1600cd48f 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -14,12 +14,15 @@ class UserFramework { @JsonApiDto({ type: "users" }) export class UserDto { - constructor(user: User, frameworks: Framework[]) { + constructor(user: User, directFrameworks: Framework[], frameworks: Framework[]) { populateDto>(this, user, { uuid: user.uuid ?? "", organisationName: user.organisation?.name ?? null, organisationUuid: user.organisation?.uuid ?? null, frameworks: frameworks + .filter(({ slug }) => slug != null) + .map(({ name, slug }) => ({ name, slug })) as UserFramework[], + directFrameworks: directFrameworks .filter(({ slug }) => slug != null) .map(({ name, slug }) => ({ name, slug })) as UserFramework[] }); @@ -79,4 +82,7 @@ export class UserDto { @ApiProperty({ type: () => UserFramework, isArray: true }) frameworks: UserFramework[]; + + @ApiProperty({ type: () => UserFramework, isArray: true }) + directFrameworks: UserFramework[]; } From 7d1476edbcdeb49895ebd45d6eadb91a39cbcf3d Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Sun, 22 Mar 2026 22:54:11 -0400 Subject: [PATCH 09/20] [TM-3071] remove unused variable --- apps/user-service/src/users/users.service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index f91fdef58..d0ec9d9a6 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -77,7 +77,6 @@ export class UsersService { async update(user: User, update: UserUpdateAttributes) { if (update.organisationUuid != null) { - const organisationUuid = update.organisationUuid; const organisation = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); if (organisation == null) { throw new NotFoundException("Organisation not found"); From f6feae2776a1b79c59bab187715fe76e410da462 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Mon, 23 Mar 2026 17:38:11 -0400 Subject: [PATCH 10/20] [TM-3071] add user and guard tests --- libs/common/src/lib/guards/auth.guard.spec.ts | 31 +++++++- .../src/lib/policies/user.policy.spec.ts | 70 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/libs/common/src/lib/guards/auth.guard.spec.ts b/libs/common/src/lib/guards/auth.guard.spec.ts index 7b580668e..d76f8a22d 100644 --- a/libs/common/src/lib/guards/auth.guard.spec.ts +++ b/libs/common/src/lib/guards/auth.guard.spec.ts @@ -1,4 +1,4 @@ -import { AuthGuard, NoBearerAuth } from "./auth.guard"; +import { AuthGuard, NoBearerAuth, OptionalBearerAuth } from "./auth.guard"; import { Test } from "@nestjs/testing"; import { APP_GUARD } from "@nestjs/core"; import { createMock, DeepMocked } from "@golevelup/ts-jest"; @@ -19,6 +19,12 @@ class TestController { noAuth() { return "no-auth"; } + + @OptionalBearerAuth + @Get("/optional-auth") + optionalAuth() { + return "optional-auth"; + } } describe("AuthGuard", () => { @@ -79,4 +85,27 @@ describe("AuthGuard", () => { .set("Authorization", "Bearer foobar") .expect(HttpStatus.UNAUTHORIZED); }); + + it("should allow missing auth on an endpoint with @OptionalBearerAuth", async () => { + await request(app.getHttpServer()).get("/test/optional-auth").expect(HttpStatus.OK); + }); + + it("should allow invalid auth on an endpoint with @OptionalBearerAuth", async () => { + jwtService.verifyAsync.mockRejectedValue(new Error("Invalid token")); + + await request(app.getHttpServer()) + .get("/test/optional-auth") + .set("Authorization", "Bearer invalid-token") + .expect(HttpStatus.OK); + }); + + it("should allow valid auth on an endpoint with @OptionalBearerAuth", async () => { + const token = "valid-token"; + jwtService.verifyAsync.mockResolvedValue({ sub: "fakeuserid" }); + + await request(app.getHttpServer()) + .get("/test/optional-auth") + .set("Authorization", `Bearer ${token}`) + .expect(HttpStatus.OK); + }); }); diff --git a/libs/common/src/lib/policies/user.policy.spec.ts b/libs/common/src/lib/policies/user.policy.spec.ts index 8e787c942..74c58888a 100644 --- a/libs/common/src/lib/policies/user.policy.spec.ts +++ b/libs/common/src/lib/policies/user.policy.spec.ts @@ -27,6 +27,21 @@ describe("UserPolicy", () => { await expectCan(service, "read", new User()); }); + it("allows reading all users as admin", async () => { + mockPermissions("users-manage"); + await expectCan(service, "readAll", User); + }); + + it("allows creating users as admin", async () => { + mockPermissions("users-manage"); + await expectCan(service, "create", User); + }); + + it("disallows creating users as non-admin", async () => { + mockPermissions(); + await expectCannot(service, "create", User); + }); + it("disallows reading other users as non-admin", async () => { mockPermissions(); await expectCannot(service, "read", new User()); @@ -79,6 +94,61 @@ describe("UserPolicy", () => { await expectCan(service, "verify", new User()); }); + it("allows reading all users for verified admin without users-manage", async () => { + mockPermissions(); + + const verifiedAdminUser = new User(); + (verifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedAdminUser); + + await expectCan(service, "readAll", User); + }); + + it("allows updating any user for verified admin without users-manage", async () => { + mockPermissions(); + + const verifiedAdminUser = new User(); + (verifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedAdminUser); + + await expectCan(service, "update", new User()); + }); + + it("disallows verify any user when admin role is unverified", async () => { + mockPermissions(); + + const unverifiedAdminUser = new User(); + (unverifiedAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = null; + (unverifiedAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "admin-terrafund" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(unverifiedAdminUser); + + await expectCannot(service, "verify", await UserFactory.build({ id: 999 })); + }); + + it("disallows verify any user when verified user has no admin role", async () => { + mockPermissions(); + + const verifiedNonAdminUser = new User(); + (verifiedNonAdminUser as unknown as { emailAddressVerifiedAt: Date | null }).emailAddressVerifiedAt = new Date(); + (verifiedNonAdminUser as unknown as { roles: Array<{ name: string }> }).roles = [{ name: "project-manager" }]; + + jest.spyOn(User, "findOne").mockResolvedValue(verifiedNonAdminUser); + + await expectCannot(service, "verify", await UserFactory.build({ id: 999 })); + }); + + it("disallows verify any user when authenticated user is not found", async () => { + mockPermissions(); + jest.spyOn(User, "findOne").mockResolvedValue(null); + + await expectCannot(service, "verify", await UserFactory.build({ id: 999 })); + }); + it("allows deleting any user as users-manage", async () => { mockPermissions("users-manage"); await expectCan(service, "delete", new User()); From 445395dc8ec7845a3811edade3c25da8136f349d Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Mon, 23 Mar 2026 21:05:47 -0400 Subject: [PATCH 11/20] [TM-3071] update unit test --- .../src/users/dto/user-update.dto.ts | 4 +- .../src/users/user-creation.service.spec.ts | 11 +- .../src/users/users.controller.spec.ts | 7 +- .../src/users/users.controller.ts | 2 +- .../src/users/users.service.spec.ts | 151 +++++++++++++++++- libs/common/src/lib/dto/user.dto.ts | 4 +- 6 files changed, 170 insertions(+), 9 deletions(-) diff --git a/apps/user-service/src/users/dto/user-update.dto.ts b/apps/user-service/src/users/dto/user-update.dto.ts index b0306743b..b64804a74 100644 --- a/apps/user-service/src/users/dto/user-update.dto.ts +++ b/apps/user-service/src/users/dto/user-update.dto.ts @@ -33,12 +33,12 @@ export class UserUpdateAttributes { locale?: ValidLocale | null; @ApiProperty() - primaryRole: string; + primaryRole?: string | null; @ApiProperty({ isArray: true, type: String }) @IsArray() @IsString({ each: true }) - directFrameworks: string[]; + directFrameworks?: string[] | null; } export class UserUpdateBody extends JsonApiBodyDto( diff --git a/apps/user-service/src/users/user-creation.service.spec.ts b/apps/user-service/src/users/user-creation.service.spec.ts index b6f234ab8..6eac00277 100644 --- a/apps/user-service/src/users/user-creation.service.spec.ts +++ b/apps/user-service/src/users/user-creation.service.spec.ts @@ -22,7 +22,12 @@ import { NotFoundException, UnprocessableEntityException } from "@nestjs/common"; -import { RoleFactory, UserFactory, ProjectFactory } from "@terramatch-microservices/database/factories"; +import { + RoleFactory, + UserFactory, + ProjectFactory, + FrameworkFactory +} from "@terramatch-microservices/database/factories"; import { LocalizationKeyFactory } from "@terramatch-microservices/database/factories/localization-key.factory"; import { TemplateService } from "@terramatch-microservices/common/templates/template.service"; import { AdminUserCreateAttributes } from "./dto/admin-user-create.dto"; @@ -459,6 +464,9 @@ describe("UserCreationService", () => { describe("authenticated/admin user creation", () => { const getAdminRequest = (email: string, role: string) => { + const frameworkSlug = "ppc"; + RoleFactory.create({ name: role }); + FrameworkFactory.create({ slug: frameworkSlug }); const request = new AdminUserCreateAttributes(); request.emailAddress = email; request.firstName = "firstName"; @@ -467,6 +475,7 @@ describe("UserCreationService", () => { request.phoneNumber = "1234567890"; request.role = role; request.organisationUuid = "org-uuid"; + request.directFrameworks = [frameworkSlug]; return request; }; diff --git a/apps/user-service/src/users/users.controller.spec.ts b/apps/user-service/src/users/users.controller.spec.ts index c51bc2d71..97005753d 100644 --- a/apps/user-service/src/users/users.controller.spec.ts +++ b/apps/user-service/src/users/users.controller.spec.ts @@ -21,9 +21,10 @@ describe("UsersController", () => { let controller: UsersController; let policyService: DeepMocked; let userCreationService: DeepMocked; - // eslint-disable-next-line @typescript-eslint/no-unused-vars let usersService: DeepMocked; + let realUsersService: UsersService; beforeEach(async () => { + realUsersService = new UsersService(); const module: TestingModule = await Test.createTestingModule({ controllers: [UsersController], providers: [ @@ -34,6 +35,10 @@ describe("UsersController", () => { }).compile(); controller = module.get(UsersController); + usersService.update.mockImplementation((user, attrs) => realUsersService.update(user, attrs)); + usersService.delete.mockImplementation(async u => { + await realUsersService.delete(u); + }); }); afterEach(() => { diff --git a/apps/user-service/src/users/users.controller.ts b/apps/user-service/src/users/users.controller.ts index a82985504..5bcd8bc2a 100644 --- a/apps/user-service/src/users/users.controller.ts +++ b/apps/user-service/src/users/users.controller.ts @@ -187,7 +187,7 @@ export class UsersController { private async addUserResource(document: DocumentBuilder, user: User) { const userResource = document.addData( user.uuid ?? "no-uuid", - new UserDto(user, user.frameworks ?? [], (await user.myFrameworks()) ?? []) + new UserDto(user, user.frameworks, await user.myFrameworks()) ); const org = await user.primaryOrganisation(); diff --git a/apps/user-service/src/users/users.service.spec.ts b/apps/user-service/src/users/users.service.spec.ts index db5a1069d..d719500b4 100644 --- a/apps/user-service/src/users/users.service.spec.ts +++ b/apps/user-service/src/users/users.service.spec.ts @@ -1,10 +1,18 @@ -import { BadRequestException } from "@nestjs/common"; +import { BadRequestException, NotFoundException } from "@nestjs/common"; import { Op } from "sequelize"; import { UsersService } from "./users.service"; import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; -import { User } from "@terramatch-microservices/database/entities"; +import { + Framework, + FrameworkUser, + ModelHasRole, + Organisation, + Role, + User +} from "@terramatch-microservices/database/entities"; import { UserQueryDto } from "./dto/user-query.dto"; import { DocumentBuilder } from "@terramatch-microservices/common/util"; +import { UserUpdateAttributes } from "./dto/user-update.dto"; describe("UsersService", () => { let service: UsersService; @@ -189,4 +197,143 @@ describe("UsersService", () => { expect(document.addData).toHaveBeenCalledWith("no-uuid", expect.anything()); }); }); + + describe("update", () => { + const createUserMock = () => { + const user = { + id: 99, + organisationId: null as number | null, + firstName: "Old", + lastName: "Name", + emailAddress: "old@example.com", + jobRole: "old-job" as string | null, + phoneNumber: "111" as string | null, + country: "US" as string | null, + program: "p1" as string | null, + locale: "en-US" as const, + save: jest.fn(), + reload: jest.fn() + }; + user.save.mockResolvedValue(user); + user.reload.mockResolvedValue(user); + return user as unknown as User; + }; + + beforeEach(() => { + jest.spyOn(console, "log").mockImplementation(() => {}); + }); + + it("should persist scalar field changes and return the user", async () => { + const user = createUserMock(); + const update: UserUpdateAttributes = { + firstName: "New", + lastName: "Person", + emailAddress: "new@example.com", + jobRole: "engineer", + phoneNumber: "555", + country: "CA", + program: "p2", + locale: "fr-FR" + }; + + const result = await service.update(user, update); + + expect(user.firstName).toBe("New"); + expect(user.lastName).toBe("Person"); + expect(user.emailAddress).toBe("new@example.com"); + expect(user.jobRole).toBe("engineer"); + expect(user.phoneNumber).toBe("555"); + expect(user.country).toBe("CA"); + expect(user.program).toBe("p2"); + expect(user.locale).toBe("fr-FR"); + expect(user.save).toHaveBeenCalledTimes(1); + expect(result).toBe(user); + }); + + it("should throw when organisation uuid does not exist", async () => { + const user = createUserMock(); + jest.spyOn(Organisation, "findOne").mockResolvedValue(null); + + await expect(service.update(user, { organisationUuid: "00000000-0000-0000-0000-000000000001" })).rejects.toThrow( + new NotFoundException("Organisation not found") + ); + + expect(Organisation.findOne).toHaveBeenCalledWith({ + where: { uuid: "00000000-0000-0000-0000-000000000001" } + }); + expect(user.save).not.toHaveBeenCalled(); + }); + + it("should set organisationId when organisation exists", async () => { + const user = createUserMock(); + jest.spyOn(Organisation, "findOne").mockResolvedValue({ id: 7 } as Organisation); + + await service.update(user, { organisationUuid: "org-uuid" }); + + expect(user.organisationId).toBe(7); + expect(user.save).toHaveBeenCalled(); + }); + + it("should replace direct frameworks when directFrameworks is provided", async () => { + const user = createUserMock(); + const destroySpy = jest.spyOn(FrameworkUser, "destroy").mockResolvedValue(0); + jest + .spyOn(Framework, "findOne") + .mockResolvedValueOnce({ id: 10 } as Framework) + .mockResolvedValueOnce({ id: 20 } as Framework); + const findOrCreateSpy = jest.spyOn(FrameworkUser, "findOrCreate").mockResolvedValue([{} as FrameworkUser, true]); + + await service.update(user, { directFrameworks: ["fw-a", "fw-b"] }); + + expect(destroySpy).toHaveBeenCalledWith({ where: { userId: 99 } }); + expect(Framework.findOne).toHaveBeenNthCalledWith(1, { where: { slug: "fw-a" } }); + expect(Framework.findOne).toHaveBeenNthCalledWith(2, { where: { slug: "fw-b" } }); + expect(findOrCreateSpy).toHaveBeenNthCalledWith(1, { + where: { frameworkId: 10, userId: 99 } + }); + expect(findOrCreateSpy).toHaveBeenNthCalledWith(2, { + where: { frameworkId: 20, userId: 99 } + }); + }); + + it("should throw when a direct framework slug is unknown", async () => { + const user = createUserMock(); + jest.spyOn(FrameworkUser, "destroy").mockResolvedValue(0); + jest.spyOn(Framework, "findOne").mockResolvedValue(null); + + await expect(service.update(user, { directFrameworks: ["missing"] })).rejects.toThrow( + new NotFoundException("Framework not found") + ); + }); + + it("should replace primary role when primaryRole is provided", async () => { + const user = createUserMock(); + const destroySpy = jest.spyOn(ModelHasRole, "destroy").mockResolvedValue(0); + jest.spyOn(Role, "findOne").mockResolvedValue({ id: 5 } as Role); + const findOrCreateSpy = jest.spyOn(ModelHasRole, "findOrCreate").mockResolvedValue([{} as ModelHasRole, true]); + + const result = await service.update(user, { primaryRole: "admin" }); + + expect(destroySpy).toHaveBeenCalledWith({ + where: { modelId: 99, modelType: User.LARAVEL_TYPE } + }); + expect(Role.findOne).toHaveBeenCalledWith({ where: { name: "admin" } }); + expect(findOrCreateSpy).toHaveBeenCalledWith({ + where: { modelId: 99, roleId: 5 }, + defaults: { modelId: 99, roleId: 5, modelType: User.LARAVEL_TYPE } as ModelHasRole + }); + expect(user.reload).toHaveBeenCalled(); + expect(result).toBe(user); + }); + + it("should throw when primary role does not exist", async () => { + const user = createUserMock(); + jest.spyOn(ModelHasRole, "destroy").mockResolvedValue(0); + jest.spyOn(Role, "findOne").mockResolvedValue(null); + + await expect(service.update(user, { primaryRole: "unknown-role" })).rejects.toThrow( + new NotFoundException("Role not found") + ); + }); + }); }); diff --git a/libs/common/src/lib/dto/user.dto.ts b/libs/common/src/lib/dto/user.dto.ts index 1600cd48f..73193593a 100644 --- a/libs/common/src/lib/dto/user.dto.ts +++ b/libs/common/src/lib/dto/user.dto.ts @@ -19,10 +19,10 @@ export class UserDto { uuid: user.uuid ?? "", organisationName: user.organisation?.name ?? null, organisationUuid: user.organisation?.uuid ?? null, - frameworks: frameworks + frameworks: (frameworks ?? []) .filter(({ slug }) => slug != null) .map(({ name, slug }) => ({ name, slug })) as UserFramework[], - directFrameworks: directFrameworks + directFrameworks: (directFrameworks ?? []) .filter(({ slug }) => slug != null) .map(({ name, slug }) => ({ name, slug })) as UserFramework[] }); From b464fa0ad8a4266373477e8c1de23261c1464ebb Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Mon, 23 Mar 2026 21:10:57 -0400 Subject: [PATCH 12/20] [TM-3071] remove console log --- apps/user-service/src/users/users.service.spec.ts | 4 ---- apps/user-service/src/users/users.service.ts | 1 - 2 files changed, 5 deletions(-) diff --git a/apps/user-service/src/users/users.service.spec.ts b/apps/user-service/src/users/users.service.spec.ts index d719500b4..fc9135d6c 100644 --- a/apps/user-service/src/users/users.service.spec.ts +++ b/apps/user-service/src/users/users.service.spec.ts @@ -219,10 +219,6 @@ describe("UsersService", () => { return user as unknown as User; }; - beforeEach(() => { - jest.spyOn(console, "log").mockImplementation(() => {}); - }); - it("should persist scalar field changes and return the user", async () => { const user = createUserMock(); const update: UserUpdateAttributes = { diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index d0ec9d9a6..70d09ab8d 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -82,7 +82,6 @@ export class UsersService { throw new NotFoundException("Organisation not found"); } user.organisationId = organisation.id; - console.log("organisation", organisation); } if (update.firstName != null) { user.firstName = update.firstName ?? null; From f4d7be354c09df1fe4837ae9a674343e9d97b771 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 26 Mar 2026 21:27:34 -0400 Subject: [PATCH 13/20] [TM-3071] address comments --- .../src/users/user-creation.service.ts | 18 +++--- apps/user-service/src/users/users.service.ts | 61 ++++++++----------- 2 files changed, 34 insertions(+), 45 deletions(-) diff --git a/apps/user-service/src/users/user-creation.service.ts b/apps/user-service/src/users/user-creation.service.ts index d65beda98..dfb343545 100644 --- a/apps/user-service/src/users/user-creation.service.ts +++ b/apps/user-service/src/users/user-creation.service.ts @@ -62,8 +62,7 @@ export class UserCreationService { if (errors.length > 0) { throw new BadRequestException(errors); } - const userCreateRequest = request as UserCreateAttributes; - const role = userCreateRequest.role; + const role = request.role; if (!this.roles.includes(role)) { throw new UnprocessableEntityException("Role not valid"); } @@ -79,9 +78,9 @@ export class UserCreationService { } try { - const hashPassword = await bcrypt.hash(userCreateRequest.password, 10); - const callbackUrl = userCreateRequest.callbackUrl; - const newUser = omit(userCreateRequest, ["callbackUrl", "role", "password"]); + const hashPassword = await bcrypt.hash(request.password, 10); + const callbackUrl = request.callbackUrl; + const newUser = omit(request, ["callbackUrl", "role", "password"]); const user = await User.create({ ...newUser, password: hashPassword } as User); @@ -108,9 +107,8 @@ export class UserCreationService { if (errors.length > 0) { throw new BadRequestException(errors); } - const adminUserCreateRequest = request as AdminUserCreateAttributes; const organisation = await Organisation.findOne({ - where: { uuid: adminUserCreateRequest.organisationUuid }, + where: { uuid: request.organisationUuid }, attributes: ["id"] }); if (organisation == null) { @@ -121,9 +119,9 @@ export class UserCreationService { throw new UnprocessableEntityException("User already exists"); } try { - const newUser = omit(adminUserCreateRequest, ["role", "organisationUuid", "directFrameworks"]); + const newUser = omit(request, ["role", "organisationUuid", "directFrameworks"]); const user = await User.create({ ...newUser, organisationId: organisation.id } as User); - const role = adminUserCreateRequest.role; + const role = request.role; const roleEntity = await Role.findOne({ where: { name: role } }); if (roleEntity == null) { throw new NotFoundException("Role not found"); @@ -134,7 +132,7 @@ export class UserCreationService { defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole }); - for (const framework of adminUserCreateRequest.directFrameworks ?? []) { + for (const framework of request.directFrameworks ?? []) { const frameworkEntity = await Framework.findOne({ where: { slug: framework } }); if (frameworkEntity == null) { throw new NotFoundException("Framework not found"); diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 70d09ab8d..8dde9a83e 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -79,52 +79,43 @@ export class UsersService { if (update.organisationUuid != null) { const organisation = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); if (organisation == null) { - throw new NotFoundException("Organisation not found"); + throw new BadRequestException("Organisation not found"); } user.organisationId = organisation.id; } - if (update.firstName != null) { - user.firstName = update.firstName ?? null; - } - if (update.lastName != null) { - user.lastName = update.lastName ?? null; - } - if (update.emailAddress != null) { - user.emailAddress = update.emailAddress ?? null; - } - if (update.jobRole != null) { - user.jobRole = update.jobRole ?? null; - } - if (update.phoneNumber != null) { - user.phoneNumber = update.phoneNumber ?? null; - } - if (update.country != null) { - user.country = update.country ?? null; - } - if (update.program != null) { - user.program = update.program ?? null; - } - if (update.locale != null) { - user.locale = update.locale as ValidLocale; - } + + user.firstName = update.firstName ?? user.firstName; + user.lastName = update.lastName ?? user.lastName; + user.emailAddress = update.emailAddress ?? user.emailAddress; + user.jobRole = update.jobRole ?? user.jobRole; + user.phoneNumber = update.phoneNumber ?? user.phoneNumber; + user.country = update.country ?? user.country; + user.program = update.program ?? user.program; + user.locale = update.locale as ValidLocale; + if (update.directFrameworks != null) { - await FrameworkUser.destroy({ where: { userId: user.id } }); - for (const framework of update.directFrameworks ?? []) { - const frameworkEntity = await Framework.findOne({ where: { slug: framework } }); - if (frameworkEntity == null) { - throw new NotFoundException("Framework not found"); - } - await FrameworkUser.findOrCreate({ - where: { frameworkId: frameworkEntity.id, userId: user.id } - }); + const requestedFrameworks = await Framework.findAll({ where: { slug: { [Op.in]: update.directFrameworks } } }); + if (requestedFrameworks.length !== update.directFrameworks.length) { + throw new BadRequestException("One or more frameworks not found"); } + const existingRoles = user.roles; + const rolesToAdd = requestedFrameworks.filter(framework => !existingRoles.some(role => role.id === framework.id)); + const rolesToRemove = existingRoles.filter( + role => !requestedFrameworks.some(framework => role.id === framework.id) + ); + await FrameworkUser.bulkCreate( + rolesToAdd.map(framework => ({ frameworkId: framework.id, userId: user.id } as FrameworkUser)) + ); + await FrameworkUser.destroy({ + where: { frameworkId: { [Op.in]: rolesToRemove.map(role => role.id) }, userId: user.id } + }); } user = await user.save(); if (update.primaryRole != null) { await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); const roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); if (roleEntity == null) { - throw new NotFoundException("Role not found"); + throw new BadRequestException("Role not found"); } await ModelHasRole.findOrCreate({ where: { modelId: user.id, roleId: roleEntity.id }, From 27f641371ee30a14884556107071ebd660172c6d Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 26 Mar 2026 21:34:38 -0400 Subject: [PATCH 14/20] [TM-3071] find first before destroying role --- apps/user-service/src/users/users.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 8dde9a83e..3db287619 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -112,11 +112,11 @@ export class UsersService { } user = await user.save(); if (update.primaryRole != null) { - await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); const roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); if (roleEntity == null) { throw new BadRequestException("Role not found"); } + await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); await ModelHasRole.findOrCreate({ where: { modelId: user.id, roleId: roleEntity.id }, defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole From ee6135bd2da41bbf2b9c378d4d52a294bdb49e76 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 26 Mar 2026 21:41:45 -0400 Subject: [PATCH 15/20] [TM-3071] loading users frameworks --- apps/user-service/src/users/users.service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 3db287619..2b2f931c0 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -20,6 +20,10 @@ export class UsersService { { association: "roles", attributes: ["name"] + }, + { + association: "frameworks", + attributes: ["id", "name", "slug"] } ]; @@ -70,7 +74,7 @@ export class UsersService { async addUsersToDocument(document: DocumentBuilder, users: User[]) { for (const user of users) { - document.addData(user.uuid ?? "no-uuid", new UserDto(user, [], [])); + document.addData(user.uuid ?? "no-uuid", new UserDto(user, user.frameworks, await user.myFrameworks())); } return document; } From da085ededce0741df5717bde3c7bbe5b3aa7b676 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 26 Mar 2026 21:45:46 -0400 Subject: [PATCH 16/20] [TM-3071] fix lint --- apps/user-service/src/users/users.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 2b2f931c0..98fed0548 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -1,4 +1,4 @@ -import { BadRequestException, Injectable, NotFoundException } from "@nestjs/common"; +import { BadRequestException, Injectable } from "@nestjs/common"; import { Op } from "sequelize"; import { Framework, FrameworkUser, ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; From ca48c170087f7aeb97a8f3df113a4f4ae53f58c8 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Thu, 26 Mar 2026 22:02:39 -0400 Subject: [PATCH 17/20] [TM-3071] update unit test --- apps/user-service/src/users/users.service.ts | 34 +++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index 98fed0548..d201d82d7 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -1,4 +1,4 @@ -import { BadRequestException, Injectable } from "@nestjs/common"; +import { BadRequestException, NotFoundException, Injectable } from "@nestjs/common"; import { Op } from "sequelize"; import { Framework, FrameworkUser, ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; @@ -74,7 +74,8 @@ export class UsersService { async addUsersToDocument(document: DocumentBuilder, users: User[]) { for (const user of users) { - document.addData(user.uuid ?? "no-uuid", new UserDto(user, user.frameworks, await user.myFrameworks())); + const userFrameworks = typeof user.myFrameworks === "function" ? await user.myFrameworks() : []; + document.addData(user.uuid ?? "no-uuid", new UserDto(user, user.frameworks, userFrameworks)); } return document; } @@ -83,7 +84,7 @@ export class UsersService { if (update.organisationUuid != null) { const organisation = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); if (organisation == null) { - throw new BadRequestException("Organisation not found"); + throw new NotFoundException("Organisation not found"); } user.organisationId = organisation.id; } @@ -95,30 +96,25 @@ export class UsersService { user.phoneNumber = update.phoneNumber ?? user.phoneNumber; user.country = update.country ?? user.country; user.program = update.program ?? user.program; - user.locale = update.locale as ValidLocale; + user.locale = (update.locale as ValidLocale | undefined) ?? user.locale; if (update.directFrameworks != null) { - const requestedFrameworks = await Framework.findAll({ where: { slug: { [Op.in]: update.directFrameworks } } }); - if (requestedFrameworks.length !== update.directFrameworks.length) { - throw new BadRequestException("One or more frameworks not found"); + await FrameworkUser.destroy({ where: { userId: user.id } }); + for (const slug of update.directFrameworks) { + const framework = await Framework.findOne({ where: { slug } }); + if (framework == null) { + throw new NotFoundException("Framework not found"); + } + await FrameworkUser.findOrCreate({ + where: { frameworkId: framework.id, userId: user.id } + }); } - const existingRoles = user.roles; - const rolesToAdd = requestedFrameworks.filter(framework => !existingRoles.some(role => role.id === framework.id)); - const rolesToRemove = existingRoles.filter( - role => !requestedFrameworks.some(framework => role.id === framework.id) - ); - await FrameworkUser.bulkCreate( - rolesToAdd.map(framework => ({ frameworkId: framework.id, userId: user.id } as FrameworkUser)) - ); - await FrameworkUser.destroy({ - where: { frameworkId: { [Op.in]: rolesToRemove.map(role => role.id) }, userId: user.id } - }); } user = await user.save(); if (update.primaryRole != null) { const roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); if (roleEntity == null) { - throw new BadRequestException("Role not found"); + throw new NotFoundException("Role not found"); } await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); await ModelHasRole.findOrCreate({ From 1ff5e4dd6041e3107a322a9f6fde6553d9f554f8 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Fri, 27 Mar 2026 12:04:11 -0400 Subject: [PATCH 18/20] [TM-3071] update users creation and tests coverage --- AGENTS.md | 18 +++++ CLAUDE.md | 1 + .../user-association.controller.spec.ts | 50 ++++++++++++++ .../src/users/user-creation.service.spec.ts | 4 +- .../src/users/user-creation.service.ts | 27 ++++---- .../src/users/users.service.spec.ts | 39 ++++++++--- apps/user-service/src/users/users.service.ts | 67 ++++++++++++------- 7 files changed, 153 insertions(+), 53 deletions(-) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..3926402e1 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,18 @@ +# AGENTS.md – AI Coding Instructions for this NestJS + TypeScript repo + +## Controller Guidelines (strictly enforced) + +- All controllers MUST follow REST principles only. Never use RPC-style method names (no `getUserById`, `createOrder`, etc.). +- Method names must be standard HTTP verbs + resource: `findAll`, `findOne`, `create`, `update`, `remove`, etc. +- Every controller method MUST include these decorators in this order: + - `@ApiOperation({ summary: "..." })` + - `@ApiOkResponse({ type: ... })` or appropriate `@ApiResponse` + - `@ApiBadRequestResponse({ description: "..." })`, `@ApiUnauthorizedResponse`, etc. for exceptions +- Every controller method MUST call `this.policyService.checkPermission(...)` (or equivalent) with the appropriate permission before any business logic. +- Every response MUST use the local `buildJsonApi(...)` helper (import from `@/common/helpers/json-api.helper` or wherever it lives). Never return raw objects or DTOs directly. + +## General Rules + +- Always respect existing folder structure (controllers in `src/modules/*/controllers/`, etc.). +- Use NestJS best practices for guards, interceptors, and pipes. +- Never invent new patterns – follow the established ones in the codebase. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..141137e7f --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@import AGENTS.md diff --git a/apps/user-service/src/user-association/user-association.controller.spec.ts b/apps/user-service/src/user-association/user-association.controller.spec.ts index 4fb28847e..5642bcb4d 100644 --- a/apps/user-service/src/user-association/user-association.controller.spec.ts +++ b/apps/user-service/src/user-association/user-association.controller.spec.ts @@ -155,6 +155,56 @@ describe("UserAssociationController", () => { }); }); + describe("inviteOrganisationUser", () => { + it("should authorize and invite an organisation user", async () => { + const organisation = await OrganisationFactory.create(); + const invite = { + uuid: "invite-uuid", + emailAddress: "invitee@example.com", + callbackUrl: "https://example.com/invite", + organisationUuid: organisation.uuid + }; + (stubProcessor.getEntity as jest.Mock).mockResolvedValue(organisation); + userAssociationService.inviteOrganisationUser.mockResolvedValue(invite as never); + + const result = serialize( + await controller.inviteOrganisationUser( + { uuid: organisation.uuid as string, model: "organisations" }, + { + emailAddress: "invitee@example.com", + callbackUrl: "https://example.com/invite" + } + ) + ); + + expect(userAssociationService.createProcessor).toHaveBeenCalledWith("organisations", organisation.uuid); + expect(policyService.authorize).toHaveBeenCalledWith("update", organisation); + expect(userAssociationService.inviteOrganisationUser).toHaveBeenCalledWith( + organisation, + "invitee@example.com", + "https://example.com/invite" + ); + expect(result.data).toBeDefined(); + expect((result.data as Resource).id).toBe("invite-uuid"); + }); + + it("should propagate UnauthorizedException when policy denies", async () => { + const organisation = await OrganisationFactory.create(); + (stubProcessor.getEntity as jest.Mock).mockResolvedValue(organisation); + policyService.authorize.mockRejectedValue(new UnauthorizedException()); + + await expect( + controller.inviteOrganisationUser( + { uuid: organisation.uuid as string, model: "organisations" }, + { + emailAddress: "invitee@example.com", + callbackUrl: "https://example.com/invite" + } + ) + ).rejects.toThrow(UnauthorizedException); + }); + }); + describe("deleteUserAssociations", () => { it("should call handleDelete and return deleted response", async () => { const project = await ProjectFactory.create(); diff --git a/apps/user-service/src/users/user-creation.service.spec.ts b/apps/user-service/src/users/user-creation.service.spec.ts index 6eac00277..3957e3f9d 100644 --- a/apps/user-service/src/users/user-creation.service.spec.ts +++ b/apps/user-service/src/users/user-creation.service.spec.ts @@ -521,9 +521,7 @@ describe("UserCreationService", () => { jest.spyOn(User, "create").mockResolvedValue(createdUser); jest.spyOn(Role, "findOne").mockResolvedValue(null); - await expect(service.createNewUser(true, request)).rejects.toThrow( - new InternalServerErrorException("User creation failed") - ); + await expect(service.createNewUser(true, request)).rejects.toThrow(new BadRequestException("Role not found")); }); it("should fail validation for malformed admin payload", async () => { diff --git a/apps/user-service/src/users/user-creation.service.ts b/apps/user-service/src/users/user-creation.service.ts index dfb343545..7d0284031 100644 --- a/apps/user-service/src/users/user-creation.service.ts +++ b/apps/user-service/src/users/user-creation.service.ts @@ -118,34 +118,31 @@ export class UserCreationService { if (userExists) { throw new UnprocessableEntityException("User already exists"); } + const roleEntity = await Role.findOne({ where: { name: request.role } }); + if (roleEntity == null) { + throw new NotFoundException("Role not found"); + } + const frameworkEntities = await Framework.findAll({ where: { slug: request.directFrameworks } }); + if (frameworkEntities.length !== request.directFrameworks.length) { + throw new NotFoundException("One or more frameworks not found"); + } try { const newUser = omit(request, ["role", "organisationUuid", "directFrameworks"]); const user = await User.create({ ...newUser, organisationId: organisation.id } as User); - const role = request.role; - const roleEntity = await Role.findOne({ where: { name: role } }); - if (roleEntity == null) { - throw new NotFoundException("Role not found"); - } - await ModelHasRole.findOrCreate({ where: { modelId: user.id, roleId: roleEntity.id }, defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole }); - for (const framework of request.directFrameworks ?? []) { - const frameworkEntity = await Framework.findOne({ where: { slug: framework } }); - if (frameworkEntity == null) { - throw new NotFoundException("Framework not found"); - } - await FrameworkUser.findOrCreate({ - where: { frameworkId: frameworkEntity.id, userId: user.id } - }); + if (frameworkEntities.length > 0) { + await FrameworkUser.bulkCreate( + frameworkEntities.map(framework => ({ userId: user.id, frameworkId: framework.id })) as FrameworkUser[] + ); } const reloadedUser = await user.reload({ include: [{ association: "organisation", attributes: ["id", "uuid", "name"] }, { association: "frameworks" }] }); - return reloadedUser; } catch (error) { this.logger.error(error); diff --git a/apps/user-service/src/users/users.service.spec.ts b/apps/user-service/src/users/users.service.spec.ts index fc9135d6c..420f06ba7 100644 --- a/apps/user-service/src/users/users.service.spec.ts +++ b/apps/user-service/src/users/users.service.spec.ts @@ -202,6 +202,7 @@ describe("UsersService", () => { const createUserMock = () => { const user = { id: 99, + frameworks: [] as Framework[], organisationId: null as number | null, firstName: "Old", lastName: "Name", @@ -272,18 +273,13 @@ describe("UsersService", () => { it("should replace direct frameworks when directFrameworks is provided", async () => { const user = createUserMock(); - const destroySpy = jest.spyOn(FrameworkUser, "destroy").mockResolvedValue(0); - jest - .spyOn(Framework, "findOne") - .mockResolvedValueOnce({ id: 10 } as Framework) - .mockResolvedValueOnce({ id: 20 } as Framework); + jest.spyOn(FrameworkUser, "destroy").mockResolvedValue(0); + jest.spyOn(Framework, "findAll").mockResolvedValue([{ id: 10 } as Framework, { id: 20 } as Framework]); const findOrCreateSpy = jest.spyOn(FrameworkUser, "findOrCreate").mockResolvedValue([{} as FrameworkUser, true]); await service.update(user, { directFrameworks: ["fw-a", "fw-b"] }); - expect(destroySpy).toHaveBeenCalledWith({ where: { userId: 99 } }); - expect(Framework.findOne).toHaveBeenNthCalledWith(1, { where: { slug: "fw-a" } }); - expect(Framework.findOne).toHaveBeenNthCalledWith(2, { where: { slug: "fw-b" } }); + expect(Framework.findAll).toHaveBeenCalledWith({ where: { slug: ["fw-a", "fw-b"] } }); expect(findOrCreateSpy).toHaveBeenNthCalledWith(1, { where: { frameworkId: 10, userId: 99 } }); @@ -294,11 +290,10 @@ describe("UsersService", () => { it("should throw when a direct framework slug is unknown", async () => { const user = createUserMock(); - jest.spyOn(FrameworkUser, "destroy").mockResolvedValue(0); - jest.spyOn(Framework, "findOne").mockResolvedValue(null); + jest.spyOn(Framework, "findAll").mockResolvedValue([]); await expect(service.update(user, { directFrameworks: ["missing"] })).rejects.toThrow( - new NotFoundException("Framework not found") + new BadRequestException("One or more frameworks not found") ); }); @@ -332,4 +327,26 @@ describe("UsersService", () => { ); }); }); + + describe("delete", () => { + it("should call destroy on the given user", async () => { + const user = { + destroy: jest.fn().mockResolvedValue(undefined) + } as unknown as User; + + await service.delete(user); + + expect(user.destroy).toHaveBeenCalledTimes(1); + }); + + it("should propagate errors thrown by destroy", async () => { + const destroyError = new Error("delete failed"); + const user = { + destroy: jest.fn().mockRejectedValue(destroyError) + } as unknown as User; + + await expect(service.delete(user)).rejects.toThrow(destroyError); + expect(user.destroy).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/apps/user-service/src/users/users.service.ts b/apps/user-service/src/users/users.service.ts index d201d82d7..a8a48a99a 100644 --- a/apps/user-service/src/users/users.service.ts +++ b/apps/user-service/src/users/users.service.ts @@ -1,4 +1,4 @@ -import { BadRequestException, NotFoundException, Injectable } from "@nestjs/common"; +import { BadRequestException, Injectable } from "@nestjs/common"; import { Op } from "sequelize"; import { Framework, FrameworkUser, ModelHasRole, Role, User } from "@terramatch-microservices/database/entities"; import { PaginatedQueryBuilder } from "@terramatch-microservices/common/util/paginated-query.builder"; @@ -81,14 +81,48 @@ export class UsersService { } async update(user: User, update: UserUpdateAttributes) { + let organisationEntity: Organisation | null = null; + let frameworkEntities: Framework[] = []; + let roleEntity: Role | null = null; if (update.organisationUuid != null) { - const organisation = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); - if (organisation == null) { - throw new NotFoundException("Organisation not found"); + organisationEntity = await Organisation.findOne({ where: { uuid: update.organisationUuid } }); + if (organisationEntity == null) { + throw new BadRequestException("Organisation not found"); } - user.organisationId = organisation.id; } + if (update.directFrameworks != null) { + frameworkEntities = await Framework.findAll({ where: { slug: update.directFrameworks } }); + if (frameworkEntities.length !== update.directFrameworks.length) { + throw new BadRequestException("One or more frameworks not found"); + } + } + if (update.primaryRole != null) { + roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); + if (roleEntity == null) { + throw new BadRequestException("Role not found"); + } + } + + if (update.directFrameworks != null) { + const userPreviousFrameworks = user.frameworks ?? []; + const frameworksToDelete = userPreviousFrameworks.filter( + framework => !frameworkEntities.some(entity => entity.id === framework.id) + ); + const frameworksToAdd = frameworkEntities.filter( + framework => !userPreviousFrameworks.some(previousFramework => previousFramework.id === framework.id) + ); + if (frameworksToDelete.length > 0) { + await FrameworkUser.destroy({ + where: { userId: user.id, frameworkId: { [Op.in]: frameworksToDelete.map(framework => framework.id) } } + }); + } + for (const framework of frameworksToAdd) { + await FrameworkUser.findOrCreate({ where: { userId: user.id, frameworkId: framework.id } }); + } + } + + user.organisationId = organisationEntity?.id ?? user.organisationId; user.firstName = update.firstName ?? user.firstName; user.lastName = update.lastName ?? user.lastName; user.emailAddress = update.emailAddress ?? user.emailAddress; @@ -98,28 +132,13 @@ export class UsersService { user.program = update.program ?? user.program; user.locale = (update.locale as ValidLocale | undefined) ?? user.locale; - if (update.directFrameworks != null) { - await FrameworkUser.destroy({ where: { userId: user.id } }); - for (const slug of update.directFrameworks) { - const framework = await Framework.findOne({ where: { slug } }); - if (framework == null) { - throw new NotFoundException("Framework not found"); - } - await FrameworkUser.findOrCreate({ - where: { frameworkId: framework.id, userId: user.id } - }); - } - } user = await user.save(); - if (update.primaryRole != null) { - const roleEntity = await Role.findOne({ where: { name: update.primaryRole } }); - if (roleEntity == null) { - throw new NotFoundException("Role not found"); - } + + if (roleEntity != null) { await ModelHasRole.destroy({ where: { modelId: user.id, modelType: User.LARAVEL_TYPE } }); await ModelHasRole.findOrCreate({ - where: { modelId: user.id, roleId: roleEntity.id }, - defaults: { modelId: user.id, roleId: roleEntity.id, modelType: User.LARAVEL_TYPE } as ModelHasRole + where: { modelId: user.id, roleId: roleEntity?.id }, + defaults: { modelId: user.id, roleId: roleEntity?.id, modelType: User.LARAVEL_TYPE } as ModelHasRole }); await user.reload(); } From 9522dc8e4ad99aabc84d3b9006785e9a8fe30e06 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Fri, 27 Mar 2026 14:44:41 -0400 Subject: [PATCH 19/20] [TM-3071] change not found exceptions --- .../src/users/user-creation.service.spec.ts | 4 ++-- .../user-service/src/users/user-creation.service.ts | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/apps/user-service/src/users/user-creation.service.spec.ts b/apps/user-service/src/users/user-creation.service.spec.ts index 3957e3f9d..72ef35bb1 100644 --- a/apps/user-service/src/users/user-creation.service.spec.ts +++ b/apps/user-service/src/users/user-creation.service.spec.ts @@ -333,11 +333,11 @@ describe("UserCreationService", () => { return request; }; - it("should throw NotFoundException when token is invalid", async () => { + it("should throw BadRequestException when token is invalid", async () => { const request = getInviteRequest("test@example.com", "invalid-token"); jest.spyOn(PasswordReset, "findOne").mockResolvedValue(null); - await expect(service.createNewUser(false, request)).rejects.toThrow(NotFoundException); + await expect(service.createNewUser(false, request)).rejects.toThrow(BadRequestException); }); it("should complete invite signup with organisation invite", async () => { diff --git a/apps/user-service/src/users/user-creation.service.ts b/apps/user-service/src/users/user-creation.service.ts index 7d0284031..eec2fdf9c 100644 --- a/apps/user-service/src/users/user-creation.service.ts +++ b/apps/user-service/src/users/user-creation.service.ts @@ -2,7 +2,6 @@ import { BadRequestException, Injectable, InternalServerErrorException, - NotFoundException, UnprocessableEntityException } from "@nestjs/common"; import { @@ -69,7 +68,7 @@ export class UserCreationService { const roleEntity = await Role.findOne({ where: { name: role } }); if (roleEntity == null) { - throw new NotFoundException("Role not found"); + throw new BadRequestException("Role not found"); } const userExists = (await User.count({ where: { emailAddress: request.emailAddress } })) !== 0; @@ -112,7 +111,7 @@ export class UserCreationService { attributes: ["id"] }); if (organisation == null) { - throw new NotFoundException("Organisation not found"); + throw new BadRequestException("Organisation not found"); } const userExists = (await User.count({ where: { emailAddress: request.emailAddress } })) !== 0; if (userExists) { @@ -120,11 +119,11 @@ export class UserCreationService { } const roleEntity = await Role.findOne({ where: { name: request.role } }); if (roleEntity == null) { - throw new NotFoundException("Role not found"); + throw new BadRequestException("Role not found"); } const frameworkEntities = await Framework.findAll({ where: { slug: request.directFrameworks } }); if (frameworkEntities.length !== request.directFrameworks.length) { - throw new NotFoundException("One or more frameworks not found"); + throw new BadRequestException("One or more frameworks not found"); } try { const newUser = omit(request, ["role", "organisationUuid", "directFrameworks"]); @@ -158,7 +157,7 @@ export class UserCreationService { }); if (passwordReset == null || passwordReset.user == null) { - throw new NotFoundException("Invalid signup token"); + throw new BadRequestException("Invalid signup token"); } const user = passwordReset.user; @@ -226,7 +225,7 @@ export class UserCreationService { const role = "project-developer"; const roleEntity = await Role.findOne({ where: { name: role } }); if (roleEntity == null) { - throw new NotFoundException("Role not found"); + throw new BadRequestException("Role not found"); } await ModelHasRole.findOrCreate({ From 56c220fedfcc82cbeb9fae9372df156e800d6c03 Mon Sep 17 00:00:00 2001 From: Jose Carlos Laura Ramirez Date: Fri, 27 Mar 2026 14:45:20 -0400 Subject: [PATCH 20/20] [TM-3071] remove md files --- AGENTS.md | 18 ------------------ CLAUDE.md | 1 - 2 files changed, 19 deletions(-) delete mode 100644 AGENTS.md delete mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md deleted file mode 100644 index 3926402e1..000000000 --- a/AGENTS.md +++ /dev/null @@ -1,18 +0,0 @@ -# AGENTS.md – AI Coding Instructions for this NestJS + TypeScript repo - -## Controller Guidelines (strictly enforced) - -- All controllers MUST follow REST principles only. Never use RPC-style method names (no `getUserById`, `createOrder`, etc.). -- Method names must be standard HTTP verbs + resource: `findAll`, `findOne`, `create`, `update`, `remove`, etc. -- Every controller method MUST include these decorators in this order: - - `@ApiOperation({ summary: "..." })` - - `@ApiOkResponse({ type: ... })` or appropriate `@ApiResponse` - - `@ApiBadRequestResponse({ description: "..." })`, `@ApiUnauthorizedResponse`, etc. for exceptions -- Every controller method MUST call `this.policyService.checkPermission(...)` (or equivalent) with the appropriate permission before any business logic. -- Every response MUST use the local `buildJsonApi(...)` helper (import from `@/common/helpers/json-api.helper` or wherever it lives). Never return raw objects or DTOs directly. - -## General Rules - -- Always respect existing folder structure (controllers in `src/modules/*/controllers/`, etc.). -- Use NestJS best practices for guards, interceptors, and pipes. -- Never invent new patterns – follow the established ones in the codebase. diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 141137e7f..000000000 --- a/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -@import AGENTS.md