Skip to content

Conversation

@amywng
Copy link
Member

@amywng amywng commented Jan 25, 2026

ℹ️ Issue

Closes SSF-114

📝 Description

  • Remove order-pantry column/relationship
  • Added migration to reflect database change
  • Fixed endpoints to remain functional
  • Added service tests

✔️ Verification

Make sure all tests passed and verified updated endpoints on Postman.

🏕️ (Optional) Future Work / Notes

Did you notice anything ugly during the course of this ticket? Any bugs, design challenges, or unexpected behavior? Write it down so we can clean it up in a future ticket!

Copy link

@dburkhart07 dburkhart07 left a comment

Choose a reason for hiding this comment

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

Few small things ☕

}

async findOrderPantry(orderId: number): Promise<Pantry> {
validateId(orderId, 'Order');

Choose a reason for hiding this comment

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

Why did we delete this validation?

const qb = this.repo
.createQueryBuilder('order')
.leftJoinAndSelect('order.pantry', 'pantry')
.leftJoinAndSelect('order.request', 'request')
Copy link

@dburkhart07 dburkhart07 Jan 26, 2026

Choose a reason for hiding this comment

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

This current call when I tested it returned no info on the volunteers or pantry. With this relation being removed, however, I wonder if we still even need them. Can you check with Sam on this, and whether we even still want the pantryNames aspect of the filter, or just reclarify what fields we would want this endpoint to return?


describe('findOrderPantry', () => {
it('should return pantry for given order', async () => {
const mockFoodRequest = {

Choose a reason for hiding this comment

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

Can we type each of these as partials?

});

it('should throw NotFoundException if pantry not found', async () => {
const mockFoodRequest = {

Choose a reason for hiding this comment

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

Typing here too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants