diff --git a/app/code/Magento/Customer/Model/Address/AbstractAddress.php b/app/code/Magento/Customer/Model/Address/AbstractAddress.php index 7b32da94ddedb476eda579521c1a19c002618ec4..786c5f9770a2e56123983c7b82c63ea369bec0a4 100644 --- a/app/code/Magento/Customer/Model/Address/AbstractAddress.php +++ b/app/code/Magento/Customer/Model/Address/AbstractAddress.php @@ -554,6 +554,8 @@ class AbstractAddress extends AbstractExtensibleModel implements AddressModelInt /** * Validate address attribute values * + * + * * @return bool|array * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) @@ -562,23 +564,24 @@ class AbstractAddress extends AbstractExtensibleModel implements AddressModelInt { $errors = []; if (!\Zend_Validate::is($this->getFirstname(), 'NotEmpty')) { - $errors[] = __('Please enter the first name.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'firstname']); } if (!\Zend_Validate::is($this->getLastname(), 'NotEmpty')) { - $errors[] = __('Please enter the last name.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'lastname']); } if (!\Zend_Validate::is($this->getStreetLine(1), 'NotEmpty')) { - $errors[] = __('Please enter the street.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'street']); } if (!\Zend_Validate::is($this->getCity(), 'NotEmpty')) { - $errors[] = __('Please enter the city.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'city']); } if (!\Zend_Validate::is($this->getTelephone(), 'NotEmpty')) { - $errors[] = __('Please enter the phone number.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'telephone']); + } $_havingOptionalZip = $this->_directoryData->getCountriesWithOptionalZip(); @@ -590,11 +593,11 @@ class AbstractAddress extends AbstractExtensibleModel implements AddressModelInt 'NotEmpty' ) ) { - $errors[] = __('Please enter the zip/postal code.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'postcode']); } if (!\Zend_Validate::is($this->getCountryId(), 'NotEmpty')) { - $errors[] = __('Please enter the country.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'countryId']); } if ($this->getCountryModel()->getRegionCollection()->getSize() && !\Zend_Validate::is( @@ -604,7 +607,7 @@ class AbstractAddress extends AbstractExtensibleModel implements AddressModelInt $this->getCountryId() ) ) { - $errors[] = __('Please enter the state/province.'); + $errors[] = __('%fieldName is a required field.', ['fieldName' => 'regionId']); } if (empty($errors) || $this->getShouldIgnoreValidation()) { diff --git a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php index 67d4ea32369ba8a37fef329d2aec809037fcac75..24df57e07e84826dcd7a19cdd84e5e6ca70ac37c 100644 --- a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php +++ b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php @@ -124,8 +124,12 @@ class AddressRepository implements \Magento\Customer\Api\AddressRepositoryInterf $addressModel->updateData($address); } - $inputException = $this->_validate($addressModel); - if ($inputException->wasErrorAdded()) { + $errors = $addressModel->validate(); + if ($errors !== true) { + $inputException = new InputException(); + foreach ($errors as $error) { + $inputException->addError($error); + } throw $inputException; } $addressModel->save(); @@ -255,70 +259,6 @@ class AddressRepository implements \Magento\Customer\Api\AddressRepositoryInterf return true; } - /** - * Validate Customer Addresses attribute values. - * - * @param CustomerAddressModel $customerAddressModel the model to validate - * @return InputException - * - * @SuppressWarnings(PHPMD.NPathComplexity) - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - */ - private function _validate(CustomerAddressModel $customerAddressModel) - { - $exception = new InputException(); - if ($customerAddressModel->getShouldIgnoreValidation()) { - return $exception; - } - - if (!\Zend_Validate::is($customerAddressModel->getFirstname(), 'NotEmpty')) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'firstname'])); - } - - if (!\Zend_Validate::is($customerAddressModel->getLastname(), 'NotEmpty')) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'lastname'])); - } - - if (!\Zend_Validate::is($customerAddressModel->getStreetLine(1), 'NotEmpty')) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'street'])); - } - - if (!\Zend_Validate::is($customerAddressModel->getCity(), 'NotEmpty')) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'city'])); - } - - if (!\Zend_Validate::is($customerAddressModel->getTelephone(), 'NotEmpty')) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'telephone'])); - } - - $havingOptionalZip = $this->directoryData->getCountriesWithOptionalZip(); - if (!in_array($customerAddressModel->getCountryId(), $havingOptionalZip) - && !\Zend_Validate::is($customerAddressModel->getPostcode(), 'NotEmpty') - ) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'postcode'])); - } - - if (!\Zend_Validate::is($customerAddressModel->getCountryId(), 'NotEmpty')) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'countryId'])); - } - - if ($this->directoryData->isRegionRequired($customerAddressModel->getCountryId())) { - $regionCollection = $customerAddressModel->getCountryModel()->getRegionCollection(); - if (!$regionCollection->count() && empty($customerAddressModel->getRegion())) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'region'])); - } elseif ( - $regionCollection->count() - && !in_array( - $customerAddressModel->getRegionId(), - array_column($regionCollection->getData(), 'region_id') - ) - ) { - $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'regionId'])); - } - } - return $exception; - } - /** * Retrieve collection processor * diff --git a/app/code/Magento/Customer/Test/Unit/Model/Address/AbstractAddressTest.php b/app/code/Magento/Customer/Test/Unit/Model/Address/AbstractAddressTest.php index 32112ccdeb1aec053955226a49eb1838f8c4276b..ba833221aba18cb2b4f2b1e90a9c8b59d78327f5 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Address/AbstractAddressTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Address/AbstractAddressTest.php @@ -336,31 +336,31 @@ class AbstractAddressTest extends \PHPUnit_Framework_TestCase return [ 'firstname' => [ array_merge(array_diff_key($data, ['firstname' => '']), ['country_id' => $countryId++]), - ['Please enter the first name.'], + ['firstname is a required field.'], ], 'lastname' => [ array_merge(array_diff_key($data, ['lastname' => '']), ['country_id' => $countryId++]), - ['Please enter the last name.'], + ['lastname is a required field.'], ], 'street' => [ array_merge(array_diff_key($data, ['street' => '']), ['country_id' => $countryId++]), - ['Please enter the street.'], + ['street is a required field.'], ], 'city' => [ array_merge(array_diff_key($data, ['city' => '']), ['country_id' => $countryId++]), - ['Please enter the city.'], + ['city is a required field.'], ], 'telephone' => [ array_merge(array_diff_key($data, ['telephone' => '']), ['country_id' => $countryId++]), - ['Please enter the phone number.'], + ['telephone is a required field.'], ], 'postcode' => [ array_merge(array_diff_key($data, ['postcode' => '']), ['country_id' => $countryId++]), - ['Please enter the zip/postal code.'], + ['postcode is a required field.'], ], 'country_id' => [ array_diff_key($data, ['country_id' => '']), - ['Please enter the country.'], + ['countryId is a required field.'], ], 'validated' => [array_merge($data, ['country_id' => $countryId++]), true], ]; diff --git a/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/AddressRepositoryTest.php b/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/AddressRepositoryTest.php index b72a5f6a417d4ab77d2c025e1f43b6e58f72ccc2..5f6bc315acd16f6a79d82c5ee2566e240c6572a6 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/AddressRepositoryTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/ResourceModel/AddressRepositoryTest.php @@ -128,6 +128,7 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase 'setCustomer', 'getCountryModel', 'getShouldIgnoreValidation', + 'validate', 'save', 'getDataModel', 'getCustomerId', @@ -191,6 +192,9 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase $this->address->expects($this->once()) ->method('setCustomer') ->with($this->customer); + $this->address->expects($this->once()) + ->method('validate') + ->willReturn(true); $this->address->expects($this->once()) ->method('save'); $this->addressRegistry->expects($this->once()) @@ -208,9 +212,6 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase $this->address->expects($this->once()) ->method('getDataModel') ->willReturn($customerAddress); - $this->address->expects($this->once()) - ->method('getShouldIgnoreValidation') - ->willReturn(true); $this->repository->save($customerAddress); } @@ -222,6 +223,7 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase { $customerId = 34; $addressId = 53; + $errors[] = __('Please enter the state/province.'); $customerAddress = $this->getMockForAbstractClass( \Magento\Customer\Api\Data\AddressInterface::class, [], @@ -245,7 +247,9 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase $this->address->expects($this->once()) ->method('updateData') ->with($customerAddress); - $this->prepareMocksForInvalidAddressValidation(); + $this->address->expects($this->once()) + ->method('validate') + ->willReturn($errors); $this->repository->save($customerAddress); } @@ -258,6 +262,7 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase { $customerId = 34; $addressId = 53; + $errors[] = __('region is a required field.'); $customerAddress = $this->getMockForAbstractClass( \Magento\Customer\Api\Data\AddressInterface::class, [], @@ -281,60 +286,13 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase $this->address->expects($this->once()) ->method('updateData') ->with($customerAddress); - $countryModel = $this->getMock(\Magento\Directory\Model\Country::class, [], [], '', false); - $regionCollection = $this->getMock( - \Magento\Directory\Model\ResourceModel\Region\Collection::class, - [], - [], - '', - false - ); - $this->address->expects($this->once()) - ->method('getShouldIgnoreValidation') - ->willReturn(false); - $this->address->expects($this->atLeastOnce()) - ->method('getCountryId') - ->willReturn(1); - $this->address->expects($this->once()) - ->method('getFirstname') - ->willReturn('firstname'); - $this->address->expects($this->once()) - ->method('getLastname') - ->willReturn('lastname'); - $this->address->expects($this->once()) - ->method('getStreetLine') - ->with(1) - ->willReturn('street line'); - $this->address->expects($this->once()) - ->method('getCity') - ->willReturn('city'); - $this->address->expects($this->once()) - ->method('getTelephone') - ->willReturn('23423423423'); $this->address->expects($this->never()) ->method('getRegionId') ->willReturn(null); - - $this->directoryData->expects($this->once()) - ->method('getCountriesWithOptionalZip') - ->willReturn([1]); - $this->address->expects($this->once()) - ->method('getCountryModel') - ->willReturn($countryModel); - $countryModel->expects($this->once()) - ->method('getRegionCollection') - ->willReturn($regionCollection); - $regionCollection->expects($this->once()) - ->method('count') - ->willReturn(0); - $this->directoryData->expects($this->once()) - ->method('isRegionRequired') - ->with(1) - ->willReturn(true); $this->address->expects($this->once()) - ->method('getRegion') - ->willReturn(''); + ->method('validate') + ->willReturn($errors); $this->repository->save($customerAddress); } @@ -347,6 +305,7 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase { $customerId = 34; $addressId = 53; + $errors[] = __('regionId is a required field.'); $customerAddress = $this->getMockForAbstractClass( \Magento\Customer\Api\Data\AddressInterface::class, [], @@ -370,114 +329,14 @@ class AddressRepositoryTest extends \PHPUnit_Framework_TestCase $this->address->expects($this->once()) ->method('updateData') ->with($customerAddress); - $countryModel = $this->getMock(\Magento\Directory\Model\Country::class, [], [], '', false); - $regionCollection = $this->getMock( - \Magento\Directory\Model\ResourceModel\Region\Collection::class, - [], - [], - '', - false - ); - - $this->address->expects($this->once()) - ->method('getShouldIgnoreValidation') - ->willReturn(false); - $this->address->expects($this->atLeastOnce()) - ->method('getCountryId') - ->willReturn(1); - $this->address->expects($this->once()) - ->method('getFirstname') - ->willReturn('firstname'); - $this->address->expects($this->once()) - ->method('getLastname') - ->willReturn('lastname'); - $this->address->expects($this->once()) - ->method('getStreetLine') - ->with(1) - ->willReturn('street line'); - $this->address->expects($this->once()) - ->method('getCity') - ->willReturn('city'); - $this->address->expects($this->once()) - ->method('getTelephone') - ->willReturn('23423423423'); - $this->address->expects($this->once()) - ->method('getRegionId') - ->willReturn(2); - - $this->directoryData->expects($this->once()) - ->method('getCountriesWithOptionalZip') - ->willReturn([1]); - $this->address->expects($this->once()) - ->method('getCountryModel') - ->willReturn($countryModel); - $countryModel->expects($this->once()) - ->method('getRegionCollection') - ->willReturn($regionCollection); - $regionCollection->expects($this->atLeastOnce()) - ->method('count') - ->willReturn(2); - $regionCollection->expects($this->once()) - ->method('getData') - ->willReturn([5, 6, 7, 8, 9]); - $this->directoryData->expects($this->once()) - ->method('isRegionRequired') - ->with(1) - ->willReturn(true); $this->address->expects($this->never()) ->method('getRegion') ->willReturn(''); - - $this->repository->save($customerAddress); - } - - protected function prepareMocksForInvalidAddressValidation() - { - $countryModel = $this->getMock(\Magento\Directory\Model\Country::class, [], [], '', false); - $regionCollection = $this->getMock( - \Magento\Directory\Model\ResourceModel\Region\Collection::class, - [], - [], - '', - false - ); - - $this->address->expects($this->once()) - ->method('getShouldIgnoreValidation') - ->willReturn(false); - $this->address->expects($this->atLeastOnce()) - ->method('getCountryId'); - $this->address->expects($this->once()) - ->method('getFirstname'); - $this->address->expects($this->once()) - ->method('getLastname'); - $this->address->expects($this->once()) - ->method('getStreetLine') - ->with(1); $this->address->expects($this->once()) - ->method('getCity'); - $this->address->expects($this->once()) - ->method('getTelephone'); - $this->address->expects($this->never()) - ->method('getRegionId') - ->willReturn(null); + ->method('validate') + ->willReturn($errors); - $this->directoryData->expects($this->once()) - ->method('getCountriesWithOptionalZip') - ->willReturn([]); - $this->address->expects($this->once()) - ->method('getCountryModel') - ->willReturn($countryModel); - $countryModel->expects($this->once()) - ->method('getRegionCollection') - ->willReturn($regionCollection); - $regionCollection->expects($this->once()) - ->method('count') - ->willReturn(0); - $this->directoryData->expects($this->once()) - ->method('isRegionRequired') - ->with(null) - ->willReturn(true); + $this->repository->save($customerAddress); } public function testGetById()