Invalid Argument Type in `QuoteManagement::afterSubmit` Plugin

[drew7721]

I just ran into a small issue when attempting to place an order.

For some business logic reasons in our implementation of the checkout we have been forced to create our own implementation of the CartManagementInterface. This means that when this gets executed it throws a type error.

So, instead of QuoteManagementEntity I believe it would be best to expect \Magento\Quote\Api\CartManagementInterface - It's always best to expect the interface as enforcing the type will cause issues with customized code.

https://github.com/taxjar/taxjar-magento2-extension/blob/7981a00919780ec4acf9ae35b4f2a42a65a4b714/Plugin/Quote/Model/QuoteManagement.php#L66-L69

Also, plugins taxjar_quote_management and taxjar_quote_management_after_submit seem redundant as one is set on the interface and the other on the class that implements the interface.

https://github.com/taxjar/taxjar-magento2-extension/blob/7981a00919780ec4acf9ae35b4f2a42a65a4b714/etc/di.xml#L49-L57

I would keep this plugin on the interface only and by doing that also changing the type in the signature should allow a wider compatibility.

As an example check out Magento\Braintree\Plugin\OrderCancellation -> aroundPlaceOrder.

I've tested locally with the suggested changes and it seems to keep working but I'm not running the latest M2 version.

[sethobey]

@drew7721 I can certainly see how that plugin method signature would conflict with a custom implementation. We'll get your fix reviewed and follow up there. Thank you for taking the time to submit this issue!

[drew7721]

Thank you. In the /pull/327 I did not remove the double addition of the plugin https://github.com/taxjar/taxjar-magento2-extension/blob/7981a00919780ec4acf9ae35b4f2a42a65a4b714/etc/di.xml#L55-L57

Not sure if there was any specific reason for that to be added twice. But I did not have the time to test without the second plugin, though it should work with it just set on the interface.

I would recommend keeping just the one on the interface and testing like that to make sure all keeps working as expected (it should)

[raghuiamsingh]

@sethobey we upgraded to the latest version of TaxJar module (v1.9.8) and still getting the same error.

Argument 2 passed to Taxjar\SalesTax\Plugin\Quote\Model\QuoteManagement::afterSubmit() must implement interface Magento\Sales\Api\Data\OrderInterface, null given, called in /var/www/releases/release-test/vendor/magento/framework/Interception/Interceptor.php on line 146

Please advise.

[sethobey]

Hi @raghuiamsingh, thank you for continuing this issue!

The error you are receiving appears to be due to the afterSubmit plugin method's type hinting. Although Magento\Quote\Model\QuoteManagement::submit() should generally return an order, in cases of failure or invalid cart contents, it can return null. I see that we will also need to modify our plugin's second parameter to allow for potential null values to pass.

Magento should normally throw an exception anytime the submit method returns null instead of an order as shown here: https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Quote/Model/QuoteManagement.php#L443 so hopefully this issue has posed minimal risk to your data integrity. Either way, I will get this patched today with this PR: https://github.com/taxjar/taxjar-magento2-extension/pull/331. 👍

In the meantime, if you have a chance please feel free to validate the solution provided in that PR.

[raghuiamsingh]

@sethobey Thanks for the prompt reply. I am going to try the change in PR https://github.com/taxjar/taxjar-magento2-extension/pull/331

[raghuiamsingh]

@sethobey I tried your change, I did fix the first error but now its giving another error: Return value of Taxjar\SalesTax\Plugin\Quote\Model\QuoteManagement::afterSubmit() must implement interface Magento\Sales\Api\Data\OrderInterface, null returned

I think you need to allow null in return as well, like you did for the second parameter.

[sethobey]

@raghuiamsingh - My apologies, and thank you for validating! I've updated #331 to account for the nullable return type and added a unit test for the nullable return as well.