GraphQl customer orders query failed if try to retreive order item with deleted product

zakdma commented 4 months ago

Preconditions and environment

Magento version 2.4.7-p1 (latest)

Steps to reproduce

Install vanilla Magento instance
Go to admin and create new simple product with SKU: simple-deleted
Fill in all necessary product data to display it on front (stock, price, category etc.) and save it
Reindex all
Go to front and create new customer
Find and add simple-deleted product to cart
Go to checkout and place an order using some address data and default shipping and billing methods
Go to admin and see the order is in Sales/Orders grid
Open any GraphQl client and generate token using generateCustomerToken GraphQl mutation using previously created customer credentials https://developer.adobe.com/commerce/webapi/graphql/schema/customer/mutations/generate-token/
Use the generated token in the following requests by adding it to Authorization header according to Magento GraphQl documentation

perform query

query {
customer {
orders(currentPage: 1, pageSize: 999) {
  items {
    number
    items {
      product {
        sku
        name
      }
    }
  }
}
}
}

See the correct response like this

{
"data": {
"customer": {
  "orders": {
    "items": [
      {
        "number": "000000001",
        "items": [
          {
            "product": {
              "sku": "simple-deleted",
              "name": "simple-deleted"
            }
          }
        ]
      }
    ]
  }
}
}
}

Go to admin and delete product simple-deleted

Perform the query again

query {
customer {
orders(currentPage: 1, pageSize: 999) {
  items {
    number
    items {
      product {
        sku
        name
      }
    }
  }
}
}
}

Expected result

There is a correct response with the data requested without any error

Actual result

There is error response like this

{
  "errors": [
    {
      "message": "Internal server error",
      "locations": [
        {
          "line": 7,
          "column": 11
        }
      ],
      "path": [
        "customer",
        "orders",
        "items",
        0,
        "items",
        0,
        "product"
      ]
    }
  ],
  "data": {
    "customer": {
      "orders": {
        "items": [
          {
            "number": "000000001",
            "items": [
              {
                "product": null
              }
            ]
          }
        ]
      }
    }
  }
}

Additional information

Check exception.log and see error like this

[2024-07-03T15:09:43.938496+00:00] main.ERROR: Missing key "associatedProduct" in Order Item value data

GraphQL (40:11)
39:           }
40:           product {
              ^
41:             id
 {"exception":"[object] (GraphQL\\Error\\Error(code: 0): Missing key \"associatedProduct\" in Order Item value data at /var/www/app/vendor/webonyx/graphql-php/src/Error/Error.php:155)
[previous exception] [object] (Magento\\Framework\\Exception\\LocalizedException(code: 0): Missing key \"associatedProduct\" in Order Item value data at /var/www/app/vendor/magento/module-sales-graph-ql/Model/Resolver/ProductResolver.php:46)"} []

Release note

No response

Triage and priority

[ ] Severity: S0 - Affects critical data or functionality and leaves users without workaround.
[X] Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
[ ] Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
[ ] Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
[ ] Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

m2-assistant[bot] commented 4 months ago

Hi @zakdma. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release
For more details, review the Magento Contributor Assistant documentation.
Add a comment to assign the issue: @magento I am working on this
To learn more about issue processing workflow, refer to the Code Contributions.

Join Magento Community Engineering Slack and ask your questions in #github channel. :warning: According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting. :clock10: You can find the schedule on the Magento Community Calendar page. :telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

m2-assistant[bot] commented 4 months ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
[ ] 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

engcom-Bravo commented 4 months ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 4 months ago

Hi @engcom-Bravo. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 4 months ago

Hi @engcom-Bravo, here is your Magento Instance: https://11bfd466f8c258a1113feb50fd641584.instances-prod.magento-community.engineering Admin access: https://11bfd466f8c258a1113feb50fd641584.instances-prod.magento-community.engineering/admin_b096 Login: 0094132b Password: 5f24f247854d

engcom-Bravo commented 4 months ago

Hi @zakdma,

Thanks for your reporting and collaboration.

We have verified the issue in Latest 2.4-develop instance and the issue is reproducible.Kindly refer the screenshots.

Error in Exception Logs:

GraphQL (11:11)
10:         items {
11:           product {
              ^
12:             sku
 {"exception":"[object] (GraphQL\\Error\\Error(code: 0): Missing key \"associatedProduct\" in Order Item value data at /usr/local/var/www/magentok/magento2/vendor/webonyx/graphql-php/src/Error/Error.php:155)
[previous exception] [object] (Magento\\Framework\\Exception\\LocalizedException(code: 0): Missing key \"associatedProduct\" in Order Item value data at /usr/local/var/www/magentok/magento2/app/code/Magento/SalesGraphQl/Model/Resolver/ProductResolver.php:46)"} []

Hence Confirming the issue.

Thanks.

github-jira-sync-bot commented 4 months ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-12332 is successfully created for this GitHub issue.

m2-assistant[bot] commented 4 months ago

:white_check_mark: Confirmed by @engcom-Bravo. Thank you for verifying the issue.
Issue Available: @engcom-Bravo, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

quterorta commented 3 months ago

@magento I'm working on this

quterorta commented 3 months ago

I guess it's a normal behavior. If you delete the product and try to get the customer's order list via GraphQl, you get the next response: As you can see, the order list is still available, you just can't get product data (obviously, because you deleted it). So, hypothetically, it's possible to add order item data to product data, and the rest of it provided as null, but I think it's redundant.

quterorta commented 3 months ago

Also, this issue doesn't look like an accidental bug, this behavior was developed to throw exceptions, if an order item doesn't have associated product data (for example, in our case, when the product was deleted)

pmzandbergen commented 3 months ago

I think the exception is redundant and shouldn't be throwed. It's a nullable field, and besides the error message thrown isn't really clear to frontend / the GraphQL API user.

Simply returning null is correct. The type hint in the Order Item resolver is also wrong: https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/SalesGraphQl/Model/OrderItem/DataProvider.php#L137

/** @var ProductInterface $associatedProduct */

Should be:

/** @var ProductInterface|null $associatedProduct */

Or even better, add a private function with a correct return type.

@quterorta since I've already created a patch for this issue: Are you working on the code or do you want me to create a PR?

quterorta commented 3 months ago

@pmzandbergen Hm, I agree with you. Yeah, if you have already created a patch for it, you could create PR, no problems

pmzandbergen commented 3 months ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 3 months ago

Hi @pmzandbergen. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 3 months ago

Hi @pmzandbergen, here is your Magento Instance: https://11bfd466f8c258a1113feb50fd641584.instances-prod.magento-community.engineering Admin access: https://11bfd466f8c258a1113feb50fd641584.instances-prod.magento-community.engineering/admin_a69f Login: 62be90e3 Password: 2bf33469180f

dimitriBouteille commented 2 weeks ago

Hi @zakdma Any news for this issue ?

Maybe someone has already made a patch? Because I have this problem too :(

pmzandbergen commented 2 weeks ago

@dimitriBouteille depends on what you want to see as a fix (nullable product, product ignoring the state and scope). Simple fix could be:

--- Model/OrderItem/DataProvider.php.org    2024-07-17 10:59:31
+++ Model/OrderItem/DataProvider.php    2024-07-17 11:01:09
@@ -10,6 +10,7 @@
 use Magento\Catalog\Api\Data\ProductInterface;
 use Magento\Catalog\Api\ProductRepositoryInterface;
 use Magento\Framework\Api\SearchCriteriaBuilder;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Sales\Api\Data\OrderInterface;
 use Magento\Sales\Api\Data\OrderItemInterface;
 use Magento\Sales\Api\OrderItemRepositoryInterface;
@@ -184,15 +185,13 @@
             $orderItems
         );

-        $searchCriteria = $this->searchCriteriaBuilder
-            ->addFilter('entity_id', $productIds, 'in')
-            ->create();
-        $products = $this->productRepository->getList($searchCriteria)->getItems();
-        $productList = [];
-        foreach ($products as $product) {
-            $productList[$product->getId()] = $product;
-        }
-        return $productList;
+        return array_combine($productIds, array_map(function (mixed $productId): ?ProductInterface {
+            try {
+                return $this->productRepository->getById($productId);
+            } catch (NoSuchEntityException $e) {
+                return null;
+            }
+        }, $productIds));
     }

     /**

pmzandbergen commented 2 weeks ago

@dimitriBouteille note that this fix is not as efficient, since we’re (trying to) load(ing) each product independently.

Using the deferred data provider would be a more efficient solution. However the existing one also has a few bugs.

Planning to fix those and this one when I’ve got some spare time.

magento / magento2