diff --git a/app/code/Magento/Sales/Model/Order/CustomerAssignment.php b/app/code/Magento/Sales/Model/Order/CustomerAssignment.php new file mode 100644 index 0000000000000..8bcfc1dc49de4 --- /dev/null +++ b/app/code/Magento/Sales/Model/Order/CustomerAssignment.php @@ -0,0 +1,59 @@ +eventManager = $eventManager; + $this->orderRepository = $orderRepository; + } + + /** + * @param OrderInterface $order + * @param CustomerInterface $customer + */ + public function execute(OrderInterface $order, CustomerInterface $customer)/*: void*/ + { + $order->setCustomerId($customer->getId()); + $order->setCustomerIsGuest(false); + $this->orderRepository->save($order); + + $this->eventManager->dispatch( + 'sales_order_customer_assign_after', + [ + 'order' => $order, + 'customer' => $customer + ] + ); + } +} diff --git a/app/code/Magento/Sales/Observer/AssignOrderToCustomerObserver.php b/app/code/Magento/Sales/Observer/AssignOrderToCustomerObserver.php index f41ea6888264f..9857fa39fa51a 100644 --- a/app/code/Magento/Sales/Observer/AssignOrderToCustomerObserver.php +++ b/app/code/Magento/Sales/Observer/AssignOrderToCustomerObserver.php @@ -12,6 +12,7 @@ use Magento\Framework\Event\Observer; use Magento\Framework\Event\ObserverInterface; use Magento\Sales\Api\OrderRepositoryInterface; +use Magento\Sales\Model\Order\CustomerAssignment; /** * Assign order to customer created after issuing guest order. @@ -24,11 +25,22 @@ class AssignOrderToCustomerObserver implements ObserverInterface private $orderRepository; /** + * @var CustomerAssignment + */ + private $assignmentService; + + /** + * AssignOrderToCustomerObserver constructor. + * * @param OrderRepositoryInterface $orderRepository + * @param CustomerAssignment $assignmentService */ - public function __construct(OrderRepositoryInterface $orderRepository) - { + public function __construct( + OrderRepositoryInterface $orderRepository, + CustomerAssignment $assignmentService + ) { $this->orderRepository = $orderRepository; + $this->assignmentService = $assignmentService; } /** @@ -44,11 +56,8 @@ public function execute(Observer $observer) if (array_key_exists('__sales_assign_order_id', $delegateData)) { $orderId = $delegateData['__sales_assign_order_id']; $order = $this->orderRepository->get($orderId); - if (!$order->getCustomerId()) { - //if customer ID wasn't already assigned then assigning. - $order->setCustomerId($customer->getId()); - $order->setCustomerIsGuest(0); - $this->orderRepository->save($order); + if (!$order->getCustomerId() && $customer->getId()) { + $this->assignmentService->execute($order, $customer); } } } diff --git a/app/code/Magento/Sales/Test/Unit/Observer/AssignOrderToCustomerObserverTest.php b/app/code/Magento/Sales/Test/Unit/Observer/AssignOrderToCustomerObserverTest.php index c6e02151b9bc1..18371274049e3 100644 --- a/app/code/Magento/Sales/Test/Unit/Observer/AssignOrderToCustomerObserverTest.php +++ b/app/code/Magento/Sales/Test/Unit/Observer/AssignOrderToCustomerObserverTest.php @@ -12,6 +12,7 @@ use Magento\Framework\Event\Observer; use Magento\Sales\Api\Data\OrderInterface; use Magento\Sales\Api\OrderRepositoryInterface; +use Magento\Sales\Model\Order\CustomerAssignment; use Magento\Sales\Observer\AssignOrderToCustomerObserver; use PHPUnit\Framework\TestCase; use PHPUnit_Framework_MockObject_MockObject; @@ -27,6 +28,9 @@ class AssignOrderToCustomerObserverTest extends TestCase /** @var OrderRepositoryInterface|PHPUnit_Framework_MockObject_MockObject */ protected $orderRepositoryMock; + /** @var CustomerAssignment | PHPUnit_Framework_MockObject_MockObject */ + protected $assignmentMock; + /** * Set Up */ @@ -35,7 +39,12 @@ protected function setUp() $this->orderRepositoryMock = $this->getMockBuilder(OrderRepositoryInterface::class) ->disableOriginalConstructor() ->getMock(); - $this->sut = new AssignOrderToCustomerObserver($this->orderRepositoryMock); + + $this->assignmentMock = $this->getMockBuilder(CustomerAssignment::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->sut = new AssignOrderToCustomerObserver($this->orderRepositoryMock, $this->assignmentMock); } /** @@ -69,13 +78,14 @@ public function testAssignOrderToCustomerAfterGuestOrder($customerId) $orderMock->expects($this->once())->method('getCustomerId')->willReturn($customerId); $this->orderRepositoryMock->expects($this->once())->method('get')->with($orderId) ->willReturn($orderMock); - if (!$customerId) { - $this->orderRepositoryMock->expects($this->once())->method('save')->with($orderMock); + + if ($customerId) { + $this->assignmentMock->expects($this->once())->method('execute')->with($orderMock, $customerMock); $this->sut->execute($observerMock); - return ; + return; } - $this->orderRepositoryMock->expects($this->never())->method('save')->with($orderMock); + $this->assignmentMock->expects($this->never())->method('execute'); $this->sut->execute($observerMock); } diff --git a/app/code/Magento/SalesRule/Observer/AssignCouponDataAfterOrderCustomerAssignObserver.php b/app/code/Magento/SalesRule/Observer/AssignCouponDataAfterOrderCustomerAssignObserver.php new file mode 100644 index 0000000000000..d9699d334ff6a --- /dev/null +++ b/app/code/Magento/SalesRule/Observer/AssignCouponDataAfterOrderCustomerAssignObserver.php @@ -0,0 +1,52 @@ +updateCouponUsages = $updateCouponUsages; + } + + /** + * @inheritDoc + */ + public function execute(Observer $observer) + { + $event = $observer->getEvent(); + /** @var OrderInterface $order */ + $order = $event->getData(self::EVENT_KEY_ORDER); + + if ($order->getCustomerId()) { + $this->updateCouponUsages->execute($order, true); + } + } +} diff --git a/app/code/Magento/SalesRule/etc/events.xml b/app/code/Magento/SalesRule/etc/events.xml index 8261860bbb7ce..eec0da74f619e 100644 --- a/app/code/Magento/SalesRule/etc/events.xml +++ b/app/code/Magento/SalesRule/etc/events.xml @@ -24,4 +24,7 @@ + + + diff --git a/app/code/Magento/Shipping/Block/Adminhtml/Order/Packaging.php b/app/code/Magento/Shipping/Block/Adminhtml/Order/Packaging.php index b4ff445c63f4e..e5e419328eea4 100644 --- a/app/code/Magento/Shipping/Block/Adminhtml/Order/Packaging.php +++ b/app/code/Magento/Shipping/Block/Adminhtml/Order/Packaging.php @@ -74,6 +74,7 @@ public function getShipment() * Configuration for popup window for packaging * * @return string + * @SuppressWarnings(PHPMD.RequestAwareBlockMethod) */ public function getConfigDataJson() { @@ -86,7 +87,7 @@ public function getConfigDataJson() $itemsName = []; $itemsWeight = []; $itemsProductId = []; - + $itemsOrderItemId = []; if ($shipmentId) { $urlParams['shipment_id'] = $shipmentId; $createLabelUrl = $this->getUrl('adminhtml/order_shipment/createLabel', $urlParams); diff --git a/dev/tests/integration/testsuite/Magento/SalesRule/Model/Observer/AssignCouponDataAfterOrderCustomerAssignTest.php b/dev/tests/integration/testsuite/Magento/SalesRule/Model/Observer/AssignCouponDataAfterOrderCustomerAssignTest.php new file mode 100644 index 0000000000000..397650df416e9 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/SalesRule/Model/Observer/AssignCouponDataAfterOrderCustomerAssignTest.php @@ -0,0 +1,291 @@ +objectManager = Bootstrap::getObjectManager(); + $this->eventManager = $this->createMock(\Magento\Framework\Event\ManagerInterface::class); + $this->orderRepository = $this->objectManager->get(\Magento\Sales\Model\OrderRepository::class); + $this->delegateCustomerService = $this->objectManager->get(Order\OrderCustomerDelegate::class); + $this->customerRepository = $this->objectManager->get(\Magento\Customer\Api\CustomerRepositoryInterface::class); + $this->ruleCustomerFactory = $this->objectManager->get(\Magento\SalesRule\Model\Rule\CustomerFactory::class); + $this->assignCouponToCustomerObserver = $this->objectManager->get( + \Magento\SalesRule\Observer\AssignCouponDataAfterOrderCustomerAssignObserver::class + ); + + $this->salesRule = $this->prepareSalesRule(); + $this->coupon = $this->attachSalesruleCoupon($this->salesRule); + $this->order = $this->makeOrderWithCouponAsGuest($this->coupon); + $this->delegateOrderToBeAssigned($this->order); + $this->customer = $this->registerNewCustomer(); + $this->order->setCustomerId($this->customer->getId()); + } + + /** + * @inheritdoc + */ + protected function tearDown() + { + $this->salesRule = null; + $this->customer = null; + $this->coupon = null; + $this->order = null; + } + + /** + * @magentoDataFixture Magento/Sales/_files/order.php + */ + public function testCouponDataHasBeenAssignedTest() + { + $ruleCustomer = $this->getSalesruleCustomerUsage($this->customer, $this->salesRule); + + // Assert, that rule customer model has been created for specific customer + $this->assertEquals( + $ruleCustomer->getCustomerId(), + $this->customer->getId() + ); + + // Assert, that customer has increased coupon usage of specific rule + $this->assertEquals( + 1, + $ruleCustomer->getTimesUsed() + ); + } + + /** + * @magentoDataFixture Magento/Sales/_files/order.php + */ + public function testOrderCancelingDecreasesCouponUsages() + { + $this->processOrder($this->order); + + // Should not throw exception as bux is fixed now + $this->order->cancel(); + $ruleCustomer = $this->getSalesruleCustomerUsage($this->customer, $this->salesRule); + + // Assert, that rule customer model has been created for specific customer + $this->assertEquals( + $ruleCustomer->getCustomerId(), + $this->customer->getId() + ); + + // Assert, that customer has increased coupon usage of specific rule + $this->assertEquals( + 0, + $ruleCustomer->getTimesUsed() + ); + } + + /** + * @param Order $order + * @return \Magento\Sales\Api\Data\OrderInterface + */ + private function processOrder(Order $order) + { + $order->setState(Order::STATE_PROCESSING); + $order->setStatus(Order::STATE_PROCESSING); + return $this->orderRepository->save($order); + } + + /** + * @param Customer $customer + * @param Rule $rule + * @return Rule\Customer + */ + private function getSalesruleCustomerUsage(Customer $customer, Rule $rule) : \Magento\SalesRule\Model\Rule\Customer + { + $ruleCustomer = $this->ruleCustomerFactory->create(); + return $ruleCustomer->loadByCustomerRule($customer->getId(), $rule->getRuleId()); + } + + /** + * @return Rule + */ + private function prepareSalesRule() : Rule + { + /** @var Rule $salesRule */ + $salesRule = $this->objectManager->create(Rule::class); + $salesRule->setData( + [ + 'name' => '15$ fixed discount on whole cart', + 'is_active' => 1, + 'customer_group_ids' => [GroupManagement::NOT_LOGGED_IN_ID], + 'coupon_type' => Rule::COUPON_TYPE_SPECIFIC, + 'conditions' => [ + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Address::class, + 'attribute' => 'base_subtotal', + 'operator' => '>', + 'value' => 45, + ], + ], + 'simple_action' => Rule::CART_FIXED_ACTION, + 'discount_amount' => 15, + 'discount_step' => 0, + 'stop_rules_processing' => 1, + 'website_ids' => [ + $this->objectManager->get(StoreManagerInterface::class)->getWebsite()->getId(), + ], + ] + ); + Bootstrap::getObjectManager()->get( + \Magento\SalesRule\Model\ResourceModel\Rule::class + )->save($salesRule); + + return $salesRule; + } + + /** + * @param Rule $salesRule + * @return Coupon + */ + private function attachSalesruleCoupon(Rule $salesRule) : Coupon + { + $coupon = $this->objectManager->create(Coupon::class); + $coupon->setRuleId($salesRule->getId()) + ->setCode('CART_FIXED_DISCOUNT_15') + ->setType(0); + + Bootstrap::getObjectManager()->get(CouponRepositoryInterface::class)->save($coupon); + + return $coupon; + } + + /** + * @param Coupon $coupon + * @return Order + */ + private function makeOrderWithCouponAsGuest(Coupon $coupon) : Order + { + $order = Bootstrap::getObjectManager()->create(\Magento\Sales\Model\Order::class); + $order->loadByIncrementId('100000001') + ->setCustomerIsGuest(true) + ->setCouponCode($coupon->getCode()) + ->setCreatedAt('2014-10-25 10:10:10') + ->setAppliedRuleIds($coupon->getRuleId()) + ->save(); + + return $order; + } + + /** + * @param Order $order + */ + private function delegateOrderToBeAssigned(Order $order) + { + $this->delegateCustomerService->delegateNew($order->getId()); + } + + /** + * @return Customer + * @throws \Magento\Framework\Exception\InputException + * @throws \Magento\Framework\Exception\LocalizedException + * @throws \Magento\Framework\Exception\State\InputMismatchException + */ + private function registerNewCustomer() : Customer + { + $customer = Bootstrap::getObjectManager()->create( + \Magento\Customer\Api\Data\CustomerInterface::class + ); + + /** @var Magento\Customer\Api\Data\CustomerInterface $customer */ + $customer->setWebsiteId(1) + ->setEmail('customer@example.com') + ->setGroupId(1) + ->setStoreId(1) + ->setPrefix('Mr.') + ->setFirstname('John') + ->setMiddlename('A') + ->setLastname('Smith') + ->setSuffix('Esq.') + ->setDefaultBilling(1) + ->setDefaultShipping(1) + ->setTaxvat('12') + ->setGender(0); + + $customer = $this->customerRepository->save($customer, 'password'); + + return $customer; + } +} diff --git a/lib/internal/Magento/Framework/View/Element/Html/Link/Current.php b/lib/internal/Magento/Framework/View/Element/Html/Link/Current.php index 43bfd46c1193a..7aac210dcab89 100644 --- a/lib/internal/Magento/Framework/View/Element/Html/Link/Current.php +++ b/lib/internal/Magento/Framework/View/Element/Html/Link/Current.php @@ -5,6 +5,10 @@ */ namespace Magento\Framework\View\Element\Html\Link; +use Magento\Framework\App\DefaultPathInterface; +use Magento\Framework\View\Element\Template; +use Magento\Framework\View\Element\Template\Context; + /** * Block representing link with two possible states. * "Current" state means link leads to URL equivalent to URL of currently displayed page. @@ -17,25 +21,25 @@ * @method null|bool getCurrent() * @method \Magento\Framework\View\Element\Html\Link\Current setCurrent(bool $value) */ -class Current extends \Magento\Framework\View\Element\Template +class Current extends Template { /** * Default path * - * @var \Magento\Framework\App\DefaultPathInterface + * @var DefaultPathInterface */ protected $_defaultPath; /** * Constructor * - * @param \Magento\Framework\View\Element\Template\Context $context - * @param \Magento\Framework\App\DefaultPathInterface $defaultPath + * @param Context $context + * @param DefaultPathInterface $defaultPath * @param array $data */ public function __construct( - \Magento\Framework\View\Element\Template\Context $context, - \Magento\Framework\App\DefaultPathInterface $defaultPath, + Context $context, + DefaultPathInterface $defaultPath, array $data = [] ) { parent::__construct($context, $data); @@ -56,18 +60,20 @@ public function getHref() * Get current mca * * @return string + * @SuppressWarnings(PHPMD.RequestAwareBlockMethod) */ private function getMca() { $routeParts = [ - 'module' => $this->_request->getModuleName(), - 'controller' => $this->_request->getControllerName(), - 'action' => $this->_request->getActionName(), + (string)$this->_request->getModuleName(), + (string)$this->_request->getControllerName(), + (string)$this->_request->getActionName(), ]; $parts = []; + $pathParts = explode('/', trim($this->_request->getPathInfo(), '/')); foreach ($routeParts as $key => $value) { - if (!empty($value) && $value != $this->_defaultPath->getPart($key)) { + if (isset($pathParts[$key]) && $pathParts[$key] === $value) { $parts[] = $value; } } diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Element/Html/Link/CurrentTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Element/Html/Link/CurrentTest.php index 909748722a081..7070ec9d48c11 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/Element/Html/Link/CurrentTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Element/Html/Link/CurrentTest.php @@ -17,11 +17,6 @@ class CurrentTest extends \PHPUnit\Framework\TestCase */ protected $_requestMock; - /** - * @var \PHPUnit_Framework_MockObject_MockObject - */ - protected $_defaultPathMock; - /** * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ @@ -32,7 +27,6 @@ protected function setUp() $this->_objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); $this->_urlBuilderMock = $this->createMock(\Magento\Framework\UrlInterface::class); $this->_requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class); - $this->_defaultPathMock = $this->createMock(\Magento\Framework\App\DefaultPathInterface::class); } public function testGetUrl() @@ -60,29 +54,46 @@ public function testIsCurrentIfIsset() $this->assertTrue($link->isCurrent()); } + /** + * Test if the current url is the same as link path + * + * @return void + */ public function testIsCurrent() { - $path = 'test/path'; - $url = 'http://example.com/a/b'; - - $this->_requestMock->expects($this->once())->method('getModuleName')->will($this->returnValue('a')); - $this->_requestMock->expects($this->once())->method('getControllerName')->will($this->returnValue('b')); - $this->_requestMock->expects($this->once())->method('getActionName')->will($this->returnValue('d')); - $this->_defaultPathMock->expects($this->atLeastOnce())->method('getPart')->will($this->returnValue('d')); + $path = 'test/index'; + $url = 'http://example.com/test/index'; + + $this->_requestMock->expects($this->once()) + ->method('getPathInfo') + ->will($this->returnValue('/test/index/')); + $this->_requestMock->expects($this->once()) + ->method('getModuleName') + ->will($this->returnValue('test')); + $this->_requestMock->expects($this->once()) + ->method('getControllerName') + ->will($this->returnValue('index')); + $this->_requestMock->expects($this->once()) + ->method('getActionName') + ->will($this->returnValue('index')); + $this->_urlBuilderMock->expects($this->at(0)) + ->method('getUrl') + ->with($path) + ->will($this->returnValue($url)); + $this->_urlBuilderMock->expects($this->at(1)) + ->method('getUrl') + ->with('test/index') + ->will($this->returnValue($url)); - $this->_urlBuilderMock->expects($this->at(0))->method('getUrl')->with($path)->will($this->returnValue($url)); - $this->_urlBuilderMock->expects($this->at(1))->method('getUrl')->with('a/b')->will($this->returnValue($url)); - - $this->_requestMock->expects($this->once())->method('getControllerName')->will($this->returnValue('b')); /** @var \Magento\Framework\View\Element\Html\Link\Current $link */ $link = $this->_objectManager->getObject( \Magento\Framework\View\Element\Html\Link\Current::class, [ 'urlBuilder' => $this->_urlBuilderMock, - 'request' => $this->_requestMock, - 'defaultPath' => $this->_defaultPathMock + 'request' => $this->_requestMock ] ); + $link->setPath($path); $this->assertTrue($link->isCurrent()); }