Item resolvers should be optional

[thomas-kl1]

Item resolvers should be optional. If we remove configurable and grouped product, checkout is broken because this argument is mandatory. Only the grouper and configurable product add item resolver.

Resolved issues:

1. resolves [Issue] Item resolvers should be optional #30860: Item resolvers should be optional

Preconditions (*)

1. Customer created
2. Simple Product created

Steps to reproduce (*)

1. Execute:

bin/magento module:disable Magento_GroupedImportExport
bin/magento module:disable Magento_GroupedCatalogInventory
bin/magento module:disable Magento_GroupedProductGraphQl
bin/magento module:disable Magento_MsrpGroupedProduct
bin/magento module:disable Magento_GroupedProduct
bin/magento module:disable Magento_CatalogRuleConfigurable
bin/magento module:disable Magento_MsrpConfigurableProduct
bin/magento module:disable Magento_NewRelicReporting
bin/magento module:disable Magento_ConfigurableProductGraphQl
bin/magento module:disable Magento_ConfigurableProductSales
bin/magento module:disable Magento_ConfigurableImportExport
bin/magento module:disable Magento_SwatchesGraphQl
bin/magento module:disable Magento_Swatches
bin/magento module:disable Magento_ConfigurableProduct
bin/magento setup:upgrade

2. Login on storefront.
3. Add Simple Product to cart.
4. Go to Checkout.
5. Create an Order.

[m2-assistant]

Hi @thomas-kl1. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[ihor-sviziev]

Hi @thomas-kl1,
Could you add some steps to reproduce, actual and expected result?
Unfortunately it's not clear how to important to fix the issue and if your solution is correct.

[thomas-kl1]

Take a look on the implementation of the method:

/**
 * {@inheritdoc}
 */
public function getFinalProduct(ItemInterface $item) : ProductInterface
{
    $finalProduct = $item->getProduct();
    foreach ($this->itemResolvers as $resolver) {
        $resolvedProduct = $this->getItemResolverInstance($resolver)->getFinalProduct($item);
        if ($resolvedProduct !== $finalProduct) {
            $finalProduct = $resolvedProduct;
            break;
        }
    }
    return $finalProduct;
}

Default case should return the item product. Then it iterates over resolvers for specific type of products. However in this case it requires that itemResolvers is an array. But if the grouped and configurable products module is not there, it's breaking the checkout.. By design the catalog module should works regardless other product type implementation

[ihor-sviziev]

@thomas-kl1 I see that in constructor we have defined type - array, so we shouldn't have any other options. Empty array should be by default.

Could you provide more info, what exception do you have? What value are you getting in the $itemResolvers are you getting when the grouped and configurable products module is not there?

[thomas-kl1]

I got this: Invalid argument supplied for foreach(), value is null

[ihor-sviziev]

Hi @thomas-kl1,
It doesn't seems to me logical.

Do you have some custom extension that changes this part? Could you provide full output of exception?

Still not clear where do you having this issue, on some frontend page or during some CLI command run?

[thomas-kl1]

The issue occurs on the checkout and cart page. What does not seems logical there? You can edit composer json and replace both module, or create your own metapackage with magento2-base and catalog modules.
There is no third party modules. Only magento2 official modules

[ihor-sviziev]

@thomas-kl1 thx for explanation, I'll check it myself and will give you my feedback a bit later.

[thomas-kl1]

My bad I still had some debug trace. The actual error message was different and not that value null is used in foreach. I can post the actual exception later. Sorry

[thomas-kl1]

Here the exception message:

[2020-11-06 16:04:57] main.CRITICAL: Missing required argument $itemResolvers of Magento\Catalog\Model\Product\Configuration\Item\ItemResolverComposite. [] []

[m2-assistant]

Hi @thomas-kl1, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.