diff --git a/CRM/Core/Resources/CollectionAdderTrait.php b/CRM/Core/Resources/CollectionAdderTrait.php index 392cc4c55da9..469d1c426f88 100644 --- a/CRM/Core/Resources/CollectionAdderTrait.php +++ b/CRM/Core/Resources/CollectionAdderTrait.php @@ -99,6 +99,9 @@ public function addScript(string $code, ...$options) { public function addScriptFile(string $ext, string $file, ...$options) { $this->add(self::mergeStandardOptions($options, [ 'scriptFile' => [$ext, $file], + 'name' => "$ext:$file", + // Setting the name above may appear superfluous, but it preserves a historical quirk + // where Region::add() and Resources::addScriptFile() produce slightly different orderings. ])); return $this; } @@ -119,6 +122,9 @@ public function addScriptFile(string $ext, string $file, ...$options) { public function addScriptUrl(string $url, ...$options) { $this->add(self::mergeStandardOptions($options, [ 'scriptUrl' => $url, + 'name' => $url, + // Setting the name above may appear superfluous, but it preserves a historical quirk + // where Region::add() and Resources::addScriptUrl() produce slightly different orderings. ])); return $this; } @@ -215,6 +221,9 @@ public function addStyle(string $code, ...$options) { public function addStyleFile(string $ext, string $file, ...$options) { $this->add(self::mergeStandardOptions($options, [ 'styleFile' => [$ext, $file], + 'name' => "$ext:$file", + // Setting the name above may appear superfluous, but it preserves a historical quirk + // where Region::add() and Resources::addScriptUrl() produce slightly different orderings. ])); return $this; } @@ -235,6 +244,9 @@ public function addStyleFile(string $ext, string $file, ...$options) { public function addStyleUrl(string $url, ...$options) { $this->add(self::mergeStandardOptions($options, [ 'styleUrl' => $url, + 'name' => $url, + // Setting the name above may appear superfluous, but it preserves a historical quirk + // where Region::add() and Resources::addScriptUrl() produce slightly different orderings. ])); return $this; } diff --git a/CRM/Core/Resources/CollectionTrait.php b/CRM/Core/Resources/CollectionTrait.php index f750f01db077..4d97aeeced74 100644 --- a/CRM/Core/Resources/CollectionTrait.php +++ b/CRM/Core/Resources/CollectionTrait.php @@ -64,6 +64,7 @@ trait CRM_Core_Resources_CollectionTrait { */ public function add($snippet) { $snippet = array_merge($this->defaults, $snippet); + $snippet['id'] = $this->nextId(); if (!isset($snippet['type'])) { foreach ($this->types as $type) { // auto-detect @@ -85,18 +86,18 @@ public function add($snippet) { switch ($snippet['type']) { case 'scriptUrl': case 'styleUrl': - $snippet['sortId'] = $this->nextId(); + $snippet['sortId'] = $snippet['id']; $snippet['name'] = $snippet[$snippet['type']]; break; case 'scriptFile': case 'styleFile': - $snippet['sortId'] = $this->nextId(); + $snippet['sortId'] = $snippet['id']; $snippet['name'] = implode(':', $snippet[$snippet['type']]); break; default: - $snippet['sortId'] = $this->nextId(); + $snippet['sortId'] = $snippet['id']; $snippet['name'] = $snippet['sortId']; break; } diff --git a/tests/phpunit/CRM/Core/Resources/CollectionTestTrait.php b/tests/phpunit/CRM/Core/Resources/CollectionTestTrait.php index 14e8942350cb..61787dfd4845 100644 --- a/tests/phpunit/CRM/Core/Resources/CollectionTestTrait.php +++ b/tests/phpunit/CRM/Core/Resources/CollectionTestTrait.php @@ -42,7 +42,7 @@ public function getSnippetExamples() { }; $addCases( - // List of equivalent method calls + // List of equivalent method calls [ 'add(scriptUrl): dfl' => ['add', ['scriptUrl' => 'http://example.com/foo.js']], 'addScriptUrl(): dfl' => ['addScriptUrl', 'http://example.com/foo.js'], @@ -53,12 +53,22 @@ public function getSnippetExamples() { 'name' => 'http://example.com/foo.js', 'disabled' => FALSE, 'weight' => 1, - 'sortId' => 1, 'type' => 'scriptUrl', 'scriptUrl' => 'http://example.com/foo.js', ] ); + // For historical reasons, the `add(scriptUrl)` and `addScriptUrl()` calls + // differ very slightly in how data is ordered. + $addCases( + ['add(scriptUrl): dfl: sortId' => ['add', ['scriptUrl' => 'http://example.com/foo.js']]], + ['sortId' => ($this instanceof CRM_Core_RegionTest ? 2 : 1)] + ); + $addCases( + ['addScriptUrl(): dfl: sortId' => ['addScriptUrl', 'http://example.com/foo.js']], + ['sortId' => 'http://example.com/foo.js'] + ); + $addCases( [ 'add(scriptUrl): wgt' => ['add', ['scriptUrl' => 'http://example.com/foo.js', 'weight' => 100]], @@ -69,7 +79,6 @@ public function getSnippetExamples() { 'name' => 'http://example.com/foo.js', 'disabled' => FALSE, 'weight' => 100, - 'sortId' => 1, 'type' => 'scriptUrl', 'scriptUrl' => 'http://example.com/foo.js', ] @@ -84,7 +93,6 @@ public function getSnippetExamples() { 'name' => 'http://example.com/foo.css', 'disabled' => FALSE, 'weight' => 1, - 'sortId' => 1, 'type' => 'styleUrl', 'styleUrl' => 'http://example.com/foo.css', ] @@ -99,7 +107,6 @@ public function getSnippetExamples() { 'name' => 'civicrm:css/civicrm.css', 'disabled' => FALSE, 'weight' => 1, - 'sortId' => 1, 'type' => 'styleFile', 'styleFile' => ['civicrm', 'css/civicrm.css'], 'styleFileUrls' => [ @@ -111,7 +118,6 @@ public function getSnippetExamples() { $basicFooJs = [ 'name' => 'civicrm:js/foo.js', 'disabled' => FALSE, - 'sortId' => 1, 'type' => 'scriptFile', 'scriptFile' => ['civicrm', 'js/foo.js'], 'scriptFileUrls' => [ @@ -153,10 +159,10 @@ public function getSnippetExamples() { 'addScript()' => ['addScript', 'window.alert("Boo!");'], ], [ - 'name' => 1, + // Regions always have a 'default' with ID#1, so our test always becomes #2. + 'name' => ($this instanceof CRM_Core_RegionTest ? 2 : 1), 'disabled' => FALSE, 'weight' => 1, - 'sortId' => 1, 'type' => 'script', 'script' => 'window.alert("Boo!");', ] @@ -321,17 +327,17 @@ public function assertSameSnippet($expect, $actual, $message = '') { return $snippet; }; - // If there isn't an explicit expectation for 'region', then we won't check it. - if (!isset($expect['region']) || '*' === $expect['region']) { - if (isset($actual['region'])) { - unset($expect['region']); - unset($actual['region']); - } - } - $expect = $normalizeSnippet($expect); $actual = $normalizeSnippet($actual); - $this->assertEquals($expect, $actual, $message); + + foreach ($expect as $expectKey => $expectValue) { + if ($expectValue === '*') { + $this->assertTrue(!empty($actual[$expectKey])); + } + else { + $this->assertEquals($expectValue, $actual[$expectKey]); + } + } } }