ENGCOM-3005: [Backport] Fixes from #15947 #18113

Author: Stanislav Idolov

app/code/Magento/NewRelicReporting/Model/Config.php

@@ -5,6 +5,9 @@
  */
 namespace Magento\NewRelicReporting\Model;
 
+/**
+ * NewRelic configuration model
+ */
 class Config
 {
     /**#@+

app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php

@@ -3,6 +3,8 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\NewRelicReporting\Plugin;
 
 use Magento\Framework\App\State;
@@ -11,6 +13,9 @@
 use Magento\NewRelicReporting\Model\NewRelicWrapper;
 use Psr\Log\LoggerInterface;
 
+/**
+ * Handles setting which, when enabled, reports frontend and adminhtml as separate apps to New Relic.
+ */
 class StatePlugin
 {
     /**
@@ -31,6 +36,7 @@ class StatePlugin
     /**
      * @param Config $config
      * @param NewRelicWrapper $newRelicWrapper
+     * @param LoggerInterface $logger
      */
     public function __construct(
         Config $config,
@@ -46,32 +52,33 @@ public function __construct(
      * Set separate appname
      *
      * @param State $subject
-     * @param null $result
-     * @return void
-     *
-     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     * @param mixed $result
+     * @return mixed
      */
-    public function afterSetAreaCode(State $state, $result)
+    public function afterSetAreaCode(State $subject, $result)
     {
         if (!$this->shouldSetAppName()) {
             return $result;
         }
 
         try {
-            $this->newRelicWrapper->setAppName($this->appName($state));
+            $this->newRelicWrapper->setAppName($this->appName($subject));
         } catch (LocalizedException $e) {
             $this->logger->critical($e);
             return $result;
         }
+
+        return $result;
     }
 
     /**
-     * @param State $state
+     * Format appName.
      *
+     * @param State $state
      * @return string
      * @throws LocalizedException
      */
-    private function appName(State $state)
+    private function appName(State $state): string
     {
         $code = $state->getAreaCode();
         $current = $this->config->getNewRelicAppName();
@@ -80,22 +87,16 @@ private function appName(State $state)
     }
 
     /**
+     * Check if app name should be set.
+     *
      * @return bool
      */
-    private function shouldSetAppName()
+    private function shouldSetAppName(): bool
     {
-        if (!$this->config->isNewRelicEnabled()) {
-            return false;
-        }
-
-        if (!$this->config->getNewRelicAppName()) {
-            return false;
-        }
-
-        if (!$this->config->isSeparateApps()) {
-            return false;
-        }
-
-        return true;
+        return (
+            $this->config->isSeparateApps() &&
+            $this->config->getNewRelicAppName() &&
+            $this->config->isNewRelicEnabled()
+        );
     }
 }

dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php

@@ -3,20 +3,28 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\NewRelicReporting\Plugin;
 
 use Magento\Framework\App\State;
 use Magento\NewRelicReporting\Model\NewRelicWrapper;
 use Magento\TestFramework\ObjectManager;
 use Magento\TestFramework\Helper\Bootstrap;
 
+/**
+ * Class SeparateAppsTest
+ */
 class SeparateAppsTest extends \PHPUnit\Framework\TestCase
 {
     /**
      * @var ObjectManager
      */
     private $objectManager;
 
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
         $this->objectManager = Bootstrap::getObjectManager();
@@ -44,4 +52,25 @@ public function testAppNameIsSetWhenConfiguredCorrectly()
 
         $state->setAreaCode('90210');
     }
+
+    /**
+     * @magentoConfigFixture default/newrelicreporting/general/enable 1
+     * @magentoConfigFixture default/newrelicreporting/general/app_name beverly_hills
+     * @magentoConfigFixture default/newrelicreporting/general/separate_apps 0
+     */
+    public function testAppNameIsNotSetWhenDisabled()
+    {
+        $newRelicWrapper = $this->getMockBuilder(NewRelicWrapper::class)
+            ->setMethods(['setAppName'])
+            ->getMock();
+
+        $this->objectManager->configure([NewRelicWrapper::class => ['shared' => true]]);
+        $this->objectManager->addSharedInstance($newRelicWrapper, NewRelicWrapper::class);
+
+        $newRelicWrapper->expects($this->never())->method('setAppName');
+
+        $state = $this->objectManager->get(State::class);
+
+        $state->setAreaCode('90210');
+    }
 }