Skip to content

Commit 4c188e7

Browse files
usesi-jlambertusesi-jlambert
committed
MAGETWO-59267: Issue #6634 - Boolean Attribute Display
Per the discussion here: #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.
1 parent df7d1c7 commit 4c188e7

File tree

2 files changed

+179
-1
lines changed

2 files changed

+179
-1
lines changed

‎app/code/Magento/Catalog/Block/Product/View/Attributes.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,15 @@ public function getAdditionalData(array $excludeAttr = [])
8484

8585
if (!$product->hasData($attribute->getAttributeCode())) {
8686
$value = __('N/A');
87+
} elseif ($value instanceof Phrase) {
88+
$value = (string)$value;
8789
} elseif ((string)$value == '') {
8890
$value = __('No');
8991
} elseif ($attribute->getFrontendInput() == 'price' && is_string($value)) {
9092
$value = $this->priceCurrency->convertAndFormat($value);
9193
}
9294

93-
if ($value instanceof Phrase || (is_string($value) && strlen($value))) {
95+
if (is_string($value) && strlen($value)) {
9496
$data[$attribute->getAttributeCode()] = [
9597
'label' => __($attribute->getStoreLabel()),
9698
'value' => $value,

‎app/code/Magento/Catalog/Test/Unit/Block/Product/View/AttributesTest.php

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Catalog\Test\Unit\Block\Product\View;
7+
8+
use \PHPUnit\Framework\TestCase;
9+
use \Magento\Framework\Phrase;
10+
use \Magento\Eav\Model\Entity\Attribute\AbstractAttribute;
11+
use \Magento\Eav\Model\Entity\Attribute\Frontend\AbstractFrontend;
12+
use \Magento\Catalog\Model\Product;
13+
use \Magento\Framework\View\Element\Template\Context;
14+
use \Magento\Framework\Registry;
15+
use \Magento\Framework\Pricing\PriceCurrencyInterface;
16+
use \Magento\Catalog\Block\Product\View\Attributes as AttributesBlock;
17+
18+
/**
19+
* Test class for \Magento\Catalog\Block\Product\View\Attributes
20+
*
21+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
22+
*/
23+
class AttributesTest extends TestCase
24+
{
25+
/**
26+
* @var \Magento\Framework\Phrase
27+
*/
28+
private $phrase;
29+
30+
/**
31+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Eav\Model\Entity\Attribute\AbstractAttribute
32+
*/
33+
private $attribute;
34+
35+
/**
36+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Eav\Model\Entity\Attribute\Frontend\AbstractFrontend
37+
*/
38+
private $frontendAttribute;
39+
40+
/**
41+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Catalog\Model\Product
42+
*/
43+
private $product;
44+
45+
/**
46+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\View\Element\Template\Context
47+
*/
48+
private $context;
49+
50+
/**
51+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Registry
52+
*/
53+
private $registry;
54+
55+
/**
56+
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Pricing\PriceCurrencyInterface
57+
*/
58+
private $priceCurrencyInterface;
59+
60+
/**
61+
* @var \Magento\Catalog\Block\Product\View\Attributes
62+
*/
63+
private $attributesBlock;
64+
65+
protected function setUp()
66+
{
67+
$this->phrase = new Phrase(__(''));
68+
69+
$this->attribute = $this
70+
->getMockBuilder(AbstractAttribute::class)
71+
->disableOriginalConstructor()
72+
->getMock();
73+
74+
$this->attribute
75+
->expects($this->any())
76+
->method('getIsVisibleOnFront')
77+
->willReturn(true);
78+
79+
$this->attribute
80+
->expects($this->any())
81+
->method('getAttributeCode')
82+
->willReturn('phrase');
83+
84+
$this->frontendAttribute = $this
85+
->getMockBuilder(AbstractFrontend::class)
86+
->disableOriginalConstructor()
87+
->getMock();
88+
89+
$this->attribute
90+
->expects($this->any())
91+
->method('getFrontendInput')
92+
->willReturn('phrase');
93+
94+
$this->attribute
95+
->expects($this->any())
96+
->method('getFrontend')
97+
->willReturn($this->frontendAttribute);
98+
99+
$this->product = $this
100+
->getMockBuilder(Product::class)
101+
->disableOriginalConstructor()
102+
->getMock();
103+
104+
$this->product
105+
->expects($this->any())
106+
->method('getAttributes')
107+
->willReturn([$this->attribute]);
108+
109+
$this->product
110+
->expects($this->any())
111+
->method('hasData')
112+
->willReturn(true);
113+
114+
$this->context = $this
115+
->getMockBuilder(Context::class)
116+
->disableOriginalConstructor()
117+
->getMock();
118+
119+
$this->registry = $this
120+
->getMockBuilder(Registry::class)
121+
->disableOriginalConstructor()
122+
->getMock();
123+
124+
$this->registry
125+
->expects($this->any())
126+
->method('registry')
127+
->willReturn($this->product);
128+
129+
$this->priceCurrencyInterface = $this
130+
->getMockBuilder(PriceCurrencyInterface::class)
131+
->disableOriginalConstructor()
132+
->getMock();
133+
134+
$this->attributesBlock = new AttributesBlock(
135+
$this->context,
136+
$this->registry,
137+
$this->priceCurrencyInterface
138+
);
139+
}
140+
141+
/**
142+
* @return void
143+
*/
144+
public function testGetAttributesBooleanNoValue()
145+
{
146+
$this->phrase = new Phrase(__(''));
147+
148+
$this->frontendAttribute
149+
->expects($this->any())
150+
->method('getValue')
151+
->willReturn($this->phrase);
152+
153+
$attributes = $this->attributesBlock->getAdditionalData();
154+
155+
$this->assertTrue(empty($attributes['phrase']));
156+
}
157+
158+
/**
159+
* @return void
160+
*/
161+
public function testGetAttributesBooleanHasValue()
162+
{
163+
$this->phrase = new Phrase(__('Yes'));
164+
165+
$this->frontendAttribute
166+
->expects($this->any())
167+
->method('getValue')
168+
->willReturn($this->phrase);
169+
170+
$attributes = $this->attributesBlock->getAdditionalData();
171+
172+
$this->assertNotTrue(empty($attributes['phrase']));
173+
$this->assertNotTrue(empty($attributes['phrase']['value']));
174+
$this->assertEquals('Yes', $attributes['phrase']['value']);
175+
}
176+
}

0 commit comments

Comments
 (0)