Skip to content

Fix #17933 - Bank Transfer Payment Instructions switch back to default #26765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2020
Merged

Fix #17933 - Bank Transfer Payment Instructions switch back to default #26765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions app/code/Magento/OfflinePayments/Observer/BeforeOrderPaymentSaveObserver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\OfflinePayments\Observer;

use Magento\Framework\Event\Observer;
use Magento\Framework\Event\ObserverInterface;
use Magento\OfflinePayments\Model\Banktransfer;
use Magento\OfflinePayments\Model\Cashondelivery;
Expand All @@ -19,10 +21,10 @@ class BeforeOrderPaymentSaveObserver implements ObserverInterface
/**
* Sets current instructions for bank transfer account
*
* @param \Magento\Framework\Event\Observer $observer
* @param Observer $observer
* @return void
*/
public function execute(\Magento\Framework\Event\Observer $observer)
public function execute(Observer $observer)
{
/** @var \Magento\Sales\Model\Order\Payment $payment */
$payment = $observer->getEvent()->getPayment();
Expand All @@ -34,15 +36,22 @@ public function execute(\Magento\Framework\Event\Observer $observer)
&& empty($payment->getAdditionalInformation('instructions'))) {
$payment->setAdditionalInformation(
'instructions',
$payment->getMethodInstance()->getInstructions()
$payment->getMethodInstance()->getConfigData(
'instructions',
$payment->getOrder()->getStoreId()
)
);
} elseif ($payment->getMethod() === Checkmo::PAYMENT_METHOD_CHECKMO_CODE) {
$methodInstance = $payment->getMethodInstance();
if (!empty($methodInstance->getPayableTo())) {
$payment->setAdditionalInformation('payable_to', $methodInstance->getPayableTo());
$storeId = $payment->getOrder()->getStoreId();

$payableTo = $methodInstance->getConfigData('payable_to', $storeId);
if (!empty($payableTo)) {
$payment->setAdditionalInformation('payable_to', $payableTo);
}
if (!empty($methodInstance->getMailingAddress())) {
$payment->setAdditionalInformation('mailing_address', $methodInstance->getMailingAddress());
$mailingAddress = $methodInstance->getConfigData('mailing_address', $storeId);
if (!empty($mailingAddress)) {
$payment->setAdditionalInformation('mailing_address', $mailingAddress);
}
}
}
Expand Down
130 changes: 80 additions & 50 deletions app/code/Magento/OfflinePayments/Test/Unit/Observer/BeforeOrderPaymentSaveObserverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,87 +10,110 @@
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use Magento\OfflinePayments\Model\Banktransfer;
use Magento\OfflinePayments\Model\Cashondelivery;
use Magento\OfflinePayments\Model\Checkmo;
use Magento\OfflinePayments\Observer\BeforeOrderPaymentSaveObserver;
use Magento\Sales\Model\Order;
use Magento\Sales\Model\Order\Payment;
use PHPUnit_Framework_MockObject_MockObject as MockObject;
use Magento\OfflinePayments\Model\Checkmo;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

class BeforeOrderPaymentSaveObserverTest extends \PHPUnit\Framework\TestCase
/**
* Test class for \Magento\OfflinePayments\Observer\BeforeOrderPaymentSaveObserver
*/
class BeforeOrderPaymentSaveObserverTest extends TestCase
{
private const STORE_ID = 1;

/**
* @var BeforeOrderPaymentSaveObserver
*/
protected $_model;
private $model;

/**
* @var Payment|MockObject
*/
private $payment;
private $paymentMock;

/**
* @var Event|MockObject
*/
private $event;
private $eventMock;

/**
* @var Observer|MockObject
*/
private $observer;
private $observerMock;

/**
* @var Order|MockObject
*/
private $orderMock;

/**
* @inheritdoc
*/
protected function setUp()
{
$objectManagerHelper = new ObjectManager($this);
$this->payment = $this->getMockBuilder(Payment::class)
$this->paymentMock = $this->getMockBuilder(Payment::class)
->disableOriginalConstructor()
->getMock();

$this->event = $this->getMockBuilder(Event::class)
$this->eventMock = $this->getMockBuilder(Event::class)
->disableOriginalConstructor()
->setMethods(['getPayment'])
->getMock();

$this->event->expects(self::once())
$this->eventMock->expects(self::once())
->method('getPayment')
->willReturn($this->payment);
->willReturn($this->paymentMock);

$this->observer = $this->getMockBuilder(Observer::class)
$this->observerMock = $this->getMockBuilder(Observer::class)
->disableOriginalConstructor()
->getMock();

$this->observer->expects(self::once())
$this->observerMock->expects(self::once())
->method('getEvent')
->willReturn($this->event);
->willReturn($this->eventMock);

$this->orderMock = $this->getMockBuilder(Order::class)
->disableOriginalConstructor()
->getMock();
$this->orderMock->method('getStoreId')
->willReturn(static::STORE_ID);

$this->_model = $objectManagerHelper->getObject(BeforeOrderPaymentSaveObserver::class);
$this->paymentMock->method('getOrder')
->willReturn($this->orderMock);

$this->model = $objectManagerHelper->getObject(BeforeOrderPaymentSaveObserver::class);
}

/**
* Checks a case when payment method is either bank transfer or cash on delivery
* @param string $methodCode
* @dataProvider dataProviderBeforeOrderPaymentSaveWithInstructions
*/
public function testBeforeOrderPaymentSaveWithInstructions($methodCode)
{
$this->payment->expects(self::once())
$this->paymentMock->expects(self::once())
->method('getMethod')
->willReturn($methodCode);
$this->payment->expects(self::once())
$this->paymentMock->expects(self::once())
->method('setAdditionalInformation')
->with('instructions', 'payment configuration');
$method = $this->getMockBuilder(Banktransfer::class)
->disableOriginalConstructor()
->getMock();

$method->expects(self::once())
->method('getInstructions')
->method('getConfigData')
->with('instructions', static::STORE_ID)
->willReturn('payment configuration');
$this->payment->expects(self::once())
$this->paymentMock->expects(self::once())
->method('getMethodInstance')
->willReturn($method);

$this->_model->execute($this->observer);
$this->model->execute($this->observerMock);
}

/**
Expand All @@ -106,33 +129,37 @@ public function dataProviderBeforeOrderPaymentSaveWithInstructions()
];
}

/**
* Checks a case when payment method is Check Money
*/
public function testBeforeOrderPaymentSaveWithCheckmo()
{
$this->payment->expects(self::exactly(2))
$this->paymentMock->expects(self::exactly(2))
->method('getMethod')
->willReturn(Checkmo::PAYMENT_METHOD_CHECKMO_CODE);
$this->payment->expects(self::exactly(2))
$this->paymentMock->expects(self::exactly(2))
->method('setAdditionalInformation')
->willReturnMap(
[
['payable_to', 'payable to', $this->payment],
['mailing_address', 'mailing address', $this->payment],
['payable_to', 'payable to', $this->paymentMock],
['mailing_address', 'mailing address', $this->paymentMock],
]
);

$method = $this->getMockBuilder(Checkmo::class)
->disableOriginalConstructor()
->getMock();
$method->expects(self::exactly(2))
->method('getPayableTo')
->willReturn('payable to');
$method->expects(self::exactly(2))
->method('getMailingAddress')
->willReturn('mailing address');
$this->payment->expects(self::once())
$method->method('getConfigData')
->willReturnMap(
[
['payable_to', static::STORE_ID, 'payable to'],
['mailing_address', static::STORE_ID, 'mailing address']
]
);
$this->paymentMock->expects(self::once())
->method('getMethodInstance')
->willReturn($method);
$this->_model->execute($this->observer);
$this->model->execute($this->observerMock);
}

/**
Expand All @@ -141,36 +168,40 @@ public function testBeforeOrderPaymentSaveWithCheckmo()
*/
public function testBeforeOrderPaymentSaveWithCheckmoWithoutConfig()
{
$this->payment->expects(self::exactly(2))
$this->paymentMock->expects(self::exactly(2))
->method('getMethod')
->willReturn(Checkmo::PAYMENT_METHOD_CHECKMO_CODE);
$this->payment->expects(self::never())
$this->paymentMock->expects(self::never())
->method('setAdditionalInformation');

$method = $this->getMockBuilder(Checkmo::class)
->disableOriginalConstructor()
->getMock();
$method->expects(self::once())
->method('getPayableTo')
->willReturn(null);
$method->expects(self::once())
->method('getMailingAddress')
->willReturn(null);
$this->payment->expects(self::once())
$method->method('getConfigData')
->willReturnMap(
[
['payable_to', static::STORE_ID, null],
['mailing_address', static::STORE_ID, null]
]
);
$this->paymentMock->expects(self::once())
->method('getMethodInstance')
->willReturn($method);
$this->_model->execute($this->observer);
$this->model->execute($this->observerMock);
}

/**
* Checks a case with payment method not handled by observer
*/
public function testBeforeOrderPaymentSaveWithOthers()
{
$this->payment->expects(self::exactly(2))
$this->paymentMock->expects(self::exactly(2))
->method('getMethod')
->willReturn('somepaymentmethod');
$this->payment->expects(self::never())
$this->paymentMock->expects(self::never())
->method('setAdditionalInformation');

$this->_model->execute($this->observer);
$this->model->execute($this->observerMock);
}

/**
Expand All @@ -179,17 +210,16 @@ public function testBeforeOrderPaymentSaveWithOthers()
*/
public function testBeforeOrderPaymentSaveWithInstructionsAlreadySet($methodCode)
{
$this->payment
->method('getMethod')
$this->paymentMock->method('getMethod')
->willReturn($methodCode);

$this->payment->expects(self::once())
$this->paymentMock->expects(self::once())
->method('getAdditionalInformation')
->willReturn('Test');

$this->payment->expects(self::never())
$this->paymentMock->expects(self::never())
->method('setAdditionalInformation');

$this->_model->execute($this->observer);
$this->model->execute($this->observerMock);
}
}