fix: address review comments

safe-global · Feb 13, 2025 · 7b1474a · 7b1474a
1 parent c87c2b6
commit 7b1474a
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 61 deletions.
diff --git a/src/domain/users/user-organizations.repository.interface.ts b/src/domain/users/user-organizations.repository.interface.ts
@@ -56,7 +56,7 @@ export interface IUsersOrganizationsRepository {
     orgId: Organization['id'];
   }): Promise<void>;
 
-  findAuthorizedUserOrgs(args: {
+  findAuthorizedUserOrgsOrFail(args: {
     authPayload: AuthPayload;
     orgId: Organization['id'];
   }): Promise<Array<UserOrganization>>;

diff --git a/src/domain/users/user-organizations.repository.ts b/src/domain/users/user-organizations.repository.ts
@@ -11,7 +11,6 @@ import { IOrganizationsRepository } from '@/domain/organizations/organizations.r
 import { AuthPayload } from '@/domain/auth/entities/auth-payload.entity';
 import { IUsersRepository } from '@/domain/users/users.repository.interface';
 import { UserOrganization as DbUserOrganization } from '@/datasources/users/entities/user-organizations.entity.db';
-import { IConfigurationService } from '@/config/configuration.service.interface';
 import { IWalletsRepository } from '@/domain/wallets/wallets.repository.interface';
 import { In } from 'typeorm';
 import type {
@@ -30,8 +29,6 @@ import { type UserOrganization } from '@/domain/users/entities/user-organization
 export class UsersOrganizationsRepository
   implements IUsersOrganizationsRepository
 {
-  private readonly maxInvites: number;
-
   constructor(
     private readonly postgresDatabaseService: PostgresDatabaseService,
     @Inject(IUsersRepository)
@@ -40,12 +37,7 @@ export class UsersOrganizationsRepository
     private readonly organizationsRepository: IOrganizationsRepository,
     @Inject(IWalletsRepository)
     private readonly walletsRepository: IWalletsRepository,
-    @Inject(IConfigurationService)
-    private readonly configurationService: IConfigurationService,
-  ) {
-    this.maxInvites =
-      this.configurationService.getOrThrow<number>('users.maxInvites');
-  }
+  ) {}
 
   public async findOneOrFail(
     where:
@@ -106,10 +98,6 @@ export class UsersOrganizationsRepository
       role: UserOrganization['role'];
     }>;
   }): Promise<Array<Invitation>> {
-    if (args.users.length > this.maxInvites) {
-      throw new ConflictException('Too many invites.');
-    }
-
     this.assertSignerAddress(args.authPayload);
 
     const user = await this.usersRepository.findByWalletAddressOrFail(
@@ -250,7 +238,7 @@ export class UsersOrganizationsRepository
     });
   }
 
-  public async findAuthorizedUserOrgs(args: {
+  public async findAuthorizedUserOrgsOrFail(args: {
     authPayload: AuthPayload;
     orgId: Organization['id'];
   }): Promise<Array<UserOrganization>> {

diff --git a/...ganizations/entities/user-organization.ts → ...entities/user-organizations.dto.entity.ts b/...ganizations/entities/user-organization.ts → ...entities/user-organizations.dto.entity.ts
@@ -34,7 +34,7 @@ class UserOrganization {
   user!: UserOrganizationUser;
 }
 
-export class Members {
+export class UserOrganizationsDto {
   @ApiProperty({ type: UserOrganization, isArray: true })
   members!: Array<UserOrganization>;
 }
diff --git a/src/routes/organizations/user-organizations.controller.spec.ts b/src/routes/organizations/user-organizations.controller.spec.ts
@@ -197,7 +197,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/invite`)
-        // No auth cookie
         .send([
           {
             role: 'ADMIN',
@@ -236,7 +235,6 @@ describe('UserOrganizationsController', () => {
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/invite`)
         .set('Cookie', [`access_token=${accessToken}`])
-        // No addresses
         .send([])
         .expect(422);
     });
@@ -264,7 +262,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/invite`)
-        // Non-user auth
         .set('Cookie', [`access_token=${nonUserAccessToken}`])
         .send([
           {
@@ -299,8 +296,6 @@ describe('UserOrganizationsController', () => {
         .set('Cookie', [`access_token=${accessToken}`])
         .expect(201);
 
-      // Don't create Organization
-
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/invite`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -350,7 +345,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/invite`)
-        // Non-user org auth
         .set('Cookie', [`access_token=${nonUserOrgAccessToken}`])
         .send([
           {
@@ -488,7 +482,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/accept`)
-        // No auth cookie
         .expect(403)
         .expect({
           message: 'Forbidden resource',
@@ -516,8 +509,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const orgId = createOrganizationResponse.body.id;
 
-      // Don't create UserOrganization
-
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/accept`)
         .set('Cookie', [`access_token=${nonUserAccessToken}`])
@@ -542,8 +533,6 @@ describe('UserOrganizationsController', () => {
         .set('Cookie', [`access_token=${accessToken}`])
         .expect(201);
 
-      // Don't create Organization or UserOrganization
-
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/accept`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -719,7 +708,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/decline`)
-        // No auth cookie
         .expect(403)
         .expect({
           message: 'Forbidden resource',
@@ -747,8 +735,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const orgId = createOrganizationResponse.body.id;
 
-      // Don't create UserOrganization
-
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/decline`)
         .set('Cookie', [`access_token=${nonUserAccessToken}`])
@@ -773,8 +759,6 @@ describe('UserOrganizationsController', () => {
         .set('Cookie', [`access_token=${accessToken}`])
         .expect(201);
 
-      // Don't create Organization or UserOrganization
-
       await request(app.getHttpServer())
         .post(`/v1/organizations/${orgId}/members/decline`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -997,7 +981,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .get(`/v1/organizations/${orgId}/members`)
-        // No auth cookie
         .expect(403)
         .expect({
           message: 'Forbidden resource',
@@ -1025,8 +1008,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const orgId = createOrganizationResponse.body.id;
 
-      // Don't create UserOrganization(s)
-
       await request(app.getHttpServer())
         .get(`/v1/organizations/${orgId}/members`)
         .set('Cookie', [`access_token=${nonUserAccessToken}`])
@@ -1051,8 +1032,6 @@ describe('UserOrganizationsController', () => {
         .set('Cookie', [`access_token=${accessToken}`])
         .expect(201);
 
-      // Don't create Organization or UserOrganization(s)
-
       await request(app.getHttpServer())
         .get(`/v1/organizations/${orgId}/members`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -1148,7 +1127,6 @@ describe('UserOrganizationsController', () => {
 
       await request(app.getHttpServer())
         .patch(`/v1/organizations/${orgId}/members/${userId}/role`)
-        // No auth cookie
         .send({ role: 'ADMIN' })
         .expect(403)
         .expect({
@@ -1239,8 +1217,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const userId = inviteUsersResponse.body[0].userId;
 
-      // Don't accept invite
-
       await request(app.getHttpServer())
         .patch(`/v1/organizations/${orgId}/members/${userId}/role`)
         .set('Cookie', [`access_token=${inviteeAccessToken}`])
@@ -1356,8 +1332,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const orgId = createOrganizationResponse.body.id;
 
-      // Don't create UserOrganization or accept invite
-
       await request(app.getHttpServer())
         .patch(`/v1/organizations/${orgId}/members/${userId}/role`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -1438,8 +1412,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const orgId = createOrganizationResponse.body.id;
 
-      // No need to create UserOrganization or accept invite
-
       await request(app.getHttpServer())
         .delete(`/v1/organizations/${orgId}/members/${userId}`)
         .set('Cookie', [`access_token=${nonUserAccessToken}`])
@@ -1465,8 +1437,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const userId = createUserResponse.body.id;
 
-      // Don't create Organization or UserOrganization
-
       await request(app.getHttpServer())
         .delete(`/v1/organizations/${orgId}/members/${userId}`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -1514,8 +1484,6 @@ describe('UserOrganizationsController', () => {
         .expect(201);
       const memberUserId = inviteUsersResponse.body[1].userId;
 
-      // Don't accept invite
-
       await request(app.getHttpServer())
         .delete(`/v1/organizations/${orgId}/members/${memberUserId}`)
         .set('Cookie', [`access_token=${inviteeAccessToken}`])

diff --git a/src/routes/organizations/user-organizations.controller.ts b/src/routes/organizations/user-organizations.controller.ts
@@ -25,7 +25,7 @@ import { ValidationPipe } from '@/validation/pipes/validation.pipe';
 import { InviteUsersDtoSchema } from '@/routes/organizations/entities/invite-users.dto.entity';
 import { UpdateRoleDtoSchema } from '@/routes/organizations/entities/update-role.dto.entity';
 import { RowSchema } from '@/datasources/db/v1/entities/row.entity';
-import { Members } from '@/routes/organizations/entities/user-organization';
+import { UserOrganizationsDto } from '@/routes/organizations/entities/user-organizations.dto.entity';
 import { Invitation } from '@/routes/organizations/entities/invitation.entity';
 import type { AuthPayload } from '@/domain/auth/entities/auth-payload.entity';
 import type { Organization } from '@/domain/organizations/entities/organization.entity';
@@ -48,11 +48,10 @@ export class UserOrganizationsController {
   })
   @ApiConflictResponse({ description: 'Too many invites' })
   @ApiForbiddenResponse({ description: 'User not authorized' })
-  @ApiNotFoundResponse({
+  @ApiUnauthorizedResponse({
     description:
       'User not admin OR signer address not provided OR member is not active',
   })
-  @ApiUnauthorizedResponse({ description: 'User not admin' })
   @Post('/:orgId/members/invite')
   @UseGuards(AuthGuard)
   public async inviteUser(
@@ -109,7 +108,7 @@ export class UserOrganizationsController {
 
   @ApiOkResponse({
     description: 'Organization and members list',
-    type: Members,
+    type: UserOrganizationsDto,
   })
   @ApiForbiddenResponse({ description: 'Signer not authorized' })
   @ApiNotFoundResponse({
@@ -121,7 +120,7 @@ export class UserOrganizationsController {
     @Auth() authPayload: AuthPayload,
     @Param('orgId', ParseIntPipe, new ValidationPipe(RowSchema.shape.id))
     orgId: Organization['id'],
-  ): Promise<Members> {
+  ): Promise<UserOrganizationsDto> {
     return await this.userOrgService.get({
       authPayload,
       orgId,

diff --git a/src/routes/organizations/user-organizations.service.ts b/src/routes/organizations/user-organizations.service.ts
@@ -1,24 +1,35 @@
-import { Inject } from '@nestjs/common';
+import { ConflictException, Inject } from '@nestjs/common';
 import { IUsersOrganizationsRepository } from '@/domain/users/user-organizations.repository.interface';
 import { User } from '@/domain/users/entities/user.entity';
+import { IConfigurationService } from '@/config/configuration.service.interface';
 import type { AuthPayload } from '@/domain/auth/entities/auth-payload.entity';
 import type { Organization } from '@/domain/organizations/entities/organization.entity';
 import type { InviteUsersDto } from '@/routes/organizations/entities/invite-users.dto.entity';
 import type { Invitation } from '@/routes/organizations/entities/invitation.entity';
-import type { Members } from '@/routes/organizations/entities/user-organization';
+import type { UserOrganizationsDto } from '@/routes/organizations/entities/user-organizations.dto.entity';
 import type { UpdateRoleDto } from '@/routes/organizations/entities/update-role.dto.entity';
 
 export class UserOrganizationsService {
+  private readonly maxInvites: number;
   public constructor(
     @Inject(IUsersOrganizationsRepository)
     private readonly usersOrgRepository: IUsersOrganizationsRepository,
-  ) {}
+    @Inject(IConfigurationService)
+    private readonly configurationService: IConfigurationService,
+  ) {
+    this.maxInvites =
+      this.configurationService.getOrThrow<number>('users.maxInvites');
+  }
 
   public async inviteUser(args: {
     authPayload: AuthPayload;
     orgId: Organization['id'];
     inviteUsersDto: InviteUsersDto;
   }): Promise<Array<Invitation>> {
+    if (args.inviteUsersDto.length > this.maxInvites) {
+      throw new ConflictException('Too many invites.');
+    }
+
     return await this.usersOrgRepository.inviteUsers({
       authPayload: args.authPayload,
       orgId: args.orgId,
@@ -43,11 +54,13 @@ export class UserOrganizationsService {
   public async get(args: {
     authPayload: AuthPayload;
     orgId: Organization['id'];
-  }): Promise<Members> {
-    const userOrgs = await this.usersOrgRepository.findAuthorizedUserOrgs({
-      authPayload: args.authPayload,
-      orgId: args.orgId,
-    });
+  }): Promise<UserOrganizationsDto> {
+    const userOrgs = await this.usersOrgRepository.findAuthorizedUserOrgsOrFail(
+      {
+        authPayload: args.authPayload,
+        orgId: args.orgId,
+      },
+    );
 
     return {
       members: userOrgs.map((userOrg) => {