From 5bb5dd96231da4b61fc2f03480fb84acb389f66c Mon Sep 17 00:00:00 2001 From: dvinograd Date: Mon, 7 Nov 2016 13:05:52 +0100 Subject: [PATCH 01/32] Fix xml parser issue --- lib/internal/Magento/Framework/Xml/Parser.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Xml/Parser.php b/lib/internal/Magento/Framework/Xml/Parser.php index 15d41d5d5427c..915cef2fbae06 100644 --- a/lib/internal/Magento/Framework/Xml/Parser.php +++ b/lib/internal/Magento/Framework/Xml/Parser.php @@ -111,7 +111,10 @@ protected function _xmlToArray($currentNode = false) $value = ['_value' => $value, '_attribute' => $attributes]; } if (isset($content[$node->nodeName])) { - if (!isset($content[$node->nodeName][0]) || !is_array($content[$node->nodeName][0])) { + if ( + (is_string($content[$node->nodeName]) || !isset($content[$node->nodeName][0])) + || (is_array($value) && !is_array($content[$node->nodeName][0])) + ) { $oldValue = $content[$node->nodeName]; $content[$node->nodeName] = []; $content[$node->nodeName][] = $oldValue; From 2c1997dcc853d773e19cd886f2f2cf47c7030639 Mon Sep 17 00:00:00 2001 From: Derik Nel Date: Mon, 9 Jan 2017 16:36:22 +0200 Subject: [PATCH 02/32] Updated Allow.php Typecast the $this->_getAllowedCurrencies() to an array so that single string responses don't break the logical checks --- .../Magento/Config/Model/Config/Backend/Currency/Allow.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php b/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php index 5ee4a093fb6bf..cc2594af93b56 100644 --- a/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php +++ b/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php @@ -51,7 +51,7 @@ public function __construct( public function afterSave() { $exceptions = []; - foreach ($this->_getAllowedCurrencies() as $currencyCode) { + foreach ((array)$this->_getAllowedCurrencies() as $currencyCode) { if (!in_array($currencyCode, $this->_getInstalledCurrencies())) { $exceptions[] = __( 'Selected allowed currency "%1" is not available in installed currencies.', @@ -60,7 +60,7 @@ public function afterSave() } } - if (!in_array($this->_getCurrencyDefault(), $this->_getAllowedCurrencies())) { + if (!in_array($this->_getCurrencyDefault(), (array)$this->_getAllowedCurrencies())) { $exceptions[] = __( 'Default display currency "%1" is not available in allowed currencies.', $this->_localeCurrency->getCurrency($this->_getCurrencyDefault())->getName() From cdb1c003842baf21ff69464496e6c2ff2392f71f Mon Sep 17 00:00:00 2001 From: Derik Nel Date: Mon, 9 Jan 2017 16:38:00 +0200 Subject: [PATCH 03/32] Update DefaultCurrency.php Fixed: Something went wrong while saving this configuration: Warning: in_array() expects parameter 2 to be array, string given in /opt/vaimo/deploy/x7642/2017-01-09_13-38-36/htdocs/vendor/magento/module-config/Model/Config/Backend/Currency/DefaultCurrency.php on line 30 --- .../Config/Model/Config/Backend/Currency/DefaultCurrency.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php b/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php index 5a5e8f92e568d..f30019a8f016b 100644 --- a/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php +++ b/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php @@ -27,7 +27,7 @@ public function afterSave() ); } - if (!in_array($this->getValue(), $this->_getAllowedCurrencies())) { + if (!in_array($this->getValue(), (array)$this->_getAllowedCurrencies())) { throw new \Magento\Framework\Exception\LocalizedException( __('Sorry, the default display currency you selected is not available in allowed currencies.') ); From 1727c7e9ebb3e457ebeadc3ca31643b869851f09 Mon Sep 17 00:00:00 2001 From: Derik Nel Date: Tue, 10 Jan 2017 15:26:13 +0200 Subject: [PATCH 04/32] Update Allow.php --- .../Magento/Config/Model/Config/Backend/Currency/Allow.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php b/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php index cc2594af93b56..5ee4a093fb6bf 100644 --- a/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php +++ b/app/code/Magento/Config/Model/Config/Backend/Currency/Allow.php @@ -51,7 +51,7 @@ public function __construct( public function afterSave() { $exceptions = []; - foreach ((array)$this->_getAllowedCurrencies() as $currencyCode) { + foreach ($this->_getAllowedCurrencies() as $currencyCode) { if (!in_array($currencyCode, $this->_getInstalledCurrencies())) { $exceptions[] = __( 'Selected allowed currency "%1" is not available in installed currencies.', @@ -60,7 +60,7 @@ public function afterSave() } } - if (!in_array($this->_getCurrencyDefault(), (array)$this->_getAllowedCurrencies())) { + if (!in_array($this->_getCurrencyDefault(), $this->_getAllowedCurrencies())) { $exceptions[] = __( 'Default display currency "%1" is not available in allowed currencies.', $this->_localeCurrency->getCurrency($this->_getCurrencyDefault())->getName() From f2304f203235b40e57c73b8cd51cd94bb7a6ab4c Mon Sep 17 00:00:00 2001 From: Derik Nel Date: Tue, 10 Jan 2017 15:26:37 +0200 Subject: [PATCH 05/32] Update DefaultCurrency.php --- .../Config/Model/Config/Backend/Currency/DefaultCurrency.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php b/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php index f30019a8f016b..5a5e8f92e568d 100644 --- a/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php +++ b/app/code/Magento/Config/Model/Config/Backend/Currency/DefaultCurrency.php @@ -27,7 +27,7 @@ public function afterSave() ); } - if (!in_array($this->getValue(), (array)$this->_getAllowedCurrencies())) { + if (!in_array($this->getValue(), $this->_getAllowedCurrencies())) { throw new \Magento\Framework\Exception\LocalizedException( __('Sorry, the default display currency you selected is not available in allowed currencies.') ); From 7139c7a5ac8dde0b760aed65cbd2bf89e7d18f76 Mon Sep 17 00:00:00 2001 From: Derik Nel Date: Tue, 10 Jan 2017 15:27:04 +0200 Subject: [PATCH 06/32] Update AbstractCurrency.php --- .../Config/Model/Config/Backend/Currency/AbstractCurrency.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Config/Model/Config/Backend/Currency/AbstractCurrency.php b/app/code/Magento/Config/Model/Config/Backend/Currency/AbstractCurrency.php index d8a9d4a00cb88..c3d6348526e33 100644 --- a/app/code/Magento/Config/Model/Config/Backend/Currency/AbstractCurrency.php +++ b/app/code/Magento/Config/Model/Config/Backend/Currency/AbstractCurrency.php @@ -32,7 +32,8 @@ protected function _getAllowedCurrencies() ) ); } - return $this->getData('groups/options/fields/allow/value'); + + return (array)$this->getData('groups/options/fields/allow/value'); } /** From 47a4681f6452559682ed6f820e1a79c76bd96eb0 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 13:32:04 +0000 Subject: [PATCH 07/32] Remove Zend_Validate calls --- app/code/Magento/Contact/Controller/Index/Post.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 9ad831befd06f..ebb8085aacaa0 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -37,16 +37,16 @@ public function execute() $error = false; - if (!\Zend_Validate::is(trim($post['name']), 'NotEmpty')) { + if (trim($post['name']) === '') { $error = true; } - if (!\Zend_Validate::is(trim($post['comment']), 'NotEmpty')) { + if (trim($post['comment']) === '') { $error = true; } if (!\Zend_Validate::is(trim($post['email']), 'EmailAddress')) { $error = true; } - if (\Zend_Validate::is(trim($post['hideit']), 'NotEmpty')) { + if (trim($post['hideit']) !== '') { $error = true; } if ($error) { From e1d47cc0b94fa2d3ad20b7ddd0706039f372c948 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 13:46:44 +0000 Subject: [PATCH 08/32] Replace Zend_Validate_EmailAddress with 100% correct email validation Reference: https://hackernoon.com/the-100-correct-way-to-validate-email-addresses-7c4818f24643 --- app/code/Magento/Contact/Controller/Index/Post.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index ebb8085aacaa0..86f9a5071d8a0 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -43,7 +43,7 @@ public function execute() if (trim($post['comment']) === '') { $error = true; } - if (!\Zend_Validate::is(trim($post['email']), 'EmailAddress')) { + if (false === \strpos($post['email'], '@')) { $error = true; } if (trim($post['hideit']) !== '') { From 962c06dbe4f2384469c167d30141c7702b2cc694 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 14:05:12 +0000 Subject: [PATCH 09/32] Extract sendEmail method in contact form controller --- .../Magento/Contact/Controller/Index/Post.php | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 86f9a5071d8a0..964e38a97ab9d 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -32,9 +32,6 @@ public function execute() $this->inlineTranslation->suspend(); try { - $postObject = new \Magento\Framework\DataObject(); - $postObject->setData($post); - $error = false; if (trim($post['name']) === '') { @@ -53,22 +50,7 @@ public function execute() throw new \Exception(); } - $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; - $transport = $this->_transportBuilder - ->setTemplateIdentifier($this->scopeConfig->getValue(self::XML_PATH_EMAIL_TEMPLATE, $storeScope)) - ->setTemplateOptions( - [ - 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, - ] - ) - ->setTemplateVars(['data' => $postObject]) - ->setFrom($this->scopeConfig->getValue(self::XML_PATH_EMAIL_SENDER, $storeScope)) - ->addTo($this->scopeConfig->getValue(self::XML_PATH_EMAIL_RECIPIENT, $storeScope)) - ->setReplyTo($post['email']) - ->getTransport(); - - $transport->sendMessage(); + $this->sendEmail($post); $this->inlineTranslation->resume(); $this->messageManager->addSuccess( __('Thanks for contacting us with your comments and questions. We\'ll respond to you very soon.') @@ -101,4 +83,27 @@ private function getDataPersistor() return $this->dataPersistor; } + + /** + * @param array $post Post data from contact form + */ + private function sendEmail($post) + { + $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; + $transport = $this->_transportBuilder + ->setTemplateIdentifier($this->scopeConfig->getValue(self::XML_PATH_EMAIL_TEMPLATE, $storeScope)) + ->setTemplateOptions( + [ + 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, + 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, + ] + ) + ->setTemplateVars(['data' => new \Magento\Framework\DataObject($post)]) + ->setFrom($this->scopeConfig->getValue(self::XML_PATH_EMAIL_SENDER, $storeScope)) + ->addTo($this->scopeConfig->getValue(self::XML_PATH_EMAIL_RECIPIENT, $storeScope)) + ->setReplyTo($post['email']) + ->getTransport(); + + $transport->sendMessage(); + } } From fb902c6b91c2d419bc80a80a04e5c00b1ed5f5f5 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 19:34:02 +0000 Subject: [PATCH 10/32] Extract validatedParams method in contact form controller, use redirect result factory --- .../Magento/Contact/Controller/Index/Post.php | 66 ++++++++++--------- .../Magento/Contact/Controller/IndexTest.php | 1 + 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 964e38a97ab9d..86c23a02a9936 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -8,6 +8,7 @@ use Magento\Framework\App\Request\DataPersistorInterface; use Magento\Framework\App\ObjectManager; +use Magento\Framework\HTTP\PhpEnvironment\Request; class Post extends \Magento\Contact\Controller\Index { @@ -19,54 +20,29 @@ class Post extends \Magento\Contact\Controller\Index /** * Post user question * - * @return void - * @throws \Exception */ public function execute() { - $post = $this->getRequest()->getPostValue(); - if (!$post) { - $this->_redirect('*/*/'); - return; + if (! $this->isPostRequest()) { + return $this->resultRedirectFactory->create()->setPath('*/*/'); } $this->inlineTranslation->suspend(); try { - $error = false; - - if (trim($post['name']) === '') { - $error = true; - } - if (trim($post['comment']) === '') { - $error = true; - } - if (false === \strpos($post['email'], '@')) { - $error = true; - } - if (trim($post['hideit']) !== '') { - $error = true; - } - if ($error) { - throw new \Exception(); - } - - $this->sendEmail($post); + $this->sendEmail($this->validatedParams()); $this->inlineTranslation->resume(); $this->messageManager->addSuccess( __('Thanks for contacting us with your comments and questions. We\'ll respond to you very soon.') ); $this->getDataPersistor()->clear('contact_us'); - $this->_redirect('contact/index'); - return; } catch (\Exception $e) { $this->inlineTranslation->resume(); $this->messageManager->addError( __('We can\'t process your request right now. Sorry, that\'s all we know.') ); - $this->getDataPersistor()->set('contact_us', $post); - $this->_redirect('contact/index'); - return; + $this->getDataPersistor()->set('contact_us', $this->getRequest()->getParams()); } + return $this->resultRedirectFactory->create()->setPath('contact/index'); } /** @@ -106,4 +82,34 @@ private function sendEmail($post) $transport->sendMessage(); } + + private function isPostRequest() + { + /** @var Request $request */ + $request = $this->getRequest(); + return !empty($request->getPostValue()); + } + + private function validatedParams() + { + $request = $this->getRequest(); + $error = false; + if (trim($request->getParam('name')) === '') { + $error = true; + } + if (trim($request->getParam('comment')) === '') { + $error = true; + } + if (false === \strpos($request->getParam('email'), '@')) { + $error = true; + } + if (trim($request->getParam('hideit')) !== '') { + $error = true; + } + if ($error) { + throw new \Exception(); + } + + return $request->getParams(); + } } diff --git a/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php b/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php index ab2d1f73ff837..6615dafdb97b9 100644 --- a/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php +++ b/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php @@ -21,6 +21,7 @@ public function testPostAction() $this->getRequest()->setPostValue($params); $this->dispatch('contact/index/post'); + $this->assertRedirect($this->stringContains('contact/index')); $this->assertSessionMessages( $this->contains( "Thanks for contacting us with your comments and questions. We'll respond to you very soon." From 352dcef8352f81245e29e3526fb79713fcadabdb Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 19:45:32 +0000 Subject: [PATCH 11/32] Show meaningful error message if contact form validation fails --- .../Magento/Contact/Controller/Index/Post.php | 24 +++++---- app/code/Magento/Contact/i18n/en_US.csv | 3 ++ .../Magento/Contact/Controller/IndexTest.php | 49 +++++++++++++++++++ 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 86c23a02a9936..ae0812a247319 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -8,6 +8,7 @@ use Magento\Framework\App\Request\DataPersistorInterface; use Magento\Framework\App\ObjectManager; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\HTTP\PhpEnvironment\Request; class Post extends \Magento\Contact\Controller\Index @@ -30,17 +31,20 @@ public function execute() $this->inlineTranslation->suspend(); try { $this->sendEmail($this->validatedParams()); - $this->inlineTranslation->resume(); $this->messageManager->addSuccess( __('Thanks for contacting us with your comments and questions. We\'ll respond to you very soon.') ); $this->getDataPersistor()->clear('contact_us'); + } catch (LocalizedException $e) { + $this->messageManager->addErrorMessage($e->getMessage()); + $this->getDataPersistor()->set('contact_us', $this->getRequest()->getParams()); } catch (\Exception $e) { - $this->inlineTranslation->resume(); - $this->messageManager->addError( + $this->messageManager->addErrorMessage( __('We can\'t process your request right now. Sorry, that\'s all we know.') ); $this->getDataPersistor()->set('contact_us', $this->getRequest()->getParams()); + } finally { + $this->inlineTranslation->resume(); } return $this->resultRedirectFactory->create()->setPath('contact/index'); } @@ -90,23 +94,23 @@ private function isPostRequest() return !empty($request->getPostValue()); } + /** + * @return array + * @throws \Exception + */ private function validatedParams() { $request = $this->getRequest(); - $error = false; if (trim($request->getParam('name')) === '') { - $error = true; + throw new LocalizedException(__('Name is missing')); } if (trim($request->getParam('comment')) === '') { - $error = true; + throw new LocalizedException(__('Comment is missing')); } if (false === \strpos($request->getParam('email'), '@')) { - $error = true; + throw new LocalizedException(__('Invalid email address')); } if (trim($request->getParam('hideit')) !== '') { - $error = true; - } - if ($error) { throw new \Exception(); } diff --git a/app/code/Magento/Contact/i18n/en_US.csv b/app/code/Magento/Contact/i18n/en_US.csv index 7d42624e2f7e2..7b6a3011d714c 100644 --- a/app/code/Magento/Contact/i18n/en_US.csv +++ b/app/code/Magento/Contact/i18n/en_US.csv @@ -22,3 +22,6 @@ Contacts,Contacts "Email Sender","Email Sender" "Email Template","Email Template" "Email template chosen based on theme fallback when ""Default"" option is selected.","Email template chosen based on theme fallback when ""Default"" option is selected." +"Comment is missing","Comment is missing" +"Name is missing","Name is missing" +"Invalid email address","Invalid email address" \ No newline at end of file diff --git a/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php b/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php index 6615dafdb97b9..f68f2281e2854 100644 --- a/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php +++ b/dev/tests/integration/testsuite/Magento/Contact/Controller/IndexTest.php @@ -29,4 +29,53 @@ public function testPostAction() \Magento\Framework\Message\MessageInterface::TYPE_SUCCESS ); } + + /** + * @dataProvider dataInvalidPostAction + * @param $params + * @param $expectedMessage + */ + public function testInvalidPostAction($params, $expectedMessage) + { + $this->getRequest()->setPostValue($params); + + $this->dispatch('contact/index/post'); + $this->assertRedirect($this->stringContains('contact/index')); + $this->assertSessionMessages( + $this->contains($expectedMessage), + \Magento\Framework\Message\MessageInterface::TYPE_ERROR + ); + } + public static function dataInvalidPostAction() + { + return [ + 'missing_comment' => [ + 'params' => [ + 'name' => 'customer name', + 'comment' => '', + 'email' => 'user@example.com', + 'hideit' => '', + ], + 'expectedMessage' => "Comment is missing", + ], + 'missing_name' => [ + 'params' => [ + 'name' => '', + 'comment' => 'customer comment', + 'email' => 'user@example.com', + 'hideit' => '', + ], + 'expectedMessage' => "Name is missing", + ], + 'invalid_email' => [ + 'params' => [ + 'name' => 'customer name', + 'comment' => 'customer comment', + 'email' => 'invalidemail', + 'hideit' => '', + ], + 'expectedMessage' => "Invalid email address", + ], + ]; + } } From c8fbb7072715c3d59e62ad12e37406518f2c0b14 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 19:58:13 +0000 Subject: [PATCH 12/32] Make validation and sending methods in contacts form controller protected I intended to move it out of the controller altogether but it seems to be against what the team had in mind with the abstract controller --- app/code/Magento/Contact/Controller/Index/Post.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index ae0812a247319..7ef5b9d48f9f8 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -67,7 +67,7 @@ private function getDataPersistor() /** * @param array $post Post data from contact form */ - private function sendEmail($post) + protected function sendEmail($post) { $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; $transport = $this->_transportBuilder @@ -98,7 +98,7 @@ private function isPostRequest() * @return array * @throws \Exception */ - private function validatedParams() + protected function validatedParams() { $request = $this->getRequest(); if (trim($request->getParam('name')) === '') { From 36261ea197d5875fc99e7c1608c4ae2a516a3fcd Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 22:33:07 +0000 Subject: [PATCH 13/32] Add docblock comments --- app/code/Magento/Contact/Controller/Index/Post.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 7ef5b9d48f9f8..11d92375bbcb6 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -8,6 +8,7 @@ use Magento\Framework\App\Request\DataPersistorInterface; use Magento\Framework\App\ObjectManager; +use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\HTTP\PhpEnvironment\Request; @@ -21,6 +22,7 @@ class Post extends \Magento\Contact\Controller\Index /** * Post user question * + * @return Redirect */ public function execute() { @@ -66,6 +68,7 @@ private function getDataPersistor() /** * @param array $post Post data from contact form + * @return void */ protected function sendEmail($post) { @@ -87,6 +90,9 @@ protected function sendEmail($post) $transport->sendMessage(); } + /** + * @return bool + */ private function isPostRequest() { /** @var Request $request */ From ec3fad1b98c3462f46da353c9236a226378462b9 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 23:12:07 +0000 Subject: [PATCH 14/32] Update unit test --- .../Test/Unit/Controller/Index/PostTest.php | 77 +++++++++++-------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index a65302733ddff..834b8fcd8ec4c 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -5,6 +5,7 @@ * See COPYING.txt for license details. */ namespace Magento\Contact\Test\Unit\Controller\Index; +use Magento\Framework\Controller\Result\Redirect; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -22,24 +23,24 @@ class PostTest extends \PHPUnit_Framework_TestCase protected $scopeConfigMock; /** - * @var \Magento\Framework\App\ViewInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\Controller\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $viewMock; + protected $redirectResultFactoryMock; /** - * @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject + * @var Redirect|\PHPUnit_Framework_MockObject_MockObject */ - protected $urlMock; + private $redirectResultMock; /** - * @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $requestMock; + protected $urlMock; /** - * @var \Magento\Framework\App\Response\RedirectInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\Request\HttpRequest|\PHPUnit_Framework_MockObject_MockObject */ - protected $redirectMock; + protected $requestStub; /** * @var \Magento\Framework\Mail\Template\TransportBuilder|\PHPUnit_Framework_MockObject_MockObject @@ -76,7 +77,7 @@ protected function setUp() ); $context = $this->getMock( \Magento\Framework\App\Action\Context::class, - ['getRequest', 'getResponse', 'getView', 'getUrl', 'getRedirect', 'getMessageManager'], + ['getRequest', 'getResponse', 'getResultRedirectFactory', 'getUrl', 'getRedirect', 'getMessageManager'], [], '', false @@ -84,11 +85,20 @@ protected function setUp() $this->urlMock = $this->getMock(\Magento\Framework\UrlInterface::class, [], [], '', false); $this->messageManagerMock = $this->getMock(\Magento\Framework\Message\ManagerInterface::class, [], [], '', false); - $this->requestMock = - $this->getMock(\Magento\Framework\App\Request\Http::class, ['getPostValue'], [], '', false); - $this->redirectMock = - $this->getMock(\Magento\Framework\App\Response\RedirectInterface::class, [], [], '', false); - $this->viewMock = $this->getMock(\Magento\Framework\App\ViewInterface::class, [], [], '', false); + $this->requestStub = + $this->getMock(\Magento\Framework\App\Request\Http::class, ['getPostValue', 'getParams', 'getParam'], [], '', false); + $this->redirectResultMock = $this->getMock( + \Magento\Framework\Controller\Result\Redirect::class, + [], + [], + '', + false + ); + $this->redirectResultMock->method('setPath')->willReturnSelf(); + $this->redirectResultFactoryMock = $this->getMock(\Magento\Framework\Controller\Result\RedirectFactory::class, ['create'], [], '', false); + $this->redirectResultFactoryMock + ->method('create') + ->willReturn($this->redirectResultMock); $this->storeManagerMock = $this->getMock(\Magento\Store\Model\StoreManagerInterface::class, [], [], '', false); $this->transportBuilderMock = $this->getMock( \Magento\Framework\Mail\Template\TransportBuilder::class, @@ -108,7 +118,7 @@ protected function setUp() ->getMockForAbstractClass(); $context->expects($this->any()) ->method('getRequest') - ->willReturn($this->requestMock); + ->willReturn($this->requestStub); $context->expects($this->any()) ->method('getResponse') @@ -122,13 +132,9 @@ protected function setUp() ->method('getUrl') ->willReturn($this->urlMock); - $context->expects($this->any()) - ->method('getRedirect') - ->willReturn($this->redirectMock); - $context->expects($this->once()) - ->method('getView') - ->willReturn($this->viewMock); + ->method('getResultRedirectFactory') + ->willReturn($this->redirectResultFactoryMock); $objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); @@ -151,9 +157,9 @@ protected function setUp() public function testExecuteEmptyPost() { - $this->requestMock->expects($this->once())->method('getPostValue')->willReturn([]); - $this->redirectMock->expects($this->once())->method('redirect'); - $this->controller->execute(); +// $this->requestMock->expects($this->once())->method('getPostValue')->willReturn([]); + $this->stubRequestPostData([]); + $this->assertSame($this->redirectResultMock, $this->controller->execute()); } /** @@ -161,13 +167,11 @@ public function testExecuteEmptyPost() */ public function testExecutePostValidation($postData, $exceptionExpected) { - $this->requestMock->expects($this->any()) - ->method('getPostValue') - ->willReturn($postData); + $this->stubRequestPostData($postData); if ($exceptionExpected) { $this->messageManagerMock->expects($this->once()) - ->method('addError'); + ->method('addErrorMessage'); $this->dataPersistorMock->expects($this->once()) ->method('set') ->with('contact_us', $postData); @@ -201,9 +205,7 @@ public function testExecuteValidPost() ->method('clear') ->with('contact_us'); - $this->requestMock->expects($this->any()) - ->method('getPostValue') - ->willReturn($post); + $this->stubRequestPostData($post); $transport = $this->getMock(\Magento\Framework\Mail\TransportInterface::class, [], [], '', false); @@ -251,4 +253,17 @@ public function testExecuteValidPost() $this->controller->execute(); } + + /** + * @param array $post + */ + private function stubRequestPostData($post) + { + $this->requestStub->method('getPostValue')->willReturn($post); + $this->requestStub->method('getParams')->willReturn($post); + $this->requestStub->method('getParam')->willReturnCallback( + function ($key) use ($post) { + return $post[$key]; + }); + } } From 0c7885778ff967627c2540d0b5bb7671139cec6e Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 23:36:03 +0000 Subject: [PATCH 15/32] Fix code style violations --- .../Test/Unit/Controller/Index/PostTest.php | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index 834b8fcd8ec4c..35f7ad9ea9ef9 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -5,6 +5,7 @@ * See COPYING.txt for license details. */ namespace Magento\Contact\Test\Unit\Controller\Index; + use Magento\Framework\Controller\Result\Redirect; /** @@ -85,8 +86,13 @@ protected function setUp() $this->urlMock = $this->getMock(\Magento\Framework\UrlInterface::class, [], [], '', false); $this->messageManagerMock = $this->getMock(\Magento\Framework\Message\ManagerInterface::class, [], [], '', false); - $this->requestStub = - $this->getMock(\Magento\Framework\App\Request\Http::class, ['getPostValue', 'getParams', 'getParam'], [], '', false); + $this->requestStub = $this->getMock( + \Magento\Framework\App\Request\Http::class, + ['getPostValue', 'getParams', 'getParam'], + [], + '', + false + ); $this->redirectResultMock = $this->getMock( \Magento\Framework\Controller\Result\Redirect::class, [], @@ -95,7 +101,13 @@ protected function setUp() false ); $this->redirectResultMock->method('setPath')->willReturnSelf(); - $this->redirectResultFactoryMock = $this->getMock(\Magento\Framework\Controller\Result\RedirectFactory::class, ['create'], [], '', false); + $this->redirectResultFactoryMock = $this->getMock( + \Magento\Framework\Controller\Result\RedirectFactory::class, + ['create'], + [], + '', + false + ); $this->redirectResultFactoryMock ->method('create') ->willReturn($this->redirectResultMock); @@ -157,7 +169,6 @@ protected function setUp() public function testExecuteEmptyPost() { -// $this->requestMock->expects($this->once())->method('getPostValue')->willReturn([]); $this->stubRequestPostData([]); $this->assertSame($this->redirectResultMock, $this->controller->execute()); } @@ -264,6 +275,7 @@ private function stubRequestPostData($post) $this->requestStub->method('getParam')->willReturnCallback( function ($key) use ($post) { return $post[$key]; - }); + } + ); } } From b4f68b0c35049901f797896f6359cb7b1c26f7bc Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 20:34:41 +0000 Subject: [PATCH 16/32] Revert "Make validation and sending methods in contacts form controller protected" This reverts commit c8fbb7072715c3d59e62ad12e37406518f2c0b14. --- app/code/Magento/Contact/Controller/Index/Post.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 11d92375bbcb6..41c0dc37a4ff6 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -70,7 +70,7 @@ private function getDataPersistor() * @param array $post Post data from contact form * @return void */ - protected function sendEmail($post) + private function sendEmail($post) { $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; $transport = $this->_transportBuilder @@ -104,7 +104,7 @@ private function isPostRequest() * @return array * @throws \Exception */ - protected function validatedParams() + private function validatedParams() { $request = $this->getRequest(); if (trim($request->getParam('name')) === '') { From 5854390e3f4b490a3b61033cd72d4162ae64b225 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 20:39:46 +0000 Subject: [PATCH 17/32] Remove unnecessary dependency while constants should be preferred over magic strings, the dependency to FrontNameResolver seems arbitrary, just because it happens to have a constant for 'adminhtml' defined. --- app/code/Magento/Contact/Controller/Index/Post.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 41c0dc37a4ff6..8fe78a976f2a8 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -77,7 +77,7 @@ private function sendEmail($post) ->setTemplateIdentifier($this->scopeConfig->getValue(self::XML_PATH_EMAIL_TEMPLATE, $storeScope)) ->setTemplateOptions( [ - 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, + 'area' => 'adminhtml', 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, ] ) From 854f16d7c60140d2dcdbd7a43050d816e4f440c2 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 21:14:24 +0000 Subject: [PATCH 18/32] Moved down unnecessary dependencies from abstract base controller in contacts module Additionally, the tests have been refactored for current coding standards and PHPUnit forward compatibility --- app/code/Magento/Contact/Controller/Index.php | 28 +------- .../Magento/Contact/Controller/Index/Post.php | 36 +++++++++- .../Test/Unit/Controller/Index/IndexTest.php | 67 +++++++++---------- .../Test/Unit/Controller/IndexTest.php | 50 +++++++------- 4 files changed, 92 insertions(+), 89 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index.php b/app/code/Magento/Contact/Controller/Index.php index a646b69dba17f..103e472b73c4e 100644 --- a/app/code/Magento/Contact/Controller/Index.php +++ b/app/code/Magento/Contact/Controller/Index.php @@ -10,7 +10,7 @@ use Magento\Store\Model\ScopeInterface; /** - * Contact index controller + * Contact module base controller */ abstract class Index extends \Magento\Framework\App\Action\Action { @@ -34,45 +34,21 @@ abstract class Index extends \Magento\Framework\App\Action\Action */ const XML_PATH_ENABLED = 'contact/contact/enabled'; - /** - * @var \Magento\Framework\Mail\Template\TransportBuilder - */ - protected $_transportBuilder; - - /** - * @var \Magento\Framework\Translate\Inline\StateInterface - */ - protected $inlineTranslation; - /** * @var \Magento\Framework\App\Config\ScopeConfigInterface */ protected $scopeConfig; - /** - * @var \Magento\Store\Model\StoreManagerInterface - */ - protected $storeManager; - /** * @param \Magento\Framework\App\Action\Context $context - * @param \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder - * @param \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig - * @param \Magento\Store\Model\StoreManagerInterface $storeManager */ public function __construct( \Magento\Framework\App\Action\Context $context, - \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder, - \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation, - \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, - \Magento\Store\Model\StoreManagerInterface $storeManager + \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig ) { parent::__construct($context); - $this->_transportBuilder = $transportBuilder; - $this->inlineTranslation = $inlineTranslation; $this->scopeConfig = $scopeConfig; - $this->storeManager = $storeManager; } /** diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 8fe78a976f2a8..b4521928d6cba 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -6,11 +6,15 @@ */ namespace Magento\Contact\Controller\Index; +use Magento\Framework\App\Action\Context; +use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\App\Request\DataPersistorInterface; use Magento\Framework\App\ObjectManager; use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\HTTP\PhpEnvironment\Request; +use Magento\Framework\Mail\Template\TransportBuilder; +use Magento\Framework\Translate\Inline\StateInterface; class Post extends \Magento\Contact\Controller\Index { @@ -19,6 +23,36 @@ class Post extends \Magento\Contact\Controller\Index */ private $dataPersistor; + /** + * @var \Magento\Framework\Mail\Template\TransportBuilder + */ + private $transportBuilder; + + /** + * @var \Magento\Framework\Translate\Inline\StateInterface + */ + private $inlineTranslation; + /** + * @var Context + */ + private $context; + + public function __construct( + Context $context, + ScopeConfigInterface $scopeConfig, + TransportBuilder $transportBuilder, + StateInterface $inlineTranslation, + DataPersistorInterface $dataPersistor + ) { + parent::__construct($context, $scopeConfig); + $this->context = $context; + $this->scopeConfig = $scopeConfig; + $this->transportBuilder = $transportBuilder; + $this->inlineTranslation = $inlineTranslation; + $this->dataPersistor = $dataPersistor; + } + + /** * Post user question * @@ -73,7 +107,7 @@ private function getDataPersistor() private function sendEmail($post) { $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; - $transport = $this->_transportBuilder + $transport = $this->transportBuilder ->setTemplateIdentifier($this->scopeConfig->getValue(self::XML_PATH_EMAIL_TEMPLATE, $storeScope)) ->setTemplateOptions( [ diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php index 5b4071eb4106e..4a5a3db72bf1f 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php @@ -13,89 +13,82 @@ class IndexTest extends \PHPUnit_Framework_TestCase * * @var \Magento\Contact\Controller\Index\Index */ - protected $_controller; + private $controller; /** * Scope config mock * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_scopeConfig; + private $scopeConfig; /** * View mock * @var \Magento\Framework\App\ViewInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_view; + private $view; /** * Url mock * @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_url; + private $url; protected function setUp() { - $this->_scopeConfig = $this->getMockForAbstractClass( - \Magento\Framework\App\Config\ScopeConfigInterface::class, - ['isSetFlag'], - '', - false - ); - $context = $this->getMock( - \Magento\Framework\App\Action\Context::class, - ['getRequest', 'getResponse', 'getView', 'getUrl'], - [], - '', - false - ); + $this->scopeConfig = $this->getMockBuilder( + \Magento\Framework\App\Config\ScopeConfigInterface::class + )->setMethods( + ['isSetFlag'] + )->getMockForAbstractClass(); - $this->_url = $this->getMockForAbstractClass(\Magento\Framework\UrlInterface::class, [], '', false); + $context = $this->getMockBuilder( + \Magento\Framework\App\Action\Context::class + )->setMethods( + ['getRequest', 'getResponse', 'getView', 'getUrl'] + )->disableOriginalConstructor( + )->getMock(); + + $this->url = $this->getMockBuilder(\Magento\Framework\UrlInterface::class)->getMockForAbstractClass(); $context->expects($this->any()) ->method('getUrl') - ->will($this->returnValue($this->_url)); + ->will($this->returnValue($this->url)); $context->expects($this->any()) ->method('getRequest') ->will($this->returnValue( - $this->getMockForAbstractClass(\Magento\Framework\App\RequestInterface::class, [], '', false) + $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class)->getMockForAbstractClass() )); $context->expects($this->any()) ->method('getResponse') ->will($this->returnValue( - $this->getMockForAbstractClass(\Magento\Framework\App\ResponseInterface::class, [], '', false) + $this->getMockBuilder(\Magento\Framework\App\ResponseInterface::class)->getMockForAbstractClass() )); - $this->_view = $this->getMock( - \Magento\Framework\App\ViewInterface::class, - [], - [], - '', - false - ); + $this->view = $this->getMockBuilder( + \Magento\Framework\App\ViewInterface::class + )->disableOriginalConstructor( + )->getMock(); $context->expects($this->once()) ->method('getView') - ->will($this->returnValue($this->_view)); + ->will($this->returnValue($this->view)); - $this->_controller = new \Magento\Contact\Controller\Index\Index( + $this->controller = new \Magento\Contact\Controller\Index\Index( $context, - $this->getMock(\Magento\Framework\Mail\Template\TransportBuilder::class, [], [], '', false), - $this->getMockForAbstractClass(\Magento\Framework\Translate\Inline\StateInterface::class, [], '', false), - $this->_scopeConfig, - $this->getMockForAbstractClass(\Magento\Store\Model\StoreManagerInterface::class, [], '', false) + $this->scopeConfig ); } public function testExecute() { - $this->_view->expects($this->once()) + $this->view->expects($this->once()) ->method('loadLayout'); - $this->_view->expects($this->once()) + $this->view->expects($this->once()) ->method('renderLayout'); - $this->_controller->execute(); + $this->controller->execute(); } } diff --git a/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php b/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php index 723a054e2d252..4a1e060c8c146 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php @@ -6,6 +6,10 @@ namespace Magento\Contact\Test\Unit\Controller; +use Magento\Framework\App\Config\ScopeConfigInterface; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\ResponseInterface; + class IndexTest extends \PHPUnit_Framework_TestCase { /** @@ -13,36 +17,35 @@ class IndexTest extends \PHPUnit_Framework_TestCase * * @var \Magento\Contact\Controller\Index */ - protected $_controller; + private $controller; /** * Scope config instance * * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_scopeConfig; + private $scopeConfig; protected function setUp() { - $this->_scopeConfig = $this->getMockForAbstractClass( - \Magento\Framework\App\Config\ScopeConfigInterface::class, - ['isSetFlag'], - '', - false - ); - $context = $this->getMock( - \Magento\Framework\App\Action\Context::class, - ['getRequest', 'getResponse'], - [], - '', - false - ); + $this->scopeConfig = $this->getMockBuilder( + ScopeConfigInterface::class + )->setMethods( + ['isSetFlag'] + )->getMockForAbstractClass(); + + $context = $this->getMockBuilder( + \Magento\Framework\App\Action\Context::class + )->setMethods( + ['getRequest', 'getResponse'] + )->disableOriginalConstructor( + )->getMock(); $context->expects($this->any()) ->method('getRequest') ->will( $this->returnValue( - $this->getMockForAbstractClass(\Magento\Framework\App\RequestInterface::class, [], '', false) + $this->getMockBuilder(RequestInterface::class)->getMockForAbstractClass() ) ); @@ -50,16 +53,13 @@ protected function setUp() ->method('getResponse') ->will( $this->returnValue( - $this->getMockForAbstractClass(\Magento\Framework\App\ResponseInterface::class, [], '', false) + $this->getMockBuilder(ResponseInterface::class)->getMockForAbstractClass() ) ); - $this->_controller = new \Magento\Contact\Test\Unit\Controller\Stub\IndexStub( + $this->controller = new \Magento\Contact\Test\Unit\Controller\Stub\IndexStub( $context, - $this->getMock(\Magento\Framework\Mail\Template\TransportBuilder::class, [], [], '', false), - $this->getMockForAbstractClass(\Magento\Framework\Translate\Inline\StateInterface::class, [], '', false), - $this->_scopeConfig, - $this->getMockForAbstractClass(\Magento\Store\Model\StoreManagerInterface::class, [], '', false) + $this->scopeConfig ); } @@ -70,7 +70,7 @@ protected function setUp() */ public function testDispatch() { - $this->_scopeConfig->expects($this->once()) + $this->scopeConfig->expects($this->once()) ->method('isSetFlag') ->with( \Magento\Contact\Controller\Index::XML_PATH_ENABLED, @@ -78,8 +78,8 @@ public function testDispatch() ) ->will($this->returnValue(false)); - $this->_controller->dispatch( - $this->getMockForAbstractClass(\Magento\Framework\App\RequestInterface::class, [], '', false) + $this->controller->dispatch( + $this->getMockBuilder(RequestInterface::class)->getMockForAbstractClass() ); } } From 22004d7e734df4ac39ac2308139eafbc6838b9a6 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 21:45:32 +0000 Subject: [PATCH 19/32] Move configuration access to config model / API interface original constants and methods are still in place for backwards compatibility --- .../Magento/Contact/Api/ConfigInterface.php | 59 +++++++++++++++ app/code/Magento/Contact/Controller/Index.php | 27 +++---- .../Magento/Contact/Controller/Index/Post.php | 20 +++--- app/code/Magento/Contact/Helper/Data.php | 4 +- app/code/Magento/Contact/Model/Config.php | 71 +++++++++++++++++++ .../Test/Unit/Controller/Index/IndexTest.php | 16 ++--- .../Test/Unit/Controller/Index/PostTest.php | 50 +++++-------- .../Test/Unit/Controller/IndexTest.php | 24 ++----- app/code/Magento/Contact/etc/di.xml | 1 + .../Magento/Contact/Model/ConfigTest.php | 43 +++++++++++ 10 files changed, 235 insertions(+), 80 deletions(-) create mode 100644 app/code/Magento/Contact/Api/ConfigInterface.php create mode 100644 app/code/Magento/Contact/Model/Config.php create mode 100644 dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php diff --git a/app/code/Magento/Contact/Api/ConfigInterface.php b/app/code/Magento/Contact/Api/ConfigInterface.php new file mode 100644 index 0000000000000..6a67eafda3b7b --- /dev/null +++ b/app/code/Magento/Contact/Api/ConfigInterface.php @@ -0,0 +1,59 @@ +scopeConfig = $scopeConfig; + $this->contactsConfig = $contactsConfig; } /** @@ -60,7 +61,7 @@ public function __construct( */ public function dispatch(RequestInterface $request) { - if (!$this->scopeConfig->isSetFlag(self::XML_PATH_ENABLED, ScopeInterface::SCOPE_STORE)) { + if (!$this->contactsConfig->isEnabled()) { throw new NotFoundException(__('Page not found.')); } return parent::dispatch($request); diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index b4521928d6cba..fe3224a9118cf 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -6,10 +6,10 @@ */ namespace Magento\Contact\Controller\Index; +use Magento\Contact\Api\ConfigInterface; use Magento\Framework\App\Action\Context; -use Magento\Framework\App\Config\ScopeConfigInterface; -use Magento\Framework\App\Request\DataPersistorInterface; use Magento\Framework\App\ObjectManager; +use Magento\Framework\App\Request\DataPersistorInterface; use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\HTTP\PhpEnvironment\Request; @@ -36,20 +36,24 @@ class Post extends \Magento\Contact\Controller\Index * @var Context */ private $context; + /** + * @var ConfigInterface + */ + private $contactsConfig; public function __construct( Context $context, - ScopeConfigInterface $scopeConfig, + ConfigInterface $contactsConfig, TransportBuilder $transportBuilder, StateInterface $inlineTranslation, DataPersistorInterface $dataPersistor ) { - parent::__construct($context, $scopeConfig); + parent::__construct($context, $contactsConfig); $this->context = $context; - $this->scopeConfig = $scopeConfig; $this->transportBuilder = $transportBuilder; $this->inlineTranslation = $inlineTranslation; $this->dataPersistor = $dataPersistor; + $this->contactsConfig = $contactsConfig; } @@ -108,7 +112,7 @@ private function sendEmail($post) { $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; $transport = $this->transportBuilder - ->setTemplateIdentifier($this->scopeConfig->getValue(self::XML_PATH_EMAIL_TEMPLATE, $storeScope)) + ->setTemplateIdentifier($this->contactsConfig->emailTemplate()) ->setTemplateOptions( [ 'area' => 'adminhtml', @@ -116,8 +120,8 @@ private function sendEmail($post) ] ) ->setTemplateVars(['data' => new \Magento\Framework\DataObject($post)]) - ->setFrom($this->scopeConfig->getValue(self::XML_PATH_EMAIL_SENDER, $storeScope)) - ->addTo($this->scopeConfig->getValue(self::XML_PATH_EMAIL_RECIPIENT, $storeScope)) + ->setFrom($this->contactsConfig->emailSender()) + ->addTo($this->contactsConfig->emailRecipient()) ->setReplyTo($post['email']) ->getTransport(); diff --git a/app/code/Magento/Contact/Helper/Data.php b/app/code/Magento/Contact/Helper/Data.php index f6ad3d9d5a871..7b207f8feb8db 100644 --- a/app/code/Magento/Contact/Helper/Data.php +++ b/app/code/Magento/Contact/Helper/Data.php @@ -6,6 +6,7 @@ namespace Magento\Contact\Helper; +use Magento\Contact\Api\ConfigInterface; use Magento\Customer\Api\Data\CustomerInterface; use Magento\Customer\Helper\View as CustomerViewHelper; use Magento\Framework\App\Request\DataPersistorInterface; @@ -16,7 +17,7 @@ */ class Data extends \Magento\Framework\App\Helper\AbstractHelper { - const XML_PATH_ENABLED = 'contact/contact/enabled'; + const XML_PATH_ENABLED = ConfigInterface::XML_PATH_ENABLED; /** * Customer session @@ -59,6 +60,7 @@ public function __construct( * Check if enabled * * @return string|null + * @deprecated use \Magento\Contact\Api\ConfigInterface::isEnabled() instead */ public function isEnabled() { diff --git a/app/code/Magento/Contact/Model/Config.php b/app/code/Magento/Contact/Model/Config.php new file mode 100644 index 0000000000000..4d213db5a459e --- /dev/null +++ b/app/code/Magento/Contact/Model/Config.php @@ -0,0 +1,71 @@ +scopeConfig = $scopeConfig; + } + + /** + * @return bool + */ + public function isEnabled() + { + return $this->scopeConfig->isSetFlag( + self::XML_PATH_ENABLED, + ScopeInterface::SCOPE_STORE + ); + } + + /** + * @return string + */ + public function emailTemplate() + { + return $this->scopeConfig->getValue( + ConfigInterface::XML_PATH_EMAIL_TEMPLATE, + ScopeInterface::SCOPE_STORE + ); + } + + /** + * @return string + */ + public function emailSender() + { + return $this->scopeConfig->getValue( + ConfigInterface::XML_PATH_EMAIL_SENDER, + ScopeInterface::SCOPE_STORE + ); + } + + /** + * @return string + */ + public function emailRecipient() + { + return $this->scopeConfig->getValue( + ConfigInterface::XML_PATH_EMAIL_RECIPIENT, + ScopeInterface::SCOPE_STORE + ); + } + +} \ No newline at end of file diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php index 4a5a3db72bf1f..fd57f92698cc7 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php @@ -6,6 +6,9 @@ namespace Magento\Contact\Test\Unit\Controller\Index; +use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\Config; + class IndexTest extends \PHPUnit_Framework_TestCase { /** @@ -16,10 +19,9 @@ class IndexTest extends \PHPUnit_Framework_TestCase private $controller; /** - * Scope config mock - * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $scopeConfig; + private $configMock; /** * View mock @@ -35,11 +37,7 @@ class IndexTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->scopeConfig = $this->getMockBuilder( - \Magento\Framework\App\Config\ScopeConfigInterface::class - )->setMethods( - ['isSetFlag'] - )->getMockForAbstractClass(); + $this->configMock = $this->getMockBuilder(ConfigInterface::class)->getMockForAbstractClass(); $context = $this->getMockBuilder( \Magento\Framework\App\Action\Context::class @@ -77,7 +75,7 @@ protected function setUp() $this->controller = new \Magento\Contact\Controller\Index\Index( $context, - $this->scopeConfig + $this->configMock ); } diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index 35f7ad9ea9ef9..8dda3c5c21c95 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -6,6 +6,7 @@ */ namespace Magento\Contact\Test\Unit\Controller\Index; +use Magento\Contact\Api\ConfigInterface; use Magento\Framework\Controller\Result\Redirect; /** @@ -16,17 +17,17 @@ class PostTest extends \PHPUnit_Framework_TestCase /** * @var \Magento\Contact\Controller\Index\Index */ - protected $controller; + private $controller; /** - * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $scopeConfigMock; + private $configMock; /** * @var \Magento\Framework\Controller\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $redirectResultFactoryMock; + private $redirectResultFactoryMock; /** * @var Redirect|\PHPUnit_Framework_MockObject_MockObject @@ -36,46 +37,41 @@ class PostTest extends \PHPUnit_Framework_TestCase /** * @var \Magento\Framework\UrlInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $urlMock; + private $urlMock; /** * @var \Magento\Framework\App\Request\HttpRequest|\PHPUnit_Framework_MockObject_MockObject */ - protected $requestStub; + private $requestStub; /** * @var \Magento\Framework\Mail\Template\TransportBuilder|\PHPUnit_Framework_MockObject_MockObject */ - protected $transportBuilderMock; + private $transportBuilderMock; /** * @var \Magento\Framework\Translate\Inline\StateInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $inlineTranslationMock; + private $inlineTranslationMock; /** * @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $messageManagerMock; + private $messageManagerMock; /** * @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $storeManagerMock; + private $storeManagerMock; /** * @var \Magento\Framework\App\Request\DataPersistorInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $dataPersistorMock; + private $dataPersistorMock; protected function setUp() { - $this->scopeConfigMock = $this->getMockForAbstractClass( - \Magento\Framework\App\Config\ScopeConfigInterface::class, - ['isSetFlag'], - '', - false - ); + $this->configMock = $this->getMockBuilder(ConfigInterface::class)->getMockForAbstractClass(); $context = $this->getMock( \Magento\Framework\App\Action\Context::class, ['getRequest', 'getResponse', 'getResultRedirectFactory', 'getUrl', 'getRedirect', 'getMessageManager'], @@ -148,21 +144,11 @@ protected function setUp() ->method('getResultRedirectFactory') ->willReturn($this->redirectResultFactoryMock); - $objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); - - $this->controller = $objectManagerHelper->getObject( - \Magento\Contact\Controller\Index\Post::class, - [ - 'context' => $context, - 'transportBuilder' => $this->transportBuilderMock, - 'inlineTranslation' => $this->inlineTranslationMock, - 'scopeConfig' => $this->scopeConfigMock, - 'storeManager' => $this->storeManagerMock - ] - ); - $objectManagerHelper->setBackwardCompatibleProperty( - $this->controller, - 'dataPersistor', + $this->controller = new \Magento\Contact\Controller\Index\Post( + $context, + $this->configMock, + $this->transportBuilderMock, + $this->inlineTranslationMock, $this->dataPersistorMock ); } diff --git a/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php b/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php index 4a1e060c8c146..a2dc9edb3d4c2 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php @@ -6,7 +6,7 @@ namespace Magento\Contact\Test\Unit\Controller; -use Magento\Framework\App\Config\ScopeConfigInterface; +use Magento\Contact\Api\ConfigInterface; use Magento\Framework\App\RequestInterface; use Magento\Framework\App\ResponseInterface; @@ -20,19 +20,15 @@ class IndexTest extends \PHPUnit_Framework_TestCase private $controller; /** - * Scope config instance + * Module config instance * - * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - private $scopeConfig; + private $configMock; protected function setUp() { - $this->scopeConfig = $this->getMockBuilder( - ScopeConfigInterface::class - )->setMethods( - ['isSetFlag'] - )->getMockForAbstractClass(); + $this->configMock = $this->getMockBuilder(ConfigInterface::class)->getMockForAbstractClass(); $context = $this->getMockBuilder( \Magento\Framework\App\Action\Context::class @@ -59,7 +55,7 @@ protected function setUp() $this->controller = new \Magento\Contact\Test\Unit\Controller\Stub\IndexStub( $context, - $this->scopeConfig + $this->configMock ); } @@ -70,13 +66,7 @@ protected function setUp() */ public function testDispatch() { - $this->scopeConfig->expects($this->once()) - ->method('isSetFlag') - ->with( - \Magento\Contact\Controller\Index::XML_PATH_ENABLED, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) - ->will($this->returnValue(false)); + $this->configMock->method('isEnabled')->willReturn(false); $this->controller->dispatch( $this->getMockBuilder(RequestInterface::class)->getMockForAbstractClass() diff --git a/app/code/Magento/Contact/etc/di.xml b/app/code/Magento/Contact/etc/di.xml index 0800e42b0ec0c..870bbad8002e9 100644 --- a/app/code/Magento/Contact/etc/di.xml +++ b/app/code/Magento/Contact/etc/di.xml @@ -6,6 +6,7 @@ */ --> + diff --git a/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php b/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php new file mode 100644 index 0000000000000..63716446c0e4b --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php @@ -0,0 +1,43 @@ +configModel = Bootstrap::getObjectManager()->create(\Magento\Contact\Api\ConfigInterface::class); + } + + /** + * @magentoAppArea frontend + * @magentoAppIsolation enabled + * @magentoConfigFixture current_store contact/contact/enabled 1 + */ + public function testIsEnabled() + { + $this->assertTrue($this->configModel->isEnabled()); + } + + /** + * @magentoAppArea frontend + * @magentoAppIsolation enabled + * @magentoConfigFixture current_store contact/contact/enabled 0 + */ + public function testIsNotEnabled() + { + $this->assertFalse($this->configModel->isEnabled()); + } +} From 28d939fa7106e8312d17bc14f51d57a62afbb5e4 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 23:17:37 +0000 Subject: [PATCH 20/32] Extract mail sending from contacts controller, introduce mail service contract --- .../Magento/Contact/Api/MailInterface.php | 15 +++ .../Magento/Contact/Controller/Index/Post.php | 43 +------ app/code/Magento/Contact/Model/Mail.php | 62 ++++++++++ .../Test/Unit/Controller/Index/PostTest.php | 82 ++----------- .../Contact/Test/Unit/Model/MailTest.php | 113 ++++++++++++++++++ 5 files changed, 203 insertions(+), 112 deletions(-) create mode 100644 app/code/Magento/Contact/Api/MailInterface.php create mode 100644 app/code/Magento/Contact/Model/Mail.php create mode 100644 app/code/Magento/Contact/Test/Unit/Model/MailTest.php diff --git a/app/code/Magento/Contact/Api/MailInterface.php b/app/code/Magento/Contact/Api/MailInterface.php new file mode 100644 index 0000000000000..e39bacdcdea03 --- /dev/null +++ b/app/code/Magento/Contact/Api/MailInterface.php @@ -0,0 +1,15 @@ +context = $context; - $this->transportBuilder = $transportBuilder; - $this->inlineTranslation = $inlineTranslation; + $this->mail = $mail; $this->dataPersistor = $dataPersistor; - $this->contactsConfig = $contactsConfig; } @@ -67,8 +55,6 @@ public function execute() if (! $this->isPostRequest()) { return $this->resultRedirectFactory->create()->setPath('*/*/'); } - - $this->inlineTranslation->suspend(); try { $this->sendEmail($this->validatedParams()); $this->messageManager->addSuccess( @@ -83,8 +69,6 @@ public function execute() __('We can\'t process your request right now. Sorry, that\'s all we know.') ); $this->getDataPersistor()->set('contact_us', $this->getRequest()->getParams()); - } finally { - $this->inlineTranslation->resume(); } return $this->resultRedirectFactory->create()->setPath('contact/index'); } @@ -110,22 +94,7 @@ private function getDataPersistor() */ private function sendEmail($post) { - $storeScope = \Magento\Store\Model\ScopeInterface::SCOPE_STORE; - $transport = $this->transportBuilder - ->setTemplateIdentifier($this->contactsConfig->emailTemplate()) - ->setTemplateOptions( - [ - 'area' => 'adminhtml', - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, - ] - ) - ->setTemplateVars(['data' => new \Magento\Framework\DataObject($post)]) - ->setFrom($this->contactsConfig->emailSender()) - ->addTo($this->contactsConfig->emailRecipient()) - ->setReplyTo($post['email']) - ->getTransport(); - - $transport->sendMessage(); + $this->mail->send($post['email'], ['data' => new \Magento\Framework\DataObject($post)]); } /** diff --git a/app/code/Magento/Contact/Model/Mail.php b/app/code/Magento/Contact/Model/Mail.php new file mode 100644 index 0000000000000..ec5afa9c4f6da --- /dev/null +++ b/app/code/Magento/Contact/Model/Mail.php @@ -0,0 +1,62 @@ +contactsConfig = $contactsConfig; + $this->transportBuilder = $transportBuilder; + $this->inlineTranslation = $inlineTranslation; + } + + public function send($replyTo, $variables) + { + $this->inlineTranslation->suspend(); + try { + $transport = $this->transportBuilder + ->setTemplateIdentifier($this->contactsConfig->emailTemplate()) + ->setTemplateOptions( + [ + 'area' => 'adminhtml', + 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, + ] + ) + ->setTemplateVars($variables) + ->setFrom($this->contactsConfig->emailSender()) + ->addTo($this->contactsConfig->emailRecipient()) + ->setReplyTo($replyTo) + ->getTransport(); + + $transport->sendMessage(); + } finally { + $this->inlineTranslation->resume(); + } + } + +} \ No newline at end of file diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index 8dda3c5c21c95..6f345843fe077 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -7,6 +7,7 @@ namespace Magento\Contact\Test\Unit\Controller\Index; use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Api\MailInterface; use Magento\Framework\Controller\Result\Redirect; /** @@ -44,16 +45,6 @@ class PostTest extends \PHPUnit_Framework_TestCase */ private $requestStub; - /** - * @var \Magento\Framework\Mail\Template\TransportBuilder|\PHPUnit_Framework_MockObject_MockObject - */ - private $transportBuilderMock; - - /** - * @var \Magento\Framework\Translate\Inline\StateInterface|\PHPUnit_Framework_MockObject_MockObject - */ - private $inlineTranslationMock; - /** * @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ @@ -68,9 +59,14 @@ class PostTest extends \PHPUnit_Framework_TestCase * @var \Magento\Framework\App\Request\DataPersistorInterface|\PHPUnit_Framework_MockObject_MockObject */ private $dataPersistorMock; + /** + * @var MailInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $mailMock; protected function setUp() { + $this->mailMock = $this->getMockBuilder(MailInterface::class)->getMockForAbstractClass(); $this->configMock = $this->getMockBuilder(ConfigInterface::class)->getMockForAbstractClass(); $context = $this->getMock( \Magento\Framework\App\Action\Context::class, @@ -108,20 +104,6 @@ protected function setUp() ->method('create') ->willReturn($this->redirectResultMock); $this->storeManagerMock = $this->getMock(\Magento\Store\Model\StoreManagerInterface::class, [], [], '', false); - $this->transportBuilderMock = $this->getMock( - \Magento\Framework\Mail\Template\TransportBuilder::class, - [], - [], - '', - false - ); - $this->inlineTranslationMock = $this->getMock( - \Magento\Framework\Translate\Inline\StateInterface::class, - [], - [], - '', - false - ); $this->dataPersistorMock = $this->getMockBuilder(\Magento\Framework\App\Request\DataPersistorInterface::class) ->getMockForAbstractClass(); $context->expects($this->any()) @@ -146,9 +128,8 @@ protected function setUp() $this->controller = new \Magento\Contact\Controller\Index\Post( $context, + $this->mailMock, $this->configMock, - $this->transportBuilderMock, - $this->inlineTranslationMock, $this->dataPersistorMock ); } @@ -173,11 +154,6 @@ public function testExecutePostValidation($postData, $exceptionExpected) ->method('set') ->with('contact_us', $postData); } - $this->inlineTranslationMock->expects($this->once()) - ->method('resume'); - - $this->inlineTranslationMock->expects($this->once()) - ->method('suspend'); $this->controller->execute(); } @@ -204,50 +180,6 @@ public function testExecuteValidPost() $this->stubRequestPostData($post); - $transport = $this->getMock(\Magento\Framework\Mail\TransportInterface::class, [], [], '', false); - - $this->transportBuilderMock->expects($this->once()) - ->method('setTemplateIdentifier') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setTemplateOptions') - ->with([ - 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, - ]) - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setTemplateVars') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setFrom') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('addTo') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setReplyTo') - ->with($post['email']) - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('getTransport') - ->willReturn($transport); - - $transport->expects($this->once()) - ->method('sendMessage'); - - $this->inlineTranslationMock->expects($this->once()) - ->method('resume'); - - $this->inlineTranslationMock->expects($this->once()) - ->method('suspend'); - $this->controller->execute(); } diff --git a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php new file mode 100644 index 0000000000000..d0061c9274b4a --- /dev/null +++ b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php @@ -0,0 +1,113 @@ +configMock = $this->getMockBuilder(ConfigInterface::class)->getMockForAbstractClass(); + $this->urlMock = $this->getMock(\Magento\Framework\UrlInterface::class, [], [], '', false); + $this->transportBuilderMock = $this->getMockBuilder( + \Magento\Framework\Mail\Template\TransportBuilder::class + )->disableOriginalConstructor( + )->getMock(); + $this->inlineTranslationMock = $this->getMockBuilder( + \Magento\Framework\Translate\Inline\StateInterface::class + )->disableOriginalConstructor( + )->getMock(); + + $this->mail = new Mail( + $this->configMock, + $this->transportBuilderMock, + $this->inlineTranslationMock + ); + } + + public function testSendMail() + { + $email = 'reply-to@example.com'; + $templateVars = ['comment' => 'Comment']; + + $transport = $this->getMock(\Magento\Framework\Mail\TransportInterface::class, [], [], '', false); + + $this->transportBuilderMock->expects($this->once()) + ->method('setTemplateIdentifier') + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setTemplateOptions') + ->with([ + 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, + 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, + ]) + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setTemplateVars') + ->with($templateVars) + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setFrom') + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('addTo') + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setReplyTo') + ->with($email) + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('getTransport') + ->willReturn($transport); + + $transport->expects($this->once()) + ->method('sendMessage'); + + $this->inlineTranslationMock->expects($this->once()) + ->method('resume'); + + $this->inlineTranslationMock->expects($this->once()) + ->method('suspend'); + + $this->mail->send($email, $templateVars); + } + +} From 7a01613012fe5feafc14d1b3e9650bcf2ad73d2f Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 23:31:26 +0000 Subject: [PATCH 21/32] Add docblock comments and empty lines --- app/code/Magento/Contact/Api/ConfigInterface.php | 2 +- app/code/Magento/Contact/Api/MailInterface.php | 4 +++- .../Magento/Contact/Controller/Index/Post.php | 9 ++++++++- app/code/Magento/Contact/Model/Config.php | 6 ++++-- app/code/Magento/Contact/Model/Mail.php | 16 ++++++++++++++-- .../Test/Unit/Controller/Index/PostTest.php | 1 + .../Magento/Contact/Test/Unit/Model/MailTest.php | 1 - 7 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/code/Magento/Contact/Api/ConfigInterface.php b/app/code/Magento/Contact/Api/ConfigInterface.php index 6a67eafda3b7b..53efcf43f322b 100644 --- a/app/code/Magento/Contact/Api/ConfigInterface.php +++ b/app/code/Magento/Contact/Api/ConfigInterface.php @@ -56,4 +56,4 @@ public function emailSender(); * @return mixed */ public function emailRecipient(); -} \ No newline at end of file +} diff --git a/app/code/Magento/Contact/Api/MailInterface.php b/app/code/Magento/Contact/Api/MailInterface.php index e39bacdcdea03..1dc408d27a237 100644 --- a/app/code/Magento/Contact/Api/MailInterface.php +++ b/app/code/Magento/Contact/Api/MailInterface.php @@ -7,9 +7,11 @@ interface MailInterface { /** + * Send email from contact form + * * @param string $replyTo Reply-to email address * @param array $variables Email template variables * @return void */ public function send($replyTo, $variables); -} \ No newline at end of file +} diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index c29f4b3af1049..4e6e7bd3cc7af 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -23,15 +23,23 @@ class Post extends \Magento\Contact\Controller\Index * @var DataPersistorInterface */ private $dataPersistor; + /** * @var Context */ private $context; + /** * @var MailInterface */ private $mail; + /** + * @param Context $context + * @param MailInterface $mail + * @param ConfigInterface $contactsConfig + * @param DataPersistorInterface $dataPersistor + */ public function __construct( Context $context, MailInterface $mail, @@ -44,7 +52,6 @@ public function __construct( $this->dataPersistor = $dataPersistor; } - /** * Post user question * diff --git a/app/code/Magento/Contact/Model/Config.php b/app/code/Magento/Contact/Model/Config.php index 4d213db5a459e..f4c92957b8403 100644 --- a/app/code/Magento/Contact/Model/Config.php +++ b/app/code/Magento/Contact/Model/Config.php @@ -19,6 +19,9 @@ class Config implements ConfigInterface */ private $scopeConfig; + /** + * @param ScopeConfigInterface $scopeConfig + */ public function __construct(ScopeConfigInterface $scopeConfig) { $this->scopeConfig = $scopeConfig; @@ -67,5 +70,4 @@ public function emailRecipient() ScopeInterface::SCOPE_STORE ); } - -} \ No newline at end of file +} diff --git a/app/code/Magento/Contact/Model/Mail.php b/app/code/Magento/Contact/Model/Mail.php index ec5afa9c4f6da..90e27eda12a57 100644 --- a/app/code/Magento/Contact/Model/Mail.php +++ b/app/code/Magento/Contact/Model/Mail.php @@ -16,15 +16,22 @@ class Mail implements MailInterface * @var ConfigInterface */ private $contactsConfig; + /** * @var TransportBuilder */ private $transportBuilder; + /** * @var StateInterface */ private $inlineTranslation; + /** + * @param ConfigInterface $contactsConfig + * @param TransportBuilder $transportBuilder + * @param StateInterface $inlineTranslation + */ public function __construct( ConfigInterface $contactsConfig, TransportBuilder $transportBuilder, @@ -35,6 +42,12 @@ public function __construct( $this->inlineTranslation = $inlineTranslation; } + /** + * Send email from contact form + * + * @param string $replyTo + * @param array $variables + */ public function send($replyTo, $variables) { $this->inlineTranslation->suspend(); @@ -58,5 +71,4 @@ public function send($replyTo, $variables) $this->inlineTranslation->resume(); } } - -} \ No newline at end of file +} diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index 6f345843fe077..e6977e7ba6c22 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -59,6 +59,7 @@ class PostTest extends \PHPUnit_Framework_TestCase * @var \Magento\Framework\App\Request\DataPersistorInterface|\PHPUnit_Framework_MockObject_MockObject */ private $dataPersistorMock; + /** * @var MailInterface|\PHPUnit_Framework_MockObject_MockObject */ diff --git a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php index d0061c9274b4a..59f4348837eec 100644 --- a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php +++ b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php @@ -109,5 +109,4 @@ public function testSendMail() $this->mail->send($email, $templateVars); } - } From 026108837ce0b924b72a15ba4a420795ae28642d Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Thu, 16 Feb 2017 23:40:20 +0000 Subject: [PATCH 22/32] Add missing preference --- app/code/Magento/Contact/etc/di.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Contact/etc/di.xml b/app/code/Magento/Contact/etc/di.xml index 870bbad8002e9..316003842d98d 100644 --- a/app/code/Magento/Contact/etc/di.xml +++ b/app/code/Magento/Contact/etc/di.xml @@ -6,6 +6,7 @@ */ --> + From 6edaaf3e329f2614d4f0b0c1ea885f644a21edbb Mon Sep 17 00:00:00 2001 From: koenner01 Date: Sat, 18 Feb 2017 11:27:33 +0100 Subject: [PATCH 23/32] Fixed crosssells count always null Fix for https://github.com/magento/magento2/issues/7353 --- .../Catalog/view/frontend/templates/product/list/items.phtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/list/items.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/list/items.phtml index db299487d17d8..84be9271980b4 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/list/items.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/list/items.phtml @@ -110,7 +110,7 @@ switch ($type = $block->getType()) { case 'crosssell': /** @var \Magento\Catalog\Block\Product\ProductList\Crosssell $block */ - if ($exist = $block->getItemCount()) { + if ($exist = count($block->getItems())) { $type = 'crosssell'; $class = $type; From a5c69d0aa1e07a6cd68827c2fdc626e4499fa141 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sun, 19 Feb 2017 12:47:12 +0000 Subject: [PATCH 24/32] Move contact form interfaces from Api to Model namespace --- app/code/Magento/Contact/Controller/Index.php | 2 +- app/code/Magento/Contact/Controller/Index/Post.php | 4 ++-- app/code/Magento/Contact/Helper/Data.php | 2 +- app/code/Magento/Contact/Model/Config.php | 2 +- .../Magento/Contact/{Api => Model}/ConfigInterface.php | 6 ++++-- app/code/Magento/Contact/Model/Mail.php | 4 ++-- app/code/Magento/Contact/{Api => Model}/MailInterface.php | 7 ++++++- .../Contact/Test/Unit/Controller/Index/IndexTest.php | 2 +- .../Contact/Test/Unit/Controller/Index/PostTest.php | 4 ++-- .../Magento/Contact/Test/Unit/Controller/IndexTest.php | 2 +- app/code/Magento/Contact/Test/Unit/Model/MailTest.php | 2 +- app/code/Magento/Contact/etc/di.xml | 4 ++-- .../testsuite/Magento/Contact/Model/ConfigTest.php | 4 ++-- 13 files changed, 26 insertions(+), 19 deletions(-) rename app/code/Magento/Contact/{Api => Model}/ConfigInterface.php (94%) rename app/code/Magento/Contact/{Api => Model}/MailInterface.php (79%) diff --git a/app/code/Magento/Contact/Controller/Index.php b/app/code/Magento/Contact/Controller/Index.php index 537f4a34ab9ed..3b170f7c4841c 100644 --- a/app/code/Magento/Contact/Controller/Index.php +++ b/app/code/Magento/Contact/Controller/Index.php @@ -5,7 +5,7 @@ */ namespace Magento\Contact\Controller; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\Framework\App\Action\Context; use Magento\Framework\App\RequestInterface; use Magento\Framework\Exception\NotFoundException; diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 4e6e7bd3cc7af..8f94dfbd98213 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -6,8 +6,8 @@ */ namespace Magento\Contact\Controller\Index; -use Magento\Contact\Api\ConfigInterface; -use Magento\Contact\Api\MailInterface; +use Magento\Contact\Model\ConfigInterface; +use Magento\Contact\Model\MailInterface; use Magento\Framework\App\Action\Context; use Magento\Framework\App\ObjectManager; use Magento\Framework\App\Request\DataPersistorInterface; diff --git a/app/code/Magento/Contact/Helper/Data.php b/app/code/Magento/Contact/Helper/Data.php index 7b207f8feb8db..ff738e7e27996 100644 --- a/app/code/Magento/Contact/Helper/Data.php +++ b/app/code/Magento/Contact/Helper/Data.php @@ -6,7 +6,7 @@ namespace Magento\Contact\Helper; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\Customer\Api\Data\CustomerInterface; use Magento\Customer\Helper\View as CustomerViewHelper; use Magento\Framework\App\Request\DataPersistorInterface; diff --git a/app/code/Magento/Contact/Model/Config.php b/app/code/Magento/Contact/Model/Config.php index f4c92957b8403..24e47af6f4dba 100644 --- a/app/code/Magento/Contact/Model/Config.php +++ b/app/code/Magento/Contact/Model/Config.php @@ -5,7 +5,7 @@ */ namespace Magento\Contact\Model; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Store\Model\ScopeInterface; diff --git a/app/code/Magento/Contact/Api/ConfigInterface.php b/app/code/Magento/Contact/Model/ConfigInterface.php similarity index 94% rename from app/code/Magento/Contact/Api/ConfigInterface.php rename to app/code/Magento/Contact/Model/ConfigInterface.php index 53efcf43f322b..ac702bc6d30d3 100644 --- a/app/code/Magento/Contact/Api/ConfigInterface.php +++ b/app/code/Magento/Contact/Model/ConfigInterface.php @@ -2,10 +2,12 @@ /** * Contact module base controller */ -namespace Magento\Contact\Api; +namespace Magento\Contact\Model; /** * Contact module configuration + * + * @api */ interface ConfigInterface { @@ -53,7 +55,7 @@ public function emailSender(); /** * Return email recipient address * - * @return mixed + * @return string */ public function emailRecipient(); } diff --git a/app/code/Magento/Contact/Model/Mail.php b/app/code/Magento/Contact/Model/Mail.php index 90e27eda12a57..9a1bf1534aed6 100644 --- a/app/code/Magento/Contact/Model/Mail.php +++ b/app/code/Magento/Contact/Model/Mail.php @@ -5,8 +5,8 @@ */ namespace Magento\Contact\Model; -use Magento\Contact\Api\ConfigInterface; -use Magento\Contact\Api\MailInterface; +use Magento\Contact\Model\ConfigInterface; +use Magento\Contact\Model\MailInterface; use Magento\Framework\Mail\Template\TransportBuilder; use Magento\Framework\Translate\Inline\StateInterface; diff --git a/app/code/Magento/Contact/Api/MailInterface.php b/app/code/Magento/Contact/Model/MailInterface.php similarity index 79% rename from app/code/Magento/Contact/Api/MailInterface.php rename to app/code/Magento/Contact/Model/MailInterface.php index 1dc408d27a237..79af47f8853a3 100644 --- a/app/code/Magento/Contact/Api/MailInterface.php +++ b/app/code/Magento/Contact/Model/MailInterface.php @@ -2,8 +2,13 @@ /** * Contact module base controller */ -namespace Magento\Contact\Api; +namespace Magento\Contact\Model; +/** + * Email from contact form + * + * @api + */ interface MailInterface { /** diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php index e3fd09328fd39..3588128a1ebf7 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/IndexTest.php @@ -6,7 +6,7 @@ namespace Magento\Contact\Test\Unit\Controller\Index; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\Contact\Model\Config; use Magento\Framework\Controller\ResultFactory; use Magento\Framework\Controller\ResultInterface; diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index e6977e7ba6c22..8fd54170819f5 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -6,8 +6,8 @@ */ namespace Magento\Contact\Test\Unit\Controller\Index; -use Magento\Contact\Api\ConfigInterface; -use Magento\Contact\Api\MailInterface; +use Magento\Contact\Model\ConfigInterface; +use Magento\Contact\Model\MailInterface; use Magento\Framework\Controller\Result\Redirect; /** diff --git a/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php b/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php index a2dc9edb3d4c2..317b9ab672b20 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/IndexTest.php @@ -6,7 +6,7 @@ namespace Magento\Contact\Test\Unit\Controller; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\Framework\App\RequestInterface; use Magento\Framework\App\ResponseInterface; diff --git a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php index 59f4348837eec..de115920062eb 100644 --- a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php +++ b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php @@ -6,7 +6,7 @@ */ namespace Magento\Contact\Test\Integration\Model; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\Contact\Model\Mail; class MailTest extends \PHPUnit_Framework_TestCase diff --git a/app/code/Magento/Contact/etc/di.xml b/app/code/Magento/Contact/etc/di.xml index 316003842d98d..c70bcfd3e2ae8 100644 --- a/app/code/Magento/Contact/etc/di.xml +++ b/app/code/Magento/Contact/etc/di.xml @@ -6,8 +6,8 @@ */ --> - - + + diff --git a/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php b/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php index 63716446c0e4b..deeb2957ec7e8 100644 --- a/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php +++ b/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php @@ -6,7 +6,7 @@ namespace Magento\Contact\Test\Integration\Model; -use Magento\Contact\Api\ConfigInterface; +use Magento\Contact\Model\ConfigInterface; use Magento\TestFramework\Helper\Bootstrap; class ConfigTest extends \PHPUnit_Framework_TestCase @@ -18,7 +18,7 @@ class ConfigTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->configModel = Bootstrap::getObjectManager()->create(\Magento\Contact\Api\ConfigInterface::class); + $this->configModel = Bootstrap::getObjectManager()->create(\Magento\Contact\Model\ConfigInterface::class); } /** From f5ac9f761ec81879d35a2295af12cb6f162b2499 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sun, 19 Feb 2017 19:51:43 +0000 Subject: [PATCH 25/32] Make method signature of Mail::send() more precise --- app/code/Magento/Contact/Model/Mail.php | 3 ++- app/code/Magento/Contact/Model/MailInterface.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Contact/Model/Mail.php b/app/code/Magento/Contact/Model/Mail.php index 9a1bf1534aed6..dc8051bf807cf 100644 --- a/app/code/Magento/Contact/Model/Mail.php +++ b/app/code/Magento/Contact/Model/Mail.php @@ -47,8 +47,9 @@ public function __construct( * * @param string $replyTo * @param array $variables + * @return void */ - public function send($replyTo, $variables) + public function send($replyTo, array $variables) { $this->inlineTranslation->suspend(); try { diff --git a/app/code/Magento/Contact/Model/MailInterface.php b/app/code/Magento/Contact/Model/MailInterface.php index 79af47f8853a3..75c5453602c0e 100644 --- a/app/code/Magento/Contact/Model/MailInterface.php +++ b/app/code/Magento/Contact/Model/MailInterface.php @@ -18,5 +18,5 @@ interface MailInterface * @param array $variables Email template variables * @return void */ - public function send($replyTo, $variables); + public function send($replyTo, array $variables); } From 4c8caec53ff96dab9d8aeb67a33860c4ec57aa8b Mon Sep 17 00:00:00 2001 From: koenner01 Date: Tue, 21 Feb 2017 10:59:52 +0100 Subject: [PATCH 26/32] Create wishlist.js --- .../Wishlist/view/frontend/web/js/wishlist.js | 237 ++++++++++++++++++ 1 file changed, 237 insertions(+) create mode 100644 app/code/Magento/Wishlist/view/frontend/web/js/wishlist.js diff --git a/app/code/Magento/Wishlist/view/frontend/web/js/wishlist.js b/app/code/Magento/Wishlist/view/frontend/web/js/wishlist.js new file mode 100644 index 0000000000000..0192afcdaef2a --- /dev/null +++ b/app/code/Magento/Wishlist/view/frontend/web/js/wishlist.js @@ -0,0 +1,237 @@ +/** + * Copyright © 2013-2017 Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +define([ + 'jquery', + 'mage/template', + 'Magento_Ui/js/modal/alert', + 'jquery/ui', + 'mage/validation/validation', + 'mage/dataPost' +], function ($, mageTemplate, alert) { + 'use strict'; + + $.widget('mage.wishlist', { + options: { + dataAttribute: 'item-id', + nameFormat: 'qty[{0}]', + btnRemoveSelector: '[data-role=remove]', + qtySelector: '[data-role=qty]', + addToCartSelector: '[data-role=tocart]', + addAllToCartSelector: '[data-role=all-tocart]', + commentInputType: 'textarea', + infoList: false + }, + + /** + * Bind handlers to events. + */ + _create: function () { + var _this = this; + + if (!this.options.infoList) { + this.element + .on('addToCart', function (event, context) { + var urlParams; + + event.stopPropagation(event); + $(context).data('stop-processing', true); + urlParams = _this._getItemsToCartParams( + $(context).parents('[data-row=product-item]').find(_this.options.addToCartSelector) + ); + $.mage.dataPost().postData(urlParams); + + return false; + }) + .on('click', this.options.btnRemoveSelector, $.proxy(function (event) { + event.preventDefault(); + $.mage.dataPost().postData($(event.currentTarget).data('post-remove')); + }, this)) + .on('click', this.options.addToCartSelector, $.proxy(this._beforeAddToCart, this)) + .on('click', this.options.addAllToCartSelector, $.proxy(this._addAllWItemsToCart, this)) + .on('focusin focusout', this.options.commentInputType, $.proxy(this._focusComment, this)); + } + + // Setup validation for the form + this.element.mage('validation', { + /** @inheritdoc */ + errorPlacement: function (error, element) { + error.insertAfter(element.next()); + } + }); + }, + + /** + * Process data before add to cart + * + * - update item's qty value. + * + * @param {Event} event + * @private + */ + _beforeAddToCart: function (event) { + var elem = $(event.currentTarget), + itemId = elem.data(this.options.dataAttribute), + qtyName = $.validator.format(this.options.nameFormat, itemId), + qtyValue = elem.parents().find('[name="' + qtyName + '"]').val(), + params = elem.data('post'); + + if (params) { + params.data = $.extend({}, params.data, { + 'qty': qtyValue + }); + elem.data('post', params); + } + }, + + /** + * Add wish list items to cart. + * @private + * @param {jQuery} elem - clicked 'add to cart' button + */ + _getItemsToCartParams: function (elem) { + var itemId, url, qtyName, qtyValue; + + if (elem.data(this.options.dataAttribute)) { + itemId = elem.data(this.options.dataAttribute); + url = this.options.addToCartUrl; + qtyName = $.validator.format(this.options.nameFormat, itemId); + qtyValue = elem.parents().find('[name="' + qtyName + '"]').val(); + url.data.item = itemId; + url.data.qty = qtyValue; + + return url; + } + }, + + /** + * Add all wish list items to cart + * @private + */ + _addAllWItemsToCart: function () { + var urlParams = this.options.addAllToCartUrl, + separator = urlParams.action.indexOf('?') >= 0 ? '&' : '?'; + + this.element.find(this.options.qtySelector).each(function (index, element) { + urlParams.action += separator + $(element).prop('name') + '=' + encodeURIComponent($(element).val()); + separator = '&'; + }); + $.mage.dataPost().postData(urlParams); + }, + + /** + * Toggle comment string. + * @private + * @param {Event} e + */ + _focusComment: function (e) { + var commentInput = e.currentTarget; + + if (commentInput.value === '' || commentInput.value === this.options.commentString) { + commentInput.value = commentInput.value === this.options.commentString ? + '' : this.options.commentString; + } + } + }); + + // Extension for mage.wishlist - Select All checkbox + $.widget('mage.wishlist', $.mage.wishlist, { + options: { + selectAllCheckbox: '#select-all', + parentContainer: '#wishlist-table' + }, + + /** @inheritdoc */ + _create: function () { + var selectAllCheckboxParent, checkboxCount; + + this._super(); + selectAllCheckboxParent = $(this.options.selectAllCheckbox).parents(this.options.parentContainer); + checkboxCount = selectAllCheckboxParent + .find('input:checkbox:not(' + this.options.selectAllCheckbox + ')').length; + // If Select all checkbox is checked, check all item checkboxes, if unchecked, uncheck all item checkboxes + $(this.options.selectAllCheckbox).on('click', function () { + selectAllCheckboxParent.find('input:checkbox').attr('checked', $(this).is(':checked')); + }); + // If all item checkboxes are checked, check select all checkbox, + // if not all item checkboxes are checked, uncheck select all checkbox + selectAllCheckboxParent.on( + 'click', + 'input:checkbox:not(' + this.options.selectAllCheckbox + ')', + $.proxy(function () { + var checkedCount = selectAllCheckboxParent + .find('input:checkbox:checked:not(' + this.options.selectAllCheckbox + ')').length; + + $(this.options.selectAllCheckbox).attr('checked', checkboxCount === checkedCount); + }, this) + ); + } + }); + // Extension for mage.wishlist info add to cart + $.widget('mage.wishlist', $.mage.wishlist, { + /** @inheritdoc */ + _create: function () { + this._super(); + + if (this.options.infoList) { + this.element.on('addToCart', $.proxy(function (event, context) { + this.element.find('input:checkbox').attr('checked', false); + $(context).closest('tr').find('input:checkbox').attr('checked', true); + this.element.submit(); + }, this)); + this._checkBoxValidate(); + } + }, + + /** + * validate checkbox selection. + * @private + */ + _checkBoxValidate: function () { + this.element.validation({ + submitHandler: $.proxy(function (form) { + if ($(form).find('input:checkbox:checked').length) { + form.submit(); + } else { + alert({ + content: this.options.checkBoxValidationMessage + }); + } + }, this) + }); + } + }); + + // Extension for mage.wishlist - Add Wishlist item to Gift Registry + $.widget('mage.wishlist', $.mage.wishlist, { + options: { + formTmplSelector: '#form-tmpl', + formTmplId: '#wishlist-hidden-form' + }, + + /** @inheritdoc */ + _create: function () { + var _this = this; + + this._super(); + this.element.on('click', '[data-wishlist-to-giftregistry]', function () { + var json = $(this).data('wishlist-to-giftregistry'), + tmplJson = { + item: json.itemId, + entity: json.entity, + url: json.url + }, + html = mageTemplate(_this.options.formTmplSelector, { + data: tmplJson + }); + + $(html).appendTo('body'); + $(_this.options.formTmplId).submit(); + }); + } + }); + + return $.mage.wishlist; +}); From 74cd7ea8c4e0d4fc29ec832f7428bf1cfb209279 Mon Sep 17 00:00:00 2001 From: koenner01 Date: Tue, 21 Feb 2017 11:00:07 +0100 Subject: [PATCH 27/32] Delete wishlist.js --- .../Wishlist/view/frontend/web/wishlist.js | 237 ------------------ 1 file changed, 237 deletions(-) delete mode 100644 app/code/Magento/Wishlist/view/frontend/web/wishlist.js diff --git a/app/code/Magento/Wishlist/view/frontend/web/wishlist.js b/app/code/Magento/Wishlist/view/frontend/web/wishlist.js deleted file mode 100644 index 0192afcdaef2a..0000000000000 --- a/app/code/Magento/Wishlist/view/frontend/web/wishlist.js +++ /dev/null @@ -1,237 +0,0 @@ -/** - * Copyright © 2013-2017 Magento, Inc. All rights reserved. - * See COPYING.txt for license details. - */ - -define([ - 'jquery', - 'mage/template', - 'Magento_Ui/js/modal/alert', - 'jquery/ui', - 'mage/validation/validation', - 'mage/dataPost' -], function ($, mageTemplate, alert) { - 'use strict'; - - $.widget('mage.wishlist', { - options: { - dataAttribute: 'item-id', - nameFormat: 'qty[{0}]', - btnRemoveSelector: '[data-role=remove]', - qtySelector: '[data-role=qty]', - addToCartSelector: '[data-role=tocart]', - addAllToCartSelector: '[data-role=all-tocart]', - commentInputType: 'textarea', - infoList: false - }, - - /** - * Bind handlers to events. - */ - _create: function () { - var _this = this; - - if (!this.options.infoList) { - this.element - .on('addToCart', function (event, context) { - var urlParams; - - event.stopPropagation(event); - $(context).data('stop-processing', true); - urlParams = _this._getItemsToCartParams( - $(context).parents('[data-row=product-item]').find(_this.options.addToCartSelector) - ); - $.mage.dataPost().postData(urlParams); - - return false; - }) - .on('click', this.options.btnRemoveSelector, $.proxy(function (event) { - event.preventDefault(); - $.mage.dataPost().postData($(event.currentTarget).data('post-remove')); - }, this)) - .on('click', this.options.addToCartSelector, $.proxy(this._beforeAddToCart, this)) - .on('click', this.options.addAllToCartSelector, $.proxy(this._addAllWItemsToCart, this)) - .on('focusin focusout', this.options.commentInputType, $.proxy(this._focusComment, this)); - } - - // Setup validation for the form - this.element.mage('validation', { - /** @inheritdoc */ - errorPlacement: function (error, element) { - error.insertAfter(element.next()); - } - }); - }, - - /** - * Process data before add to cart - * - * - update item's qty value. - * - * @param {Event} event - * @private - */ - _beforeAddToCart: function (event) { - var elem = $(event.currentTarget), - itemId = elem.data(this.options.dataAttribute), - qtyName = $.validator.format(this.options.nameFormat, itemId), - qtyValue = elem.parents().find('[name="' + qtyName + '"]').val(), - params = elem.data('post'); - - if (params) { - params.data = $.extend({}, params.data, { - 'qty': qtyValue - }); - elem.data('post', params); - } - }, - - /** - * Add wish list items to cart. - * @private - * @param {jQuery} elem - clicked 'add to cart' button - */ - _getItemsToCartParams: function (elem) { - var itemId, url, qtyName, qtyValue; - - if (elem.data(this.options.dataAttribute)) { - itemId = elem.data(this.options.dataAttribute); - url = this.options.addToCartUrl; - qtyName = $.validator.format(this.options.nameFormat, itemId); - qtyValue = elem.parents().find('[name="' + qtyName + '"]').val(); - url.data.item = itemId; - url.data.qty = qtyValue; - - return url; - } - }, - - /** - * Add all wish list items to cart - * @private - */ - _addAllWItemsToCart: function () { - var urlParams = this.options.addAllToCartUrl, - separator = urlParams.action.indexOf('?') >= 0 ? '&' : '?'; - - this.element.find(this.options.qtySelector).each(function (index, element) { - urlParams.action += separator + $(element).prop('name') + '=' + encodeURIComponent($(element).val()); - separator = '&'; - }); - $.mage.dataPost().postData(urlParams); - }, - - /** - * Toggle comment string. - * @private - * @param {Event} e - */ - _focusComment: function (e) { - var commentInput = e.currentTarget; - - if (commentInput.value === '' || commentInput.value === this.options.commentString) { - commentInput.value = commentInput.value === this.options.commentString ? - '' : this.options.commentString; - } - } - }); - - // Extension for mage.wishlist - Select All checkbox - $.widget('mage.wishlist', $.mage.wishlist, { - options: { - selectAllCheckbox: '#select-all', - parentContainer: '#wishlist-table' - }, - - /** @inheritdoc */ - _create: function () { - var selectAllCheckboxParent, checkboxCount; - - this._super(); - selectAllCheckboxParent = $(this.options.selectAllCheckbox).parents(this.options.parentContainer); - checkboxCount = selectAllCheckboxParent - .find('input:checkbox:not(' + this.options.selectAllCheckbox + ')').length; - // If Select all checkbox is checked, check all item checkboxes, if unchecked, uncheck all item checkboxes - $(this.options.selectAllCheckbox).on('click', function () { - selectAllCheckboxParent.find('input:checkbox').attr('checked', $(this).is(':checked')); - }); - // If all item checkboxes are checked, check select all checkbox, - // if not all item checkboxes are checked, uncheck select all checkbox - selectAllCheckboxParent.on( - 'click', - 'input:checkbox:not(' + this.options.selectAllCheckbox + ')', - $.proxy(function () { - var checkedCount = selectAllCheckboxParent - .find('input:checkbox:checked:not(' + this.options.selectAllCheckbox + ')').length; - - $(this.options.selectAllCheckbox).attr('checked', checkboxCount === checkedCount); - }, this) - ); - } - }); - // Extension for mage.wishlist info add to cart - $.widget('mage.wishlist', $.mage.wishlist, { - /** @inheritdoc */ - _create: function () { - this._super(); - - if (this.options.infoList) { - this.element.on('addToCart', $.proxy(function (event, context) { - this.element.find('input:checkbox').attr('checked', false); - $(context).closest('tr').find('input:checkbox').attr('checked', true); - this.element.submit(); - }, this)); - this._checkBoxValidate(); - } - }, - - /** - * validate checkbox selection. - * @private - */ - _checkBoxValidate: function () { - this.element.validation({ - submitHandler: $.proxy(function (form) { - if ($(form).find('input:checkbox:checked').length) { - form.submit(); - } else { - alert({ - content: this.options.checkBoxValidationMessage - }); - } - }, this) - }); - } - }); - - // Extension for mage.wishlist - Add Wishlist item to Gift Registry - $.widget('mage.wishlist', $.mage.wishlist, { - options: { - formTmplSelector: '#form-tmpl', - formTmplId: '#wishlist-hidden-form' - }, - - /** @inheritdoc */ - _create: function () { - var _this = this; - - this._super(); - this.element.on('click', '[data-wishlist-to-giftregistry]', function () { - var json = $(this).data('wishlist-to-giftregistry'), - tmplJson = { - item: json.itemId, - entity: json.entity, - url: json.url - }, - html = mageTemplate(_this.options.formTmplSelector, { - data: tmplJson - }); - - $(html).appendTo('body'); - $(_this.options.formTmplId).submit(); - }); - } - }); - - return $.mage.wishlist; -}); From e162c3d20914b4ba15175094ce7b8e9f15ef31d1 Mon Sep 17 00:00:00 2001 From: koenner01 Date: Tue, 21 Feb 2017 11:00:21 +0100 Subject: [PATCH 28/32] Update requirejs-config.js --- app/code/Magento/Wishlist/view/frontend/requirejs-config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Wishlist/view/frontend/requirejs-config.js b/app/code/Magento/Wishlist/view/frontend/requirejs-config.js index e078023ac56ea..c235273b700e1 100644 --- a/app/code/Magento/Wishlist/view/frontend/requirejs-config.js +++ b/app/code/Magento/Wishlist/view/frontend/requirejs-config.js @@ -6,7 +6,7 @@ var config = { map: { '*': { - wishlist: 'Magento_Wishlist/wishlist', + wishlist: 'Magento_Wishlist/js/wishlist', addToWishlist: 'Magento_Wishlist/js/add-to-wishlist', wishlistSearch: 'Magento_Wishlist/js/search' } From 8a0f0c69875073ca39069141718760232c48ad8d Mon Sep 17 00:00:00 2001 From: Josef Behr Date: Tue, 21 Feb 2017 17:23:16 +0100 Subject: [PATCH 29/32] Update Save.php If quantity is changed using mass action for attribute changes, but no stock item exists for the product, saving the newly created stockItem in StockItemRepository->save($stockItem) returns without saving the stockItem, since $stockItem->getProductId() is null, due to it being added to the array with key '0' (see line 165). To fix this, the array key has been changed to "product_id", such that StockItemRepository successfully creates the stockItem. Also, in my opinion, StockItemRepository->save() should not just return without notice, especially as it already encapsulates the functionality in a try/catch block. Throwing an exception would actually tell the user that something went wrong. --- .../Controller/Adminhtml/Product/Action/Attribute/Save.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Action/Attribute/Save.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Action/Attribute/Save.php index e04d71b99cb9b..d8f4e204c1bb4 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Action/Attribute/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Action/Attribute/Save.php @@ -162,7 +162,7 @@ public function execute() $this->attributeHelper->getStoreWebsiteId($storeId) ); if (!$stockItemDo->getProductId()) { - $inventoryData[] = $productId; + $inventoryData['product_id'] = $productId; } $stockItemId = $stockItemDo->getId(); From 7ff48237a7d524ae62cf063c0b976fbfd8a7dd05 Mon Sep 17 00:00:00 2001 From: Eugene Tulika Date: Wed, 22 Feb 2017 11:54:49 -0600 Subject: [PATCH 30/32] MAGETWO-65047 [GitHub][PR] Refactor contact module magento/magento2#8420 - fix code style issues --- app/code/Magento/Contact/Controller/Index.php | 2 +- app/code/Magento/Contact/Controller/Index/Post.php | 4 +--- app/code/Magento/Contact/Model/Config.php | 9 ++++----- app/code/Magento/Contact/Model/ConfigInterface.php | 3 ++- app/code/Magento/Contact/Model/Mail.php | 2 -- app/code/Magento/Contact/Model/MailInterface.php | 3 ++- .../Contact/Test/Unit/Controller/Index/IndexTest.php | 2 -- 7 files changed, 10 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index.php b/app/code/Magento/Contact/Controller/Index.php index 3b170f7c4841c..672059740afdc 100644 --- a/app/code/Magento/Contact/Controller/Index.php +++ b/app/code/Magento/Contact/Controller/Index.php @@ -42,7 +42,7 @@ abstract class Index extends \Magento\Framework\App\Action\Action /** * @param Context $context - * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig + * @param ConfigInterface $contactsConfig */ public function __construct( Context $context, diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 8f94dfbd98213..7350e12a69712 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -14,8 +14,6 @@ use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\HTTP\PhpEnvironment\Request; -use Magento\Framework\Mail\Template\TransportBuilder; -use Magento\Framework\Translate\Inline\StateInterface; class Post extends \Magento\Contact\Controller\Index { @@ -59,7 +57,7 @@ public function __construct( */ public function execute() { - if (! $this->isPostRequest()) { + if (!$this->isPostRequest()) { return $this->resultRedirectFactory->create()->setPath('*/*/'); } try { diff --git a/app/code/Magento/Contact/Model/Config.php b/app/code/Magento/Contact/Model/Config.php index 24e47af6f4dba..44f3dc9052c1e 100644 --- a/app/code/Magento/Contact/Model/Config.php +++ b/app/code/Magento/Contact/Model/Config.php @@ -5,7 +5,6 @@ */ namespace Magento\Contact\Model; -use Magento\Contact\Model\ConfigInterface; use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Store\Model\ScopeInterface; @@ -28,7 +27,7 @@ public function __construct(ScopeConfigInterface $scopeConfig) } /** - * @return bool + * {@inheritdoc} */ public function isEnabled() { @@ -39,7 +38,7 @@ public function isEnabled() } /** - * @return string + * {@inheritdoc} */ public function emailTemplate() { @@ -50,7 +49,7 @@ public function emailTemplate() } /** - * @return string + * {@inheritdoc} */ public function emailSender() { @@ -61,7 +60,7 @@ public function emailSender() } /** - * @return string + * {@inheritdoc} */ public function emailRecipient() { diff --git a/app/code/Magento/Contact/Model/ConfigInterface.php b/app/code/Magento/Contact/Model/ConfigInterface.php index ac702bc6d30d3..92c17f6b75183 100644 --- a/app/code/Magento/Contact/Model/ConfigInterface.php +++ b/app/code/Magento/Contact/Model/ConfigInterface.php @@ -1,6 +1,7 @@ Date: Wed, 22 Feb 2017 13:20:28 -0600 Subject: [PATCH 31/32] MAGETWO-65047 [GitHub][PR] Refactor contact module magento/magento2#8420 - fix integrity tests issues --- app/code/Magento/Contact/Controller/Index/Post.php | 4 ++-- .../testsuite/Magento/Contact/Model/ConfigTest.php | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Contact/Controller/Index/Post.php b/app/code/Magento/Contact/Controller/Index/Post.php index 7350e12a69712..59cd5d534a0d9 100644 --- a/app/code/Magento/Contact/Controller/Index/Post.php +++ b/app/code/Magento/Contact/Controller/Index/Post.php @@ -34,14 +34,14 @@ class Post extends \Magento\Contact\Controller\Index /** * @param Context $context - * @param MailInterface $mail * @param ConfigInterface $contactsConfig + * @param MailInterface $mail * @param DataPersistorInterface $dataPersistor */ public function __construct( Context $context, - MailInterface $mail, ConfigInterface $contactsConfig, + MailInterface $mail, DataPersistorInterface $dataPersistor ) { parent::__construct($context, $contactsConfig); diff --git a/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php b/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php index deeb2957ec7e8..e5336c3fdd9b2 100644 --- a/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php +++ b/dev/tests/integration/testsuite/Magento/Contact/Model/ConfigTest.php @@ -4,9 +4,8 @@ * See COPYING.txt for license details. */ -namespace Magento\Contact\Test\Integration\Model; +namespace Magento\Contact\Model; -use Magento\Contact\Model\ConfigInterface; use Magento\TestFramework\Helper\Bootstrap; class ConfigTest extends \PHPUnit_Framework_TestCase From b8b5a6be7fc94e5775b35602eac93cff3e00cd33 Mon Sep 17 00:00:00 2001 From: Eugene Tulika Date: Wed, 22 Feb 2017 13:59:49 -0600 Subject: [PATCH 32/32] MAGETWO-65047 [GitHub][PR] Refactor contact module magento/magento2#8420 - fix integration tests --- .../Magento/Contact/Test/Unit/Controller/Index/PostTest.php | 2 +- app/code/Magento/Contact/Test/Unit/Model/MailTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php index 8fd54170819f5..f3c644a45a103 100644 --- a/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php +++ b/app/code/Magento/Contact/Test/Unit/Controller/Index/PostTest.php @@ -129,8 +129,8 @@ protected function setUp() $this->controller = new \Magento\Contact\Controller\Index\Post( $context, - $this->mailMock, $this->configMock, + $this->mailMock, $this->dataPersistorMock ); } diff --git a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php index de115920062eb..fd7a114aa5e32 100644 --- a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php +++ b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php @@ -4,7 +4,7 @@ * Copyright © 2013-2017 Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento\Contact\Test\Integration\Model; +namespace Magento\Contact\Test\Unit\Model; use Magento\Contact\Model\ConfigInterface; use Magento\Contact\Model\Mail;