MAGETWO-64833: [Github] PayPal Express Checkout Discards Address Line 2 For Billing and Shipping Addresses #8313

isavchuk-magento · isavchuk-magento · commit b424fdc22c4d · 2017-03-02T17:38:52.000+02:00
- Fixed handling for the 'street2' additional param from the Paypal response
- Removed code that allows to mix system address data and data from Paypal response what could lead to unpredictable mixing data
- Added integration tests
diff --git a/app/code/Magento/Paypal/Model/Api/Nvp.php b/app/code/Magento/Paypal/Model/Api/Nvp.php
@@ -1482,9 +1482,9 @@ protected function _exportAddressses($data)
     protected function _applyStreetAndRegionWorkarounds(\Magento\Framework\DataObject $address)
     {
         // merge street addresses into 1
-        if ($address->hasStreet2()) {
-            $address->setStreet(implode("\n", [$address->getStreet(), $address->getStreetLine(2)]));
-            $address->unsStreet2();
+        if ($address->getData('street2') !== null) {
+            $address->setStreet(implode("\n", [$address->getData('street'), $address->getData('street2')]));
+            $address->unsetData('street2');
         }
         // attempt to fetch region_id from directory
         if ($address->getCountryId() && $address->getRegion()) {
diff --git a/app/code/Magento/Paypal/Model/Express/Checkout.php b/app/code/Magento/Paypal/Model/Express/Checkout.php
@@ -905,30 +905,15 @@ public function getCheckoutMethod()
     protected function _setExportedAddressData($address, $exportedAddress)
     {
         // Exported data is more priority if we came from Express Checkout button
-        $isButton  = (bool)$this->_quote->getPayment()->getAdditionalInformation(self::PAYMENT_INFO_BUTTON);
+        $isButton = (bool)$this->_quote->getPayment()->getAdditionalInformation(self::PAYMENT_INFO_BUTTON);
         if (!$isButton) {
-            foreach ($exportedAddress->getExportedKeys() as $key) {
-                $oldData = $address->getDataUsingMethod($key);
-                $isEmpty = null;
-                if (is_array($oldData)) {
-                    foreach ($oldData as $val) {
-                        if (!empty($val)) {
-                            $isEmpty = false;
-                            break;
-                        }
-                        $isEmpty = true;
-                    }
-                }
-                if (empty($oldData) || $isEmpty === true) {
-                    $address->setDataUsingMethod($key, $exportedAddress->getData($key));
-                }
-            }
-        } else {
-            foreach ($exportedAddress->getExportedKeys() as $key) {
-                $data = $exportedAddress->getData($key);
-                if (!empty($data)) {
-                    $address->setDataUsingMethod($key, $data);
-                }
+            return;
+        }
+
+        foreach ($exportedAddress->getExportedKeys() as $key) {
+            $data = $exportedAddress->getData($key);
+            if (!empty($data)) {
+                $address->setDataUsingMethod($key, $data);
             }
         }
     }
diff --git a/dev/tests/integration/testsuite/Magento/Paypal/Model/Express/CheckoutTest.php b/dev/tests/integration/testsuite/Magento/Paypal/Model/Express/CheckoutTest.php
@@ -5,9 +5,15 @@
  */
 namespace Magento\Paypal\Model\Express;
 
+use Magento\Framework\ObjectManagerInterface;
 use Magento\Quote\Model\Quote;
 use Magento\Checkout\Model\Type\Onepage;
 use Magento\TestFramework\Helper\Bootstrap;
+use Magento\Paypal\Model\Config;
+use Magento\Paypal\Model\Express\Checkout;
+use Magento\Paypal\Model\Api\Type\Factory;
+use Magento\Paypal\Model\Api\Nvp;
+use Magento\Paypal\Model\Info;
 
 /**
  * Class CheckoutTest
@@ -17,9 +23,34 @@
 class CheckoutTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @var \Magento\Framework\ObjectManagerInterface
+     * @var ObjectManagerInterface
      */
-    protected $_objectManager;
+    private $_objectManager;
+
+    /**
+     * @var Info
+     */
+    private $paypalInfo;
+
+    /**
+     * @var Config
+     */
+    private $paypalConfig;
+
+    /**
+     * @var Factory
+     */
+    private $apiTypeFactory;
+
+    /**
+     * @var Nvp
+     */
+    private $api;
+
+    /**
+     * @var Checkout
+     */
+    private $checkoutModel;
 
     /**
      * Set up
@@ -29,6 +60,31 @@ class CheckoutTest extends \PHPUnit_Framework_TestCase
     protected function setUp()
     {
         $this->_objectManager = Bootstrap::getObjectManager();
+
+        $this->paypalInfo = $this->getMockBuilder(Info::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->paypalConfig = $this->getMockBuilder(Config::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->api = $this->getMockBuilder(Nvp::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['call', 'getExportedShippingAddress', 'getExportedBillingAddress'])
+            ->getMock();
+
+        $this->api->expects($this->any())
+            ->method('call')
+            ->will($this->returnValue([]));
+
+        $this->apiTypeFactory = $this->getMockBuilder(Factory::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->apiTypeFactory->expects($this->any())
+            ->method('create')
+            ->will($this->returnValue($this->api));
     }
 
     /**
@@ -111,7 +167,7 @@ public function testPlaceGuestQuote()
     protected function _getCheckout(Quote $quote)
     {
         return $this->_objectManager->create(
-            \Magento\Paypal\Model\Express\Checkout::class,
+            Checkout::class,
             [
                 'params' => [
                     'config' => $this->getMock(\Magento\Paypal\Model\Config::class, [], [], '', false),
@@ -131,43 +187,30 @@ protected function _getCheckout(Quote $quote)
     public function testReturnFromPaypal()
     {
         $quote = $this->_getFixtureQuote();
-        $paypalConfigMock = $this->getMock(\Magento\Paypal\Model\Config::class, [], [], '', false);
-        $apiTypeFactory = $this->getMock(\Magento\Paypal\Model\Api\Type\Factory::class, [], [], '', false);
-        $paypalInfo = $this->getMock(\Magento\Paypal\Model\Info::class, [], [], '', false);
-        $checkoutModel = $this->_objectManager->create(
-            \Magento\Paypal\Model\Express\Checkout::class,
+        $this->checkoutModel = $this->_objectManager->create(
+            Checkout::class,
             [
-                'params' => ['quote' => $quote, 'config' => $paypalConfigMock],
-                'apiTypeFactory' => $apiTypeFactory,
-                'paypalInfo' => $paypalInfo
+                'params' => ['quote' => $quote, 'config' => $this->paypalConfig],
+                'apiTypeFactory' => $this->apiTypeFactory,
+                'paypalInfo' => $this->paypalInfo
             ]
         );
 
-        $api = $this->getMock(
-            \Magento\Paypal\Model\Api\Nvp::class,
-            ['call', 'getExportedShippingAddress', 'getExportedBillingAddress'],
-            [],
-            '',
-            false
-        );
-        $api->expects($this->any())->method('call')->will($this->returnValue([]));
-        $apiTypeFactory->expects($this->any())->method('create')->will($this->returnValue($api));
-
         $exportedBillingAddress = $this->_getExportedAddressFixture($quote->getBillingAddress()->getData());
-        $api->expects($this->any())
+        $this->api->expects($this->any())
             ->method('getExportedBillingAddress')
             ->will($this->returnValue($exportedBillingAddress));
 
         $exportedShippingAddress = $this->_getExportedAddressFixture($quote->getShippingAddress()->getData());
-        $api->expects($this->any())
+        $this->api->expects($this->any())
             ->method('getExportedShippingAddress')
             ->will($this->returnValue($exportedShippingAddress));
 
-        $paypalInfo->expects($this->once())->method('importToPayment')->with($api, $quote->getPayment());
+        $this->paypalInfo->expects($this->once())->method('importToPayment')->with($this->api, $quote->getPayment());
 
         $quote->getPayment()->setAdditionalInformation(Checkout::PAYMENT_INFO_BUTTON, 1);
 
-        $checkoutModel->returnFromPaypal('token');
+        $this->checkoutModel->returnFromPaypal('token');
 
         $billingAddress = $quote->getBillingAddress();
 
@@ -182,7 +225,6 @@ public function testReturnFromPaypal()
         $this->assertNull($shippingAddress->getSuffix());
         $this->assertTrue($shippingAddress->getShouldIgnoreValidation());
         $this->assertContains('exported', $shippingAddress->getFirstname());
-
         $paymentAdditionalInformation = $quote->getPayment()->getAdditionalInformation();
         $this->assertArrayHasKey(Checkout::PAYMENT_INFO_TRANSPORT_SHIPPING_METHOD, $paymentAdditionalInformation);
         $this->assertArrayHasKey(Checkout::PAYMENT_INFO_TRANSPORT_PAYER_ID, $paymentAdditionalInformation);
@@ -191,6 +233,111 @@ public function testReturnFromPaypal()
         $this->assertTrue($quote->getTotalsCollectedFlag());
     }
 
+    /**
+     * The case when handling address data from Paypal button.
+     * System's address fields are replacing from export Paypal data.
+     *
+     * @magentoDataFixture Magento/Paypal/_files/quote_payment_express_with_customer.php
+     * @magentoAppIsolation enabled
+     * @magentoDbIsolation enabled
+     */
+    public function testReturnFromPaypalButton()
+    {
+        $quote = $this->_getFixtureQuote();
+        $this->prepareCheckoutModel($quote);
+        $quote->getPayment()->setAdditionalInformation(Checkout::PAYMENT_INFO_BUTTON, 1);
+
+        $this->checkoutModel->returnFromPaypal('token');
+
+        $shippingAddress = $quote->getShippingAddress();
+
+        $prefix = 'exported';
+        $this->assertEquals([$prefix .$this->getExportedData()['street']], $shippingAddress->getStreet());
+        $this->assertEquals($prefix . $this->getExportedData()['firstname'], $shippingAddress->getFirstname());
+        $this->assertEquals($prefix . $this->getExportedData()['city'], $shippingAddress->getCity());
+        $this->assertEquals($prefix . $this->getExportedData()['telephone'], $shippingAddress->getTelephone());
+        // This fields not in exported keys list. Fields the same as quote shipping and billing address.
+        $this->assertNotEquals($prefix . $this->getExportedData()['region'], $shippingAddress->getRegion());
+        $this->assertNotEquals($prefix . $this->getExportedData()['email'], $shippingAddress->getEmail());
+    }
+
+    /**
+     * The case when handling address data from the checkout.
+     * System's address fields are not replacing from export Paypal data.
+     *
+     * @magentoDataFixture Magento/Paypal/_files/quote_payment_express_with_customer.php
+     * @magentoAppIsolation enabled
+     * @magentoDbIsolation enabled
+     */
+    public function testReturnFromPaypalIfCheckout()
+    {
+        $quote = $this->_getFixtureQuote();
+        $this->prepareCheckoutModel($quote);
+        $quote->getPayment()->setAdditionalInformation(Checkout::PAYMENT_INFO_BUTTON, 0);
+
+        $this->checkoutModel->returnFromPaypal('token');
+
+        $shippingAddress = $quote->getShippingAddress();
+
+        $prefix = 'exported';
+
+        $this->assertNotEquals([$prefix .$this->getExportedData()['street']], $shippingAddress->getStreet());
+        $this->assertNotEquals($prefix . $this->getExportedData()['firstname'], $shippingAddress->getFirstname());
+        $this->assertNotEquals($prefix . $this->getExportedData()['city'], $shippingAddress->getCity());
+        $this->assertNotEquals($prefix . $this->getExportedData()['telephone'], $shippingAddress->getTelephone());
+    }
+
+    /**
+     * Initialize a checkout model mock.
+     *
+     * @param Quote $quote
+     */
+    private function prepareCheckoutModel(Quote $quote)
+    {
+        $this->checkoutModel = $this->_objectManager->create(
+            Checkout::class,
+            [
+                'params'         => ['quote' => $quote, 'config' => $this->paypalConfig],
+                'apiTypeFactory' => $this->apiTypeFactory,
+                'paypalInfo'     => $this->paypalInfo
+            ]
+        );
+
+        $exportedBillingAddress = $this->_getExportedAddressFixture($this->getExportedData());
+        $this->api->expects($this->any())
+            ->method('getExportedBillingAddress')
+            ->will($this->returnValue($exportedBillingAddress));
+
+        $exportedShippingAddress = $this->_getExportedAddressFixture($this->getExportedData());
+        $this->api->expects($this->any())
+            ->method('getExportedShippingAddress')
+            ->will($this->returnValue($exportedShippingAddress));
+
+        $this->paypalInfo->expects($this->once())
+            ->method('importToPayment')
+            ->with($this->api, $quote->getPayment());
+    }
+
+    /**
+     * A Paypal response stub.
+     *
+     * @return array
+     */
+    private function getExportedData()
+    {
+        return
+            [
+                'company'    => 'Testcompany',
+                'email'      => 'buyeraccountmpi@gmail.com',
+                'firstname'  => 'testFirstName',
+                'country_id' => 'US',
+                'region'     => 'testRegion',
+                'city'       => 'testSity',
+                'street'     => 'testStreet',
+                'telephone'  => '223344',
+            ];
+    }
+
     /**
      * Prepare fixture for exported address
      *

Original file line number	Diff line number	Diff line change
`@@ -1482,9 +1482,9 @@ protected function _exportAddressses($data)`
`1482`	`1482`	`protected function _applyStreetAndRegionWorkarounds(\Magento\Framework\DataObject $address)`
`1483`	`1483`	`{`
`1484`	`1484`	`// merge street addresses into 1`
`1485`		`- if ($address->hasStreet2()) {`
`1486`		`- $address->setStreet(implode("\n", [$address->getStreet(), $address->getStreetLine(2)]));`
`1487`		`- $address->unsStreet2();`
	`1485`	`+ if ($address->getData('street2') !== null) {`
	`1486`	`+ $address->setStreet(implode("\n", [$address->getData('street'), $address->getData('street2')]));`
	`1487`	`+ $address->unsetData('street2');`
`1488`	`1488`	`}`
`1489`	`1489`	`// attempt to fetch region_id from directory`
`1490`	`1490`	`if ($address->getCountryId() && $address->getRegion()) {`