MAGETWO-60248: Billing Address and Default Shipping Address checkboxes on Customer page are saved incorrectly

Author: OlgaVasyltsun

app/code/Magento/Customer/Controller/Adminhtml/Index/Save.php

@@ -13,9 +13,7 @@
 use Magento\Customer\Model\Metadata\Form;
 use Magento\Framework\Exception\LocalizedException;
 
-/**
- * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
- */
+
 class Save extends \Magento\Customer\Controller\Adminhtml\Index
 {
     /**
@@ -81,6 +79,7 @@ protected function _extractData(
     ) {
         $metadataForm = $metadataForm ? $metadataForm : $this->getMetadataForm($entityType, $formCode, $scope);
         $formData = $metadataForm->extractData($request, $scope);
+        $formData = $metadataForm->compactData($formData);
 
         // Initialize additional attributes
         /** @var \Magento\Framework\DataObject $object */
@@ -90,11 +89,6 @@ protected function _extractData(
             $formData[$attributeCode] = isset($requestData[$attributeCode]) ? $requestData[$attributeCode] : false;
         }
 
-        $result = $metadataForm->compactData($formData);
-
-        // Re-initialize additional attributes
-        $formData = array_replace($formData, $result);
-
         // Unset unused attributes
         $formAttributes = $metadataForm->getAttributes();
         foreach ($formAttributes as $attribute) {

app/code/Magento/Customer/Test/Unit/Controller/Adminhtml/Index/SaveTest.php

@@ -302,33 +302,30 @@ public function testExecuteWithExistentCustomer()
             ],
             'subscription' => $subscription,
         ];
-        $filteredData = [
+        $extractedData = [
             'entity_id' => $customerId,
             'code' => 'value',
             'coolness' => false,
             'disable_auto_group_change' => 'false',
         ];
-        $dataToCompact = [
+        $compactedData = [
             'entity_id' => $customerId,
             'code' => 'value',
             'coolness' => false,
             'disable_auto_group_change' => 'false',
-            CustomerInterface::DEFAULT_BILLING => false,
-            CustomerInterface::DEFAULT_SHIPPING => false,
-            'confirmation' => false,
-            'sendemail_store_id' => false,
-            'extension_attributes' => false,
+            CustomerInterface::DEFAULT_BILLING => 2,
+            CustomerInterface::DEFAULT_SHIPPING => 2
         ];
-        $addressFilteredData = [
+        $addressExtractedData = [
             'entity_id' => $addressId,
-            'default_billing' => 'true',
-            'default_shipping' => 'true',
+/*            'default_billing' => 'true',
+            'default_shipping' => 'true',*/
             'code' => 'value',
             'coolness' => false,
             'region' => 'region',
             'region_id' => 'region_id',
         ];
-        $addressDataToCompact = [
+        $addressCompactedData = [
             'entity_id' => $addressId,
             'default_billing' => 'true',
             'default_shipping' => 'true',
@@ -428,11 +425,11 @@ public function testExecuteWithExistentCustomer()
         $customerFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'customer')
-            ->willReturn($filteredData);
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('compactData')
-            ->with($dataToCompact)
-            ->willReturn($filteredData);
+            ->with($extractedData)
+            ->willReturn($compactedData);
         $customerFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);
@@ -443,11 +440,11 @@ public function testExecuteWithExistentCustomer()
         $customerAddressFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'address/' . $addressId)
-            ->willReturn($addressFilteredData);
+            ->willReturn($addressExtractedData);
         $customerAddressFormMock->expects($this->once())
             ->method('compactData')
-            ->with($addressDataToCompact)
-            ->willReturn($addressFilteredData);
+            ->with($addressExtractedData)
+            ->willReturn($addressCompactedData);
         $customerAddressFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);
@@ -635,33 +632,13 @@ public function testExecuteWithNewCustomer()
             ],
             'subscription' => $subscription,
         ];
-        $filteredData = [
+        $extractedData = [
             'coolness' => false,
             'disable_auto_group_change' => 'false',
         ];
-        $dataToCompact = [
-            'coolness' => false,
-            'disable_auto_group_change' => 'false',
-            CustomerInterface::DEFAULT_BILLING => false,
-            CustomerInterface::DEFAULT_SHIPPING => false,
-            'confirmation' => false,
-            'sendemail_store_id' => false,
-            'extension_attributes' => false,
-        ];
-        $addressFilteredData = [
+        $addressExtractedData = [
             'entity_id' => $addressId,
-            'default_billing' => 'false',
-            'default_shipping' => 'false',
-            'code' => 'value',
-            'coolness' => false,
-            'region' => 'region',
-            'region_id' => 'region_id',
-        ];
-        $addressDataToCompact = [
-            'entity_id' => $addressId,
-            'default_billing' => 'false',
-            'default_shipping' => 'false',
-            'code' => 'value',
+             'code' => 'value',
             'coolness' => false,
             'region' => 'region',
             'region_id' => 'region_id',
@@ -739,11 +716,11 @@ public function testExecuteWithNewCustomer()
         $customerFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'customer')
-            ->willReturn($filteredData);
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('compactData')
-            ->with($dataToCompact)
-            ->willReturn($filteredData);
+            ->with($extractedData)
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);
@@ -754,11 +731,11 @@ public function testExecuteWithNewCustomer()
         $customerAddressFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'address/' . $addressId)
-            ->willReturn($addressFilteredData);
+            ->willReturn($addressExtractedData);
         $customerAddressFormMock->expects($this->once())
             ->method('compactData')
-            ->with($addressDataToCompact)
-            ->willReturn($addressFilteredData);
+            ->with($addressExtractedData)
+            ->willReturn($addressExtractedData);
         $customerAddressFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);
@@ -912,19 +889,10 @@ public function testExecuteWithNewCustomerAndValidationException()
             ],
             'subscription' => $subscription,
         ];
-        $filteredData = [
+        $extractedData = [
             'coolness' => false,
             'disable_auto_group_change' => 'false',
         ];
-        $dataToCompact = [
-            'coolness' => false,
-            'disable_auto_group_change' => 'false',
-            CustomerInterface::DEFAULT_BILLING => false,
-            CustomerInterface::DEFAULT_SHIPPING => false,
-            'confirmation' => false,
-            'sendemail_store_id' => false,
-            'extension_attributes' => false,
-        ];
 
         /** @var AttributeMetadataInterface|\PHPUnit_Framework_MockObject_MockObject $customerFormMock */
         $attributeMock = $this->getMockBuilder('Magento\Customer\Api\Data\AttributeMetadataInterface')
@@ -973,11 +941,11 @@ public function testExecuteWithNewCustomerAndValidationException()
         $customerFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'customer')
-            ->willReturn($filteredData);
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('compactData')
-            ->with($dataToCompact)
-            ->willReturn($filteredData);
+            ->with($extractedData)
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);
@@ -1064,18 +1032,9 @@ public function testExecuteWithNewCustomerAndLocalizedException()
             ],
             'subscription' => $subscription,
         ];
-        $filteredData = [
-            'coolness' => false,
-            'disable_auto_group_change' => 'false',
-        ];
-        $dataToCompact = [
+        $extractedData = [
             'coolness' => false,
             'disable_auto_group_change' => 'false',
-            CustomerInterface::DEFAULT_BILLING => false,
-            CustomerInterface::DEFAULT_SHIPPING => false,
-            'confirmation' => false,
-            'sendemail_store_id' => false,
-            'extension_attributes' => false,
         ];
 
         /** @var AttributeMetadataInterface|\PHPUnit_Framework_MockObject_MockObject $customerFormMock */
@@ -1126,11 +1085,11 @@ public function testExecuteWithNewCustomerAndLocalizedException()
         $customerFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'customer')
-            ->willReturn($filteredData);
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('compactData')
-            ->with($dataToCompact)
-            ->willReturn($filteredData);
+            ->with($extractedData)
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);
@@ -1216,19 +1175,10 @@ public function testExecuteWithNewCustomerAndException()
             ],
             'subscription' => $subscription,
         ];
-        $filteredData = [
+        $extractedData = [
             'coolness' => false,
             'disable_auto_group_change' => 'false',
         ];
-        $dataToCompact = [
-            'coolness' => false,
-            'disable_auto_group_change' => 'false',
-            CustomerInterface::DEFAULT_BILLING => false,
-            CustomerInterface::DEFAULT_SHIPPING => false,
-            'confirmation' => false,
-            'sendemail_store_id' => false,
-            'extension_attributes' => false,
-        ];
 
         /** @var AttributeMetadataInterface|\PHPUnit_Framework_MockObject_MockObject $customerFormMock */
         $attributeMock = $this->getMockBuilder('Magento\Customer\Api\Data\AttributeMetadataInterface')
@@ -1277,11 +1227,11 @@ public function testExecuteWithNewCustomerAndException()
         $customerFormMock->expects($this->once())
             ->method('extractData')
             ->with($this->requestMock, 'customer')
-            ->willReturn($filteredData);
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('compactData')
-            ->with($dataToCompact)
-            ->willReturn($filteredData);
+            ->with($extractedData)
+            ->willReturn($extractedData);
         $customerFormMock->expects($this->once())
             ->method('getAttributes')
             ->willReturn($attributes);