From 47a4681f6452559682ed6f820e1a79c76bd96eb0 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sat, 4 Feb 2017 13:32:04 +0000 Subject: [PATCH 01/18] 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 02/18] 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 03/18] 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 04/18] 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 05/18] 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 06/18] 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 07/18] 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 08/18] 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 09/18] 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 10/18] 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 11/18] 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 12/18] 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 13/18] 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 14/18] 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 15/18] 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 16/18] 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 a5c69d0aa1e07a6cd68827c2fdc626e4499fa141 Mon Sep 17 00:00:00 2001 From: Fabian Schmengler Date: Sun, 19 Feb 2017 12:47:12 +0000 Subject: [PATCH 17/18] 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 18/18] 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); }