Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions .claude/settings.local.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
Comment thread
dburkhart07 marked this conversation as resolved.
Outdated
"permissions": {
"allow": [
"Bash(grep -E \"\\\\.\\(ts|js\\)$\")",
"Bash(yarn test:*)"
]
}
}
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;
}
118 changes: 117 additions & 1 deletion apps/backend/src/foodRequests/request.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ 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 { UpdateRequestDto } from './dtos/update-request.dto';
import {
DonationItemDetailsDto,
MatchingItemsDto,
MatchingManufacturersDto,
} from './dtos/matching.dto';
import { FoodManufacturer } from '../foodManufacturers/manufacturers.entity';
import { Pantry } from '../pantries/pantries.entity';
import { BadRequestException } from '@nestjs/common';

const mockRequestsService = mock<RequestsService>();

Expand Down Expand Up @@ -44,6 +46,8 @@ describe('RequestsController', () => {
mockRequestsService.find.mockReset();
mockRequestsService.create.mockReset();
mockRequestsService.getOrderDetails.mockReset();
mockRequestsService.updateRequest.mockReset();
mockRequestsService.delete.mockReset();

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

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

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

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

it('should throw BadRequestException when request is not active', async () => {
Comment thread
dburkhart07 marked this conversation as resolved.
Outdated
mockRequestsService.updateRequest.mockRejectedValue(
new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be updated`,
),
);

const updateRequestDto: UpdateRequestDto = {
requestedSize: RequestSize.MEDIUM,
};

await expect(
controller.updateRequest(1, updateRequestDto),
).rejects.toThrow(
new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be updated`,
),
);
expect(mockRequestsService.updateRequest).toHaveBeenCalledWith(
1,
updateRequestDto,
);
});

it('should throw BadRequestException when request has orders', async () => {
mockRequestsService.updateRequest.mockRejectedValue(
new BadRequestException(
`Request 2 cannot be updated if it still has orders associated with it`,
),
);

const updateRequestDto: UpdateRequestDto = {
requestedSize: RequestSize.MEDIUM,
};

await expect(
controller.updateRequest(2, updateRequestDto),
).rejects.toThrow(
new BadRequestException(
`Request 2 cannot be updated if it still has orders associated with it`,
),
);
expect(mockRequestsService.updateRequest).toHaveBeenCalledWith(
2,
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);
});

it('should throw BadRequestException when request is not active', async () => {
mockRequestsService.delete.mockRejectedValue(
new BadRequestException(
`Request must be ${FoodRequestStatus.ACTIVE} in order to be deleted`,
),
);

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

it('should throw BadRequestException when request has orders', async () => {
mockRequestsService.delete.mockRejectedValue(
new BadRequestException(
`Request 2 cannot be deleted if it still has orders associated with it`,
),
);

await expect(controller.deleteRequest(2)).rejects.toThrow(
new BadRequestException(
`Request 2 cannot be deleted if it still has orders associated with it`,
),
);
expect(mockRequestsService.delete).toHaveBeenCalledWith(2);
});
});

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 @@ -6,6 +6,8 @@ import {
Post,
Body,
ValidationPipe,
Patch,
Delete,
} from '@nestjs/common';
import { ApiBody } from '@nestjs/swagger';
import { RequestsService } from './request.service';
Expand All @@ -20,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 @@ -108,4 +111,19 @@ export class RequestsController {
requestData.additionalInformation,
);
}

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

@Delete('/:requestId')
Comment thread
dburkhart07 marked this conversation as resolved.
async deleteRequest(
@Param('requestId', ParseIntPipe) requestId: number,
): Promise<void> {
return this.requestsService.delete(requestId);
}
}
131 changes: 131 additions & 0 deletions apps/backend/src/foodRequests/request.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { FoodType } from '../donationItems/types';
import { DonationItem } from '../donationItems/donationItems.entity';
import { testDataSource } from '../config/typeormTestDataSource';
import {
BadRequestException,
InternalServerErrorException,
NotFoundException,
} from '@nestjs/common';
Expand Down Expand Up @@ -581,4 +582,134 @@ describe('RequestsService', () => {
);
});
});

describe('updateRequest', () => {
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.updateRequest(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.updateRequest(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.updateRequest(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.updateRequest(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.updateRequest(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.updateRequest(1, {})).rejects.toThrow(
new BadRequestException(
'At least one field must be provided to update',
),
);
});
});

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'),
);
});
});
});
Loading
Loading