vendure-ecommerce / vendure

The commerce platform with customization in its DNA.
https://www.vendure.io
Other
5.84k stars 1.04k forks source link

Edge case: the property "discounts" on the Order entity requires the Order.lines relation to be joined. #3068

Open mschipperheyn opened 2 months ago

mschipperheyn commented 2 months ago

Describe the bug I started running into this issue in our populate script after some heavy refactoring.

The problem is here . The getOrderOrThrow allows a custom relations key to be passed in. However, when discounts() is accessed on the Order entity, an error will be thrown if the lines key is not hydrated.

It's hard to debug because in my case it's being thrown from an order-confirmation email that gets triggered by my code. To Reproduce Steps to reproduce the behavior:

  1. Call addItemToOrder with a custom relations key that doesn't include lines

Expected behavior Order should hydrate the necessary keys it needs to process instead of throwing errors. The relations key on the getOrderOrThrow should merge required keys with incoming keys using a Array.from(new Set(...relations, ...['lines', etc]))

Environment (please complete the following information):

mschipperheyn commented 2 months ago

Additional information about this issue.

I was actually working with an orderline directly (which for our scenario is necessary) like so, which causes the issue

await entityHydrator.hydrate(ctx, orderLine, {
        relations: [
          'order',
          'customFields.assignedCustomer',
          'customFields.preferredGroup',
          'productVariant',
        ],
      })

If I add this, the issue doesn't occur

await entityHydrator.hydrate(ctx, orderLine, {
        relations: [
          'order',
          'order.shippingLines',
          'order.lines',
          'customFields.assignedCustomer',
          'customFields.preferredGroup',
          'productVariant',
        ],
      })

@michaelbromley if you agree that hydration keys should be merged to avoid this issue at the order service level getOrderOrThrow method by merging relations instead of replacing them, I can go ahead and write that.