From 0aded46e5903558260a839cce649d6d74aca7e7f Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 25 Aug 2020 16:53:47 -0700
Subject: [PATCH] (REF) CRM_Core_Resources::addBundle() - Improve readability.
 Add test.

---
 CRM/Core/Resources.php                   | 24 +++++--------
 CRM/Core/Resources/Bundle.php            |  7 ++--
 tests/phpunit/CRM/Core/ResourcesTest.php | 45 ++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/CRM/Core/Resources.php b/CRM/Core/Resources.php
index 6d73cccb00..4a48798230 100644
--- a/CRM/Core/Resources.php
+++ b/CRM/Core/Resources.php
@@ -219,23 +219,17 @@ class CRM_Core_Resources implements CRM_Core_Resources_CollectionAdderInterface
     }
     $this->addedBundles[$bundle->name] = TRUE;
 
-    // If an item is already assigned to a region, we'll respect that.
-    // Otherwise, we'll use defaults.
-    $pickRegion = function ($snippet) {
-      if (isset($snippet['settings'])) {
-        return $this->getSettingRegion($snippet['region'] ?? NULL)->_name;
+    // Ensure that every asset has a region.
+    $bundle->filter(function($snippet) {
+      if (empty($snippet['region'])) {
+        $snippet['region'] = isset($snippet['settings'])
+          ? $this->getSettingRegion()->_name
+          : self::DEFAULT_REGION;
       }
-      else {
-        return $snippet['region'] ?? self::DEFAULT_REGION;
-      }
-    };
-
-    $byRegion = [];
-    foreach ($bundle->getAll() as $snippet) {
-      $snippet['region'] = $pickRegion($snippet);
-      $byRegion[$snippet['region']][$snippet['name']] = $snippet;
-    }
+      return $snippet;
+    });
 
+    $byRegion = CRM_Utils_Array::index(['region', 'name'], $bundle->getAll());
     foreach ($byRegion as $regionName => $snippets) {
       CRM_Core_Region::instance($regionName)->merge($snippets);
     }
diff --git a/CRM/Core/Resources/Bundle.php b/CRM/Core/Resources/Bundle.php
index 311dc2a05d..62c1a04269 100644
--- a/CRM/Core/Resources/Bundle.php
+++ b/CRM/Core/Resources/Bundle.php
@@ -29,10 +29,13 @@ class CRM_Core_Resources_Bundle implements CRM_Core_Resources_CollectionInterfac
   public $name;
 
   /**
+   * @param string|NULL $name
+   * @param string[]|NULL $types
+   *   List of resource-types to permit in this bundle. NULL for a default list.
    */
-  public function __construct($name = NULL) {
+  public function __construct($name = NULL, $types = NULL) {
     $this->name = $name;
-    $this->types = ['script', 'scriptFile', 'scriptUrl', 'settings', 'style', 'styleFile', 'styleUrl'];
+    $this->types = $types ?: ['script', 'scriptFile', 'scriptUrl', 'settings', 'style', 'styleFile', 'styleUrl'];
   }
 
 }
diff --git a/tests/phpunit/CRM/Core/ResourcesTest.php b/tests/phpunit/CRM/Core/ResourcesTest.php
index e3e8627bd8..2d726ab811 100644
--- a/tests/phpunit/CRM/Core/ResourcesTest.php
+++ b/tests/phpunit/CRM/Core/ResourcesTest.php
@@ -60,6 +60,51 @@ class CRM_Core_ResourcesTest extends CiviUnitTestCase {
     $_GET = $this->originalGet;
   }
 
+  /**
+   * Make two bundles (multi-regional). Add them to CRM_Core_Resources.
+   * Ensure that the resources land in the right regions.
+   */
+  public function testAddBundle() {
+    $foo = new CRM_Core_Resources_Bundle('foo', ['scriptUrl', 'styleUrl', 'markup']);
+    $bar = new CRM_Core_Resources_Bundle('bar', ['scriptUrl', 'styleUrl', 'markup']);
+
+    $foo->addScriptUrl('http://example.com/foo.js', 100, 'testAddBundle_foo');
+    $foo->add(['markup' => 'Hello, foo', 'region' => 'page-header']);
+    $bar->addScriptUrl('http://example.com/bar.js', 100, 'testAddBundle_bar');
+    $bar->add(['markup' => 'Hello, bar', 'region' => 'page-header']);
+    $foo->addStyleUrl('http://example.com/shoes.css');
+
+    $this->res->addBundle($foo);
+    $this->res->addBundle([$bar]);
+
+    $getPropsByRegion = function($region, $key) {
+      $props = [];
+      foreach (CRM_Core_Region::instance($region)->getAll() as $snippet) {
+        if (isset($snippet[$key])) {
+          $props[] = $snippet[$key];
+        }
+      }
+      return $props;
+    };
+
+    $this->assertEquals(
+      ['http://example.com/foo.js'],
+      $getPropsByRegion('testAddBundle_foo', 'scriptUrl')
+    );
+    $this->assertEquals(
+      ['http://example.com/bar.js'],
+      $getPropsByRegion('testAddBundle_bar', 'scriptUrl')
+    );
+    $this->assertEquals(
+      ['', 'Hello, foo', 'Hello, bar'],
+      $getPropsByRegion('page-header', 'markup')
+    );
+    $this->assertEquals(
+      ['http://example.com/shoes.css'],
+      $getPropsByRegion('page-footer', 'styleUrl')
+    );
+  }
+
   public function testAddScriptFile() {
     $this->res
       ->addScriptFile('com.example.ext', 'foo%20bar.js', 0, 'testAddScriptFile')
-- 
2.25.1