Problem/Motivation

drupal-check results on commit hash: 7649aa25d500c5ce3cd56d87f41fc143e9bfa323

 ------ --------------------------------------------------------------------------------------------------- 
  Line   src/EntityEmbedDisplay/EntityEmbedDisplayManager.php                                               
 ------ --------------------------------------------------------------------------------------------------- 
  32     config:entity_view_mode_list cache tag might be unclear and does not contain the cache key in it.  
 ------ --------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------- 
  Line   src/Form/EntityEmbedDialog.php                                                 
 ------ ------------------------------------------------------------------------------- 
  380    Call to deprecated method link() of class Drupal\Core\Entity\EntityInterface.  
  385    Call to deprecated method url() of class Drupal\Core\Entity\EntityInterface.   
  775    Call to deprecated method strlen() of class Drupal\Component\Utility\Unicode.  
 ------ ------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedDialogTest.php                                         
 ------ --------------------------------------------------------------------------------------- 
  28     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  30     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  34     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  39     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  48     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  54     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  57     Call to deprecated method assertFieldByName() of class Drupal\Tests\BrowserTestBase.   
  67     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  76     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  82     Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
  85     Call to deprecated method assertFieldByName() of class Drupal\Tests\BrowserTestBase.   
  88     Call to deprecated method assertFieldByXPath() of class Drupal\Tests\BrowserTestBase.  
  102    Call to deprecated method assertResponse() of class Drupal\Tests\BrowserTestBase.      
 ------ --------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedEntityBrowserTest.php                                       
 ------ -------------------------------------------------------------------------------------------- 
         Class Drupal\entity_embed\Tests\EntityEmbedEntityBrowserTest was not found while trying to  
         analyse it - autoloading is probably not configured properly.                               
 ------ -------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedFilterTest.php                                     
 ------ ----------------------------------------------------------------------------------- 
  47     Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  48     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  49     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  50     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  60     Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  61     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  62     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  69     Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  70     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  71     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  72     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  85     Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  86     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  87     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  88     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  89     Call to deprecated method assertCacheTag() of class Drupal\Tests\BrowserTestBase.  
  99     Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  100    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  111    Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  112    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  113    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  114    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  115    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  125    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  126    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  127    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  138    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  139    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  140    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  144    Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  145    Call to deprecated method assertNoRaw() of class Drupal\Tests\BrowserTestBase.     
  156    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  157    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  165    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  166    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  180    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  181    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  182    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  185    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  197    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.      
  198    Call to deprecated method assertNoLink() of class Drupal\Tests\BrowserTestBase.    
  199    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  200    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.    
  201    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
  211    Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.       
 ------ ----------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedHooksTest.php                                        
 ------ ------------------------------------------------------------------------------------- 
  61     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.        
  62     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  64     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.        
  65     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  78     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  81     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  82     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.        
  83     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  84     Call to deprecated method assertLinkByHref() of class Drupal\Tests\BrowserTestBase.  
 ------ ------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedTestBase.php                                    
 ------ -------------------------------------------------------------------------------- 
  124    Call to deprecated method assertEqual() of class Drupal\Tests\BrowserTestBase.  
 ------ -------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedTwigTest.php                                           
 ------ --------------------------------------------------------------------------------------- 
  31     Call to deprecated method assertEqual() of class Drupal\Tests\BrowserTestBase.         
  40     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.          
  44     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.          
  45     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  46     Call to deprecated method assertLinkByHref() of class Drupal\Tests\BrowserTestBase.    
  50     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.          
  51     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  52     Call to deprecated method assertNoLinkByHref() of class Drupal\Tests\BrowserTestBase.  
 ------ --------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityEmbedUpdateHookTest.php                              
 ------ -------------------------------------------------------------------------------- 
  43     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.    
  44     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.    
  77     Call to deprecated method assertEqual() of class Drupal\Tests\BrowserTestBase.  
 ------ -------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/EntityReferenceFieldFormatterTest.php                             
 ------ --------------------------------------------------------------------------------------- 
  28     Call to deprecated function entity_create().                                           
  68     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.     
  77     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.     
  78     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.     
  87     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.     
  88     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.     
  89     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.     
  109    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.          
  110    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  111    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  112    Call to deprecated method assertLinkByHref() of class Drupal\Tests\BrowserTestBase.    
  122    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.          
  123    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  124    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  125    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
  126    Call to deprecated method assertNoLinkByHref() of class Drupal\Tests\BrowserTestBase.  
  136    Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.          
  137    Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.        
 ------ --------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/FileFieldFormatterTest.php                                      
 ------ ------------------------------------------------------------------------------------- 
  66     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  67     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  81     Call to deprecated method assertText() of class Drupal\Tests\BrowserTestBase.        
  82     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  83     Call to deprecated method assertLinkByHref() of class Drupal\Tests\BrowserTestBase.  
 ------ ------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/ImageFieldFormatterTest.php                                     
 ------ ------------------------------------------------------------------------------------- 
  67     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  73     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  74     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  75     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  76     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  77     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  78     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  79     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  80     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.   
  93     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.         
  94     Call to deprecated method assertNoText() of class Drupal\Tests\BrowserTestBase.      
  95     Call to deprecated method assertLinkByHref() of class Drupal\Tests\BrowserTestBase.  
 ------ ------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------ 
  Line   tests/src/Functional/ViewModeFieldFormatterTest.php                                 
 ------ ------------------------------------------------------------------------------------ 
  34     Call to deprecated method assertIdentical() of class Drupal\Tests\BrowserTestBase.  
  52     Call to deprecated method assertRaw() of class Drupal\Tests\BrowserTestBase.        
 ------ ------------------------------------------------------------------------------------ 

 ------ --------------------------------------------------------------------------------------- 
  Line   tests/src/FunctionalJavascript/EntityEmbedDialogTest.php                               
 ------ --------------------------------------------------------------------------------------- 
  29     Call to deprecated method assertFieldByName() of class Drupal\Tests\BrowserTestBase.   
  32     Call to deprecated method assertFieldByXPath() of class Drupal\Tests\BrowserTestBase.  
 ------ --------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------- 
  Line   tests/src/FunctionalJavascript/EntityEmbedTestBase.php                          
 ------ -------------------------------------------------------------------------------- 
  127    Call to deprecated method assertEqual() of class Drupal\Tests\BrowserTestBase.  
 ------ --------------------------------------------------------------------------------

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#26 entity-embed--drupal-9-deprecation-fixes-3042600-26.patch37.17 KBoknate
#26 3042600-interdiff--20-26.txt14.83 KBoknate
#20 entity-embed--drupal-9-deprecation-fixes-3042600-20.patch49.66 KBoknate
#20 3042600--interdiff-18-20.txt3.97 KBoknate
#18 entity-embed--drupal-9-deprecation-fixes-3042600-18.patch47.43 KBoknate
#18 3042600--interdiff-16-18.txt5.02 KBoknate
#16 entity-embed--drupal-9-deprecation-fixes-3042600-16.patch46.59 KBoknate
#16 3042600--interdiff-15-16.txt14.83 KBoknate
#15 entity-embed--drupal-9-deprecation-fixes-3042600-15.patch38.81 KBoknate
#14 entity-embed-3042600-14.patch756 bytesoknate
#4 drupal_9_deprecated_code-3042600-4.patch50.98 KBsergiu stici
#2 drupal_9_deprecated_code-3042600-2.patch51.01 KBsergiu stici

Comments

oheller created an issue. See original summary.

sergiu stici’s picture

sergiu stici commented
Status: Active » Needs review
StatusFileSize
new drupal_9_deprecated_code-3042600-2.patch51.01 KB

Here is the patch, please review.

Status: Needs review » Needs work

The last submitted patch, 2: drupal_9_deprecated_code-3042600-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

sergiu stici’s picture

sergiu stici commented
Status: Needs work » Needs review
StatusFileSize
new drupal_9_deprecated_code-3042600-4.patch50.98 KB

Corrected code sniffer, and any errors. Here is the patch. Please review.

Status: Needs review » Needs work

The last submitted patch, 4: drupal_9_deprecated_code-3042600-4.patch, failed testing. View results

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks for working on this! 🙏 🥳

It'll be great to get this fixed, but we first need tests to be passing again of course :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Part of this has been fixed in #3023051: Stop using deprecated assertIdentical() and assertEqual() functions in tests already.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3023051: Stop using deprecated assertIdentical() and assertEqual() functions in tests, +#2974668: Convert module to use short array syntax (new coding standard)
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: -Drupal 9 compatibility +Coding standards, +Drupal 9
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3051101: Drupal 9 Deprecated Code Report

Closed #3051101: Drupal 9 Deprecated Code Report as a duplicate of this.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Novice, +php-novice
waverate’s picture

waverate commented
Issue tags: +Drupal 9 compatibility
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3052245: Replace deprecated method link() with toLink()

Closed #3052245: Replace deprecated method link() with toLink() as a duplicate.

oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs work » Needs review
StatusFileSize
new entity-embed-3042600-14.patch756 bytes

First change:

-    $this->assertEqual(get_class($ext), 'Drupal\\entity_embed\\Twig\\EntityEmbedTwigExtension', 'Extension loaded successfully.');
+    $this->assertEquals('Drupal\\entity_embed\\Twig\\EntityEmbedTwigExtension', get_class($ext), 'Extension loaded successfully.');
oknate’s picture

oknate
Location Greater New York City Area
commented
Title: Drupal 9 Deprecated Code Report » Remove deprecated code in preparation for Drupal 9
StatusFileSize
new entity-embed--drupal-9-deprecation-fixes-3042600-15.patch38.81 KB

Working on this.

Still to do: I need to add back fixes for assertResponse:

-    $this->assertResponse(403, 'Embed dialog is not accessible with a filter that does not have the embed button assigned to it.');
+    $this->assertSession()->statusCodeEquals(403, 'Embed dialog is not accessible with a filter that does not have the embed button assigned to it.');
oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new 3042600--interdiff-15-16.txt14.83 KB
new entity-embed--drupal-9-deprecation-fixes-3042600-16.patch46.59 KB

This patch takes drupal-check output down to one error:

 ------ --------------------------------------------------------------------------------------------------- 
  Line   modules/contrib/entity_embed/src/EntityEmbedDisplay/EntityEmbedDisplayManager.php                  
 ------ --------------------------------------------------------------------------------------------------- 
  32     config:entity_view_mode_list cache tag might be unclear and does not contain the cache key in it.  
 ------ ---------------------------------------------------------------------------------------------------

Status: Needs review » Needs work

The last submitted patch, 16: entity-embed--drupal-9-deprecation-fixes-3042600-16.patch, failed testing. View results

oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs work » Needs review
StatusFileSize
new 3042600--interdiff-16-18.txt5.02 KB
new entity-embed--drupal-9-deprecation-fixes-3042600-18.patch47.43 KB

Fixing some coding standard errors:

tests/src/Functional/EntityEmbedDialogTest.php ✗ 2 more
37	Line exceeds 80 characters; contains 81 characters
97	Line exceeds 80 characters; contains 85 characters
tests/src/Functional/EntityEmbedFilterTest.php ✗ 2 more
67	Line exceeds 80 characters; contains 81 characters
170	Line exceeds 80 characters; contains 97 characters
tests/src/Functional/EntityEmbedUpdateHookTest.php
61	Public method name "EntityEmbedUpdateHookTest::testAllowedHTML" is not in lowerCamel format
tests/src/Functional/ImageFieldFormatterTest.php ✗ 1 more
94	Line exceeds 80 characters; contains 88 characters

I'm not sure why the last test run failed. Investigating.

Status: Needs review » Needs work

The last submitted patch, 18: entity-embed--drupal-9-deprecation-fixes-3042600-18.patch, failed testing. View results

oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs work » Needs review
StatusFileSize
new 3042600--interdiff-18-20.txt3.97 KB
new entity-embed--drupal-9-deprecation-fixes-3042600-20.patch49.66 KB

The failure was due to this error:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Entity\Exception\UndefinedLinkTemplateException</em>: No link template &#039;canonical&#039; found for the &#039;file&#039; entity type in <em class="placeholder">Drupal\Core\Entity\Entity-&gt;toUrl()</em> (line <em class="placeholder">224</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/Entity.php</em>). <pre class="backtrace">Drupal\entity_embed\Form\EntityEmbedDialog-&gt;buildEmbedStep(Array, Object) (Line: 199)
Drupal\entity_embed\Form\EntityEmbedDialog-&gt;buildForm(Array, Object, Object, Object)

So it seems that changing $entity->link to $entity->toLink is more complex, and might be better handled in a separate issue, because some entities don't have canonical link template, this means toLink and toUrl fail. So for these types of entities, such as files, we have to add custom handling in order to display a link in the dialog.

While touching this, I'm adding _blank as the target on the links. There's no use case where you'd want the EntityEmbedDialog to go away by clicking on a link, when the back button means you have to start over.

I'm moving $entity->toUrl()->toString() into the catch statement, as it throws an error with file entity.

This requires special handling for the file:

+      if ($entity->getEntityTypeId() == 'file') {
+        $entity_label = '<a href="' . file_create_url($entity->getFileUri()) . '" target="_blank">' . $entity->label() . '</a>';
+      }

Here's all the code to get the entity link to display in the dialog:

-    $entity_label = '';
     try {
-      $entity_label = $entity->toLink()->toString();
+      if ($entity->getEntityType()->hasLinkTemplate('canonical')) {
+        $options = [
+          'attributes' => [
+            'target' => '_blank',
+          ],
+        ];
+        $entity_label = $entity->toLink($entity->label(), 'canonical', $options)->toString();
+      }
+      else {
+        // Construct markup of the link to the entity manually if toLink() fails.
+        // @see https://www.drupal.org/node/2402533
+        $entity_label = '<a href="' . $entity->toUrl()->toString() . '" target="_blank">' . $entity->label() . '</a>';
+      }
     }
     catch (\Exception $e) {
-      // Construct markup of the link to the entity manually if toLink() fails.
-      // @see https://www.drupal.org/node/2402533
-      $entity_label = '<a href="' . $entity->toUrl()->toString() . '">' . $entity->label() . '</a>';
+      // $entity->toUrl()->toString doesn't work for files.
+      if ($entity->getEntityTypeId() == 'file') {
+        $entity_label = '<a href="' . file_create_url($entity->getFileUri()) . '" target="_blank">' . $entity->label() . '</a>';
+      }
+      else {
+        $entity_label = $entity->label();
+      }

Also, I was getting this unrelated error locally, due to incorrect sRGB profile in the image '/js/plugins/drupalentity/entity.png'. I don't know if we need to fix that, but it was annoying me when testing, so I switched out which png I'm using as a test image.

1) Drupal\Tests\entity_embed\FunctionalJavascript\ImageFieldFormatterTest::testInvalidImageError
PHPUnit\Framework\Exception: libpng warning: iCCP: known incorrect sRGB profile

I fixed this by switching out images here:

-    // Create a sample image to embed.
-    $entity_embed_path = $this->container->get('module_handler')
-      ->getModule('entity_embed')
-      ->getPath();
-    file_unmanaged_copy($entity_embed_path . '/js/plugins/drupalentity/entity.png', 'public://example1.png');
+    file_unmanaged_copy(\Drupal::root() . '/core/misc/druplicon.png', 'public://example1.png');
oknate’s picture

oknate
Location Greater New York City Area
commented
Title: Remove deprecated code in preparation for Drupal 9 » Update deprecated code in preparation for Drupal 9
tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
commented
Issue tags: +DrupalCampBelarus2019
tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
commented
Title: Update deprecated code in preparation for Drupal 9 » Drupal 9 Deprecated Code Report
oknate’s picture

oknate
Location Greater New York City Area
commented

Because I keep stepping on my own toes, I'm creating a meta issue with some of my test updates:
#3055928: [META] 8.x-1.0-beta4 plan

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

Thanks for pushing this forward!

  1. +++ b/src/Form/EntityEmbedDialog.php
@@ -374,14 +374,29 @@ class EntityEmbedDialog extends FormBase {
+        // Construct markup of the link to the entity manually if toLink() fails.
+        // @see https://www.drupal.org/node/2402533
+        $entity_label = '<a href="' . $entity->toUrl()->toString() . '" target="_blank">' . $entity->label() . '</a>';

    It didn't fail, it just didn't have a canonical link template.

    I think the else statement should contain what is currently in the catch statement? 

  2. +++ b/src/Form/EntityEmbedDialog.php
@@ -374,14 +374,29 @@ class EntityEmbedDialog extends FormBase {
+        $entity_label = '<a href="' . file_create_url($entity->getFileUri()) . '" target="_blank">' . $entity->label() . '</a>';

    Let's use \Drupal\file\FileInterface::createFileUrl() instead.

    EDIT: nope, that'd make this module require Drupal 8.7.x, which we don't want. 

  3. +++ b/tests/src/Functional/EntityEmbedFilterTest.php
@@ -44,118 +44,158 @@ class EntityEmbedFilterTest extends EntityEmbedTestBase {
+    // Verify placeholder does not appear in the output when
+    // embed is successful.

    80 cols formatting: more things fit on the first line. 

  4. +++ b/tests/src/Functional/EntityEmbedFilterTest.php
@@ -44,118 +44,158 @@ class EntityEmbedFilterTest extends EntityEmbedTestBase {
-    $settings = [];
-    $settings['type'] = 'page';
-    $settings['title'] = 'Test that entity-uuid is preferred over entity-id when both attributes are present';
-    $settings['body'] = [['value' => $content, 'format' => 'custom_format']];
-    $node = $this->drupalCreateNode($settings);
+    $node = $this->drupalCreateNode([
+      'type' => 'page',
+      'title' => 'Test that entity-uuid is preferred over entity-id when both attributes are present',
+      'body' => [
+        [
+          'value' => $content,
+          'format' => 'custom_format',
+        ],
+      ],
+    ]);

    Are these kinds of changes truly necessary for code deprecation fixes? I don't see it in the issue summary's drupal-check output. At most, this would be coding standards improvements, which is a separate concern.

    Reverting these changes would make the patch much simpler.

Everything else looks fine :)

oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new 3042600-interdiff--20-26.txt14.83 KB
new entity-embed--drupal-9-deprecation-fixes-3042600-26.patch37.17 KB

Responses to #25
1), that comment about it failing was already there! I removed it, since I think it's unnecessary. I updated the logic, since we know files don't have a canonical link template, we can prevent the error that triggers the catch, so we can move that into the catch statement, as per your suggestion. We might not need the catch statement, but I'm leaving it for now. We should manually test, as I don't think we have test coverage for all the scenarios here. I just eyeballed the change and haven't tested it yet.
2) There's nothing to be done there.
3) Added more text to the first line.
4) I removed the unrelated stylistic change. I'm refactoring away that code anyway in #3052482: Expand EntityEmbedFilter's test coverage: test many more edge cases, change to kernel test currently anyway.

oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs work » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -DrupalCampBelarus2019

#26: that all sounds good, thanks!

From 49K patch to 37K, nice.

I didn't manually test this (per #26.1), but I did thoroughly review this and am confident that this continues to work correctly for File entities, based on my testing with a debugger that

+++ b/src/Form/EntityEmbedDialog.php
@@ -374,14 +374,24 @@ class EntityEmbedDialog extends FormBase {
+      if ($entity->getEntityType()->hasLinkTemplate('canonical')) {

returns FALSE for File entities :)

Thanks, @oknate!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Drupal 9 Deprecated Code Report » Remove all deprecated API calls

  • Wim Leers committed 462e5ca on 8.x-1.x authored by oknate 
    Issue #3042600 by oknate, Sergiu Stici, Wim Leers, oheller, waverate:...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

🚢

Status: Fixed » Closed (fixed)

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

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue tags: -Drupal 9, -

Per a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.

vinodhini.e’s picture

vinodhini.e commented
Priority: Normal » Critical

Line src/EntityEmbedDisplay/EntityEmbedDisplayManager.php
------ ---------------------------------------------------------------------------------------------------
32 config:entity_view_mode_list cache tag might be unclear and does not contain the cache key in it.

Facing this issue. Can anyone help me with this?