From 4b0fdfcf4d83b4dc180cefa65a717fbddb790dc9 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 19 Aug 2020 00:23:08 +0200 Subject: [PATCH 1/3] Minor Refactor --- .../Quote/Model/ChangeQuoteControl.php | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/Quote/Model/ChangeQuoteControl.php b/app/code/Magento/Quote/Model/ChangeQuoteControl.php index b88898a816d66..92e25ca6c7d3a 100644 --- a/app/code/Magento/Quote/Model/ChangeQuoteControl.php +++ b/app/code/Magento/Quote/Model/ChangeQuoteControl.php @@ -3,7 +3,6 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ - declare(strict_types=1); namespace Magento\Quote\Model; @@ -12,13 +11,10 @@ use Magento\Quote\Api\ChangeQuoteControlInterface; use Magento\Quote\Api\Data\CartInterface; -/** - * {@inheritdoc} - */ class ChangeQuoteControl implements ChangeQuoteControlInterface { /** - * @var UserContextInterface $userContext + * @var UserContextInterface */ private $userContext; @@ -31,25 +27,20 @@ public function __construct(UserContextInterface $userContext) } /** - * {@inheritdoc} + * @inheritdoc */ public function isAllowed(CartInterface $quote): bool { switch ($this->userContext->getUserType()) { case UserContextInterface::USER_TYPE_CUSTOMER: - $isAllowed = ($quote->getCustomerId() == $this->userContext->getUserId()); - break; + return ($quote->getCustomerId() == $this->userContext->getUserId()); case UserContextInterface::USER_TYPE_GUEST: - $isAllowed = ($quote->getCustomerId() === null); - break; + return ($quote->getCustomerId() === null); case UserContextInterface::USER_TYPE_ADMIN: case UserContextInterface::USER_TYPE_INTEGRATION: - $isAllowed = true; - break; - default: - $isAllowed = false; + return true; } - return $isAllowed; + return false; } } From 6b01af91fdc4720f73302f89256c1c91f345c429 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 19 Aug 2020 00:31:46 +0200 Subject: [PATCH 2/3] Refactor Unit Tests too --- .../Unit/Model/ChangeQuoteControlTest.php | 52 +++---------------- 1 file changed, 8 insertions(+), 44 deletions(-) diff --git a/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php b/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php index f302372344c11..a241272652cb9 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php @@ -16,57 +16,36 @@ /** * Unit test for \Magento\Quote\Model\ChangeQuoteControl - * - * Class \Magento\Quote\Test\Unit\Model\ChangeQuoteControlTest */ class ChangeQuoteControlTest extends TestCase { - /** - * @var ObjectManager - */ - protected $objectManager; - /** * @var ChangeQuoteControl */ protected $model; /** - * @var MockObject + * @var MockObject|UserContextInterface */ protected $userContextMock; /** - * @var MockObject + * @var MockObject|CartInterface */ protected $quoteMock; protected function setUp(): void { - $this->objectManager = new ObjectManager($this); $this->userContextMock = $this->getMockForAbstractClass(UserContextInterface::class); - $this->model = $this->objectManager->getObject( - ChangeQuoteControl::class, - [ - 'userContext' => $this->userContextMock - ] - ); - - $this->quoteMock = $this->getMockForAbstractClass( - CartInterface::class, - [], - '', - false, - true, - true, - ['getCustomerId'] - ); + $this->model = new ChangeQuoteControl($this->userContextMock); + + $this->quoteMock = $this->getMockBuilder(CartInterface::class) + ->disableOriginalConstructor() + ->addMethods(['getCustomerId']) + ->getMockForAbstractClass(); } - /** - * Test if the quote is belonged to customer - */ public function testIsAllowedIfTheQuoteIsBelongedToCustomer() { $quoteCustomerId = 1; @@ -80,9 +59,6 @@ public function testIsAllowedIfTheQuoteIsBelongedToCustomer() $this->assertTrue($this->model->isAllowed($this->quoteMock)); } - /** - * Test if the quote is not belonged to customer - */ public function testIsAllowedIfTheQuoteIsNotBelongedToCustomer() { $currentCustomerId = 1; @@ -98,9 +74,6 @@ public function testIsAllowedIfTheQuoteIsNotBelongedToCustomer() $this->assertFalse($this->model->isAllowed($this->quoteMock)); } - /** - * Test if the quote is belonged to guest and the context is guest - */ public function testIsAllowedIfQuoteIsBelongedToGuestAndContextIsGuest() { $quoteCustomerId = null; @@ -111,9 +84,6 @@ public function testIsAllowedIfQuoteIsBelongedToGuestAndContextIsGuest() $this->assertTrue($this->model->isAllowed($this->quoteMock)); } - /** - * Test if the quote is belonged to customer and the context is guest - */ public function testIsAllowedIfQuoteIsBelongedToCustomerAndContextIsGuest() { $quoteCustomerId = 1; @@ -124,9 +94,6 @@ public function testIsAllowedIfQuoteIsBelongedToCustomerAndContextIsGuest() $this->assertFalse($this->model->isAllowed($this->quoteMock)); } - /** - * Test if the context is admin - */ public function testIsAllowedIfContextIsAdmin() { $this->userContextMock->expects($this->any())->method('getUserType') @@ -134,9 +101,6 @@ public function testIsAllowedIfContextIsAdmin() $this->assertTrue($this->model->isAllowed($this->quoteMock)); } - /** - * Test if the context is integration - */ public function testIsAllowedIfContextIsIntegration() { $this->userContextMock->expects($this->any())->method('getUserType') From bc5e644ed1131a6ad4d9bf900270076cbad0098a Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Wed, 19 Aug 2020 00:34:22 +0200 Subject: [PATCH 3/3] Refactor Unit Tests too (remove "expects any()") --- .../Unit/Model/ChangeQuoteControlTest.php | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php b/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php index a241272652cb9..a467f3e25d698 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/ChangeQuoteControlTest.php @@ -49,11 +49,11 @@ protected function setUp(): void public function testIsAllowedIfTheQuoteIsBelongedToCustomer() { $quoteCustomerId = 1; - $this->quoteMock->expects($this->any())->method('getCustomerId') + $this->quoteMock->method('getCustomerId') ->willReturn($quoteCustomerId); - $this->userContextMock->expects($this->any())->method('getUserType') + $this->userContextMock->method('getUserType') ->willReturn(UserContextInterface::USER_TYPE_CUSTOMER); - $this->userContextMock->expects($this->any())->method('getUserId') + $this->userContextMock->method('getUserId') ->willReturn($quoteCustomerId); $this->assertTrue($this->model->isAllowed($this->quoteMock)); @@ -64,11 +64,11 @@ public function testIsAllowedIfTheQuoteIsNotBelongedToCustomer() $currentCustomerId = 1; $quoteCustomerId = 2; - $this->quoteMock->expects($this->any())->method('getCustomerId') + $this->quoteMock->method('getCustomerId') ->willReturn($quoteCustomerId); - $this->userContextMock->expects($this->any())->method('getUserType') + $this->userContextMock->method('getUserType') ->willReturn(UserContextInterface::USER_TYPE_CUSTOMER); - $this->userContextMock->expects($this->any())->method('getUserId') + $this->userContextMock->method('getUserId') ->willReturn($currentCustomerId); $this->assertFalse($this->model->isAllowed($this->quoteMock)); @@ -77,9 +77,9 @@ public function testIsAllowedIfTheQuoteIsNotBelongedToCustomer() public function testIsAllowedIfQuoteIsBelongedToGuestAndContextIsGuest() { $quoteCustomerId = null; - $this->quoteMock->expects($this->any())->method('getCustomerId') + $this->quoteMock->method('getCustomerId') ->willReturn($quoteCustomerId); - $this->userContextMock->expects($this->any())->method('getUserType') + $this->userContextMock->method('getUserType') ->willReturn(UserContextInterface::USER_TYPE_GUEST); $this->assertTrue($this->model->isAllowed($this->quoteMock)); } @@ -87,23 +87,23 @@ public function testIsAllowedIfQuoteIsBelongedToGuestAndContextIsGuest() public function testIsAllowedIfQuoteIsBelongedToCustomerAndContextIsGuest() { $quoteCustomerId = 1; - $this->quoteMock->expects($this->any())->method('getCustomerId') + $this->quoteMock->method('getCustomerId') ->willReturn($quoteCustomerId); - $this->userContextMock->expects($this->any())->method('getUserType') + $this->userContextMock->method('getUserType') ->willReturn(UserContextInterface::USER_TYPE_GUEST); $this->assertFalse($this->model->isAllowed($this->quoteMock)); } public function testIsAllowedIfContextIsAdmin() { - $this->userContextMock->expects($this->any())->method('getUserType') + $this->userContextMock->method('getUserType') ->willReturn(UserContextInterface::USER_TYPE_ADMIN); $this->assertTrue($this->model->isAllowed($this->quoteMock)); } public function testIsAllowedIfContextIsIntegration() { - $this->userContextMock->expects($this->any())->method('getUserType') + $this->userContextMock->method('getUserType') ->willReturn(UserContextInterface::USER_TYPE_INTEGRATION); $this->assertTrue($this->model->isAllowed($this->quoteMock)); }