Fix > Exception #0 (BadMethodCallException): Missing required argument $msrpPriceCalculators of Magento\Msrp\Pricing\MsrpPriceCalculator.

[lfluvisotto]

Description (*)

Fix > Exception #0 (BadMethodCallException): Missing required argument $msrpPriceCalculators of Magento\Msrp\Pricing\MsrpPriceCalculator.

The possible solution is a workaround for the case of modules Magento_MsrpConfigurableProduct and Magento_MsrpGroupedProduct are disabled.

Fixed Issues (if relevant)

#22090 MsrpPriceCalculator exception after upgrade to 2.3.1

#22190: Exception (BadMethodCallException): Missing required argument $msrpPriceCalculators of Magento\Msrp\Pricing\MsrpPriceCalculator.

Manual testing scenarios (*)

1. Disable Magento_MsrpConfigurableProduct and Magento_MsrpGroupedProduct modules, Magento_Msrp should work properly.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[m2-assistant]

Hi @lfluvisotto. 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-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[ghost]

@lfluvisotto unfortunately, only members of the maintainers team are allowed to assign developers to the pull request

[ghost]

@lfluvisotto unfortunately, only members of the maintainers team are allowed to assign developers to the pull request

[davidverholen]

@lfluvisotto I think it should be sufficient to set the default value in the constructor to an empty array
This way the value is not required anymore which would fix the first error.

/**
 * @param array $msrpPriceCalculators
 */
public function __construct(array $msrpPriceCalculators = [])
{
     $this->msrpPriceCalculators = $this->getMsrpPriceCalculators($msrpPriceCalculators);
}

The foreach in getMsrpPriceCalculators then just does nothing (if the other modules are disabled) and the getMsrpPriceValue should fall back to the default case $msrp = (float)$product->getMsrp();

[lfluvisotto]

@davidverholen

Sure, nice, I did the change.

[magento-engcom-team]

Hi @orlangur, thank you for the review.
ENGCOM-4706 has been created to process this Pull Request

[VasylShvorak]

✔️ QA passed

[cds-magento2]

Is there an eta for a fix release? We are trying to upgrade to 2.3.1 and getting this error.
We are also seeing this error: report.CRITICAL:
Type Error occurred when creating object: Magento\Msrp\Pricing\MsrpPriceCalculator {"exception":"[object] (Magento\Framework\Exception\RuntimeException(code: 0): Type Error occurred when creating object: Magento\Msrp\Pricing\MsrpPriceCalculator at /opt/magento/demo/releases/20190416153530/vendor/magento/framework/ObjectManager/Factory/AbstractFactory.php:124)"} []

[orlangur]

@cds-magento2 you can use fix from this PR, it is pretty trivial. Fix will be released no earlier than 2.3.2 is out.

[janmyszkier]

this PR doesn't solve the initial isue which is passing null into the method in the first place.
if you pass a null it will still try to pas it to $this->getMsrpPriceCalculators as null (because you HAVE PROVIDED null. defaul values only work when you didn't pass anything.

\Magento\Msrp\Pricing\MsrpPriceCalculator::getMsrpPriceCalculators will also fail for the same reason the argument being null because it also requires an array and will throw the same error as
\Magento\Msrp\Pricing\MsrpPriceCalculator::__construct

Please make a testcase for PR a requirement.
simple code as this:

<?php
declare(strict_types=1);
class Test {
  function test1($argument = []){
    var_dump($argument);
  }
}

$test = new Test();
$test->test1(null);

would confirm the value is STILL null after providing the default values of [] and this PR doesn't completely solve the issue.

[davidverholen]

@janmyszkier are you sure? why would someone pass null to it? The object manager should (and normally does) pass nothing if there is no di configuration and so the default value should be taken

[orlangur]

@janmyszkier,

the initial issue which is passing null into the method in the first place

I believe you didn't understand root cause of the issue correctly, try more :)

[m2-assistant]

Hi @lfluvisotto, 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.

[janmyszkier]

@orlangur truth is: the issue with passing null is different from initial comment in this thread (my bad), the issue with null is still there for multicurrency setup though but since I fixed this internally with an override I'll wait for 2.3.2 and maybe submit separate issue then.