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 ☕

@amywng amywng requested a review from dburkhart07 January 27, 2026 19:07
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.

2 very small things. Also, while we are at it, since we are rewriting this relationship, could we slightly restructure the getAllOrders controller test? Just making it so that the mockOrders actually follow the new relationship (they should contain a partial request that contains a partial pantry that contains a pantry name).

'order.createdAt',
'order.shippedAt',
'order.deliveredAt',
'request.pantryId',

Choose a reason for hiding this comment

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

We already return the pantryId below, so we should get rid of one of these. I'm more in favor of getting rid of pantry.pantryId and keeping the request.pantryId.


@ManyToOne(() => Pantry, { nullable: false })
@JoinColumn({ name: 'pantry_id', referencedColumnName: 'pantryId' })
pantry: Pantry;

Choose a reason for hiding this comment

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

We should change the frontend type too with adding this.

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