Skip to content

Commit

Permalink
Merge pull request #466 from magento-south/BUGS
Browse files Browse the repository at this point in the history
[South] Bug fixes
- MAGETWO-56529 Magento\Framework\Session\SaveHandlerTest test failed
- MAGETWO-55794 [FT] Product is not assigned to category
- MAGETWO-56370 [GITHUB] Broken File Type Custom Option #5434
- MAGETWO-59146 [Github] Disable email communication set to yes: email did get sent. #5988
- MAGETWO-56871 [GITHUB] Non-admin do not have permission to search for Categories in Cart Price Rules #6168
  • Loading branch information
slavvka authored Oct 6, 2016
2 parents a9510d3 + 53be3cc commit 4d69ecb
Show file tree
Hide file tree
Showing 18 changed files with 401 additions and 84 deletions.
18 changes: 14 additions & 4 deletions app/code/Magento/Catalog/Model/Product/Option/Type/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
namespace Magento\Catalog\Model\Product\Option\Type;

use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\Filesystem;
use Magento\Framework\Exception\LocalizedException;
use Magento\Catalog\Model\Product\Exception as ProductException;
Expand Down Expand Up @@ -69,17 +70,23 @@ class File extends \Magento\Catalog\Model\Product\Option\Type\DefaultType
*/
protected $validatorFile;

/**
* @var Filesystem
*/
private $filesystem;

/**
* @param \Magento\Checkout\Model\Session $checkoutSession
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
* @param \Magento\Quote\Model\Quote\Item\OptionFactory $itemOptionFactory
* @param \Magento\Catalog\Model\Product\Option\UrlBuilder $urlBuilder
* @param \Magento\Framework\Escaper $escaper
* @param \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDatabase
* @param File\ValidatorInfo $validatorInfo
* @param File\ValidatorFile $validatorFile
* @param \Magento\Catalog\Model\Product\Option\UrlBuilder $urlBuilder
* @param \Magento\Framework\Escaper $escaper
* @param array $data
* @throws \Magento\Framework\Exception\FileSystemException
* @param Filesystem $filesystem
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
\Magento\Checkout\Model\Session $checkoutSession,
Expand All @@ -90,12 +97,15 @@ public function __construct(
\Magento\Catalog\Model\Product\Option\Type\File\ValidatorFile $validatorFile,
\Magento\Catalog\Model\Product\Option\UrlBuilder $urlBuilder,
\Magento\Framework\Escaper $escaper,
array $data = []
array $data = [],
Filesystem $filesystem = null
) {
$this->_itemOptionFactory = $itemOptionFactory;
$this->_urlBuilder = $urlBuilder;
$this->_escaper = $escaper;
$this->_coreFileStorageDatabase = $coreFileStorageDatabase;
$this->filesystem = $filesystem ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Filesystem::class);
$this->_rootDirectory = $this->filesystem->getDirectoryRead(DirectoryList::MEDIA);
$this->validatorInfo = $validatorInfo;
$this->validatorFile = $validatorFile;
parent::__construct($checkoutSession, $scopeConfig, $data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
*/
namespace Magento\Catalog\Test\Unit\Model\Product\Option\Type;

use Magento\Catalog\Model\Product\Configuration\Item\Option\OptionInterface;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\Filesystem;
use Magento\Framework\Filesystem\Directory\ReadInterface;
use Magento\Framework\Filesystem\DriverPool;

class FileTest extends \PHPUnit_Framework_TestCase
{
/**
Expand All @@ -13,7 +19,7 @@ class FileTest extends \PHPUnit_Framework_TestCase
protected $objectManager;

/**
* @var \Magento\Framework\Filesystem\Directory\ReadInterface|\PHPUnit_Framework_MockObject_MockObject
* @var ReadInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $rootDirectory;

Expand All @@ -22,14 +28,26 @@ class FileTest extends \PHPUnit_Framework_TestCase
*/
protected $coreFileStorageDatabase;

/**
* @var Filesystem|\PHPUnit_Framework_MockObject_MockObject
*/
private $filesystemMock;

protected function setUp()
{
$this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);

$this->rootDirectory = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\ReadInterface::class)
$this->filesystemMock = $this->getMockBuilder(Filesystem::class)
->disableOriginalConstructor()
->setMethods(['isFile', 'isReadable', 'getAbsolutePath'])
->getMockForAbstractClass();
->getMock();

$this->rootDirectory = $this->getMockBuilder(ReadInterface::class)
->getMock();

$this->filesystemMock->expects($this->once())
->method('getDirectoryRead')
->with(DirectoryList::MEDIA, DriverPool::FILE)
->willReturn($this->rootDirectory);

$this->coreFileStorageDatabase = $this->getMock(
\Magento\MediaStorage\Helper\File\Storage\Database::class,
Expand All @@ -48,26 +66,27 @@ protected function getFileObject()
return $this->objectManager->getObject(
\Magento\Catalog\Model\Product\Option\Type\File::class,
[
'saleableItem' => $this->rootDirectory,
'priceCurrency' => $this->coreFileStorageDatabase
'filesystem' => $this->filesystemMock,
'coreFileStorageDatabase' => $this->coreFileStorageDatabase
]
);
}

public function testCopyQuoteToOrder()
{
$optionMock = $this->getMockBuilder(
\Magento\Catalog\Model\Product\Configuration\Item\Option\OptionInterface::class
)->disableOriginalConstructor()->setMethods(['getValue'])->getMockForAbstractClass();
$optionMock = $this->getMockBuilder(OptionInterface::class)
->disableOriginalConstructor()
->setMethods(['getValue'])
->getMockForAbstractClass();

$quotePath = '/quote/path/path/uploaded.file';
$orderPath = '/order/path/path/uploaded.file';

$optionMock->expects($this->any())
->method('getValue')
->will($this->returnValue(['quote_path' => $quotePath, 'order_path' => $orderPath]));
->will($this->returnValue(serialize(['quote_path' => $quotePath, 'order_path' => $orderPath])));

$this->rootDirectory->expects($this->any())
$this->rootDirectory->expects($this->once())
->method('isFile')
->with($this->equalTo($quotePath))
->will($this->returnValue(true));
Expand Down
3 changes: 3 additions & 0 deletions app/code/Magento/Catalog/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -834,4 +834,7 @@
<argument name="collectionProcessor" xsi:type="object">Magento\Eav\Model\Api\SearchCriteria\CollectionProcessor</argument>
</arguments>
</type>
<type name="Magento\Quote\Model\Quote\Item\ToOrderItem">
<plugin name="copy_quote_files_to_order" type="Magento\Catalog\Model\Plugin\QuoteItemProductOption"/>
</type>
</config>
3 changes: 0 additions & 3 deletions app/code/Magento/Catalog/etc/frontend/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
<argument name="fetchStrategy" xsi:type="object">Magento\Catalog\Model\ResourceModel\Category\Collection\FetchStrategy</argument>
</arguments>
</type>
<type name="Magento\Quote\Model\Quote\Item\ToOrderItem">
<plugin name="copy_quote_files_to_order" type="Magento\Catalog\Model\Plugin\QuoteItemProductOption"/>
</type>
<type name="Magento\Catalog\Model\Indexer\AbstractFlatState">
<arguments>
<argument name="isAvailable" xsi:type="boolean">true</argument>
Expand Down
51 changes: 51 additions & 0 deletions app/code/Magento/Email/Model/Mail/TransportInterfacePlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php
/**
* Copyright © 2016 Magento. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Email\Model\Mail;

use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\Exception\MailException;
use Magento\Framework\Mail\TransportInterface;
use Magento\Store\Model\ScopeInterface;

class TransportInterfacePlugin
{
/**
* Config path to mail sending setting that shows if email communications are disabled
*/
const XML_PATH_SYSTEM_SMTP_DISABLE = 'system/smtp/disable';

/**
* @var ScopeConfigInterface
*/
private $scopeConfig;

/**
* @param ScopeConfigInterface $scopeConfig
*/
public function __construct(
ScopeConfigInterface $scopeConfig
) {
$this->scopeConfig = $scopeConfig;
}

/**
* Omit email sending if disabled
*
* @param TransportInterface $subject
* @param \Closure $proceed
* @return void
* @throws MailException
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function aroundSendMessage(
TransportInterface $subject,
\Closure $proceed
) {
if (!$this->scopeConfig->isSetFlag(self::XML_PATH_SYSTEM_SMTP_DISABLE, ScopeInterface::SCOPE_STORE)) {
$proceed();
}
}
}
23 changes: 5 additions & 18 deletions app/code/Magento/Email/Model/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,11 @@
*/
namespace Magento\Email\Model;

use Magento\Store\Model\ScopeInterface;
use Magento\Store\Model\StoreManagerInterface;

/**
* Template model
*
* Example:
*
* // Loading of template
* \Magento\Email\Model\TemplateFactory $templateFactory
* $templateFactory->create()->load($this->_scopeConfig->getValue(
* 'path_to_email_template_id_config',
* \Magento\Store\Model\ScopeInterface::SCOPE_STORE
* ));
* $variables = array(
* 'someObject' => $this->_coreResourceEmailTemplate
* 'someString' => 'Some string value'
* );
* $emailTemplate->send('some@domain.com', 'Name Of User', $variables);
*
* @method \Magento\Email\Model\ResourceModel\Template _getResource()
* @method \Magento\Email\Model\ResourceModel\Template getResource()
* @method string getTemplateCode()
Expand Down Expand Up @@ -63,6 +48,8 @@ class Template extends AbstractTemplate implements \Magento\Framework\Mail\Templ

/**
* Config path to mail sending setting that shows if email communications are disabled
* @deprecated
* @see \Magento\Email\Model\Mail\TransportInterfacePlugin::XML_PATH_SYSTEM_SMTP_DISABLE
*/
const XML_PATH_SYSTEM_SMTP_DISABLE = 'system/smtp/disable';

Expand Down Expand Up @@ -196,8 +183,7 @@ public function setId($value)
*/
public function isValidForSend()
{
return !$this->scopeConfig->isSetFlag(Template::XML_PATH_SYSTEM_SMTP_DISABLE, ScopeInterface::SCOPE_STORE)
&& $this->getSenderName() && $this->getSenderEmail() && $this->getTemplateSubject();
return $this->getSenderName() && $this->getSenderEmail() && $this->getTemplateSubject();
}

/**
Expand Down Expand Up @@ -344,7 +330,8 @@ public function beforeSave()
if ($this->_getResource()->checkCodeUsage($this)) {
throw new \Magento\Framework\Exception\MailException(__('Duplicate Of Template Name'));
}
return parent::beforeSave();
parent::beforeSave();
return $this;
}

/**
Expand Down
19 changes: 1 addition & 18 deletions app/code/Magento/Email/Test/Unit/Model/TemplateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/
namespace Magento\Email\Test\Unit\Model;

use Magento\Email\Model\Template\Filter;
use Magento\Framework\App\Area;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\App\TemplateTypesInterface;
Expand Down Expand Up @@ -426,18 +425,13 @@ public function testGetAndSetId()
}

/**
* @param $isSMTPDisabled bool
* @param $senderName string
* @param $senderEmail string
* @param $templateSubject string
* @dataProvider isValidForSendDataProvider
*/
public function testIsValidForSend($isSMTPDisabled, $senderName, $senderEmail, $templateSubject, $expectedValue)
public function testIsValidForSend($senderName, $senderEmail, $templateSubject, $expectedValue)
{
$this->scopeConfig->expects($this->once())
->method('isSetFlag')
->with('system/smtp/disable', ScopeInterface::SCOPE_STORE)
->will($this->returnValue($isSMTPDisabled));
$model = $this->getModelMock(['getSenderName', 'getSenderEmail', 'getTemplateSubject']);
$model->expects($this->any())
->method('getSenderName')
Expand All @@ -455,35 +449,24 @@ public function isValidForSendDataProvider()
{
return [
'should be valid' => [
'isSMTPDisabled' => false,
'senderName' => 'sender name',
'senderEmail' => 'email@example.com',
'templateSubject' => 'template subject',
'expectedValue' => true
],
'no smtp so not valid' => [
'isSMTPDisabled' => true,
'senderName' => 'sender name',
'senderEmail' => 'email@example.com',
'templateSubject' => 'template subject',
'expectedValue' => false
],
'no sender name so not valid' => [
'isSMTPDisabled' => false,
'senderName' => '',
'senderEmail' => 'email@example.com',
'templateSubject' => 'template subject',
'expectedValue' => false
],
'no sender email so not valid' => [
'isSMTPDisabled' => false,
'senderName' => 'sender name',
'senderEmail' => '',
'templateSubject' => 'template subject',
'expectedValue' => false
],
'no subject so not valid' => [
'isSMTPDisabled' => false,
'senderName' => 'sender name',
'senderEmail' => 'email@example.com',
'templateSubject' => '',
Expand Down
1 change: 1 addition & 0 deletions app/code/Magento/Email/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@
</type>
<type name="Magento\Framework\Mail\TransportInterface">
<plugin name="WindowsSmtpConfig" type="Magento\Email\Model\Plugin\WindowsSmtpConfig" />
<plugin name="disableSending" type="Magento\Email\Model\Mail\TransportInterfacePlugin" />
</type>
</config>
8 changes: 3 additions & 5 deletions app/code/Magento/Newsletter/Model/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ public function validate()
public function beforeSave()
{
$this->validate();
return parent::beforeSave();
parent::beforeSave();
return $this;
}

/**
Expand Down Expand Up @@ -238,9 +239,6 @@ protected function getFilterFactory()
*/
public function isValidForSend()
{
return !$this->scopeConfig->isSetFlag(
\Magento\Email\Model\Template::XML_PATH_SYSTEM_SMTP_DISABLE,
\Magento\Store\Model\ScopeInterface::SCOPE_STORE
) && $this->getTemplateSenderName() && $this->getTemplateSenderEmail() && $this->getTemplateSubject();
return $this->getTemplateSenderName() && $this->getTemplateSenderEmail() && $this->getTemplateSubject();
}
}
Loading

0 comments on commit 4d69ecb

Please sign in to comment.