Skip to content

Commit

Permalink
fix: further tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@iamacook
iamacook committed Jan 31, 2025
1 parent 78e2ed8 commit c6a9410
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 58 deletions.
45 changes: 33 additions & 12 deletions src/domain/users/users.repository.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
BadRequestException,
ConflictException,
Injectable,
NotFoundException,
Expand Down Expand Up @@ -98,11 +97,29 @@ export class UsersRepository implements IUsersRepository {

const userRepository =
await this.postgresDatabaseService.getRepository(DbUser);
// TODO: Revert to using singular delete when solving join
const walletRepository =
await this.postgresDatabaseService.getRepository(Wallet);

const wallet = await walletRepository.findOne({
where: { address: authPayload.signer_address },
relations: { user: true },
select: {
user: {
id: true,
},
},
});

if (!wallet?.user) {
throw new NotFoundException('User not found');
}

const deleteResult = await userRepository.delete({
wallets: { address: authPayload.signer_address },
id: wallet.user.id,
});

// TODO: Check if possible - a todo test case remains for this
if (!deleteResult.affected) {
throw new ConflictException(
`Could not delete user. Wallet=${authPayload.signer_address}`,
Expand All @@ -120,27 +137,31 @@ export class UsersRepository implements IUsersRepository {
throw new ConflictException('Cannot remove the current wallet');
}

const userRepository =
await this.postgresDatabaseService.getRepository(DbUser);
// TODO: Revert to finding user by wallet when solving join
const walletRepository =
await this.postgresDatabaseService.getRepository(Wallet);

const user = await userRepository.findOne({
where: { wallets: { address: args.authPayload.signer_address } },
relations: { wallets: true },
const wallet = await walletRepository.findOne({
where: { address: args.authPayload.signer_address },
relations: { user: true },
select: {
user: {
id: true,
wallets: true,
},
},
});

if (!user) {
if (!wallet) {
// TODO: If we remain with finding wallet with users, update this message and Swagger
throw new NotFoundException('User not found');
}

if (user.wallets.length === 1) {
throw new BadRequestException('Cannot delete the last wallet of a user');
}
// We don't need to check if the wallet is the last one, as we can't delete the current wallet

const deleteResult = await walletRepository.delete({
address: args.walletAddress,
user: { id: user.id },
user: { id: wallet.user.id },
});

if (!deleteResult.affected) {
Expand Down
104 changes: 62 additions & 42 deletions src/routes/users/users.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ import { PostgresDatabaseModule } from '@/datasources/db/v1/postgres-database.mo
import { IJwtService } from '@/datasources/jwt/jwt.service.interface';
import { TestNetworkModule } from '@/datasources/network/__tests__/test.network.module';
import { NetworkModule } from '@/datasources/network/network.module';
import { NetworkService } from '@/datasources/network/network.service.interface';
import { TestQueuesApiModule } from '@/datasources/queues/__tests__/test.queues-api.module';
import { QueuesApiModule } from '@/datasources/queues/queues-api.module';
import { TestTargetedMessagingDatasourceModule } from '@/datasources/targeted-messaging/__tests__/test.targeted-messaging.datasource.module';
import { TargetedMessagingDatasourceModule } from '@/datasources/targeted-messaging/targeted-messaging.datasource.module';
import { NotificationsRepositoryV2Module } from '@/domain/notifications/v2/notifications.repository.module';
import { IUsersRepository } from '@/domain/users/users.repository.interface';
import { TestNotificationsRepositoryV2Module } from '@/domain/notifications/v2/test.notification.repository.module';
import { TestLoggingModule } from '@/logging/__tests__/test.logging.module';
import { RequestScopedLoggingModule } from '@/logging/logging.module';
Expand All @@ -34,13 +32,10 @@ import { checkGuardIsApplied } from '@/__tests__/util/check-guard';
import { AuthGuard } from '@/routes/auth/guards/auth.guard';
import type { INestApplication } from '@nestjs/common';
import type { Server } from 'net';
import type { INetworkService } from '@/datasources/network/network.service.interface';

describe('UsersController', () => {
let app: INestApplication<Server>;
let jwtService: IJwtService;
let networkService: jest.MockedObjectDeep<INetworkService>;
let usersRepository: IUsersRepository;

beforeEach(async () => {
jest.resetAllMocks();
Expand Down Expand Up @@ -81,8 +76,6 @@ describe('UsersController', () => {
.compile();

jwtService = moduleFixture.get<IJwtService>(IJwtService);
networkService = moduleFixture.get(NetworkService);
usersRepository = moduleFixture.get(IUsersRepository);

app = await new TestAppProvider().provide(moduleFixture);
await app.init();
Expand Down Expand Up @@ -115,16 +108,18 @@ describe('UsersController', () => {
.get('/v1/users')
.set('Cookie', [`access_token=${accessToken}`])
.expect(200)
.expect({
id: 1,
status: 1,
wallets: [
{
id: 1,
address: authPayloadDto.signer_address,
},
],
});
.expect(({ body }) =>
expect(body).toEqual({
id: expect.any(Number),
status: 1,
wallets: [
{
id: expect.any(Number),
address: authPayloadDto.signer_address,
},
],
}),
);
});

// Note: we could extensively test JWT validity but it is covered in the AuthGuard tests
Expand Down Expand Up @@ -168,13 +163,11 @@ describe('UsersController', () => {
});

describe('DELETE /v1/users', () => {
// TODO: Check wallet/user entities are removed in integration test (and other tests don't)
it('should delete the user', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);

// TODO: Throws 500 because:
// - Cannot query across one-to-many for property wallets
// - Cannot query across one-to-many for property wallets
await request(app.getHttpServer())
.post('/v1/users/wallet')
.set('Cookie', [`access_token=${accessToken}`])
Expand All @@ -183,11 +176,8 @@ describe('UsersController', () => {
await request(app.getHttpServer())
.delete('/v1/users')
.set('Cookie', [`access_token=${accessToken}`])
.expect({
statusCode: 200,
});

// TODO: Check that wallet/user entities are removed from the database
.expect(200)
.expect({});
});

// Note: we could extensively test JWT validity but it is covered in the AuthGuard tests
Expand Down Expand Up @@ -215,21 +205,22 @@ describe('UsersController', () => {
});
});

it('should return a 409 if no user is affected', async () => {
it('should return a 404 if the user is not found', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);

// TODO: Throws 500 because:
// - Cannot query across one-to-many for property wallets
await request(app.getHttpServer())
.post('/v1/users/wallet')
.delete('/v1/users')
.set('Cookie', [`access_token=${accessToken}`])
.expect({
statusCode: 409,
message: `Could not delete user. Wallet=${authPayloadDto.signer_address}`,
error: 'Conflict',
statusCode: 404,
message: 'User not found',
error: 'Not Found',
});
});

// In the current implementation, this won't occur due to no foreign key constraints
it.todo('should return a 409 if no user is affected');
});

describe('POST /v1/users/wallet', () => {
Expand All @@ -241,9 +232,11 @@ describe('UsersController', () => {
.post('/v1/users/wallet')
.set('Cookie', [`access_token=${accessToken}`])
.expect(201)
.expect({
id: 1,
});
.expect(({ body }) =>
expect(body).toEqual({
id: expect.any(Number),
}),
);
});

// Note: we could extensively test JWT validity but it is covered in the AuthGuard tests
Expand Down Expand Up @@ -292,8 +285,21 @@ describe('UsersController', () => {
});

describe('DELETE /v1/users/wallet/:walletAddress', () => {
// TODO: Check that the wallet was removed from the user and deleted from the database
it.todo('should delete a wallet from a user');
// TODO: Check wallet was deleted in integration test (and other tests don't)
it.skip('should delete a wallet from a user', async () => {
const walletAddress = getAddress(faker.finance.ethereumAddress());
const authPayloadDto = authPayloadDtoBuilder()
.with('signer_address', walletAddress)
.build();
const accessToken = jwtService.sign(authPayloadDto);

await request(app.getHttpServer())
.post('/v1/users/wallet')
.set('Cookie', [`access_token=${accessToken}`])
.expect(201);

// TODO: We need a new endpoint to add a wallet to a user to test this
});

// Note: we could extensively test JWT validity but it is covered in the AuthGuard tests
it('should return a 403 if not authenticated', async () => {
Expand Down Expand Up @@ -325,7 +331,7 @@ describe('UsersController', () => {
});
});

it('should return a 409 if the authenticated one', async () => {
it('should return a 409 if it is the authenticated one', async () => {
const walletAddress = getAddress(faker.finance.ethereumAddress());
const authPayloadDto = authPayloadDtoBuilder()
.with('signer_address', walletAddress)
Expand All @@ -352,8 +358,6 @@ describe('UsersController', () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);

// TODO: Throws 500 because:
// - Cannot read properties of undefined (reading 'joinColumns')
await request(app.getHttpServer())
.delete(`/v1/users/wallet/${walletAddress}`)
.set('Cookie', [`access_token=${accessToken}`])
Expand All @@ -364,8 +368,24 @@ describe('UsersController', () => {
});
});

it.todo('should return a 400 if the wallet is the last one');
it('should return a 409 if the wallet could not be removed', async () => {
const walletAddress = getAddress(faker.finance.ethereumAddress());
const authPayloadDto = authPayloadDtoBuilder().build();
const accessToken = jwtService.sign(authPayloadDto);

await request(app.getHttpServer())
.post('/v1/users/wallet')
.set('Cookie', [`access_token=${accessToken}`])
.expect(201);

it.todo('should return a 409 if the wallet could not be removed');
await request(app.getHttpServer())
.delete(`/v1/users/wallet/${walletAddress}`)
.set('Cookie', [`access_token=${accessToken}`])
.expect({
statusCode: 409,
message: `User could not be removed from wallet. Wallet=${walletAddress}`,
error: 'Conflict',
});
});
});
});
5 changes: 1 addition & 4 deletions src/routes/users/users.controller.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
ApiBadRequestResponse,
ApiConflictResponse,
ApiNotFoundResponse,
ApiOkResponse,
Expand Down Expand Up @@ -37,6 +36,7 @@ export class UsersController {
return await this.usersService.getUserWithWallets(authPayload);
}

@ApiNotFoundResponse({ description: 'User not found' })
@ApiConflictResponse({ description: 'Could not delete user' })
@Delete()
@UseGuards(AuthGuard)
Expand All @@ -61,9 +61,6 @@ export class UsersController {
'Cannot remove the current wallet OR User could not be remove from wallet',
})
@ApiNotFoundResponse({ description: 'User not found' })
@ApiBadRequestResponse({
description: 'Cannot delete the last wallet of a user',
})
@ApiOkResponse({ description: 'Wallet removed from user and deleted' })
@Delete('/wallet/:walletAddress')
@UseGuards(AuthGuard)
Expand Down

0 comments on commit c6a9410

Please sign in to comment.