From 4c188e706cc4d5523162ea66295e33add1bf98e5 Mon Sep 17 00:00:00 2001
From: usesi-jlambert <jlambert@usesi.com>
Date: Tue, 22 Aug 2017 12:54:55 -0400
Subject: [PATCH] MAGETWO-59267: Issue #6634 - Boolean Attribute Display

Per the discussion here:
https://github.com/magento/magento2/issues/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.
---
 .../Catalog/Block/Product/View/Attributes.php |   4 +-
 .../Block/Product/View/AttributesTest.php     | 176 ++++++++++++++++++
 2 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 app/code/Magento/Catalog/Test/Unit/Block/Product/View/AttributesTest.php

diff --git a/app/code/Magento/Catalog/Block/Product/View/Attributes.php b/app/code/Magento/Catalog/Block/Product/View/Attributes.php
index 52039e23e0335..88416786dee7a 100644
--- a/app/code/Magento/Catalog/Block/Product/View/Attributes.php
+++ b/app/code/Magento/Catalog/Block/Product/View/Attributes.php
@@ -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,
diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/AttributesTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/AttributesTest.php
new file mode 100644
index 0000000000000..902cd80f542e7
--- /dev/null
+++ b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/AttributesTest.php
@@ -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']);
+    }
+}