Updated: Comment #N

Problem/Motivation

We still have calls to \Drupal:: in code that would be otherwise cleanly unit testable.
This results in hacks like setting the container during a unit test.

Proposed resolution

Continue adoption of our pattern of wrapping the \Drupal:: call in a method that can be bypassed by setting a property.

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

CommentFileSizeAuthor
#8 allow_drupal_call_in-2183929-8.patch3.47 KBcilefen
#8 interdiff-5-8.txt1.05 KBcilefen
#5 allow_drupal_call_in-2183929-5.patch3.78 KBcilefen
#5 interdiff-4-5.txt1.78 KBcilefen
#4 allow_drupal_call_in-2183929-4.patch4.26 KBcilefen
block-container.patch4.29 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: tim.plunkett » Unassigned

It works, still applies.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented 
+++ b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestBlockInstantiation.php
@@ -64,4 +65,17 @@ public function build() {
+  }
...
+    return $this;
...
+    $this->transliteration = $transliteration;
...
+  public function setTransliteration(TransliterationInterface $transliteration) {
...
+   * @return $this
+   */
...
+   *
...
+   *   The transliteration service.
...
+   * @param \Drupal\Component\Transliteration\TransliterationInterface $transliteration
...
+  /**
+   * Sets the transliteration service.
+   *

Is there a reason to not provide this on the real block?

ParisLiakos’s picture

ParisLiakos commented
Status: Needs review » Needs work

agreed

cilefen’s picture

cilefen commented
Status: Needs work » Needs review
StatusFileSize
new allow_drupal_call_in-2183929-4.patch4.26 KB

reroll

cilefen’s picture

cilefen commented
StatusFileSize
new interdiff-4-5.txt1.78 KB
new allow_drupal_call_in-2183929-5.patch3.78 KB

Moved setTransliteration() to BlockBase.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
  1. +++ b/core/lib/Drupal/Core/Block/BlockBase.php
@@ -18,9 +18,10 @@
-use Drupal\Core\Language\LanguageInterface;
+use Drupal\Core\Language\Language;

@@ -407,8 +415,7 @@ public function getMachineNameSuggestion() {
-    $transliterated = $transliteration_service->transliterate($admin_label, LanguageInterface::LANGCODE_DEFAULT, '_');
+    $transliterated = $this->transliteration()->transliterate($admin_label, Language::LANGCODE_DEFAULT, '_');

    This should stay LanguageInterface 

  2. +++ b/core/lib/Drupal/Core/Block/BlockBase.php
@@ -532,5 +561,4 @@ protected function eventDispatcher() {
   }
-
 }

    We leave a blank line before the end of the class.

The last submitted patch, 4: allow_drupal_call_in-2183929-4.patch, failed testing.

cilefen’s picture

cilefen commented
StatusFileSize
new interdiff-5-8.txt1.05 KB
new allow_drupal_call_in-2183929-8.patch3.47 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

RTBC if green. Thanks @cilefen!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

This looks consistent with what we do elsewhere.

Committed and pushed to 8.x. Thanks!

  • webchick committed 2b24e72 on 8.0.x 
    Issue #2183929 by cilefen, tim.plunkett: Allow \Drupal:: call in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.