Skip to content

Commit

Permalink
fix: address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
iamacook committed Feb 12, 2025
1 parent e5662e8 commit cfd384b
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 43 deletions.
3 changes: 3 additions & 0 deletions src/config/entities/__tests__/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,7 @@ export default (): ReturnType<typeof configuration> => ({
},
},
},
users: {
maxInvites: faker.number.int({ min: 5, max: 10 }),
},
});
3 changes: 3 additions & 0 deletions src/config/entities/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,4 +431,7 @@ export default () => ({
},
},
},
users: {
maxInvites: 50,
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export interface IUsersOrganizationsRepository {
orgId: Organization['id'];
}): Promise<void>;

get(args: {
findAuthorizedUserOrgs(args: {
authPayload: AuthPayload;
orgId: Organization['id'];
}): Promise<Array<UserOrganization>>;
Expand Down
72 changes: 53 additions & 19 deletions src/domain/users/user-organizations.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,25 @@ import { IUsersRepository } from '@/domain/users/users.repository.interface';
import { UserOrganization as DbUserOrganization } from '@/datasources/users/entities/user-organizations.entity.db';
import { IWalletsRepository } from '@/domain/wallets/wallets.repository.interface';
import { In } from 'typeorm';
import type { FindOptionsWhere, FindOptionsRelations } from 'typeorm';
import type {
FindOptionsWhere,
FindOptionsRelations,
EntityManager,
} from 'typeorm';
import type { IUsersOrganizationsRepository } from '@/domain/users/user-organizations.repository.interface';
import { UserOrganizationRole } from '@/domain/users/entities/user-organization.entity';
import type { Organization } from '@/domain/organizations/entities/organization.entity';
import type { User } from '@/domain/users/entities/user.entity';
import type { UserOrganization } from '@/domain/users/entities/user-organization.entity';
import type { Invitation } from '@/domain/users/entities/invitation.entity';
import { IConfigurationService } from '@/config/configuration.service.interface';

@Injectable()
export class UsersOrganizationsRepository
implements IUsersOrganizationsRepository
{
private readonly maxInvites: number;

constructor(
private readonly postgresDatabaseService: PostgresDatabaseService,
@Inject(IUsersRepository)
Expand All @@ -33,7 +40,12 @@ 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:
Expand Down Expand Up @@ -73,7 +85,11 @@ export class UsersOrganizationsRepository
role: UserOrganization['role'];
}>;
}): Promise<Array<Invitation>> {
const signer = await this.getOrgAndSignerOrFail(args);
if (args.users.length > this.maxInvites) {
throw new ConflictException('Too many invites.');
}

const signer = await this.findSignerAndOrgOrFail(args);

this.assertUserOrgIsActive(signer.userOrg);

Expand All @@ -91,13 +107,15 @@ export class UsersOrganizationsRepository

await this.postgresDatabaseService.transaction(async (entityManager) => {
for (const userToInvite of args.users) {
// Find existing User via Wallet
const invitedUser = invitedUsers.find(({ user }) => {
return user.wallets.some((wallet) => {
return isAddressEqual(wallet.address, userToInvite.address);
});
});
let invitedUserId = invitedUser?.user.id;

// Otherwise create User and Wallet
if (!invitedUserId) {
invitedUserId = await this.usersRepository.create(
'PENDING',
Expand Down Expand Up @@ -136,20 +154,20 @@ export class UsersOrganizationsRepository
authPayload: AuthPayload;
orgId: Organization['id'];
}): Promise<void> {
const signer = await this.getOrgAndSignerOrFail(args);
const signer = await this.findSignerAndOrgOrFail(args);

this.assertUserOrgIsInvited(signer.userOrg);

await this.postgresDatabaseService.transaction(async (entityManager) => {
await entityManager.update(DbUserOrganization, signer.userOrg.id, {
await this.updateStatus({
userOrgId: signer.userOrg.id,
status: 'ACTIVE',
entityManager,
});

await this.usersRepository.update({
await this.usersRepository.updateStatus({
userId: signer.user.id,
user: {
status: 'ACTIVE',
},
status: 'ACTIVE',
entityManager,
});
});
Expand All @@ -159,22 +177,34 @@ export class UsersOrganizationsRepository
authPayload: AuthPayload;
orgId: Organization['id'];
}): Promise<void> {
const signer = await this.getOrgAndSignerOrFail(args);
const signer = await this.findSignerAndOrgOrFail(args);

this.assertUserOrgIsInvited(signer.userOrg);

const userOrganizationRepository =
await this.postgresDatabaseService.getRepository(DbUserOrganization);
await userOrganizationRepository.update(signer.userOrg.id, {
status: 'DECLINED',
await this.postgresDatabaseService.transaction(async (entityManager) => {
await this.updateStatus({
userOrgId: signer.userOrg.id,
status: 'DECLINED',
entityManager,
});
});
}

private async updateStatus(args: {
userOrgId: UserOrganization['id'];
status: UserOrganization['status'];
entityManager: EntityManager;
}): Promise<void> {
await args.entityManager.update(DbUserOrganization, args.userOrgId, {
status: args.status,
});
}

public async get(args: {
public async findAuthorizedUserOrgs(args: {
authPayload: AuthPayload;
orgId: Organization['id'];
}): Promise<Array<UserOrganization>> {
const signer = await this.getOrgAndSignerOrFail(args);
const signer = await this.findSignerAndOrgOrFail(args);
return signer.org.userOrganizations;
}

Expand All @@ -184,7 +214,7 @@ export class UsersOrganizationsRepository
userId: User['id'];
role: UserOrganization['role'];
}): Promise<void> {
const signer = await this.getOrgAndSignerOrFail(args);
const signer = await this.findSignerAndOrgOrFail(args);

this.assertUserOrgIsActive(signer.userOrg);
this.assertUserOrgAdmin(signer.userOrg);
Expand All @@ -210,7 +240,7 @@ export class UsersOrganizationsRepository
userId: User['id'];
orgId: Organization['id'];
}): Promise<void> {
const signer = await this.getOrgAndSignerOrFail(args);
const signer = await this.findSignerAndOrgOrFail(args);

this.assertUserOrgIsActive(signer.userOrg);
this.assertUserOrgAdmin(signer.userOrg);
Expand All @@ -226,7 +256,11 @@ export class UsersOrganizationsRepository
await userOrganizationRepository.delete(updateUserOrg.id);
}

private async getOrgAndSignerOrFail(args: {
// The following helper is used across every above method but they don't
// necessarily require all orgs, e.g. some only invited, others active
// TODO: Revisit implementation, maybe splitting into method-specific ones
// that use WHERE clauses instead of assertions
private async findSignerAndOrgOrFail(args: {
authPayload: AuthPayload;
orgId: Organization['id'];
}): Promise<{
Expand Down
8 changes: 7 additions & 1 deletion src/domain/users/users.repository.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,14 @@ export interface IUsersRepository {
findByWalletAddress(address: `0x${string}`): Promise<User | undefined>;

update(args: {
userId: number;
userId: User['id'];
user: Partial<User>;
entityManager: EntityManager;
}): Promise<void>;

updateStatus(args: {
userId: User['id'];
status: User['status'];
entityManager: EntityManager;
}): Promise<void>;
}
18 changes: 17 additions & 1 deletion src/domain/users/users.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,28 @@ export class UsersRepository implements IUsersRepository {
}

public async update(args: {
user: Partial<DbUser> & { id: User['id'] };
userId: User['id'];
user: Partial<User>;
entityManager: EntityManager;
}): Promise<void> {
await args.entityManager.update(DbUser, args.user.id, args.user);
}

public async updateStatus(args: {
userId: User['id'];
status: User['status'];
entityManager: EntityManager;
}): Promise<void> {
await this.update({
userId: args.userId,
user: {
id: args.userId,
status: args.status,
},
entityManager: args.entityManager,
});
}

private assertSignerAddress(
authPayload: AuthPayload,
): asserts authPayload is AuthPayload & { signer_address: `0x${string}` } {
Expand Down

This file was deleted.

5 changes: 1 addition & 4 deletions src/routes/organizations/entities/invite-users.dto.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ import { UserOrganizationRole } from '@/domain/users/entities/user-organization.
import { AddressSchema } from '@/validation/entities/schemas/address.schema';
import { getStringEnumKeys } from '@/domain/common/utils/enum';

const MAX_INVITE_USERS = 50;

export const InviteUsersDtoSchema = z
.array(
z.object({
address: AddressSchema,
role: z.enum(getStringEnumKeys(UserOrganizationRole)),
}),
)
.min(1)
.max(MAX_INVITE_USERS);
.min(1);

export type InviteUsersDto = z.infer<typeof InviteUsersDtoSchema>;
4 changes: 2 additions & 2 deletions src/routes/organizations/entities/user-organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ class UserOrganization {
status!: keyof typeof UserOrganizationStatus;

@ApiProperty()
createdAt!: string;
createdAt!: Date;

@ApiProperty()
updatedAt!: string;
updatedAt!: Date;

@ApiProperty({ type: UserOrganizationUser })
user!: UserOrganizationUser;
Expand Down
32 changes: 31 additions & 1 deletion src/routes/organizations/user-organizations.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ import { authPayloadDtoBuilder } from '@/domain/auth/entities/__tests__/auth-pay
import { DB_MAX_SAFE_INTEGER } from '@/domain/common/constants';
import type { INestApplication } from '@nestjs/common';
import type { Server } from 'net';
import { IConfigurationService } from '@/config/configuration.service.interface';

describe('UserOrganizationsController', () => {
let app: INestApplication<Server>;
let jwtService: IJwtService;
let maxInvites: number;

beforeEach(async () => {
jest.resetAllMocks();
Expand Down Expand Up @@ -77,6 +79,10 @@ describe('UserOrganizationsController', () => {
.compile();

jwtService = moduleFixture.get<IJwtService>(IJwtService);
const configService = moduleFixture.get<IConfigurationService>(
IConfigurationService,
);
maxInvites = configService.getOrThrow('users.maxInvites');

app = await new TestAppProvider().provide(moduleFixture);
await app.init();
Expand Down Expand Up @@ -147,6 +153,29 @@ describe('UserOrganizationsController', () => {
);
});

it('should throw a 409 if there are too many invites', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);
const orgId = faker.number.int();
const invites = Array.from({ length: maxInvites + 1 }).map(() => {
return {
role: faker.helpers.arrayElement(['ADMIN', 'MEMBER']),
address: getAddress(faker.finance.ethereumAddress()),
};
});

await request(app.getHttpServer())
.post(`/v1/organizations/${orgId}/members/invite`)
.set('Cookie', [`access_token=${accessToken}`])
.send(invites)
.expect(409)
.expect({
message: 'Too many invites.',
error: 'Conflict',
statusCode: 409,
});
});

it('should throw a 403 if the user is not authenticated', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);
Expand Down Expand Up @@ -1035,6 +1064,7 @@ describe('UserOrganizationsController', () => {
});
});

// TODO: Investigate
it('should throw a 404 if the user organization does not exist', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);
Expand Down Expand Up @@ -1114,7 +1144,7 @@ describe('UserOrganizationsController', () => {
.patch(`/v1/organizations/${orgId}/members/${userId}/role`)
.set('Cookie', [`access_token=${accessToken}`])
.send({ role: 'ADMIN' })
.expect(201)
.expect(200)
.expect({});
});

Expand Down
1 change: 1 addition & 0 deletions src/routes/organizations/user-organizations.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class UserOrganizationsController {
type: Invitation,
isArray: true,
})
@ApiConflictResponse({ description: 'Too many invites' })
@ApiForbiddenResponse({ description: 'User not authorized' })
@ApiNotFoundResponse({
description:
Expand Down
6 changes: 3 additions & 3 deletions src/routes/organizations/user-organizations.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class UserOrganizationsService {
authPayload: AuthPayload;
orgId: Organization['id'];
}): Promise<Members> {
const userOrgs = await this.usersOrgRepository.get({
const userOrgs = await this.usersOrgRepository.findAuthorizedUserOrgs({
authPayload: args.authPayload,
orgId: args.orgId,
});
Expand All @@ -55,8 +55,8 @@ export class UserOrganizationsService {
id: userOrg.id,
role: userOrg.role,
status: userOrg.status,
createdAt: userOrg.createdAt.toISOString(),
updatedAt: userOrg.updatedAt.toISOString(),
createdAt: userOrg.createdAt,
updatedAt: userOrg.updatedAt,
user: {
id: userOrg.user.id,
status: userOrg.user.status,
Expand Down

0 comments on commit cfd384b

Please sign in to comment.