Skip to content

Commit

Permalink
MAGETWO-59267: Issue magento#6634 - Boolean Attribute Display
Browse files Browse the repository at this point in the history
Per the discussion here:
magento#6634

I found when I implemented the proposed fix that was implemented in this file in my own Magento installation, all boolean attributes were returned even if they were not assigned to a product and had no value in the database. So my product pages were full of NULLs and had a large number of non-relevant attributes listed because of the large number of custom attributes we are utilizing for our product catalog. The small change I have made here allows attributes that have been assigned a value to be displayed, but non-relevant/unset attributes are not, therefore I feel it is a much cleaner solution to the problem.
  • Loading branch information
usesi-jlambert committed Sep 5, 2017
1 parent df7d1c7 commit 4c188e7
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 1 deletion.
4 changes: 3 additions & 1 deletion app/code/Magento/Catalog/Block/Product/View/Attributes.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,15 @@ public function getAdditionalData(array $excludeAttr = [])

if (!$product->hasData($attribute->getAttributeCode())) {
$value = __('N/A');
} elseif ($value instanceof Phrase) {
$value = (string)$value;
} elseif ((string)$value == '') {
$value = __('No');
} elseif ($attribute->getFrontendInput() == 'price' && is_string($value)) {
$value = $this->priceCurrency->convertAndFormat($value);
}

if ($value instanceof Phrase || (is_string($value) && strlen($value))) {
if (is_string($value) && strlen($value)) {
$data[$attribute->getAttributeCode()] = [
'label' => __($attribute->getStoreLabel()),
'value' => $value,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Catalog\Test\Unit\Block\Product\View;

use \PHPUnit\Framework\TestCase;
use \Magento\Framework\Phrase;
use \Magento\Eav\Model\Entity\Attribute\AbstractAttribute;
use \Magento\Eav\Model\Entity\Attribute\Frontend\AbstractFrontend;
use \Magento\Catalog\Model\Product;
use \Magento\Framework\View\Element\Template\Context;
use \Magento\Framework\Registry;
use \Magento\Framework\Pricing\PriceCurrencyInterface;
use \Magento\Catalog\Block\Product\View\Attributes as AttributesBlock;

/**
* Test class for \Magento\Catalog\Block\Product\View\Attributes
*
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class AttributesTest extends TestCase
{
/**
* @var \Magento\Framework\Phrase
*/
private $phrase;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Eav\Model\Entity\Attribute\AbstractAttribute
*/
private $attribute;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Eav\Model\Entity\Attribute\Frontend\AbstractFrontend
*/
private $frontendAttribute;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Catalog\Model\Product
*/
private $product;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\View\Element\Template\Context
*/
private $context;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Registry
*/
private $registry;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Pricing\PriceCurrencyInterface
*/
private $priceCurrencyInterface;

/**
* @var \Magento\Catalog\Block\Product\View\Attributes
*/
private $attributesBlock;

protected function setUp()
{
$this->phrase = new Phrase(__(''));

$this->attribute = $this
->getMockBuilder(AbstractAttribute::class)
->disableOriginalConstructor()
->getMock();

$this->attribute
->expects($this->any())
->method('getIsVisibleOnFront')
->willReturn(true);

$this->attribute
->expects($this->any())
->method('getAttributeCode')
->willReturn('phrase');

$this->frontendAttribute = $this
->getMockBuilder(AbstractFrontend::class)
->disableOriginalConstructor()
->getMock();

$this->attribute
->expects($this->any())
->method('getFrontendInput')
->willReturn('phrase');

$this->attribute
->expects($this->any())
->method('getFrontend')
->willReturn($this->frontendAttribute);

$this->product = $this
->getMockBuilder(Product::class)
->disableOriginalConstructor()
->getMock();

$this->product
->expects($this->any())
->method('getAttributes')
->willReturn([$this->attribute]);

$this->product
->expects($this->any())
->method('hasData')
->willReturn(true);

$this->context = $this
->getMockBuilder(Context::class)
->disableOriginalConstructor()
->getMock();

$this->registry = $this
->getMockBuilder(Registry::class)
->disableOriginalConstructor()
->getMock();

$this->registry
->expects($this->any())
->method('registry')
->willReturn($this->product);

$this->priceCurrencyInterface = $this
->getMockBuilder(PriceCurrencyInterface::class)
->disableOriginalConstructor()
->getMock();

$this->attributesBlock = new AttributesBlock(
$this->context,
$this->registry,
$this->priceCurrencyInterface
);
}

/**
* @return void
*/
public function testGetAttributesBooleanNoValue()
{
$this->phrase = new Phrase(__(''));

$this->frontendAttribute
->expects($this->any())
->method('getValue')
->willReturn($this->phrase);

$attributes = $this->attributesBlock->getAdditionalData();

$this->assertTrue(empty($attributes['phrase']));
}

/**
* @return void
*/
public function testGetAttributesBooleanHasValue()
{
$this->phrase = new Phrase(__('Yes'));

$this->frontendAttribute
->expects($this->any())
->method('getValue')
->willReturn($this->phrase);

$attributes = $this->attributesBlock->getAdditionalData();

$this->assertNotTrue(empty($attributes['phrase']));
$this->assertNotTrue(empty($attributes['phrase']['value']));
$this->assertEquals('Yes', $attributes['phrase']['value']);
}
}

0 comments on commit 4c188e7

Please sign in to comment.