ENGCOM-7785: Create or update customer don't validate customer group id #28903

slavvka · web-flow · commit bc633b6881ad · 2020-07-14T22:06:33.000-05:00
diff --git a/app/code/Magento/Customer/Model/ResourceModel/CustomerRepository.php b/app/code/Magento/Customer/Model/ResourceModel/CustomerRepository.php
@@ -10,6 +10,7 @@
 use Magento\Customer\Api\CustomerRepositoryInterface;
 use Magento\Customer\Api\Data\CustomerInterface;
 use Magento\Customer\Api\Data\CustomerSearchResultsInterfaceFactory;
+use Magento\Customer\Api\GroupRepositoryInterface;
 use Magento\Customer\Model\Customer as CustomerModel;
 use Magento\Customer\Model\Customer\NotificationStorage;
 use Magento\Customer\Model\CustomerFactory;
@@ -27,6 +28,8 @@
 use Magento\Framework\Api\SearchCriteriaInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Event\ManagerInterface;
+use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Store\Model\StoreManagerInterface;
 
 /**
@@ -119,6 +122,11 @@ class CustomerRepository implements CustomerRepositoryInterface
      */
     private $delegatedStorage;
 
+    /**
+     * @var GroupRepositoryInterface
+     */
+    private $groupRepository;
+
     /**
      * @param CustomerFactory $customerFactory
      * @param CustomerSecureFactory $customerSecureFactory
@@ -136,6 +144,7 @@ class CustomerRepository implements CustomerRepositoryInterface
      * @param CollectionProcessorInterface $collectionProcessor
      * @param NotificationStorage $notificationStorage
      * @param DelegatedStorage|null $delegatedStorage
+     * @param GroupRepositoryInterface|null $groupRepository
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -154,7 +163,8 @@ public function __construct(
         JoinProcessorInterface $extensionAttributesJoinProcessor,
         CollectionProcessorInterface $collectionProcessor,
         NotificationStorage $notificationStorage,
-        DelegatedStorage $delegatedStorage = null
+        DelegatedStorage $delegatedStorage = null,
+        ?GroupRepositoryInterface $groupRepository = null
     ) {
         $this->customerFactory = $customerFactory;
         $this->customerSecureFactory = $customerSecureFactory;
@@ -172,6 +182,7 @@ public function __construct(
         $this->collectionProcessor = $collectionProcessor;
         $this->notificationStorage = $notificationStorage;
         $this->delegatedStorage = $delegatedStorage ?? ObjectManager::getInstance()->get(DelegatedStorage::class);
+        $this->groupRepository = $groupRepository ?: ObjectManager::getInstance()->get(GroupRepositoryInterface::class);
     }
 
     /**
@@ -216,6 +227,7 @@ public function save(CustomerInterface $customer, $passwordHash = null)
                 $prevCustomerData ? $prevCustomerData->getStoreId() : $this->storeManager->getStore()->getId()
             );
         }
+        $this->validateGroupId($customer->getGroupId());
         $this->setCustomerGroupId($customerModel, $customerArr, $prevCustomerDataArr);
         // Need to use attribute set or future updates can cause data loss
         if (!$customerModel->getAttributeSetId()) {
@@ -268,10 +280,7 @@ public function save(CustomerInterface $customer, $passwordHash = null)
                     $savedAddressIds[] = $address->getId();
                 }
             }
-            $addressIdsToDelete = array_diff($existingAddressIds, $savedAddressIds);
-            foreach ($addressIdsToDelete as $addressId) {
-                $this->addressRepository->deleteById($addressId);
-            }
+            $this->deleteAddressesByIds(array_diff($existingAddressIds, $savedAddressIds));
         }
         $this->customerRegistry->remove($customerId);
         $savedCustomer = $this->get($customer->getEmail(), $customer->getWebsiteId());
@@ -286,6 +295,39 @@ public function save(CustomerInterface $customer, $passwordHash = null)
         return $savedCustomer;
     }
 
+    /**
+     * Delete addresses by ids
+     *
+     * @param array $addressIds
+     * @return void
+     */
+    private function deleteAddressesByIds(array $addressIds): void
+    {
+        foreach ($addressIds as $id) {
+            $this->addressRepository->deleteById($id);
+        }
+    }
+
+    /**
+     * Validate customer group id if exist
+     *
+     * @param int|null $groupId
+     * @return bool
+     * @throws LocalizedException
+     */
+    private function validateGroupId(?int $groupId): bool
+    {
+        if ($groupId) {
+            try {
+                $this->groupRepository->getById($groupId);
+            } catch (NoSuchEntityException $e) {
+                throw new LocalizedException(__('The specified customer group id does not exist.'));
+            }
+        }
+
+        return true;
+    }
+
     /**
      * Set secure data to customer model
      *
diff --git a/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/CustomerRepositoryTest.php b/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/CustomerRepositoryTest.php
@@ -249,7 +249,8 @@ public function testSave()
                 'getEmail',
                 'getWebsiteId',
                 'getAddresses',
-                'setAddresses'
+                'setAddresses',
+                'getGroupId',
             ]
         );
         $customerSecureData = $this->getMockBuilder(CustomerSecure::class)
@@ -433,7 +434,8 @@ public function testSaveWithPasswordHash()
                 'getEmail',
                 'getWebsiteId',
                 'getAddresses',
-                'setAddresses'
+                'setAddresses',
+                'getGroupId'
             ]
         );
         $customerModel->expects($this->atLeastOnce())
diff --git a/app/code/Magento/Customer/i18n/en_US.csv b/app/code/Magento/Customer/i18n/en_US.csv
@@ -542,3 +542,4 @@ Addresses,Addresses
 "The store view is not in the associated website.","The store view is not in the associated website."
 "The Store View selected for sending Welcome email from is not related to the customer's associated website.","The Store View selected for sending Welcome email from is not related to the customer's associated website."
 "Add/Update Address","Add/Update Address"
+"The specified customer group id does not exist.","The specified customer group id does not exist."
diff --git a/dev/tests/api-functional/testsuite/Magento/Customer/Api/CustomerRepositoryTest.php b/dev/tests/api-functional/testsuite/Magento/Customer/Api/CustomerRepositoryTest.php
@@ -34,13 +34,8 @@ class CustomerRepositoryTest extends WebapiAbstract
     const SERVICE_VERSION = 'V1';
     const SERVICE_NAME = 'customerCustomerRepositoryV1';
     const RESOURCE_PATH = '/V1/customers';
-    const RESOURCE_PATH_CUSTOMER_TOKEN = "/V1/integration/customer/token";
 
-    /**
-     * Sample values for testing
-     */
-    const ATTRIBUTE_CODE = 'attribute_code';
-    const ATTRIBUTE_VALUE = 'attribute_value';
+    private const STUB_INVALID_CUSTOMER_GROUP_ID = 777;
 
     /**
      * @var CustomerRepositoryInterface
@@ -412,6 +407,89 @@ public function testUpdateCustomerException(): void
         }
     }
 
+    /**
+     * Test customer update with invalid customer group id
+     *
+     * @magentoApiDataFixture Magento/Customer/_files/customer.php
+     *
+     * @return void
+     */
+    public function testUpdateCustomerWithInvalidGroupId(): void
+    {
+        $customerId = 1;
+        $customerData = $this->dataObjectProcessor->buildOutputDataArray(
+            $this->getCustomerData($customerId),
+            Customer::class
+        );
+        $customerData[Customer::GROUP_ID] = self::STUB_INVALID_CUSTOMER_GROUP_ID;
+
+        $serviceInfo = [
+            'rest' => [
+                'resourcePath' => self::RESOURCE_PATH . '/' . $customerId,
+                'httpMethod' => Request::HTTP_METHOD_PUT,
+            ],
+            'soap' => [
+                'service' => self::SERVICE_NAME,
+                'serviceVersion' => self::SERVICE_VERSION,
+                'operation' => self::SERVICE_NAME . 'Save',
+            ],
+        ];
+
+        $requestData['customer'] = $customerData;
+        $expectedMessage = 'The specified customer group id does not exist.';
+
+        try {
+            $this->_webApiCall($serviceInfo, $requestData);
+            $this->fail('Expected exception was not raised');
+        } catch (\SoapFault $e) {
+            $this->assertStringContainsString($expectedMessage, $e->getMessage());
+        } catch (\Exception $e) {
+            $errorObj = $this->processRestExceptionResult($e);
+            $this->assertEquals(HTTPExceptionCodes::HTTP_BAD_REQUEST, $e->getCode());
+            $this->assertEquals($expectedMessage, $errorObj['message']);
+        }
+    }
+
+    /**
+     * Test customer create with invalid customer group id
+     *
+     * @return void
+     */
+    public function testCreateCustomerWithInvalidGroupId(): void
+    {
+        $customerData = $this->dataObjectProcessor->buildOutputDataArray(
+            $this->customerHelper->createSampleCustomerDataObject(),
+            Customer::class
+        );
+        $customerData[Customer::GROUP_ID] = self::STUB_INVALID_CUSTOMER_GROUP_ID;
+
+        $serviceInfo = [
+            'rest' => [
+                'resourcePath' => self::RESOURCE_PATH,
+                'httpMethod' => Request::HTTP_METHOD_POST,
+            ],
+            'soap' => [
+                'service' => self::SERVICE_NAME,
+                'serviceVersion' => self::SERVICE_VERSION,
+                'operation' => self::SERVICE_NAME . 'Save',
+            ],
+        ];
+
+        $requestData = ['customer' => $customerData];
+        $expectedMessage = 'The specified customer group id does not exist.';
+
+        try {
+            $this->_webApiCall($serviceInfo, $requestData);
+            $this->fail('Expected exception was not raised');
+        } catch (\SoapFault $e) {
+            $this->assertStringContainsString($expectedMessage, $e->getMessage());
+        } catch (\Exception $e) {
+            $errorObj = $this->processRestExceptionResult($e);
+            $this->assertEquals(HTTPExceptionCodes::HTTP_BAD_REQUEST, $e->getCode());
+            $this->assertEquals($expectedMessage, $errorObj['message']);
+        }
+    }
+
     /**
      * Test creating a customer with absent required address fields
      *
