Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions apps/backend/src/foodRequests/dtos/update-request.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {
ArrayNotEmpty,
IsEnum,
IsNotEmpty,
IsOptional,
IsString,
} from 'class-validator';
import { RequestSize } from '../types';
import { FoodType } from '../../donationItems/types';

export class UpdateRequestDto {
@IsOptional()
@IsEnum(RequestSize)
requestedSize?: RequestSize;

@IsOptional()
@ArrayNotEmpty()
@IsEnum(FoodType, { each: true })
requestedFoodTypes?: FoodType[];

@IsOptional()
@IsString()
@IsNotEmpty()
additionalInformation?: string;
}
40 changes: 38 additions & 2 deletions apps/backend/src/foodRequests/request.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { RequestsController } from './request.controller';
import { Test, TestingModule } from '@nestjs/testing';
import { mock } from 'jest-mock-extended';
import { FoodRequest } from './request.entity';
import { RequestSize } from './types';
import { FoodRequestStatus, RequestSize } from './types';
import { OrderStatus } from '../orders/types';
import { FoodType } from '../donationItems/types';
import { OrderDetailsDto } from '../orders/dtos/order-details.dto';
import { CreateRequestDto } from './dtos/create-request.dto';
import { FoodRequestStatus } from './types';
import { UpdateRequestDto } from './dtos/update-request.dto';
import {
DonationItemDetailsDto,
MatchingItemsDto,
Expand Down Expand Up @@ -45,6 +45,8 @@ describe('RequestsController', () => {
mockRequestsService.find.mockReset();
mockRequestsService.create.mockReset();
mockRequestsService.getOrderDetails.mockReset();
mockRequestsService.update.mockReset();
mockRequestsService.delete.mockReset();

const module: TestingModule = await Test.createTestingModule({
controllers: [RequestsController],
Expand Down Expand Up @@ -240,6 +242,40 @@ describe('RequestsController', () => {
});
});

describe('PATCH /:requestId', () => {
it('should update request with valid information', async () => {
const updatedRequest = {
...foodRequest1,
requestedSize: RequestSize.MEDIUM,
};
mockRequestsService.update.mockResolvedValue(
updatedRequest as FoodRequest,
);

const updateRequestDto: UpdateRequestDto = {
requestedSize: RequestSize.MEDIUM,
};
const result = await controller.updateRequest(1, updateRequestDto);

expect(result).toEqual(updatedRequest);
expect(mockRequestsService.update).toHaveBeenCalledWith(
1,
updateRequestDto,
);
});
});

describe('DELETE /:requestId', () => {
it('should delete a request by id', async () => {
mockRequestsService.delete.mockResolvedValue(undefined);

const result = await controller.deleteRequest(1);

expect(result).toBeUndefined();
expect(mockRequestsService.delete).toHaveBeenCalledWith(1);
});
});

describe('GET /:requestId/matching-manufacturers/:foodManufacturerId/available-items', () => {
it('should call requestsService.getAvailableItems and return grouped items', async () => {
const requestId = 1;
Expand Down
18 changes: 18 additions & 0 deletions apps/backend/src/foodRequests/request.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Body,
ValidationPipe,
Patch,
Delete,
} from '@nestjs/common';
import { ApiBody } from '@nestjs/swagger';
import { RequestsService } from './request.service';
Expand All @@ -21,6 +22,7 @@ import {
MatchingItemsDto,
MatchingManufacturersDto,
} from './dtos/matching.dto';
import { UpdateRequestDto } from './dtos/update-request.dto';

@Controller('requests')
export class RequestsController {
Expand Down Expand Up @@ -109,6 +111,22 @@ export class RequestsController {
requestData.additionalInformation,
);
}

Comment thread
dburkhart07 marked this conversation as resolved.
@Patch('/:requestId')
async updateRequest(
@Param('requestId', ParseIntPipe) requestId: number,
@Body(new ValidationPipe()) body: UpdateRequestDto,
): Promise<FoodRequest> {
return this.requestsService.update(requestId, body);
}

@Delete('/:requestId')
Comment thread
dburkhart07 marked this conversation as resolved.
async deleteRequest(
@Param('requestId', ParseIntPipe) requestId: number,
): Promise<void> {
return this.requestsService.delete(requestId);
}

@Roles(Role.VOLUNTEER)
@Patch('/:requestId/close')
async closeRequest(
Expand Down
133 changes: 132 additions & 1 deletion apps/backend/src/foodRequests/request.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { FoodType } from '../donationItems/types';
import { DonationItem } from '../donationItems/donationItems.entity';
import { testDataSource } from '../config/typeormTestDataSource';
import {
BadRequestException,
InternalServerErrorException,
NotFoundException,
BadRequestException,
} from '@nestjs/common';
import { EmailsService } from '../emails/email.service';
import { mock } from 'jest-mock-extended';
Expand Down Expand Up @@ -582,6 +582,137 @@ describe('RequestsService', () => {
);
});
});

describe('update', () => {
it('should update request attributes', async () => {
await testDataSource.query(
`DELETE FROM allocations WHERE order_id IN (SELECT order_id FROM orders WHERE request_id = 1)`,
);
await testDataSource.query(`DELETE FROM orders WHERE request_id = 1`);

const result = await service.update(1, {
requestedSize: RequestSize.MEDIUM,
});

expect(result.requestedSize).toBe(RequestSize.MEDIUM);
expect(result.requestedFoodTypes).toEqual([
FoodType.SEED_BUTTERS,
FoodType.GLUTEN_FREE_BREAD,
FoodType.DRIED_BEANS,
FoodType.DAIRY_FREE_ALTERNATIVES,
]);

const fromDb = await testDataSource
.getRepository(FoodRequest)
.findOneBy({ requestId: 1 });
expect(fromDb?.requestedSize).toBe(RequestSize.MEDIUM);
});

it('should throw NotFoundException when request is not found', async () => {
await expect(
service.update(9999, { requestedSize: RequestSize.MEDIUM }),
).rejects.toThrow(new NotFoundException('Request 9999 not found'));
});

it('should update all request attributes when all fields are provided', async () => {
await testDataSource.query(
`DELETE FROM allocations WHERE order_id IN (SELECT order_id FROM orders WHERE request_id = 1)`,
);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test that has other fields in the dto which will be whitelisted from the validation pipe setting - meaning payload with fields we didn't define (ex. pantryid) will be ignored

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, i got a chance to look more into it, and i feel like this test is not really necessary. we know that the whitelisting works from the main.ts code:
app.useGlobalPipes( new ValidationPipe({ whitelist: true, }), );
this falls under the same category to me as not testing the validateId on every single test, since we know it works (and tested it elsewhere). we know that this whitelisting will work fine. if we really want to test this, we should make one test file for main and do it there, rather than testing this for every single dto we use in every single service.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for looking into it. that makes sense, i'll note to test that in a future ticket!

await testDataSource.query(`DELETE FROM orders WHERE request_id = 1`);

const result = await service.update(1, {
requestedSize: RequestSize.SMALL,
requestedFoodTypes: [FoodType.GRANOLA],
additionalInformation: 'Updated information',
});

expect(result.requestedSize).toBe(RequestSize.SMALL);
expect(result.requestedFoodTypes).toEqual([FoodType.GRANOLA]);
expect(result.additionalInformation).toBe('Updated information');

const fromDb = await testDataSource
.getRepository(FoodRequest)
.findOneBy({ requestId: 1 });
expect(fromDb?.requestedSize).toBe(RequestSize.SMALL);
expect(fromDb?.requestedFoodTypes).toEqual([FoodType.GRANOLA]);
expect(fromDb?.additionalInformation).toBe('Updated information');
});

it('should throw BadRequestException when request is not active', async () => {
await testDataSource.query(
`UPDATE food_requests SET status = 'closed' WHERE request_id = 1`,
);

await expect(
service.update(1, { requestedSize: RequestSize.MEDIUM }),
).rejects.toThrow(
new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be updated`,
),
);
});

it('should throw BadRequestException when request has orders', async () => {
await expect(
service.update(2, { requestedSize: RequestSize.MEDIUM }),
).rejects.toThrow(
new BadRequestException(
`Request 2 cannot be updated if it still has orders associated with it`,
),
);
});

it('should throw BadRequestException when all DTO fields are undefined', async () => {
await expect(service.update(1, {})).rejects.toThrow(
new BadRequestException(
'At least one field must be provided to update request',
),
);
});
});

describe('delete', () => {
it('should delete a request by id', async () => {
await testDataSource.query(
`DELETE FROM allocations WHERE order_id IN (SELECT order_id FROM orders WHERE request_id = 1)`,
);
await testDataSource.query(`DELETE FROM orders WHERE request_id = 1`);

await service.delete(1);

const fromDb = await testDataSource
.getRepository(FoodRequest)
.findOneBy({ requestId: 1 });
expect(fromDb).toBeNull();
});

it('should throw BadRequestException when request is not active', async () => {
await testDataSource.query(
`UPDATE food_requests SET status = 'closed' WHERE request_id = 1`,
);

await expect(service.delete(1)).rejects.toThrow(
new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be deleted`,
),
);
});

it('should throw BadRequestException when request has orders', async () => {
await expect(service.delete(2)).rejects.toThrow(
new BadRequestException(
`Request 2 cannot be deleted if it still has orders associated with it`,
),
);
});

it('should throw NotFoundException when request is not found', async () => {
await expect(service.delete(9999)).rejects.toThrow(
new NotFoundException('Request 9999 not found'),
);
});
});

describe('closeRequest', () => {
it('should close an active request', async () => {
const result = await service.closeRequest(3);
Expand Down
69 changes: 68 additions & 1 deletion apps/backend/src/foodRequests/request.service.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {
BadRequestException,
Injectable,
InternalServerErrorException,
NotFoundException,
BadRequestException,
} from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
Expand All @@ -22,6 +22,7 @@ import { FoodType } from '../donationItems/types';
import { DonationItem } from '../donationItems/donationItems.entity';
import { EmailsService } from '../emails/email.service';
import { emailTemplates } from '../emails/emailTemplates';
import { UpdateRequestDto } from './dtos/update-request.dto';

@Injectable()
export class RequestsService {
Expand Down Expand Up @@ -304,6 +305,72 @@ export class RequestsService {
await this.repo.save(request);
}

async update(requestId: number, dto: UpdateRequestDto): Promise<FoodRequest> {
validateId(requestId, 'Request');

if (
dto.requestedSize == undefined &&
dto.requestedFoodTypes == undefined &&
dto.additionalInformation == undefined
) {
throw new BadRequestException(
'At least one field must be provided to update request',
);
}

const request = await this.repo.findOne({
where: { requestId },
relations: ['orders'],
});

if (!request) {
Comment thread
dburkhart07 marked this conversation as resolved.
throw new NotFoundException(`Request ${requestId} not found`);
}

if (request.status != FoodRequestStatus.ACTIVE) {
throw new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be updated`,
);
}

if (request.orders && request.orders.length > 0) {
throw new BadRequestException(
`Request ${requestId} cannot be updated if it still has orders associated with it`,
Comment thread
amywng marked this conversation as resolved.
);
}

Object.assign(request, dto);

return this.repo.save(request);
}

async delete(requestId: number) {
validateId(requestId, 'Request');

const request = await this.repo.findOne({
where: { requestId },
relations: ['orders'],
});

if (!request) {
throw new NotFoundException(`Request ${requestId} not found`);
}

if (request.status != FoodRequestStatus.ACTIVE) {
throw new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be deleted`,
);
}

if (request.orders && request.orders.length > 0) {
throw new BadRequestException(
`Request ${requestId} cannot be deleted if it still has orders associated with it`,
);
}

await this.repo.remove(request);
}

async closeRequest(requestId: number): Promise<FoodRequest> {
validateId(requestId, 'Request');

Expand Down
12 changes: 4 additions & 8 deletions apps/backend/src/users/users.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,10 @@ export class UsersService {
async update(id: number, dto: UpdateUserInfoDto): Promise<User> {
validateId(id, 'User');

const { firstName, lastName, phone } = dto;

if (
firstName === undefined &&
lastName === undefined &&
phone === undefined
dto.firstName === undefined &&
dto.lastName === undefined &&
dto.phone === undefined
) {
throw new BadRequestException(
'At least one field must be provided to update',
Expand All @@ -148,9 +146,7 @@ export class UsersService {
throw new NotFoundException(`User ${id} not found`);
}

if (firstName !== undefined) user.firstName = firstName;
if (lastName !== undefined) user.lastName = lastName;
if (phone !== undefined) user.phone = phone;
Object.assign(user, dto);

return this.repo.save(user);
}
Expand Down
Loading