Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hydrating an active order only applies prices to the variant of the first order line #2013

Closed
martijnvdbrug opened this issue Jan 30, 2023 · 4 comments
Assignees
Labels
type: bug 🐛 Something isn't working

Comments

@martijnvdbrug
Copy link
Collaborator

Describe the bug
When hydrating an active order with applyProductVariantPrices=true, only the variant of the first order line has pricing applied, the other will have price 0;

To Reproduce
Steps to reproduce the behavior: (see e2e test below)

  1. Create an active order with 3 items
  2. Fetch the active order
  3. Hydrate the active order
  4. order.lines[0].productVariant.price will be 155880, while order.lines[1].productVariant.price will be 0.

Adding the variants in a different order will result in the same issue: it's always only the first variant that will have pricing applied.

Expected behavior
I would expect all the variants to have the correct pricing.

Environment (please complete the following information):

  • @vendure/core version: 1.9.2
  • Nodejs version: 16
  • Database (mysql/postgres etc): SQLite

Additional context
You should be able to just copy and paste the test case below.

Products.csv:

name   , slug   , description                                                                                                                                                                                                                                                                  , assets                           , facets                                  , optionGroups       , optionValues    , sku      , price   , taxCategory , stockOnHand , trackInventory , variantAssets , variantFacets
Laptop , laptop , "Now equipped with seventh-generation Intel Core processors, Laptop is snappier than ever. From daily tasks like launching apps and opening files to more advanced computing, you can power through your day thanks to faster SSDs and Turbo Boost processing up to 3.6GHz." ,                                  , category:electronics|category:computers , "screen size|RAM"  , "13 inch|8GB"   , L2201308 , 1299.00 , standard    , 100         , false          ,               ,
       ,        ,                                                                                                                                                                                                                                                                              ,                                  ,                                         ,                    , "15 inch|8GB"   , L2201508 , 1399.00 , standard    , 100         , false          ,               ,
       ,        ,                                                                                                                                                                                                                                                                              ,                                  ,                                         ,                    , "13 inch|16GB"  , L2201316 , 2199.00 , standard    , 100         , false          ,               ,
       ,        ,                                                                                                                                                                                                                                                                              ,                                  ,                                         ,                    , "15 inch|16GB"  , L2201516 , 2299.00 , standard    , 100         , false          ,               ,

hydration-error.spec.ts

import {
  createTestEnvironment,
  registerInitializer,
  SimpleGraphQLClient,
  SqljsInitializer,
  testConfig
} from "@vendure/testing";
import {
  ActiveOrderService,
  ChannelService,
  DefaultLogger, EntityHydrator,
  LogLevel,
  mergeConfig, Order,
  RequestContext
} from "@vendure/core";
import { TestServer } from "@vendure/testing/lib/test-server";
import { gql } from "graphql-tag";

jest.setTimeout(20000);

import { LanguageCode } from '@vendure/common/lib/generated-types';
import { InitialData } from '@vendure/core';

const initialData: InitialData = {
  defaultLanguage: LanguageCode.en,
  defaultZone: 'Europe',
  taxRates: [
    { name: 'Standard Tax', percentage: 20 },
    { name: 'Reduced Tax', percentage: 10 },
    { name: 'Zero Tax', percentage: 0 },
  ],
  shippingMethods: [
    { name: 'Standard Shipping', price: 500 },
    { name: 'Express Shipping', price: 1000 },
  ],
  countries: [
    { name: 'Australia', code: 'AU', zone: 'Oceania' },
    { name: 'Austria', code: 'AT', zone: 'Europe' },
    { name: 'Canada', code: 'CA', zone: 'Americas' },
    { name: 'China', code: 'CN', zone: 'Asia' },
    { name: 'South Africa', code: 'ZA', zone: 'Africa' },
    { name: 'United Kingdom', code: 'GB', zone: 'Europe' },
    { name: 'United States of America', code: 'US', zone: 'Americas' },
    { name: 'Nederland', code: 'NL', zone: 'Europe' },
  ],
  collections: [
    {
      name: 'Plants',
      filters: [
        {
          code: 'facet-value-filter',
          args: { facetValueNames: ['plants'], containsAny: false },
        },
      ],
    },
  ],
  paymentMethods: [],
  /*  paymentMethods: [
    {
      name: testPaymentMethod.code,
      handler: { code: testPaymentMethod.code, arguments: [] },
    },
  ],*/
};

const ADD_ITEM_TO_ORDER = gql`
    mutation AddItemToOrder(
        $productVariantId: ID!
        $quantity: Int!
    ) {
        addItemToOrder(
            productVariantId: $productVariantId
            quantity: $quantity
        ) {
            ... on Order {
                id
                code
            }
            ... on ErrorResult {
                errorCode
                message
            }
        }
    }
`;

describe("Order export plugin", function() {
  let server: TestServer;
  let adminClient: SimpleGraphQLClient;
  let shopClient: SimpleGraphQLClient;

  beforeAll(async () => {
    registerInitializer("sqljs", new SqljsInitializer("__data__"));
    const config = mergeConfig(testConfig, {
      logger: new DefaultLogger({ level: LogLevel.Debug }),
      plugins: []
    });
    ({ server, adminClient, shopClient } = createTestEnvironment(config));
    await server.init({
      initialData,
      // productsCsvPath: `${__dirname}/subscriptions.csv`
      productsCsvPath: '../test/src/products-import.csv',
    });
  }, 60000);

  let order: Order | undefined;

  it("Create order with 3 items", async () => {
    await shopClient.asUserWithCredentials(
      "hayden.zieme12@hotmail.com",
      "test"
    );
    await shopClient.query(ADD_ITEM_TO_ORDER, {
      productVariantId: "1",
      quantity: 1
    });
    await shopClient.query(ADD_ITEM_TO_ORDER, {
      productVariantId: "2",
      quantity: 1
    });
    await shopClient.query(ADD_ITEM_TO_ORDER, {
      productVariantId: "3",
      quantity: 1
    });
    const channel = await server.app.get(ChannelService).getDefaultChannel();
    // This is ugly, but in our real life example we use a CTX constructed by Vendure
    const ctx = new RequestContext({
      channel,
      authorizedAsOwnerOnly: true,
      apiType: "shop",
      isAuthorized: true,
      session: {
        activeOrderId: 1,
        activeChannelId: 1,
        user: {
          id: 2
        }
      } as any
    });
    order = await server.app.get(ActiveOrderService).getActiveOrder(ctx, undefined);
    await server.app.get(EntityHydrator).hydrate(ctx, order!, {
      relations: ["lines.productVariant"],
      applyProductVariantPrices: true
    });
  });

  it("Variant of orderLine 1 has a price", async () => {
    expect(order!.lines[0].productVariant.priceWithTax).toBeGreaterThan(0);
  });

  it("Variant of orderLine 2 has a price", async () => {
    expect(order!.lines[1].productVariant.priceWithTax).toBeGreaterThan(0);
  });

  it("Variant of orderLine 3 has a price", async () => {
    expect(order!.lines[1].productVariant.priceWithTax).toBeGreaterThan(0);
  });
});
@michaelbromley
Copy link
Member

Hi Martijn,

It took me a while to get round to this, but I just added your tests to the existing entity-hydrator.e2e-spec.ts suite and it passed. So I'm suspecting that this somehow got fixed between 1.9.2 -> 2.0.5?

Have you run into this in the latest versions?

In any case, I'll leave the e2e tests in the suite to ensure this does not regress.

michaelbromley added a commit that referenced this issue Aug 7, 2023
@martijnvdbrug
Copy link
Collaborator Author

martijnvdbrug commented Aug 9, 2023

Thanks for looking into this. Unfortunately the issue still persists on my end. I've created a reproducable setup here:
(It is using Vendure 2.0.5)
https://github.com/Pinelab-studio/_temp_aug_2023_vendure_issue2013_repro

You can

  1. git clone git@github.com:Pinelab-studio/_temp_aug_2023_vendure_issue2013_repro.git
  2. yarn
  3. yarn test
  4. And see the last 2 tests failing:
 ❯ testing.spec.ts (4) 5162ms
   ❯ Hydration issue (4) 5162ms
     ✓ Create order with 3 items 302ms
     ✓ Variant of orderLine 1 has a price
     × Variant of orderLine 2 has a price
     × Variant of orderLine 3 has a price

@michaelbromley
Copy link
Member

michaelbromley commented Aug 10, 2023

Thanks. This lead me to discover that I can reproduce the error when using postgres / sqlite - but it works with mysql, mariadb & sqljs! I'll look into how to fix...

@michaelbromley michaelbromley moved this from 📋 Backlog to 🏗 In progress in Vendure OS Roadmap Aug 11, 2023
@michaelbromley michaelbromley moved this from 🏗 In progress to ✅ Done in Vendure OS Roadmap Aug 11, 2023
@martijnvdbrug
Copy link
Collaborator Author

martijnvdbrug commented Aug 16, 2023

Strange 😞 I remember last time we had an issue that was DB specific, it was caused by the order in which arrays were returned...

EDIT: nvm, I see it's already closed, nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Something isn't working
Projects
Status: 🚀 Shipped
Development

No branches or pull requests

2 participants