From 70d3468b24ffbbd4233123c8d60916a440975b39 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Thu, 4 Aug 2016 09:34:15 +0200 Subject: [PATCH 01/55] change category image attribute implementation to work with any attribute code rather than working only with 'image' --- .../Adminhtml/Category/Image/Upload.php | 4 +- .../Controller/Adminhtml/Category/Save.php | 61 +++++++-------- app/code/Magento/Catalog/Model/Category.php | 10 +-- .../Category/Attribute/Backend/Image.php | 74 ++++++++++++++----- .../Catalog/Model/Category/DataProvider.php | 19 +++-- .../base/web/js/form/element/file-uploader.js | 10 ++- 6 files changed, 115 insertions(+), 63 deletions(-) diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Image/Upload.php b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Image/Upload.php index 285f4f1e3ffad..25bd24ef70b94 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Image/Upload.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Image/Upload.php @@ -50,8 +50,10 @@ protected function _isAllowed() */ public function execute() { + $imageId = $this->_request->getParam('param_name', 'image'); + try { - $result = $this->imageUploader->saveFileToTmpDir('image'); + $result = $this->imageUploader->saveFileToTmpDir($imageId); $result['cookie'] = [ 'name' => $this->_getSession()->getName(), diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php index 55ae96faeb7e6..a793ad8478412 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php @@ -6,6 +6,7 @@ namespace Magento\Catalog\Controller\Adminhtml\Category; use Magento\Store\Model\StoreManagerInterface; +use \Magento\Catalog\Api\Data\CategoryAttributeInterface; /** * Class Save @@ -48,6 +49,11 @@ class Save extends \Magento\Catalog\Controller\Adminhtml\Category */ private $storeManager; + /** + * @var \Magento\Eav\Model\Config + */ + private $eavConfig; + /** * Constructor * @@ -56,43 +62,22 @@ class Save extends \Magento\Catalog\Controller\Adminhtml\Category * @param \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory * @param \Magento\Framework\View\LayoutFactory $layoutFactory * @param StoreManagerInterface $storeManager + * @param \Magento\Eav\Model\Config $eavConfig */ public function __construct( \Magento\Backend\App\Action\Context $context, \Magento\Framework\Controller\Result\RawFactory $resultRawFactory, \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, \Magento\Framework\View\LayoutFactory $layoutFactory, - StoreManagerInterface $storeManager + StoreManagerInterface $storeManager, + \Magento\Eav\Model\Config $eavConfig ) { parent::__construct($context); $this->resultRawFactory = $resultRawFactory; $this->resultJsonFactory = $resultJsonFactory; $this->layoutFactory = $layoutFactory; $this->storeManager = $storeManager; - } - - /** - * Filter category data - * - * @param array $rawData - * @return array - */ - protected function _filterCategoryPostData(array $rawData) - { - $data = $rawData; - // @todo It is a workaround to prevent saving this data in category model and it has to be refactored in future - if (isset($data['image']) && is_array($data['image'])) { - if (!empty($data['image']['delete'])) { - $data['image'] = null; - } else { - if (isset($data['image'][0]['name']) && isset($data['image'][0]['tmp_name'])) { - $data['image'] = $data['image'][0]['name']; - } else { - unset($data['image']); - } - } - } - return $data; + $this->eavConfig = $eavConfig; } /** @@ -126,7 +111,7 @@ public function execute() $this->storeManager->setCurrentStore($store->getCode()); $parentId = isset($categoryPostData['parent']) ? $categoryPostData['parent'] : null; if ($categoryPostData) { - $category->addData($this->_filterCategoryPostData($categoryPostData)); + $category->addData($categoryPostData); if ($isNewCategory) { $parentCategory = $this->getParentCategory($parentId, $storeId); $category->setPath($parentCategory->getPath()); @@ -248,18 +233,28 @@ public function execute() } /** - * Image data preprocessing - * - * @param array $data - * + * @param $data * @return array */ public function imagePreprocessing($data) { - if (empty($data['image'])) { - unset($data['image']); - $data['image']['delete'] = true; + $entityType = $this->eavConfig->getEntityType(CategoryAttributeInterface::ENTITY_TYPE_CODE); + + foreach ($entityType->getAttributeCollection() as $attributeModel) { + $attributeCode = $attributeModel->getAttributeCode(); + $backendModel = $attributeModel->getBackend(); + + if (isset($data[$attributeCode])) { + continue; + } + + if (!$backendModel instanceof \Magento\Catalog\Model\Category\Attribute\Backend\Image) { + continue; + } + + $data[$attributeCode] = false; } + return $data; } diff --git a/app/code/Magento/Catalog/Model/Category.php b/app/code/Magento/Catalog/Model/Category.php index b1f9dbf8c327a..86606278b1923 100644 --- a/app/code/Magento/Catalog/Model/Category.php +++ b/app/code/Magento/Catalog/Model/Category.php @@ -652,14 +652,14 @@ public function formatUrlKey($str) } /** - * Retrieve image URL - * - * @return string + * @param string $attributeCode + * @return bool|string + * @throws \Magento\Framework\Exception\LocalizedException */ - public function getImageUrl() + public function getImageUrl($attributeCode = 'image') { $url = false; - $image = $this->getImage(); + $image = $this->getData($attributeCode); if ($image) { if (is_string($image)) { $url = $this->_storeManager->getStore()->getBaseUrl( diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index 5eb4461ace5f0..b558c4528d8b1 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -13,6 +13,8 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend { + const ADDITIONAL_DATA_SUFFIX = '_additional_data'; + /** * @var \Magento\MediaStorage\Model\File\UploaderFactory * @@ -21,8 +23,6 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend protected $_uploaderFactory; /** - * Filesystem facade - * * @var \Magento\Framework\Filesystem * * @deprecated @@ -30,8 +30,6 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend protected $_filesystem; /** - * File Uploader factory - * * @var \Magento\MediaStorage\Model\File\UploaderFactory * * @deprecated @@ -46,15 +44,11 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend protected $_logger; /** - * Image uploader - * * @var \Magento\Catalog\Model\ImageUploader */ private $imageUploader; /** - * Image constructor. - * * @param \Psr\Log\LoggerInterface $logger * @param \Magento\Framework\Filesystem $filesystem * @param \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory @@ -70,8 +64,50 @@ public function __construct( } /** - * Get image uploader + * @param $value + * @return string|bool + */ + protected function getUploadedImageName($value) + { + if (!is_array($value)) { + return false; + } + + if (!count($value)) { + return false; + } + + $imageData = reset($value); + + if (!isset($imageData['name'])) { + return false; + } + + return $imageData['name']; + } + + /** + * Avoiding saving potential upload data to DB * + * @param \Magento\Framework\DataObject $object + * @return $this + */ + public function beforeSave($object) + { + $attributeName = $this->getAttribute()->getName(); + $value = $object->getData($attributeName); + + if ($value === false || (is_array($value) && isset($value['delete']) && $value['delete'] === true)) { + $object->setData($attributeName, ''); + } else if ($imageName = $this->getUploadedImageName($value)) { + $object->setData($attributeName . self::ADDITIONAL_DATA_SUFFIX, $value); + $object->setData($attributeName, $imageName); + } + + return parent::beforeSave($object); + } + + /** * @return \Magento\Catalog\Model\ImageUploader * * @deprecated @@ -83,6 +119,7 @@ private function getImageUploader() \Magento\Catalog\CategoryImageUpload::class ); } + return $this->imageUploader; } @@ -94,15 +131,18 @@ private function getImageUploader() */ public function afterSave($object) { - $image = $object->getData($this->getAttribute()->getName(), null); - - if ($image !== null) { - try { - $this->getImageUploader()->moveFileFromTmp($image); - } catch (\Exception $e) { - $this->_logger->critical($e); - } + $value = $object->getData($this->getAttribute()->getName() . self::ADDITIONAL_DATA_SUFFIX); + + if (!$imageName = $this->getUploadedImageName($value)) { + return $this; } + + try { + $this->getImageUploader()->moveFileFromTmp($imageName); + } catch (\Exception $e) { + $this->_logger->critical($e); + } + return $this; } } diff --git a/app/code/Magento/Catalog/Model/Category/DataProvider.php b/app/code/Magento/Catalog/Model/Category/DataProvider.php index 8626d183e2fac..f2eb84561aa3f 100644 --- a/app/code/Magento/Catalog/Model/Category/DataProvider.php +++ b/app/code/Magento/Catalog/Model/Category/DataProvider.php @@ -206,11 +206,20 @@ public function getData() $categoryData = $this->addUseDefaultSettings($category, $categoryData); $categoryData = $this->addUseConfigSettings($categoryData); $categoryData = $this->filterFields($categoryData); - if (isset($categoryData['image'])) { - unset($categoryData['image']); - $categoryData['image'][0]['name'] = $category->getData('image'); - $categoryData['image'][0]['url'] = $category->getImageUrl(); - } + + foreach ($category->getAttributes() as $attributeCode => $attribute) { + $backendModel = $attribute->getBackend(); + + if ($backendModel instanceof \Magento\Catalog\Model\Category\Attribute\Backend\Image) { + if (isset($categoryData[$attributeCode])) { + unset($categoryData[$attributeCode]); + + $categoryData[$attributeCode][0]['name'] = $category->getData($attributeCode); + $categoryData[$attributeCode][0]['url'] = $category->getImageUrl($attributeCode); + } + } + } + $this->loadedData[$category->getId()] = $categoryData; } return $this->loadedData; diff --git a/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js b/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js index 1805dcd2f004e..5d06886fe91bd 100644 --- a/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js +++ b/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js @@ -294,7 +294,7 @@ define([ /** * Handler which is invoked prior to the start of a file upload. * - * @param {Event} e - Event obejct. + * @param {Event} e - Event object. * @param {Object} data - File data that will be uploaded. */ onBeforeFileUpload: function (e, data) { @@ -302,7 +302,13 @@ define([ allowed = this.isFileAllowed(file); if (allowed.passed) { - $(e.target).fileupload('process', data).done(function () { + var $target = $(e.target); + + $target.on('fileuploadsend', function(event, postData) { + postData.data.set('param_name', this.paramName); + }.bind(data)); + + $target.fileupload('process', data).done(function () { data.submit(); }); } else { From b9419fb97b0d7c56d557da8d7de859571d308f29 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Thu, 4 Aug 2016 15:48:37 +0200 Subject: [PATCH 02/55] change code formatting to okease automatic code-sniffer --- .../Catalog/Controller/Adminhtml/Category/Save.php | 7 ++++--- .../Model/Category/Attribute/Backend/Image.php | 4 ++-- .../Catalog/Model/Category/DataProvider.php | 14 ++++++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php index a793ad8478412..492bf1c021cf0 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php @@ -233,10 +233,11 @@ public function execute() } /** - * @param $data - * @return array + * @param array $data Category data + * @return mixed + * @throws \Magento\Framework\Exception\LocalizedException */ - public function imagePreprocessing($data) + public function imagePreprocessing(array $data) { $entityType = $this->eavConfig->getEntityType(CategoryAttributeInterface::ENTITY_TYPE_CODE); diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index b558c4528d8b1..adaf752f1613e 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -64,8 +64,8 @@ public function __construct( } /** - * @param $value - * @return string|bool + * @param array|bool|null $value Attribute value + * @return bool */ protected function getUploadedImageName($value) { diff --git a/app/code/Magento/Catalog/Model/Category/DataProvider.php b/app/code/Magento/Catalog/Model/Category/DataProvider.php index f2eb84561aa3f..d3d34594cfdb3 100644 --- a/app/code/Magento/Catalog/Model/Category/DataProvider.php +++ b/app/code/Magento/Catalog/Model/Category/DataProvider.php @@ -210,15 +210,17 @@ public function getData() foreach ($category->getAttributes() as $attributeCode => $attribute) { $backendModel = $attribute->getBackend(); + if (!isset($categoryData[$attributeCode])) { + continue; + } + if ($backendModel instanceof \Magento\Catalog\Model\Category\Attribute\Backend\Image) { - if (isset($categoryData[$attributeCode])) { - unset($categoryData[$attributeCode]); + unset($categoryData[$attributeCode]); - $categoryData[$attributeCode][0]['name'] = $category->getData($attributeCode); - $categoryData[$attributeCode][0]['url'] = $category->getImageUrl($attributeCode); - } + $categoryData[$attributeCode][0]['name'] = $category->getData($attributeCode); + $categoryData[$attributeCode][0]['url'] = $category->getImageUrl($attributeCode); } - } + } $this->loadedData[$category->getId()] = $categoryData; } From 0995b37bcd97c8a57f51b860f4ad5a9010d1fd63 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Thu, 4 Aug 2016 17:28:12 +0200 Subject: [PATCH 03/55] Increased test-coverage --- .../Adminhtml/Category/Image/UploadTest.php | 70 ++++++++++ .../Adminhtml/Category/SaveTest.php | 83 +++++++++++- .../Catalog/Test/Unit/Model/CategoryTest.php | 120 ++++++++++++++---- 3 files changed, 248 insertions(+), 25 deletions(-) create mode 100644 app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php new file mode 100644 index 0000000000000..cde1859cf8a74 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php @@ -0,0 +1,70 @@ +objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + } + + public function uploadedImageNameProvider() + { + return [ + ['image1', 'image1'], + ['image2', 'image2'], + [null, 'image'], + ]; + } + + /** + * @param $name + * @param $savedName + * + * @dataProvider uploadedImageNameProvider + */ + public function testExecuteShouldSaveUploadedImageWithSpecifiedNameToTmpFolder($name, $savedName) + { + $request = $this->objectManager->getObject(Request::class); + + $uploader = $this->getMock( + \Magento\Catalog\Model\ImageUploader::class, ['saveFileToTmpDir'], [], '', false); + + $resultFactory = $this->getMock( + \Magento\Framework\Controller\ResultFactory::class, ['create'], [], '', false); + + $resultFactory->expects($this->once()) + ->method('create') + ->will($this->returnValue(new \Magento\Framework\DataObject())); + + $model = $this->objectManager->getObject(Model::class, [ + 'context' => $this->objectManager->getObject(\Magento\Backend\App\Action\Context::class, [ + 'request' => $request, + 'resultFactory' => $resultFactory + ]), + 'imageUploader' => $uploader + ]); + + $uploader->expects($this->once()) + ->method('saveFileToTmpDir') + ->with($savedName) + ->will($this->returnValue([])); + + $request->setParam('param_name', $name); + + $model->execute(); + } +} \ No newline at end of file diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php index 51d99f7219575..9d1bcf3125e99 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php @@ -84,7 +84,6 @@ class SaveTest extends \PHPUnit_Framework_TestCase */ protected function setUp() { - $this->markTestSkipped('Due to MAGETWO-48956'); $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); $this->contextMock = $this->getMock( @@ -201,6 +200,8 @@ protected function setUp() */ public function testExecute($categoryId, $storeId, $parentId) { + $this->markTestSkipped('Due to MAGETWO-48956'); + $rootCategoryId = \Magento\Catalog\Model\Category::TREE_ROOT_ID; $products = [['any_product']]; $postData = [ @@ -577,4 +578,84 @@ public function dataProviderExecute() ] ]; } + + public function attributeValueDataProvider() + { + return [ + [['attribute1' => null, 'attribute2' => 123]], + [['attribute2' => 123]] + ]; + } + + /** + * @dataProvider attributeValueDataProvider + * + * @param $data + */ + public function testImagePreprocessingShouldSetAttributesWithImageBackendToFalse($data) + { + $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); + + $collection = new \Magento\Framework\DataObject(['attribute_collection' => [ + new \Magento\Framework\DataObject([ + 'attribute_code' => 'attribute1', + 'backend' => $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class) + ]), + new \Magento\Framework\DataObject([ + 'attribute_code' => 'attribute2', + 'backend' => new \Magento\Framework\DataObject() + ]) + ]]); + + $eavConfig->expects($this->once()) + ->method('getEntityType') + ->with(\Magento\Catalog\Api\Data\CategoryAttributeInterface::ENTITY_TYPE_CODE) + ->will($this->returnValue($collection)); + + $model = $this->objectManager->getObject(\Magento\Catalog\Controller\Adminhtml\Category\Save::class, [ + 'eavConfig' => $eavConfig + ]); + + $result = $model->imagePreprocessing($data); + + $this->assertEquals([ + 'attribute1' => false, + 'attribute2' => 123 + ], $result); + } + + public function testImagePreprocessingShouldNotSetValueToFalseWhenValueSet() + { + $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); + + $collection = new \Magento\Framework\DataObject(['attribute_collection' => [ + new \Magento\Framework\DataObject([ + 'attribute_code' => 'attribute1', + 'backend' => $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class) + ]), + new \Magento\Framework\DataObject([ + 'attribute_code' => 'attribute2', + 'backend' => new \Magento\Framework\DataObject() + ]) + ]]); + + $eavConfig->expects($this->once()) + ->method('getEntityType') + ->with(\Magento\Catalog\Api\Data\CategoryAttributeInterface::ENTITY_TYPE_CODE) + ->will($this->returnValue($collection)); + + $model = $this->objectManager->getObject(\Magento\Catalog\Controller\Adminhtml\Category\Save::class, [ + 'eavConfig' => $eavConfig + ]); + + $result = $model->imagePreprocessing([ + 'attribute1' => 'somevalue', + 'attribute2' => null + ]); + + $this->assertEquals([ + 'attribute1' => 'somevalue', + 'attribute2' => null + ], $result); + } } diff --git a/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php index 471037f1d3f45..66403d3964890 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php @@ -9,6 +9,7 @@ namespace Magento\Catalog\Test\Unit\Model; use Magento\Catalog\Model\Indexer; +use Magento\Catalog\Model\Category as Model; /** * @SuppressWarnings(PHPMD.TooManyFields) @@ -89,9 +90,16 @@ class CategoryTest extends \PHPUnit_Framework_TestCase */ protected $attributeValueFactory; + /** + * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager + */ + protected $objectManager; + protected function setUp() { - $this->context = $this->getMock( + $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + + $this->context = $this->getMock( \Magento\Framework\Model\Context::class, ['getEventDispatcher', 'getCacheManager'], [], @@ -108,21 +116,21 @@ protected function setUp() $this->registry = $this->getMock(\Magento\Framework\Registry::class); $this->storeManager = $this->getMock(\Magento\Store\Model\StoreManagerInterface::class); - $this->categoryTreeResource = $this->getMock( - \Magento\Catalog\Model\ResourceModel\Category\Tree::class, - [], - [], - '', - false + $this->categoryTreeResource = $this->getMock( + \Magento\Catalog\Model\ResourceModel\Category\Tree::class, + [], + [], + '', + false ); - $this->categoryTreeFactory = $this->getMock( + $this->categoryTreeFactory = $this->getMock( \Magento\Catalog\Model\ResourceModel\Category\TreeFactory::class, ['create'], [], '', false); $this->categoryRepository = $this->getMock(\Magento\Catalog\Api\CategoryRepositoryInterface::class); - $this->storeCollectionFactory = $this->getMock( + $this->storeCollectionFactory = $this->getMock( \Magento\Store\Model\ResourceModel\Store\CollectionFactory::class, ['create'], [], @@ -130,7 +138,7 @@ protected function setUp() false ); $this->url = $this->getMock(\Magento\Framework\UrlInterface::class); - $this->productCollectionFactory = $this->getMock( + $this->productCollectionFactory = $this->getMock( \Magento\Catalog\Model\ResourceModel\Product\CollectionFactory::class, ['create'], [], @@ -138,7 +146,7 @@ protected function setUp() false ); $this->catalogConfig = $this->getMock(\Magento\Catalog\Model\Config::class, [], [], '', false); - $this->filterManager = $this->getMock( + $this->filterManager = $this->getMock( \Magento\Framework\Filter\FilterManager::class, ['translitUrl'], [], @@ -148,7 +156,7 @@ protected function setUp() $this->flatState = $this->getMock(\Magento\Catalog\Model\Indexer\Category\Flat\State::class, [], [], '', false); $this->flatIndexer = $this->getMock(\Magento\Framework\Indexer\IndexerInterface::class); $this->productIndexer = $this->getMock(\Magento\Framework\Indexer\IndexerInterface::class); - $this->categoryUrlPathGenerator = $this->getMock( + $this->categoryUrlPathGenerator = $this->getMock( \Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator::class, [], [], @@ -156,19 +164,19 @@ protected function setUp() false ); $this->urlFinder = $this->getMock(\Magento\UrlRewrite\Model\UrlFinderInterface::class); - $this->resource = $this->getMock( + $this->resource = $this->getMock( \Magento\Catalog\Model\ResourceModel\Category::class, [], [], '', false ); - $this->indexerRegistry = $this->getMock( - \Magento\Framework\Indexer\IndexerRegistry::class, - ['get'], - [], - '', - false + $this->indexerRegistry = $this->getMock( + \Magento\Framework\Indexer\IndexerRegistry::class, + ['get'], + [], + '', + false ); $this->metadataServiceMock = $this->getMock(\Magento\Catalog\Api\CategoryAttributeRepositoryInterface::class); @@ -198,7 +206,7 @@ public function testFormatUrlKey() public function testMoveWhenCannotFindParentCategory() { $this->markTestIncomplete('MAGETWO-31165'); - $parentCategory = $this->getMock( + $parentCategory = $this->getMock( \Magento\Catalog\Model\Category::class, ['getId', 'setStoreId', 'load'], [], @@ -223,7 +231,7 @@ public function testMoveWhenCannotFindParentCategory() */ public function testMoveWhenCannotFindNewCategory() { - $parentCategory = $this->getMock( + $parentCategory = $this->getMock( \Magento\Catalog\Model\Category::class, ['getId', 'setStoreId', 'load'], [], @@ -250,7 +258,7 @@ public function testMoveWhenCannotFindNewCategory() public function testMoveWhenParentCategoryIsSameAsChildCategory() { $this->markTestIncomplete('MAGETWO-31165'); - $parentCategory = $this->getMock( + $parentCategory = $this->getMock( \Magento\Catalog\Model\Category::class, ['getId', 'setStoreId', 'load'], [], @@ -277,7 +285,7 @@ public function testMovePrimaryWorkflow() ->method('get') ->with('catalog_category_product') ->will($this->returnValue($indexer)); - $parentCategory = $this->getMock( + $parentCategory = $this->getMock( \Magento\Catalog\Model\Category::class, ['getId', 'setStoreId', 'load'], [], @@ -313,7 +321,7 @@ public function testGetUseFlatResourceTrue() protected function getCategoryModel() { - return (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))->getObject( + return $this->objectManager->getObject( \Magento\Catalog\Model\Category::class, [ 'context' => $this->context, @@ -487,4 +495,68 @@ public function testGetCustomAttributes() $this->category->getCustomAttribute($descriptionAttributeCode)->getValue() ); } + + public function imageAttributeNameAndUrlProvider() + { + return [ + ['testimage', 'http://www.test123.com/catalog/category/testimage'], + [false, false] + ]; + } + + /** + * @param $value + * @param $url + * + * @dataProvider imageAttributeNameAndUrlProvider + */ + public function testGetImageUrlShouldGenerateMediaUrlForSpecifiedAttributeValue($value, $url) + { + $storeManager = $this->getMock(\Magento\Store\Model\StoreManager::class, ['getStore'], [], '', false); + $store = $this->getMock(\Magento\Store\Model\Store::class, ['getBaseUrl'], [], '', false); + + $storeManager->expects($this->any()) + ->method('getStore') + ->will($this->returnValue($store)); + + $store->expects($this->any()) + ->method('getBaseUrl') + ->with(\Magento\Framework\UrlInterface::URL_TYPE_MEDIA) + ->will($this->returnValue('http://www.test123.com/')); + + $model = $this->objectManager->getObject(Model::class, [ + 'storeManager' => $storeManager + ]); + + $model->setData('attribute1', $value); + + $result = $model->getImageUrl('attribute1'); + + $this->assertEquals($url, $result); + } + + public function testGetImageUrlShouldGenerateMediaUrlForImageAttributeValue() + { + $storeManager = $this->getMock(\Magento\Store\Model\StoreManager::class, ['getStore'], [], '', false); + $store = $this->getMock(\Magento\Store\Model\Store::class, ['getBaseUrl'], [], '', false); + + $storeManager->expects($this->any()) + ->method('getStore') + ->will($this->returnValue($store)); + + $store->expects($this->any()) + ->method('getBaseUrl') + ->with(\Magento\Framework\UrlInterface::URL_TYPE_MEDIA) + ->will($this->returnValue('http://www.test123.com/')); + + $model = $this->objectManager->getObject(Model::class, [ + 'storeManager' => $storeManager + ]); + + $model->setData('image', 'myimage'); + + $result = $model->getImageUrl(); + + $this->assertEquals('http://www.test123.com/catalog/category/myimage', $result); + } } From ed35ab9976ddf86ba19338d3c83f8e114374c79e Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Thu, 4 Aug 2016 21:31:29 +0200 Subject: [PATCH 04/55] Code reformatted after style-checking failures --- .../Category/Attribute/Backend/Image.php | 34 +++++----------- .../Catalog/Model/Category/DataProvider.php | 40 ++++++++++++------- .../Adminhtml/Category/Image/UploadTest.php | 16 ++++---- .../Adminhtml/Category/SaveTest.php | 9 ++++- 4 files changed, 52 insertions(+), 47 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index adaf752f1613e..4669f72b0bbd4 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -64,26 +64,16 @@ public function __construct( } /** - * @param array|bool|null $value Attribute value - * @return bool + * @param array $value Attribute value + * @return string */ protected function getUploadedImageName($value) { - if (!is_array($value)) { - return false; + if (is_array($value) && isset($value[0]['name'])) { + return $value[0]['name']; } - if (!count($value)) { - return false; - } - - $imageData = reset($value); - - if (!isset($imageData['name'])) { - return false; - } - - return $imageData['name']; + return ''; } /** @@ -133,14 +123,12 @@ public function afterSave($object) { $value = $object->getData($this->getAttribute()->getName() . self::ADDITIONAL_DATA_SUFFIX); - if (!$imageName = $this->getUploadedImageName($value)) { - return $this; - } - - try { - $this->getImageUploader()->moveFileFromTmp($imageName); - } catch (\Exception $e) { - $this->_logger->critical($e); + if ($imageName = $this->getUploadedImageName($value)) { + try { + $this->getImageUploader()->moveFileFromTmp($imageName); + } catch (\Exception $e) { + $this->_logger->critical($e); + } } return $this; diff --git a/app/code/Magento/Catalog/Model/Category/DataProvider.php b/app/code/Magento/Catalog/Model/Category/DataProvider.php index d3d34594cfdb3..8ea60d54ec556 100644 --- a/app/code/Magento/Catalog/Model/Category/DataProvider.php +++ b/app/code/Magento/Catalog/Model/Category/DataProvider.php @@ -17,6 +17,7 @@ use Magento\Ui\DataProvider\EavValidationRules; use Magento\Catalog\Model\CategoryFactory; use Magento\Framework\Exception\NoSuchEntityException; +use Magento\Catalog\Model\Category\Attribute\Backend\Image as ImageBackendModel; /** * Class DataProvider @@ -206,21 +207,7 @@ public function getData() $categoryData = $this->addUseDefaultSettings($category, $categoryData); $categoryData = $this->addUseConfigSettings($categoryData); $categoryData = $this->filterFields($categoryData); - - foreach ($category->getAttributes() as $attributeCode => $attribute) { - $backendModel = $attribute->getBackend(); - - if (!isset($categoryData[$attributeCode])) { - continue; - } - - if ($backendModel instanceof \Magento\Catalog\Model\Category\Attribute\Backend\Image) { - unset($categoryData[$attributeCode]); - - $categoryData[$attributeCode][0]['name'] = $category->getData($attributeCode); - $categoryData[$attributeCode][0]['url'] = $category->getImageUrl($attributeCode); - } - } + $categoryData = $this->convertValues($category, $categoryData); $this->loadedData[$category->getId()] = $categoryData; } @@ -382,6 +369,29 @@ protected function filterFields($categoryData) return array_diff_key($categoryData, array_flip($this->ignoreFields)); } + /** + * @param \Magento\Catalog\Model\Category $category + * @param array $categoryData + * @return array + */ + protected function convertValues($category, $categoryData) + { + foreach ($category->getAttributes() as $attributeCode => $attribute) { + if (!isset($categoryData[$attributeCode])) { + continue; + } + + if ($attribute->getBackend() instanceof ImageBackendModel) { + unset($categoryData[$attributeCode]); + + $categoryData[$attributeCode][0]['name'] = $category->getData($attributeCode); + $categoryData[$attributeCode][0]['url'] = $category->getImageUrl($attributeCode); + } + } + + return $categoryData; + } + /** * Category's fields default values * diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php index cde1859cf8a74..40c40a17b8e52 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php @@ -7,6 +7,10 @@ use \Magento\Catalog\Controller\Adminhtml\Category\Image\Upload as Model; use \Magento\Framework\App\Request\Http as Request; +use \Magento\Catalog\Model\ImageUploader; +use \Magento\Framework\Controller\ResultFactory; +use \Magento\Framework\DataObject; +use \Magento\Backend\App\Action\Context; /** * Class UploadTest @@ -40,18 +44,16 @@ public function testExecuteShouldSaveUploadedImageWithSpecifiedNameToTmpFolder($ { $request = $this->objectManager->getObject(Request::class); - $uploader = $this->getMock( - \Magento\Catalog\Model\ImageUploader::class, ['saveFileToTmpDir'], [], '', false); + $uploader = $this->getMock(ImageUploader::class, ['saveFileToTmpDir'], [], '', false); - $resultFactory = $this->getMock( - \Magento\Framework\Controller\ResultFactory::class, ['create'], [], '', false); + $resultFactory = $this->getMock(ResultFactory::class, ['create'], [], '', false); $resultFactory->expects($this->once()) ->method('create') - ->will($this->returnValue(new \Magento\Framework\DataObject())); + ->will($this->returnValue(new DataObject())); $model = $this->objectManager->getObject(Model::class, [ - 'context' => $this->objectManager->getObject(\Magento\Backend\App\Action\Context::class, [ + 'context' => $this->objectManager->getObject(Context::class, [ 'request' => $request, 'resultFactory' => $resultFactory ]), @@ -67,4 +69,4 @@ public function testExecuteShouldSaveUploadedImageWithSpecifiedNameToTmpFolder($ $model->execute(); } -} \ No newline at end of file +} diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php index 9d1bcf3125e99..1a2556b603602 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php @@ -5,6 +5,8 @@ */ namespace Magento\Catalog\Test\Unit\Controller\Adminhtml\Category; +use \Magento\Catalog\Controller\Adminhtml\Category\Save as Model; + /** * Class SaveTest * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -628,10 +630,13 @@ public function testImagePreprocessingShouldNotSetValueToFalseWhenValueSet() { $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); + $imageBackendModel = $this->objectManager->getObject( + \Magento\Catalog\Model\Category\Attribute\Backend\Image::class); + $collection = new \Magento\Framework\DataObject(['attribute_collection' => [ new \Magento\Framework\DataObject([ 'attribute_code' => 'attribute1', - 'backend' => $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class) + 'backend' => $imageBackendModel ]), new \Magento\Framework\DataObject([ 'attribute_code' => 'attribute2', @@ -644,7 +649,7 @@ public function testImagePreprocessingShouldNotSetValueToFalseWhenValueSet() ->with(\Magento\Catalog\Api\Data\CategoryAttributeInterface::ENTITY_TYPE_CODE) ->will($this->returnValue($collection)); - $model = $this->objectManager->getObject(\Magento\Catalog\Controller\Adminhtml\Category\Save::class, [ + $model = $this->objectManager->getObject(Model::class, [ 'eavConfig' => $eavConfig ]); From 0887c0103cd172568d78399c554e6528483796ac Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Thu, 4 Aug 2016 21:46:29 +0200 Subject: [PATCH 05/55] More code styling fixes --- .../Test/Unit/Controller/Adminhtml/Category/SaveTest.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php index 1a2556b603602..9ef502b9640ca 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php @@ -598,10 +598,14 @@ public function testImagePreprocessingShouldSetAttributesWithImageBackendToFalse { $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); + $imageBackendModel = $this->objectManager->getObject( + \Magento\Catalog\Model\Category\Attribute\Backend\Image::class + ); + $collection = new \Magento\Framework\DataObject(['attribute_collection' => [ new \Magento\Framework\DataObject([ 'attribute_code' => 'attribute1', - 'backend' => $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class) + 'backend' => $imageBackendModel ]), new \Magento\Framework\DataObject([ 'attribute_code' => 'attribute2', @@ -631,7 +635,8 @@ public function testImagePreprocessingShouldNotSetValueToFalseWhenValueSet() $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); $imageBackendModel = $this->objectManager->getObject( - \Magento\Catalog\Model\Category\Attribute\Backend\Image::class); + \Magento\Catalog\Model\Category\Attribute\Backend\Image::class + ); $collection = new \Magento\Framework\DataObject(['attribute_collection' => [ new \Magento\Framework\DataObject([ From e68d4ea412a46899a52100b7425d90117d5a675f Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Sat, 6 Aug 2016 18:15:34 +0200 Subject: [PATCH 06/55] test-coverage increased --- .../Category/Attribute/Backend/ImageTest.php | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php new file mode 100644 index 0000000000000..cace3c1b39118 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php @@ -0,0 +1,224 @@ +objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + + $this->attribute = $this->getMockForAbstractClass( + \Magento\Eav\Model\Entity\Attribute\AbstractAttribute::class, + [], + 'TestAttribute', + false, + false, + true, + ['getName'] + ); + + $this->attribute->expects($this->once()) + ->method('getName') + ->will($this->returnValue('test_attribute')); + + $this->imageUploader = $this->getMock( + \Magento\Catalog\Model\ImageUploader::class, + ['moveFileFromTmp'], + [], + '', + false + ); + } + + public function deletionValuesProvider() + { + return [ + [false], + [['delete' => true]] + ]; + } + + /** + * @dataProvider deletionValuesProvider + * + * @param $value + */ + public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueRequiresDeletion($value) + { + $model = $this->objectManager->getObject(Model::class); + $model->setAttribute($this->attribute); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => $value + ]); + + $model->beforeSave($object); + + $this->assertEquals('', $object->getTestAttribute()); + } + + public function testBeforeSaveShouldSetAttributeValueToUploadedImageName() + { + $model = $this->objectManager->getObject(Model::class); + $model->setAttribute($this->attribute); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => [ + ['name' => 'test123.jpg'] + ] + ]); + + $model->beforeSave($object); + + $this->assertEquals('test123.jpg', $object->getTestAttribute()); + } + + public function testBeforeSaveShouldSetAttributeUploadInformationToTemporaryAttribute() + { + $model = $this->objectManager->getObject(Model::class); + $model->setAttribute($this->attribute); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => [ + ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.test.com/test123.jpg'] + ] + ]); + + $model->beforeSave($object); + + $this->assertEquals([ + ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.test.com/test123.jpg'] + ], $object->getTestAttributeAdditionalData()); + } + + public function stringValueProvider() + { + return [ + ['test123'], + [12345], + [true], + ['some' => 'value'] + ]; + } + + /** + * @dataProvider stringValueProvider + * + * @param $value + */ + public function testBeforeSaveShouldNotModifyAttributeValueWhenNotUploadData($value) + { + $model = $this->objectManager->getObject(Model::class); + $model->setAttribute($this->attribute); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => $value + ]); + + $model->beforeSave($object); + + $this->assertEquals($value, $object->getTestAttribute()); + } + + /** + * @dataProvider stringValueProvider + * + * @param $value + */ + public function testBeforeSaveShouldNotSetAdditionalDataWhenNotUploadData($value) + { + $model = $this->objectManager->getObject(Model::class); + $model->setAttribute($this->attribute); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => $value + ]); + + $model->beforeSave($object); + + $this->assertNull($object->getTestAttributeAdditionalData()); + } + + protected function setUpModelForAfterSave() + { + $objectManagerMock = $this->getMock( + \Magento\Framework\App\ObjectManager::class, + ['get'], + [], + '', + false + ); + + $imageUploaderMock = $this->imageUploader; + + $objectManagerMock->expects($this->any()) + ->method('get') + ->will($this->returnCallback(function($class, $params = []) use ($imageUploaderMock) { + if ($class == \Magento\Catalog\CategoryImageUpload::class) { + return $imageUploaderMock; + } + + return $this->objectManager->get($class, $params); + })); + + $model = $this->objectManager->getObject(Model::class, [ + 'objectManager' => $objectManagerMock + ]); + + return $model->setAttribute($this->attribute); + } + + public function testAfterSaveShouldUploadImageWhenAdditionalDataSet() + { + $model = $this->setUpModelForAfterSave(); + + $this->imageUploader->expects($this->once()) + ->method('moveFileFromTmp') + ->with($this->equalTo('test1234.jpg')); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute_additional_data' => [ + ['name' => 'test1234.jpg'] + ] + ]); + + $model->afterSave($object); + } + + public function testAfterSaveShouldNotUploadImageWhenAdditionalDataNotSet() + { + $model = $this->setUpModelForAfterSave(); + + $this->imageUploader->expects($this->never()) + ->method('moveFileFromTmp'); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => [ + ['name' => 'test1234.jpg'] + ] + ]); + + $model->afterSave($object); + } +} From 459bb1c1e7bd8e315b2d611381ea8605267f7596 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Sat, 6 Aug 2016 18:26:36 +0200 Subject: [PATCH 07/55] made image attribut backend class afterSave method testable --- .../Model/Category/Attribute/Backend/Image.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index 4669f72b0bbd4..97c719b632a47 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -48,6 +48,11 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend */ private $imageUploader; + /** + * @var \Magento\Framework\App\ObjectManager + */ + private $objectManager; + /** * @param \Psr\Log\LoggerInterface $logger * @param \Magento\Framework\Filesystem $filesystem @@ -56,11 +61,13 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend public function __construct( \Psr\Log\LoggerInterface $logger, \Magento\Framework\Filesystem $filesystem, - \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory + \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory, + \Magento\Framework\App\ObjectManager $objectManager ) { $this->_filesystem = $filesystem; $this->_fileUploaderFactory = $fileUploaderFactory; $this->_logger = $logger; + $this->objectManager = $objectManager; } /** @@ -105,9 +112,7 @@ public function beforeSave($object) private function getImageUploader() { if ($this->imageUploader === null) { - $this->imageUploader = \Magento\Framework\App\ObjectManager::getInstance()->get( - \Magento\Catalog\CategoryImageUpload::class - ); + $this->imageUploader = $this->objectManager->get(\Magento\Catalog\CategoryImageUpload::class); } return $this->imageUploader; From 58c0575e40e55db58b41baf65caec614b47d2d01 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Sat, 6 Aug 2016 21:56:09 +0200 Subject: [PATCH 08/55] Switched ObjectManager dependency to be declared as interface rather than concrete class --- .../Catalog/Model/Category/Attribute/Backend/Image.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index 97c719b632a47..dbbc46729a2cc 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -49,7 +49,7 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend private $imageUploader; /** - * @var \Magento\Framework\App\ObjectManager + * @var \Magento\Framework\ObjectManagerInterface */ private $objectManager; @@ -57,12 +57,13 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend * @param \Psr\Log\LoggerInterface $logger * @param \Magento\Framework\Filesystem $filesystem * @param \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory + * @param \Magento\Framework\ObjectManagerInterface */ public function __construct( \Psr\Log\LoggerInterface $logger, \Magento\Framework\Filesystem $filesystem, \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory, - \Magento\Framework\App\ObjectManager $objectManager + \Magento\Framework\ObjectManagerInterface $objectManager ) { $this->_filesystem = $filesystem; $this->_fileUploaderFactory = $fileUploaderFactory; From 2e5de1dddfc76de2e25c632286e6258450391dac Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Sat, 6 Aug 2016 22:41:05 +0200 Subject: [PATCH 09/55] Code simplified + test-coverage increased + fixed things that scrutinizer reported --- .../Category/Attribute/Backend/Image.php | 14 +- .../Category/Attribute/Backend/ImageTest.php | 138 +++++++++++++----- 2 files changed, 110 insertions(+), 42 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index dbbc46729a2cc..efd3a4afb4a60 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -13,7 +13,7 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend { - const ADDITIONAL_DATA_SUFFIX = '_additional_data'; + const ADDITIONAL_DATA_PREFIX = '_additional_data_'; /** * @var \Magento\MediaStorage\Model\File\UploaderFactory @@ -57,7 +57,7 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend * @param \Psr\Log\LoggerInterface $logger * @param \Magento\Framework\Filesystem $filesystem * @param \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory - * @param \Magento\Framework\ObjectManagerInterface + * @param \Magento\Framework\ObjectManagerInterface $objectManager */ public function __construct( \Psr\Log\LoggerInterface $logger, @@ -95,11 +95,11 @@ public function beforeSave($object) $attributeName = $this->getAttribute()->getName(); $value = $object->getData($attributeName); - if ($value === false || (is_array($value) && isset($value['delete']) && $value['delete'] === true)) { - $object->setData($attributeName, ''); - } else if ($imageName = $this->getUploadedImageName($value)) { - $object->setData($attributeName . self::ADDITIONAL_DATA_SUFFIX, $value); + if ($imageName = $this->getUploadedImageName($value)) { + $object->setData(self::ADDITIONAL_DATA_PREFIX . $attributeName, $value); $object->setData($attributeName, $imageName); + } else if (!is_string($value)) { + $object->setData($attributeName, ''); } return parent::beforeSave($object); @@ -127,7 +127,7 @@ private function getImageUploader() */ public function afterSave($object) { - $value = $object->getData($this->getAttribute()->getName() . self::ADDITIONAL_DATA_SUFFIX); + $value = $object->getData(self::ADDITIONAL_DATA_PREFIX . $this->getAttribute()->getName()); if ($imageName = $this->getUploadedImageName($value)) { try { diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php index cace3c1b39118..dafd97fd42721 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php @@ -24,6 +24,11 @@ class ImageTest extends \PHPUnit_Framework_TestCase */ protected $imageUploader; + /** + * @var \Psr\Log\LoggerInterface + */ + protected $logger; + protected function setUp() { $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); @@ -42,6 +47,16 @@ protected function setUp() ->method('getName') ->will($this->returnValue('test_attribute')); + $this->logger = $this->getMockForAbstractClass( + \Psr\Log\LoggerInterface::class, + [], + 'TestLogger', + false, + false, + true, + ['critical'] + ); + $this->imageUploader = $this->getMock( \Magento\Catalog\Model\ImageUploader::class, ['moveFileFromTmp'], @@ -51,7 +66,7 @@ protected function setUp() ); } - public function deletionValuesProvider() + public function deletionValueProvider() { return [ [false], @@ -60,7 +75,7 @@ public function deletionValuesProvider() } /** - * @dataProvider deletionValuesProvider + * @dataProvider deletionValueProvider * * @param $value */ @@ -78,6 +93,36 @@ public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueRequir $this->assertEquals('', $object->getTestAttribute()); } + public function invalidValueProvider() + { + return [ + [1234], + [true], + [new \stdClass()], + [function() {}], + [['a' => 1, 'b' => 2]] + ]; + } + + /** + * @dataProvider invalidValueProvider + * + * @param $value + */ + public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueInvalid($value) + { + $model = $this->objectManager->getObject(Model::class); + $model->setAttribute($this->attribute); + + $object = new \Magento\Framework\DataObject([ + 'test_attribute' => $value + ]); + + $model->beforeSave($object); + + $this->assertEquals('', $object->getTestAttribute()); + } + public function testBeforeSaveShouldSetAttributeValueToUploadedImageName() { $model = $this->objectManager->getObject(Model::class); @@ -109,55 +154,35 @@ public function testBeforeSaveShouldSetAttributeUploadInformationToTemporaryAttr $this->assertEquals([ ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.test.com/test123.jpg'] - ], $object->getTestAttributeAdditionalData()); + ], $object->getData('_additional_data_test_attribute')); } - public function stringValueProvider() - { - return [ - ['test123'], - [12345], - [true], - ['some' => 'value'] - ]; - } - - /** - * @dataProvider stringValueProvider - * - * @param $value - */ - public function testBeforeSaveShouldNotModifyAttributeValueWhenNotUploadData($value) + public function testBeforeSaveShouldNotModifyAttributeValueWhenStringValue() { $model = $this->objectManager->getObject(Model::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ - 'test_attribute' => $value + 'test_attribute' => 'test123.jpg' ]); $model->beforeSave($object); - $this->assertEquals($value, $object->getTestAttribute()); + $this->assertEquals('test123.jpg', $object->getTestAttribute()); } - /** - * @dataProvider stringValueProvider - * - * @param $value - */ - public function testBeforeSaveShouldNotSetAdditionalDataWhenNotUploadData($value) + public function testBeforeSaveShouldNotSetAdditionalDataWhenStringValue() { $model = $this->objectManager->getObject(Model::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ - 'test_attribute' => $value + 'test_attribute' => 'test123.jpg' ]); $model->beforeSave($object); - $this->assertNull($object->getTestAttributeAdditionalData()); + $this->assertNull($object->getData('_additional_data_test_attribute')); } protected function setUpModelForAfterSave() @@ -174,7 +199,7 @@ protected function setUpModelForAfterSave() $objectManagerMock->expects($this->any()) ->method('get') - ->will($this->returnCallback(function($class, $params = []) use ($imageUploaderMock) { + ->will($this->returnCallback(function ($class, $params = []) use ($imageUploaderMock) { if ($class == \Magento\Catalog\CategoryImageUpload::class) { return $imageUploaderMock; } @@ -183,13 +208,29 @@ protected function setUpModelForAfterSave() })); $model = $this->objectManager->getObject(Model::class, [ - 'objectManager' => $objectManagerMock + 'objectManager' => $objectManagerMock, + 'logger' => $this->logger ]); return $model->setAttribute($this->attribute); } - public function testAfterSaveShouldUploadImageWhenAdditionalDataSet() + public function attributeValidValueProvider() + { + return [ + [[['name' => 'test1234.jpg']]], + ['test1234.jpg'], + [''], + [false] + ]; + } + + /** + * @dataProvider attributeValidValueProvider + * + * @param $value + */ + public function testAfterSaveShouldUploadImageWhenAdditionalDataSet($value) { $model = $this->setUpModelForAfterSave(); @@ -198,7 +239,8 @@ public function testAfterSaveShouldUploadImageWhenAdditionalDataSet() ->with($this->equalTo('test1234.jpg')); $object = new \Magento\Framework\DataObject([ - 'test_attribute_additional_data' => [ + 'test_attribute' => $value, + '_additional_data_test_attribute' => [ ['name' => 'test1234.jpg'] ] ]); @@ -206,7 +248,12 @@ public function testAfterSaveShouldUploadImageWhenAdditionalDataSet() $model->afterSave($object); } - public function testAfterSaveShouldNotUploadImageWhenAdditionalDataNotSet() + /** + * @dataProvider attributeValidValueProvider + * + * @param $value + */ + public function testAfterSaveShouldNotUploadImageWhenAdditionalDataNotSet($value) { $model = $this->setUpModelForAfterSave(); @@ -214,7 +261,28 @@ public function testAfterSaveShouldNotUploadImageWhenAdditionalDataNotSet() ->method('moveFileFromTmp'); $object = new \Magento\Framework\DataObject([ - 'test_attribute' => [ + 'test_attribute' => $value + ]); + + $model->afterSave($object); + } + + public function testAfterSaveShouldCreateCriticalLogEntryOnUploadExceptions() + { + $model = $this->setUpModelForAfterSave(); + + $exception = new \Exception(); + + $this->imageUploader->expects($this->any()) + ->method('moveFileFromTmp') + ->will($this->throwException($exception)); + + $this->logger->expects($this->once()) + ->method('critical') + ->with($this->equalTo($exception)); + + $object = new \Magento\Framework\DataObject([ + '_additional_data_test_attribute' => [ ['name' => 'test1234.jpg'] ] ]); From c82aa4f0ef908efdc58a7e4c64dd4c10c8e6ee32 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Sat, 6 Aug 2016 22:53:48 +0200 Subject: [PATCH 10/55] fixes to code styling --- .../Unit/Model/Category/Attribute/Backend/ImageTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php index dafd97fd42721..2bd41e6bca343 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php @@ -95,11 +95,15 @@ public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueRequir public function invalidValueProvider() { + $closure = function () { + return false; + }; + return [ [1234], [true], [new \stdClass()], - [function() {}], + [$closure], [['a' => 1, 'b' => 2]] ]; } From 388029e9a6cf090736bd734f5a87c31815fe4e01 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Wed, 17 Aug 2016 17:51:07 +0200 Subject: [PATCH 11/55] Moved PageCache form_key reset to frontend area --- app/code/Magento/PageCache/etc/events.xml | 5 ----- app/code/Magento/PageCache/etc/frontend/events.xml | 13 +++++++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 app/code/Magento/PageCache/etc/frontend/events.xml diff --git a/app/code/Magento/PageCache/etc/events.xml b/app/code/Magento/PageCache/etc/events.xml index 7ac67a931c7ac..8d88feeab18bc 100644 --- a/app/code/Magento/PageCache/etc/events.xml +++ b/app/code/Magento/PageCache/etc/events.xml @@ -48,11 +48,6 @@ - - - diff --git a/app/code/Magento/PageCache/etc/frontend/events.xml b/app/code/Magento/PageCache/etc/frontend/events.xml new file mode 100644 index 0000000000000..b30e81f6527ab --- /dev/null +++ b/app/code/Magento/PageCache/etc/frontend/events.xml @@ -0,0 +1,13 @@ + + + + + + + \ No newline at end of file From f38a54986ece19e40813896207490a6627152d96 Mon Sep 17 00:00:00 2001 From: Allan Paiste Date: Wed, 17 Aug 2016 17:54:49 +0200 Subject: [PATCH 12/55] Missing line added to the end of the file --- app/code/Magento/PageCache/etc/frontend/events.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/PageCache/etc/frontend/events.xml b/app/code/Magento/PageCache/etc/frontend/events.xml index b30e81f6527ab..196d16c9a3c99 100644 --- a/app/code/Magento/PageCache/etc/frontend/events.xml +++ b/app/code/Magento/PageCache/etc/frontend/events.xml @@ -10,4 +10,4 @@ - \ No newline at end of file + From 06034a1e4a3a1c2a76e3263138d8ef5018c00292 Mon Sep 17 00:00:00 2001 From: Leonid Poluyanov Date: Wed, 28 Sep 2016 16:10:30 +0300 Subject: [PATCH 13/55] MAGETWO-57989: Unable to create custom image attribute in category --- .../Controller/Adminhtml/Category/Save.php | 14 +- .../Category/Attribute/Backend/Image.php | 19 +- .../Catalog/Model/Category/DataProvider.php | 4 +- .../Adminhtml/Category/Image/UploadTest.php | 25 ++- .../Adminhtml/Category/SaveTest.php | 39 ++-- .../Category/Attribute/Backend/ImageTest.php | 115 ++++++------ .../Catalog/Test/Unit/Model/CategoryTest.php | 172 +++++++++++------- .../base/web/js/form/element/file-uploader.js | 9 +- 8 files changed, 223 insertions(+), 174 deletions(-) diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php index 492bf1c021cf0..b8fe7df8189de 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Category/Save.php @@ -6,7 +6,7 @@ namespace Magento\Catalog\Controller\Adminhtml\Category; use Magento\Store\Model\StoreManagerInterface; -use \Magento\Catalog\Api\Data\CategoryAttributeInterface; +use Magento\Catalog\Api\Data\CategoryAttributeInterface; /** * Class Save @@ -70,14 +70,15 @@ public function __construct( \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, \Magento\Framework\View\LayoutFactory $layoutFactory, StoreManagerInterface $storeManager, - \Magento\Eav\Model\Config $eavConfig + \Magento\Eav\Model\Config $eavConfig = null ) { parent::__construct($context); $this->resultRawFactory = $resultRawFactory; $this->resultJsonFactory = $resultJsonFactory; $this->layoutFactory = $layoutFactory; $this->storeManager = $storeManager; - $this->eavConfig = $eavConfig; + $this->eavConfig = $eavConfig + ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Eav\Model\Config::class); } /** @@ -233,9 +234,10 @@ public function execute() } /** - * @param array $data Category data - * @return mixed - * @throws \Magento\Framework\Exception\LocalizedException + * Sets image attribute data to false if image was removed + * + * @param array $data + * @return array */ public function imagePreprocessing(array $data) { diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index efd3a4afb4a60..4d5a811211bf7 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -48,34 +48,29 @@ class Image extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend */ private $imageUploader; - /** - * @var \Magento\Framework\ObjectManagerInterface - */ - private $objectManager; - /** * @param \Psr\Log\LoggerInterface $logger * @param \Magento\Framework\Filesystem $filesystem * @param \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory - * @param \Magento\Framework\ObjectManagerInterface $objectManager */ public function __construct( \Psr\Log\LoggerInterface $logger, \Magento\Framework\Filesystem $filesystem, - \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory, - \Magento\Framework\ObjectManagerInterface $objectManager + \Magento\MediaStorage\Model\File\UploaderFactory $fileUploaderFactory ) { $this->_filesystem = $filesystem; $this->_fileUploaderFactory = $fileUploaderFactory; $this->_logger = $logger; - $this->objectManager = $objectManager; } /** + * Gets image name from $value array. + * Will return empty string in a case when $value is not an array + * * @param array $value Attribute value * @return string */ - protected function getUploadedImageName($value) + private function getUploadedImageName($value) { if (is_array($value) && isset($value[0]['name'])) { return $value[0]['name']; @@ -86,6 +81,7 @@ protected function getUploadedImageName($value) /** * Avoiding saving potential upload data to DB + * Will set empty image attribute value if image was not uploaded * * @param \Magento\Framework\DataObject $object * @return $this @@ -113,7 +109,8 @@ public function beforeSave($object) private function getImageUploader() { if ($this->imageUploader === null) { - $this->imageUploader = $this->objectManager->get(\Magento\Catalog\CategoryImageUpload::class); + $this->imageUploader = \Magento\Framework\App\ObjectManager::getInstance() + ->get(\Magento\Catalog\CategoryImageUpload::class); } return $this->imageUploader; diff --git a/app/code/Magento/Catalog/Model/Category/DataProvider.php b/app/code/Magento/Catalog/Model/Category/DataProvider.php index 8ea60d54ec556..aefd31e21ad62 100644 --- a/app/code/Magento/Catalog/Model/Category/DataProvider.php +++ b/app/code/Magento/Catalog/Model/Category/DataProvider.php @@ -370,11 +370,13 @@ protected function filterFields($categoryData) } /** + * Converts category image data to acceptable for rendering format + * * @param \Magento\Catalog\Model\Category $category * @param array $categoryData * @return array */ - protected function convertValues($category, $categoryData) + private function convertValues($category, $categoryData) { foreach ($category->getAttributes() as $attributeCode => $attribute) { if (!isset($categoryData[$attributeCode])) { diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php index 40c40a17b8e52..e49da3972e0a0 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/Image/UploadTest.php @@ -5,27 +5,26 @@ */ namespace Magento\Catalog\Test\Unit\Controller\Adminhtml\Category\Image; -use \Magento\Catalog\Controller\Adminhtml\Category\Image\Upload as Model; -use \Magento\Framework\App\Request\Http as Request; -use \Magento\Catalog\Model\ImageUploader; -use \Magento\Framework\Controller\ResultFactory; -use \Magento\Framework\DataObject; -use \Magento\Backend\App\Action\Context; +use Magento\Catalog\Controller\Adminhtml\Category\Image\Upload as Model; +use Magento\Framework\App\Request\Http as Request; +use Magento\Catalog\Model\ImageUploader; +use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\DataObject; +use Magento\Backend\App\Action\Context; /** * Class UploadTest - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class UploadTest extends \PHPUnit_Framework_TestCase { - protected $objectManager; + private $objectManager; protected function setUp() { $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); } - public function uploadedImageNameProvider() + public function executeDataProvider() { return [ ['image1', 'image1'], @@ -35,12 +34,12 @@ public function uploadedImageNameProvider() } /** - * @param $name - * @param $savedName + * @param string $name + * @param string $savedName * - * @dataProvider uploadedImageNameProvider + * @dataProvider executeDataProvider */ - public function testExecuteShouldSaveUploadedImageWithSpecifiedNameToTmpFolder($name, $savedName) + public function testExecute($name, $savedName) { $request = $this->objectManager->getObject(Request::class); diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php index 9ef502b9640ca..58557e25d970b 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Category/SaveTest.php @@ -16,67 +16,67 @@ class SaveTest extends \PHPUnit_Framework_TestCase /** * @var \Magento\Backend\Model\View\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $resultRedirectFactoryMock; + private $resultRedirectFactoryMock; /** * @var \Magento\Framework\Controller\Result\RawFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $resultRawFactoryMock; + private $resultRawFactoryMock; /** * @var \Magento\Framework\Controller\Result\JsonFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $resultJsonFactoryMock; + private $resultJsonFactoryMock; /** * @var \Magento\Framework\View\LayoutFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $layoutFactoryMock; + private $layoutFactoryMock; /** * @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject */ - protected $contextMock; + private $contextMock; /** * @var \Magento\Framework\View\Page\Title|\PHPUnit_Framework_MockObject_MockObject */ - protected $titleMock; + private $titleMock; /** * @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $requestMock; + private $requestMock; /** * @var \Magento\Framework\ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $objectManagerMock; + private $objectManagerMock; /** * @var \Magento\Framework\Event\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $eventManagerMock; + private $eventManagerMock; /** * @var \Magento\Framework\App\ResponseInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $responseMock; + private $responseMock; /** * @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $messageManagerMock; + private $messageManagerMock; /** * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ - protected $objectManager; + private $objectManager; /** * @var \Magento\Catalog\Controller\Adminhtml\Category\Save */ - protected $save; + private $save; /** * Set up @@ -581,7 +581,10 @@ public function dataProviderExecute() ]; } - public function attributeValueDataProvider() + /** + * @return array + */ + public function imagePreprocessingDataProvider() { return [ [['attribute1' => null, 'attribute2' => 123]], @@ -590,11 +593,11 @@ public function attributeValueDataProvider() } /** - * @dataProvider attributeValueDataProvider + * @dataProvider imagePreprocessingDataProvider * - * @param $data + * @param array $data */ - public function testImagePreprocessingShouldSetAttributesWithImageBackendToFalse($data) + public function testImagePreprocessingWithoutValue($data) { $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); @@ -630,7 +633,7 @@ public function testImagePreprocessingShouldSetAttributesWithImageBackendToFalse ], $result); } - public function testImagePreprocessingShouldNotSetValueToFalseWhenValueSet() + public function testImagePreprocessingWithValue() { $eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, ['getEntityType'], [], '', false); diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php index 2bd41e6bca343..aa78fea896687 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php @@ -5,29 +5,27 @@ */ namespace Magento\Catalog\Test\Unit\Model\Category\Attribute\Backend; -use \Magento\Catalog\Model\Category\Attribute\Backend\Image as Model; - class ImageTest extends \PHPUnit_Framework_TestCase { /** * @var \Magento\Eav\Model\Entity\Attribute\AbstractAttribute */ - protected $attribute; + private $attribute; /** * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ - protected $objectManager; + private $objectManager; /** * @var \Magento\Catalog\Model\ImageUploader */ - protected $imageUploader; + private $imageUploader; /** * @var \Psr\Log\LoggerInterface */ - protected $logger; + private $logger; protected function setUp() { @@ -66,7 +64,10 @@ protected function setUp() ); } - public function deletionValueProvider() + /** + * @return array + */ + public function deletedValueDataProvider() { return [ [false], @@ -75,13 +76,13 @@ public function deletionValueProvider() } /** - * @dataProvider deletionValueProvider + * @dataProvider deletedValueDataProvider * - * @param $value + * @param array $value */ - public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueRequiresDeletion($value) + public function testBeforeSaveValueDeletion($value) { - $model = $this->objectManager->getObject(Model::class); + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ @@ -93,7 +94,10 @@ public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueRequir $this->assertEquals('', $object->getTestAttribute()); } - public function invalidValueProvider() + /** + * @return array + */ + public function invalidValueDataProvider() { $closure = function () { return false; @@ -109,13 +113,13 @@ public function invalidValueProvider() } /** - * @dataProvider invalidValueProvider + * @dataProvider invalidValueDataProvider * - * @param $value + * @param array $value */ - public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueInvalid($value) + public function testBeforeSaveValueInvalid($value) { - $model = $this->objectManager->getObject(Model::class); + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ @@ -127,9 +131,9 @@ public function testBeforeSaveShouldSetAttributeValueToBlankWhenImageValueInvali $this->assertEquals('', $object->getTestAttribute()); } - public function testBeforeSaveShouldSetAttributeValueToUploadedImageName() + public function testBeforeSaveAttributeFileName() { - $model = $this->objectManager->getObject(Model::class); + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ @@ -143,27 +147,27 @@ public function testBeforeSaveShouldSetAttributeValueToUploadedImageName() $this->assertEquals('test123.jpg', $object->getTestAttribute()); } - public function testBeforeSaveShouldSetAttributeUploadInformationToTemporaryAttribute() + public function testBeforeSaveTemporaryAttribute() { - $model = $this->objectManager->getObject(Model::class); + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ 'test_attribute' => [ - ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.test.com/test123.jpg'] + ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg'] ] ]); $model->beforeSave($object); $this->assertEquals([ - ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.test.com/test123.jpg'] + ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg'] ], $object->getData('_additional_data_test_attribute')); } - public function testBeforeSaveShouldNotModifyAttributeValueWhenStringValue() + public function testBeforeSaveAttributeStringValue() { - $model = $this->objectManager->getObject(Model::class); + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ @@ -173,23 +177,13 @@ public function testBeforeSaveShouldNotModifyAttributeValueWhenStringValue() $model->beforeSave($object); $this->assertEquals('test123.jpg', $object->getTestAttribute()); - } - - public function testBeforeSaveShouldNotSetAdditionalDataWhenStringValue() - { - $model = $this->objectManager->getObject(Model::class); - $model->setAttribute($this->attribute); - - $object = new \Magento\Framework\DataObject([ - 'test_attribute' => 'test123.jpg' - ]); - - $model->beforeSave($object); - $this->assertNull($object->getData('_additional_data_test_attribute')); } - protected function setUpModelForAfterSave() + /** + * @return \Magento\Catalog\Model\Category\Attribute\Backend\Image + */ + private function setUpModelForAfterSave() { $objectManagerMock = $this->getMock( \Magento\Framework\App\ObjectManager::class, @@ -211,15 +205,16 @@ protected function setUpModelForAfterSave() return $this->objectManager->get($class, $params); })); - $model = $this->objectManager->getObject(Model::class, [ + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class, [ 'objectManager' => $objectManagerMock, 'logger' => $this->logger ]); + $this->objectManager->setBackwardCompatibleProperty($model, 'imageUploader', $this->imageUploader); return $model->setAttribute($this->attribute); } - public function attributeValidValueProvider() + public function attributeValueDataProvider() { return [ [[['name' => 'test1234.jpg']]], @@ -230,11 +225,11 @@ public function attributeValidValueProvider() } /** - * @dataProvider attributeValidValueProvider + * @dataProvider attributeValueDataProvider * - * @param $value + * @param array $value */ - public function testAfterSaveShouldUploadImageWhenAdditionalDataSet($value) + public function testAfterSaveWithAdditionalData($value) { $model = $this->setUpModelForAfterSave(); @@ -242,36 +237,38 @@ public function testAfterSaveShouldUploadImageWhenAdditionalDataSet($value) ->method('moveFileFromTmp') ->with($this->equalTo('test1234.jpg')); - $object = new \Magento\Framework\DataObject([ - 'test_attribute' => $value, - '_additional_data_test_attribute' => [ - ['name' => 'test1234.jpg'] + $object = new \Magento\Framework\DataObject( + [ + 'test_attribute' => $value, + '_additional_data_test_attribute' => [['name' => 'test1234.jpg']] ] - ]); + ); $model->afterSave($object); } /** - * @dataProvider attributeValidValueProvider + * @dataProvider attributeValueDataProvider * - * @param $value + * @param array $value */ - public function testAfterSaveShouldNotUploadImageWhenAdditionalDataNotSet($value) + public function testAfterSaveWithoutAdditionalData($value) { $model = $this->setUpModelForAfterSave(); $this->imageUploader->expects($this->never()) ->method('moveFileFromTmp'); - $object = new \Magento\Framework\DataObject([ - 'test_attribute' => $value - ]); + $object = new \Magento\Framework\DataObject( + [ + 'test_attribute' => $value + ] + ); $model->afterSave($object); } - public function testAfterSaveShouldCreateCriticalLogEntryOnUploadExceptions() + public function testAfterSaveWithExceptions() { $model = $this->setUpModelForAfterSave(); @@ -285,11 +282,11 @@ public function testAfterSaveShouldCreateCriticalLogEntryOnUploadExceptions() ->method('critical') ->with($this->equalTo($exception)); - $object = new \Magento\Framework\DataObject([ - '_additional_data_test_attribute' => [ - ['name' => 'test1234.jpg'] + $object = new \Magento\Framework\DataObject( + [ + '_additional_data_test_attribute' => [['name' => 'test1234.jpg']] ] - ]); + ); $model->afterSave($object); } diff --git a/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php index 66403d3964890..ce4cc93b0181b 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/CategoryTest.php @@ -9,7 +9,7 @@ namespace Magento\Catalog\Test\Unit\Model; use Magento\Catalog\Model\Indexer; -use Magento\Catalog\Model\Category as Model; +use Magento\Catalog\Model\Category; /** * @SuppressWarnings(PHPMD.TooManyFields) @@ -17,83 +17,125 @@ */ class CategoryTest extends \PHPUnit_Framework_TestCase { - /** @var \Magento\Catalog\Model\Category */ - protected $category; + /** + * @var \Magento\Catalog\Model\Category + */ + private $category; - /** @var \Magento\Framework\Model\Context|\PHPUnit_Framework_MockObject_MockObject */ - protected $context; + /** + * @var \Magento\Framework\Model\Context|\PHPUnit_Framework_MockObject_MockObject + */ + private $context; - /** @var \Magento\Framework\Event\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $eventManager; + /** + * @var \Magento\Framework\Event\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $eventManager; - /** @var \Magento\Framework\App\CacheInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $cacheManager; + /** + * @var \Magento\Framework\App\CacheInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $cacheManager; - /** @var \Magento\Framework\Registry|\PHPUnit_Framework_MockObject_MockObject */ - protected $registry; + /** + * @var \Magento\Framework\Registry|\PHPUnit_Framework_MockObject_MockObject + */ + private $registry; - /** @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $storeManager; + /** + * @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $storeManager; - /** @var \Magento\Catalog\Model\ResourceModel\Category\Tree|\PHPUnit_Framework_MockObject_MockObject */ - protected $categoryTreeResource; + /** + * @var \Magento\Catalog\Model\ResourceModel\Category\Tree|\PHPUnit_Framework_MockObject_MockObject + */ + private $categoryTreeResource; - /** @var \PHPUnit_Framework_MockObject_MockObject */ - protected $categoryTreeFactory; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $categoryTreeFactory; - /** @var \Magento\Catalog\Api\CategoryRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $categoryRepository; + /** + * @var \Magento\Catalog\Api\CategoryRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $categoryRepository; - /** @var \PHPUnit_Framework_MockObject_MockObject */ - protected $storeCollectionFactory; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $storeCollectionFactory; - /** @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $url; + /** + * @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $url; - /** @var \PHPUnit_Framework_MockObject_MockObject */ - protected $productCollectionFactory; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $productCollectionFactory; - /** @var \Magento\Catalog\Model\Config|\PHPUnit_Framework_MockObject_MockObject */ - protected $catalogConfig; + /** + * @var \Magento\Catalog\Model\Config|\PHPUnit_Framework_MockObject_MockObject + */ + private $catalogConfig; - /** @var \Magento\Framework\Filter\FilterManager|\PHPUnit_Framework_MockObject_MockObject */ - protected $filterManager; + /** + * @var \Magento\Framework\Filter\FilterManager|\PHPUnit_Framework_MockObject_MockObject + */ + private $filterManager; - /** @var \Magento\Catalog\Model\Indexer\Category\Flat\State|\PHPUnit_Framework_MockObject_MockObject */ - protected $flatState; + /** + * @var \Magento\Catalog\Model\Indexer\Category\Flat\State|\PHPUnit_Framework_MockObject_MockObject + */ + private $flatState; - /** @var \Magento\Framework\Indexer\IndexerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $flatIndexer; + /** + * @var \Magento\Framework\Indexer\IndexerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $flatIndexer; - /** @var \Magento\Framework\Indexer\IndexerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $productIndexer; + /** + * @var \Magento\Framework\Indexer\IndexerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productIndexer; - /** @var \Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator|\PHPUnit_Framework_MockObject_MockObject */ - protected $categoryUrlPathGenerator; + /** + * @var \Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator|\PHPUnit_Framework_MockObject_MockObject + */ + private $categoryUrlPathGenerator; - /** @var \Magento\UrlRewrite\Model\UrlFinderInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $urlFinder; + /** + * @var \Magento\UrlRewrite\Model\UrlFinderInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $urlFinder; - /** @var \Magento\Framework\Model\ResourceModel\AbstractResource|\PHPUnit_Framework_MockObject_MockObject */ - protected $resource; + /** + * @var \Magento\Framework\Model\ResourceModel\AbstractResource|\PHPUnit_Framework_MockObject_MockObject + */ + private $resource; - /** @var \Magento\Framework\Indexer\IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject */ - protected $indexerRegistry; + /** + * @var \Magento\Framework\Indexer\IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject + */ + private $indexerRegistry; /** - * @var \PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Catalog\Api\CategoryAttributeRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $metadataServiceMock; + private $metadataServiceMock; /** * @var \PHPUnit_Framework_MockObject_MockObject */ - protected $attributeValueFactory; + private $attributeValueFactory; /** * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ - protected $objectManager; + private $objectManager; protected function setUp() { @@ -496,21 +538,24 @@ public function testGetCustomAttributes() ); } - public function imageAttributeNameAndUrlProvider() + /** + * @return array + */ + public function getImageWithAttributeCodeDataProvider() { return [ - ['testimage', 'http://www.test123.com/catalog/category/testimage'], + ['testimage', 'http://www.example.com/catalog/category/testimage'], [false, false] ]; } /** - * @param $value - * @param $url + * @param string|bool $value + * @param string|bool $url * - * @dataProvider imageAttributeNameAndUrlProvider + * @dataProvider getImageWithAttributeCodeDataProvider */ - public function testGetImageUrlShouldGenerateMediaUrlForSpecifiedAttributeValue($value, $url) + public function testGetImageWithAttributeCode($value, $url) { $storeManager = $this->getMock(\Magento\Store\Model\StoreManager::class, ['getStore'], [], '', false); $store = $this->getMock(\Magento\Store\Model\Store::class, ['getBaseUrl'], [], '', false); @@ -522,11 +567,15 @@ public function testGetImageUrlShouldGenerateMediaUrlForSpecifiedAttributeValue( $store->expects($this->any()) ->method('getBaseUrl') ->with(\Magento\Framework\UrlInterface::URL_TYPE_MEDIA) - ->will($this->returnValue('http://www.test123.com/')); + ->will($this->returnValue('http://www.example.com/')); - $model = $this->objectManager->getObject(Model::class, [ - 'storeManager' => $storeManager - ]); + /** @var \Magento\Catalog\Model\Category $model */ + $model = $this->objectManager->getObject( + \Magento\Catalog\Model\Category::class, + [ + 'storeManager' => $storeManager + ] + ); $model->setData('attribute1', $value); @@ -535,7 +584,7 @@ public function testGetImageUrlShouldGenerateMediaUrlForSpecifiedAttributeValue( $this->assertEquals($url, $result); } - public function testGetImageUrlShouldGenerateMediaUrlForImageAttributeValue() + public function testGetImageWithoutAttributeCode() { $storeManager = $this->getMock(\Magento\Store\Model\StoreManager::class, ['getStore'], [], '', false); $store = $this->getMock(\Magento\Store\Model\Store::class, ['getBaseUrl'], [], '', false); @@ -547,9 +596,10 @@ public function testGetImageUrlShouldGenerateMediaUrlForImageAttributeValue() $store->expects($this->any()) ->method('getBaseUrl') ->with(\Magento\Framework\UrlInterface::URL_TYPE_MEDIA) - ->will($this->returnValue('http://www.test123.com/')); + ->will($this->returnValue('http://www.example.com/')); - $model = $this->objectManager->getObject(Model::class, [ + /** @var \Magento\Catalog\Model\Category $model */ + $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category::class, [ 'storeManager' => $storeManager ]); @@ -557,6 +607,6 @@ public function testGetImageUrlShouldGenerateMediaUrlForImageAttributeValue() $result = $model->getImageUrl(); - $this->assertEquals('http://www.test123.com/catalog/category/myimage', $result); + $this->assertEquals('http://www.example.com/catalog/category/myimage', $result); } } diff --git a/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js b/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js index 5d06886fe91bd..917dc62f9f49b 100644 --- a/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js +++ b/app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js @@ -299,16 +299,15 @@ define([ */ onBeforeFileUpload: function (e, data) { var file = data.files[0], - allowed = this.isFileAllowed(file); + allowed = this.isFileAllowed(file), + target = $(e.target); if (allowed.passed) { - var $target = $(e.target); - - $target.on('fileuploadsend', function(event, postData) { + target.on('fileuploadsend', function (event, postData) { postData.data.set('param_name', this.paramName); }.bind(data)); - $target.fileupload('process', data).done(function () { + target.fileupload('process', data).done(function () { data.submit(); }); } else { From 726928a4236135ffe5ce91035d8dd46a10f6f3af Mon Sep 17 00:00:00 2001 From: Ruslan Kostiv Date: Thu, 29 Sep 2016 21:33:06 +0300 Subject: [PATCH 14/55] MAGETWO-55849: Customer can be deleted without Merchant permissions verification --- app/code/Magento/User/Block/User/Edit.php | 6 +- .../User/Controller/Adminhtml/User/Delete.php | 10 + .../Controller/Adminhtml/User/DeleteTest.php | 225 ++++++++++++++++++ .../templates/user/roles_grid_js.phtml | 9 + .../app/Magento/User/Test/Repository/User.xml | 4 + .../TestCase/DeleteAdminUserEntityTest.php | 5 +- .../TestCase/DeleteAdminUserEntityTest.xml | 2 + 7 files changed, 257 insertions(+), 4 deletions(-) create mode 100644 app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php diff --git a/app/code/Magento/User/Block/User/Edit.php b/app/code/Magento/User/Block/User/Edit.php index f6c5b484861de..94b1b2f2e6cf9 100644 --- a/app/code/Magento/User/Block/User/Edit.php +++ b/app/code/Magento/User/Block/User/Edit.php @@ -50,7 +50,7 @@ protected function _construct() $this->buttonList->update('save', 'label', __('Save User')); $this->buttonList->remove('delete'); - $objId = $this->getRequest()->getParam($this->_objectId); + $objId = (int)$this->getRequest()->getParam($this->_objectId); if (!empty($objId)) { $this->addButton( @@ -59,10 +59,10 @@ protected function _construct() 'label' => __('Delete User'), 'class' => 'delete', 'onclick' => sprintf( - 'deleteConfirm("%s", "%s", %s)', + 'deleteUserAccount("%s", "%s", %s)', __('Are you sure you want to do this?'), $this->getUrl('adminhtml/*/delete'), - json_encode(['data' => ['user_id' => $objId]]) + $objId ), ] ); diff --git a/app/code/Magento/User/Controller/Adminhtml/User/Delete.php b/app/code/Magento/User/Controller/Adminhtml/User/Delete.php index d892c8533a297..3ff336eeecff8 100644 --- a/app/code/Magento/User/Controller/Adminhtml/User/Delete.php +++ b/app/code/Magento/User/Controller/Adminhtml/User/Delete.php @@ -6,6 +6,9 @@ */ namespace Magento\User\Controller\Adminhtml\User; +use Magento\User\Block\User\Edit\Tab\Main as UserEdit; +use Magento\Framework\Exception\AuthenticationException; + class Delete extends \Magento\User\Controller\Adminhtml\User { /** @@ -13,8 +16,10 @@ class Delete extends \Magento\User\Controller\Adminhtml\User */ public function execute() { + /** @var \Magento\User\Model\User */ $currentUser = $this->_objectManager->get(\Magento\Backend\Model\Auth\Session::class)->getUser(); $userId = (int)$this->getRequest()->getPost('user_id'); + if ($userId) { if ($currentUser->getId() == $userId) { $this->messageManager->addError(__('You cannot delete your own account.')); @@ -22,6 +27,11 @@ public function execute() return; } try { + $currentUserPassword = (string)$this->getRequest()->getPost(UserEdit::CURRENT_USER_PASSWORD_FIELD); + if (empty($currentUserPassword)) { + throw new AuthenticationException(__('You have entered an invalid password for current user.')); + } + $currentUser->performIdentityCheck($currentUserPassword); /** @var \Magento\User\Model\User $model */ $model = $this->_userFactory->create(); $model->setId($userId); diff --git a/app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php b/app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php new file mode 100644 index 0000000000000..5e2d0d6bcfdbf --- /dev/null +++ b/app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php @@ -0,0 +1,225 @@ +objectManagerMock = $this->getMockBuilder(\Magento\Framework\ObjectManager\ObjectManager::class) + ->disableOriginalConstructor() + ->setMethods(['get', 'create']) + ->getMock(); + + $this->responseMock = $this->getMockBuilder(\Magento\Framework\App\ResponseInterface::class) + ->disableOriginalConstructor() + ->setMethods(['setRedirect']) + ->getMockForAbstractClass(); + + $this->requestMock = $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class) + ->disableOriginalConstructor() + ->setMethods(['getPost']) + ->getMockForAbstractClass(); + + $this->authSessionMock = $this->getMockBuilder(Session::class) + ->disableOriginalConstructor() + ->setMethods(['getUser']) + ->getMock(); + + $this->userMock = $this->getMockBuilder(\Magento\User\Model\User::class) + ->disableOriginalConstructor() + ->setMethods(['getId', 'performIdentityCheck', 'delete']) + ->getMock(); + + $this->userFactoryMock = $this->getMockBuilder(\Magento\User\Model\UserFactory::class) + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + + $this->messageManagerMock = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $objectManager = new ObjectManagerHelper($this); + $context = $objectManager->getObject( + \Magento\Backend\App\Action\Context::class, + [ + 'request' => $this->requestMock, + 'response' => $this->responseMock, + 'objectManager' => $this->objectManagerMock, + 'messageManager' => $this->messageManagerMock, + ] + ); + + $this->controller = $objectManager->getObject( + \Magento\User\Controller\Adminhtml\User\Delete::class, + [ + 'context' => $context, + 'userFactory' => $this->userFactoryMock, + ] + ); + } + + /** + * Test method \Magento\User\Controller\Adminhtml\User\Delete::execute + * + * @param string $currentUserPassword + * @param int $userId + * @param int $currentUserId + * @param string $resultMethod + * + * @dataProvider executeDataProvider + * @return void + * + */ + public function testExecute($currentUserPassword, $userId, $currentUserId, $resultMethod) + { + $currentUserMock = $this->userMock; + $this->authSessionMock->expects($this->any())->method('getUser')->will($this->returnValue($currentUserMock)); + + $currentUserMock->expects($this->any())->method('getId')->willReturn($currentUserId); + + $this->objectManagerMock + ->expects($this->any()) + ->method('get') + ->with(Session::class) + ->willReturn($this->authSessionMock); + + $this->requestMock->expects($this->any()) + ->method('getPost') + ->willReturnMap([ + ['user_id', $userId], + [UserEdit::CURRENT_USER_PASSWORD_FIELD, $currentUserPassword], + ]); + + $userMock = clone $currentUserMock; + + $this->userFactoryMock->expects($this->any())->method('create')->will($this->returnValue($userMock)); + $this->responseMock->expects($this->any())->method('setRedirect')->willReturnSelf(); + $this->userMock->expects($this->any())->method('delete')->willReturnSelf(); + $this->messageManagerMock->expects($this->once())->method($resultMethod); + + $this->controller->execute(); + } + + /** + * @return void + */ + public function testEmptyPasswordThrowsException() + { + try { + $currentUserId = 1; + $userId = 2; + + $currentUserMock = $this->userMock; + $this->authSessionMock->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($currentUserMock)); + + $currentUserMock->expects($this->any())->method('getId')->willReturn($currentUserId); + + $this->objectManagerMock + ->expects($this->any()) + ->method('get') + ->with(Session::class) + ->willReturn($this->authSessionMock); + + $this->requestMock->expects($this->any()) + ->method('getPost') + ->willReturnMap([ + ['user_id', $userId], + [UserEdit::CURRENT_USER_PASSWORD_FIELD, ''], + ]); + + $this->controller->execute(); + } catch (AuthenticationException $e) { + $this->assertEquals($e->getMessage(), 'You have entered an invalid password for current user.'); + } + } + + /** + * Data Provider for execute method + * + * @return array + */ + public function executeDataProvider() + { + return [ + [ + 'currentUserPassword' => '123123q', + 'userId' => 1, + 'currentUserId' => 2, + 'resultMethod' => 'addSuccess', + ], + [ + 'currentUserPassword' => '123123q', + 'userId' => 0, + 'currentUserId' => 2, + 'resultMethod' => 'addError', + ], + [ + 'currentUserPassword' => '123123q', + 'userId' => 1, + 'currentUserId' => 1, + 'resultMethod' => 'addError', + ], + ]; + } +} diff --git a/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml b/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml index bac0dd0b3f869..9795c7fd1672f 100644 --- a/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml +++ b/app/code/Magento/User/view/adminhtml/templates/user/roles_grid_js.phtml @@ -70,4 +70,13 @@ require([ }); + function deleteUserAccount(message, url, objId) { + if (jQuery.validator.validateElement(jQuery('[name="current_password"]'))) { + postData = {'data' : { + 'user_id': objId, + 'current_password': jQuery('[name="current_password"]').val() + }} + deleteConfirm(message, url, postData); + } + } diff --git a/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml b/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml index 25b812bf77665..dbb7a4aeb53bb 100644 --- a/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml +++ b/dev/tests/functional/tests/app/Magento/User/Test/Repository/User.xml @@ -41,5 +41,9 @@ %current_password% Active + + + 123123q + diff --git a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php index 57de56c7f56f7..67ede3855e7a7 100644 --- a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php +++ b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.php @@ -101,11 +101,13 @@ public function __inject( * * @param User $user * @param string $isDefaultUser + * @param User $systemAdmin * @return void */ public function testDeleteAdminUserEntity( User $user, - $isDefaultUser + $isDefaultUser, + User $systemAdmin = null ) { $filter = [ 'username' => $user->getUsername(), @@ -118,6 +120,7 @@ public function testDeleteAdminUserEntity( } $this->userIndex->open(); $this->userIndex->getUserGrid()->searchAndOpen($filter); + $this->userEdit->getUserForm()->fill($systemAdmin); $this->userEdit->getPageActions()->delete(); $this->userEdit->getModalBlock()->acceptAlert(); } diff --git a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml index 782c3243eaa6b..fd10c53dfd533 100644 --- a/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml +++ b/dev/tests/functional/tests/app/Magento/User/Test/TestCase/DeleteAdminUserEntityTest.xml @@ -9,11 +9,13 @@ 0 + system_admin 1 + system_admin From 41b7e36575283d64831b16e2e0e908dceb191f52 Mon Sep 17 00:00:00 2001 From: Ruslan Kostiv Date: Fri, 30 Sep 2016 13:08:42 +0300 Subject: [PATCH 15/55] MAGETWO-55395: Custom address attribute not appearing on Checkout summary --- .../template/shipping-address/address-renderer/default.html | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/code/Magento/Checkout/view/frontend/web/template/shipping-address/address-renderer/default.html b/app/code/Magento/Checkout/view/frontend/web/template/shipping-address/address-renderer/default.html index eeecbf3466366..a2b83ead6b354 100644 --- a/app/code/Magento/Checkout/view/frontend/web/template/shipping-address/address-renderer/default.html +++ b/app/code/Magento/Checkout/view/frontend/web/template/shipping-address/address-renderer/default.html @@ -13,6 +13,11 @@
+ + + + +