Company name taken from a forgone request

[jaykobi]

I sometimes have to create a lot of orders by code. It became evident now, that quite a lot (several hundreds) requests were send where the company name in the autorization request was the name of a foregone request. This leads to payments being matched to wrong orders.
Except for the company name, all customer data always was correct. The afftectet requests have consequent order_ids and very close txids.

I searched my database but all my sales_order_address and quote_addresses were correct. Now I checked in the extension code and I found the function addAdress in AddressRequest (which is extended by AuthrizationRequest)

protected function addAddress($oAddress, $blIsShipping = false)
    {
        $sPre = '';
        if ($blIsShipping === true) {
            $sPre = 'shipping_'; // add shipping prefix for shipping addresses
        }
        $this->addParameter($sPre.'firstname', $oAddress->getFirstname());
        $this->addParameter($sPre.'lastname', $oAddress->getLastname());
        if ($oAddress->getCompany()) {// company name existing?
            $this->addParameter($sPre.'company', $oAddress->getCompany());
        }

        $aStreet = $oAddress->getStreet();
        $sStreet = is_array($aStreet) ? implode(' ', $aStreet) : $aStreet; // street may be an array
        $this->addParameter($sPre.'street', trim($sStreet));
        $this->addParameter($sPre.'zip', $oAddress->getPostcode());
        $this->addParameter($sPre.'city', $oAddress->getCity());
        $this->addParameter($sPre.'country', $oAddress->getCountryId());

        if (CountryHelper::isStateNeeded($oAddress->getCountryId()) && $oAddress->getRegionCode()) {
            $this->addParameter($sPre.'state', $this->customerHelper->getRegionCode($oAddress));
        }
    }

The company ist not reset when current address does not have one. Without diffing further inot this I assume this is the reason for the explained behavoiur.

Here are some example txid pairs

236124436,236124433
236124452,236124450
236124460,236124456
236124475,236124478

I have a lot more examples if you need them ;)

A similar behaviour occurs with bankdata. I found requests with clearingtype "rec" that carry the IBAN form the request before.

It can be that my code has the bug, but I create empty quotes with Magento\Quote\Api\CartManagement::createEMptyCart() and then add sales items and customer information. As I said, my tables show no sign of any informaton from the foregone requests. And that the only affected customer information is company just fits so very nicely with the code quoted above.

[fjbender]

Thanks for your precise analysis. To save us the hassle of writing code that creates a lot of orders, can you test a workaround, where you explicitly reset the company parameter (or the entire address for that matter) and see if the issue goes away?

Thanks
Florian

[jaykobi]

I think for thr addresses it's just a matter or removing the if clause. I will have to check where the setting of bankdata occurs to remove that issue, too. I will post my findings here during the next days.

[jaykobi]

I looked a little deeper into the extension code and I found, that the request object itself gets injected when creating an order. Magentos DI component injects singletons so the same object is reused every time. Every parameter that was present on the last request and is ot overriden will continue to exist.

To create a blank request object for each order we have to inject a factory that creates the request object for us. These factory methods are created by Magento automatically when requested. I only tested it with AuthorizationRequest (and it works) but there is no reason the same pattern should not be applied to Debit, Capture,... as well.

To reproduce the problem and test the solution you will not have to create lots of orders. Just two in one "PHP run" will be enough.

diff --git a/Model/Methods/BaseMethod.php b/Model/Methods/BaseMethod.php
index deb08ee..89ebea1 100644
--- a/Model/Methods/BaseMethod.php
+++ b/Model/Methods/BaseMethod.php
@@ -208,7 +208,7 @@ abstract class BaseMethod extends AbstractMethod
      * @param \Magento\Checkout\Model\Session                         $checkoutSession
      * @param \Payone\Core\Model\Api\Request\Debit                    $debitRequest
      * @param \Payone\Core\Model\Api\Request\Capture                  $captureRequest
-     * @param \Payone\Core\Model\Api\Request\Authorization            $authorizationRequest
+     * @param \Payone\Core\Model\Api\Request\AuthorizationFactory     $authorizationRequestFactory
      * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
      * @param \Magento\Framework\Data\Collection\AbstractDb           $resourceCollection
      * @param array                                                   $data
@@ -227,7 +227,7 @@ abstract class BaseMethod extends AbstractMethod
         \Magento\Checkout\Model\Session $checkoutSession,
         \Payone\Core\Model\Api\Request\Debit $debitRequest,
         \Payone\Core\Model\Api\Request\Capture $captureRequest,
-        \Payone\Core\Model\Api\Request\Authorization $authorizationRequest,
+        \Payone\Core\Model\Api\Request\AuthorizationFactory $authorizationRequestFactory,
         \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
         \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
         array $data = []
@@ -239,7 +239,7 @@ abstract class BaseMethod extends AbstractMethod
         $this->checkoutSession = $checkoutSession;
         $this->debitRequest = $debitRequest;
         $this->captureRequest = $captureRequest;
-        $this->authorizationRequest = $authorizationRequest;
+        $this->authorizationRequest = $authorizationRequestFactory->create();
     }

[fjbender]

Wow, thanks :)

We'll see that we put this in one of the upcoming sprints.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.