Drupal 9 Deprecated Code Report for Pathauto

Here is the patch, please review.

Patch at #2 works. Removes deprecation errors.

Setting this to Needs Work to get the tests to pass.

Test fails against Drupal 8.7
1) Drupal\Tests\pathauto\Kernel\PathautoKernelTest::testParentChildPathTokens
Error: Call to undefined method Drupal\Core\Entity\EntityDisplayRepository::getViewDisplay()

Should work against Drupal 8.8 as EntityDisplayRepository::getViewDisplay() is introduced in 8.8.

I will only be able to commit deprecation updates that work on 8.6, to able to commit your work, I suggest you split the parts that are deprecated on 8.6 or earlier into a separate issue.

Tried to fix the final issue according to: https://api.drupal.org/api/drupal/core%21includes%21entity.inc/function/...

+++ b/tests/src/Kernel/PathautoKernelTest.php
@@ -374,7 +374,16 @@ class PathautoKernelTest extends KernelTestBase {
 
-    $display = entity_get_display('taxonomy_term', 'tags', 'default');
+    $values = array(
+      'targetEntityType' => 'taxonomy_term',
+      'bundle' => 'tags',
+      'mode' => 'default',
+      'status' => TRUE,
+    );
+    $display = \Drupal::entityTypeManager()
+      ->getStorage('entity_view_display')
+      ->create($values);
+

This is exactly why we introduced the new API in 8.8, to avoid duplicating that much code. There's no reason to update this now, just remove that part from the patch.

Adjusted patch according to @Berdir's remarks

Not sure why the tests are failing. Seems not related to the patch.

> 20:43:27 Fatal error: Uncaught Error: Call to undefined method Drupal\pathauto\Tests\PathautoUserWebTest::assertEquals() in /var/www/html/modules/contrib/pathauto/src/Tests/PathautoTestHelperTrait.php:122

Looks pretty related to me :)

These test aren't converted yet, so assertEquals() doesn't exist. We need to convert them to phpunit first.

You're welcome to give that a try, I'd do that in a new issue if there is none yet. Simpletest base classes were officially deprecated today, but not sure how much documentation there already is on converting them. Basically it is about moving them into the new location, update base classes and so on.

@Berdir,
Thanks, I got the point. Will try to look into it.

Converted the simpletests to phpunit.

Changed how ajax requests are processed in tests

Note to others following this: test conversion now happens in #3061563: Convert simpletest to PHPUnit tests., don't reroll the patch here ;)

I rerolled at #11, and then ran drupal-checker and the only failure was `entity_get_display()`. So hopefully this works after the changes in #3061563.

the phpunit assert methods have inverted arguments expected should come first.

Here's the change from #21.

RTBC on green, thanks @katherined!

Follow a new patch that replace entity_get_display().

Ok, so if we fix 8.8 deprecations then lets do it properly, and use this issue to prepare for a release that is compatible with 8.8 and 9.0.

1. Update the version constraints to require ^8.8 || ^9.0 to all info.yml files.
2. Add the defaultTheme property to browser tests
3. fix remaining entity manager usages (grep entity.manager -r .)
4. Remove the BC layer that we added for 8.7 (service provider and the alternative storage helper class)
5. Fix other remaining dynamic deprecations that drupal-check can't find, run phpunit tests with deprecations enabled.

Also, there are several existing overlapping and duplicate issues in the issue queue like #3004108: ContextDefinition object for an entity type is deprecated, #3069603: [8.8+] Removing deprecated method entity_get_display() and #3086586: Add new key core_version_requirement in pathauto.info.yml.

I'd really appreciate help to consolidate that into this issue, close them as duplicate and merge existing patches together, so we can test it all together. That's just as much work as fixing the code :)

Note that I will commit this then likely around 9.0 beta. We'll also need to make dependencies like ctools and token ready for D9 in the same way.

Took care of all the 8.7 deprecations in #3004108: ContextDefinition object for an entity type is deprecated, this can now increase the version requirement to 8.8 and deal with the final parts.

Changing version to 8.8 in info files.

So far so good, the next step is removing \Drupal\pathauto\PathautoServiceProvider, \Drupal\pathauto\LegacyAliasStorageHelper. Then remove usages of \Drupal\Core\Path\AliasRepositoryInterface, \Drupal\Core\Path\AliasManagerInterface and replace it with type hints on path_alias.module services.

Added changes per #31.

#32 looks good to me.

➜  d9 git:(8.8.x) ✗ ./vendor/mglaman/drupal-check/drupal-check --drupal-root=./ modules/contrib/pathauto

 54/54 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


 [OK] No errors

#3042582-32: Drupal 9 Deprecated Code Report for Pathauto works fine. Checked with upgrade_status module too.

These tools don't find everything though, there are still quite a few cases of the old services left, for example in \Drupal\Tests\pathauto\Functional\PathautoTestHelperTrait.

  26x: The \Drupal\Core\Path\AliasManager class is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Instead, use \Drupal\path_alias\AliasManager. See https://drupal.org/node/3092086
    5x in PathautoEntityWithStringIdTest::testEntityWithStringId from Drupal\Tests\pathauto\Kernel
    1x in PathautoMassDeleteTest::testDeleteAll from Drupal\Tests\pathauto\Functional
    1x in PathautoKernelTest::testUpdateActions from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testDefaultRevision from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPatternStatus from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathAliasUniquifyWordsafe from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testProgrammaticEntityCreation from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testNoExistingPathAliases from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testParentChildPathTokens from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathTokens from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testNoTokensNoAlias from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testSameTitleDifferentLanguages from Drupal\Tests\pathauto\Kernel
    1x in PathautoNodeWebTest::testNodeEditing from Drupal\Tests\pathauto\Functional
    1x in PathautoBulkUpdateTest::testBulkUpdate from Drupal\Tests\pathauto\Functional
    1x in PathautoUiTest::testPatternsWorkflow from Drupal\Tests\pathauto\FunctionalJavascript
    1x in PathautoLocaleTest::testLanguageAliases from Drupal\Tests\pathauto\FunctionalJavascript
    1x in PathautoUserWebTest::testUserOperations from Drupal\Tests\pathauto\Functional
    1x in PathautoSettingsFormWebTest::testPunctuationSettings from Drupal\Tests\pathauto\Functional
    1x in PathautoSettingsFormWebTest::testSettingsForm from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testNodeState from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testNodeOperations from Drupal\Tests\pathauto\Functional
    1x in PathautoKernelTest::testCreateNodeWhileAccessingPath from Drupal\Tests\pathauto\Kernel

  18x: \Drupal\Core\Path\AliasStorage is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use the "path_alias.repository" service instead, or the entity storage handler for the "path_alias" entity type for CRUD methods. See https://www.drupal.org/node/3013865.
    5x in PathautoEntityWithStringIdTest::testEntityWithStringId from Drupal\Tests\pathauto\Kernel
    1x in PathautoBulkUpdateTest::testBulkUpdate from Drupal\Tests\pathauto\Functional
    1x in PathautoEnablingEntityTypesTest::testEnablingEntityTypes from Drupal\Tests\pathauto\Functional
    1x in PathautoMassDeleteTest::testDeleteAll from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testNodeEditing from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testNodeState from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testCustomAliasWithoutPattern from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testCustomAliasAfterAutomaticAlias from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testCustomAliasAfterRemovingPattern from Drupal\Tests\pathauto\Functional
    1x in PathautoLocaleTest::testLanguageAliases from Drupal\Tests\pathauto\FunctionalJavascript
    1x in PathautoKernelTest::testPathDeleteMultiple from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testUpdateActions from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testNoTokensNoAlias from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathTokens from Drupal\Tests\pathauto\Kernel

  16x: The "path.alias_manager" service is deprecated. Use "path_alias.manager" instead. See https://drupal.org/node/3092086
    5x in PathautoEntityWithStringIdTest::testEntityWithStringId from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testSameTitleDifferentLanguages from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testUpdateActions from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testNoTokensNoAlias from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathTokens from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testParentChildPathTokens from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testNoExistingPathAliases from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testProgrammaticEntityCreation from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathAliasUniquifyWordsafe from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPatternStatus from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testDefaultRevision from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testCreateNodeWhileAccessingPath from Drupal\Tests\pathauto\Kernel

  9x: The "path.alias_storage" service is deprecated. Use the "path_alias.repository" service instead, or the entity storage handler for the "path_alias" entity type for CRUD methods. See https://www.drupal.org/node/3013865
    5x in PathautoEntityWithStringIdTest::testEntityWithStringId from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathDeleteMultiple from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testUpdateActions from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testNoTokensNoAlias from Drupal\Tests\pathauto\Kernel
    1x in PathautoKernelTest::testPathTokens from Drupal\Tests\pathauto\Kernel

  1x: Support for asserting against non-boolean values in ::assertTrue is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertNotEmpty(). See https://www.drupal.org/node/3082086
    1x in PathautoLocaleTest::testLanguageAliases from Drupal\Tests\pathauto\FunctionalJavascript

  1x: Support for asserting against non-boolean values in ::assertFalse is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertEmpty(). See https://www.drupal.org/node/3082086
    1x in PathautoUiTest::testPatternsWorkflow from Drupal\Tests\pathauto\FunctionalJavascript

Fixed above items.

+++ b/tests/src/Functional/PathautoTestHelperTrait.php
@@ -77,8 +77,9 @@ trait PathautoTestHelperTrait {
 
   public function saveAlias($source, $alias, $langcode = Language::LANGCODE_NOT_SPECIFIED) {
-    \Drupal::service('path.alias_storage')->delete(['source' => $source, 'langcode' => $langcode]);
-    return \Drupal::service('path.alias_storage')->save($source, $alias, $langcode);
+    $alias_to_delete = \Drupal::service('entity_type.manager')->getStorage('path_alias')->loadByProperties(['path' => $source, 'langcode' => $langcode]);
+    \Drupal::service('entity_type.manager')->getStorage('path_alias')->delete($alias_to_delete);
+    return \Drupal::service('entity_type.manager')->getStorage('path_alias')->create(['path' => $source, 'alias' => $alias, 'langcode' => $langcode])->save();
   }

the delete and re-save is pretty strange, are we really using that?

There's also \Drupal\Tests\Traits\Core\PathAliasTestTrait that we could use with a few useful methods for loading,creating as well as asserting that an alias exists.

Well, I've added some core PathAliasTestTrait methods, but not assertPathAliasExists, because it loads the alias differently from PathautoTestHelperTrait similar method already used in the tests.

JavaScript test fix.

This change should take care of

26x: Support for asserting against non-boolean values in ::assertFalse is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertEmpty(). See https://www.drupal.org/node/3082086
    11x in PathautoNodeWebTest::testNodeState from Drupal\Tests\pathauto\Functional
    5x in PathautoEntityWithStringIdTest::testEntityWithStringId from Drupal\Tests\pathauto\Kernel
    3x in PathautoKernelTest::testUpdateActions from Drupal\Tests\pathauto\Kernel
    2x in PathautoKernelTest::testPathDeleteMultiple from Drupal\Tests\pathauto\Kernel
    1x in PathautoBulkUpdateTest::testBulkUpdate from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testNodeEditing from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testCustomAliasAfterAutomaticAlias from Drupal\Tests\pathauto\Functional
    1x in PathautoNodeWebTest::testCustomAliasAfterRemovingPattern from Drupal\Tests\pathauto\Functional
    1x in PathautoKernelTest::testNoTokensNoAlias from Drupal\Tests\pathauto\Kernel

Fixed the deprecation warning below in this patch.

Parameter $entity_manager of method Drupal​\​pathauto​\​Plugin​\​migrate​\​source​\​PathautoPattern::__construct() has typehint with deprecated interface Drupal​\​Core​\​Entity​\​EntityManagerInterface. Deprecated in drupal:8.0.0 and is removed from drupal:9.0.0.

Note: the changes from the issue https://www.drupal.org/project/pathauto/issues/3108739 has been added to the patch #42 to create new patch.

Working on this issue contributionweekend2020

Fix
377 Call to deprecated function entity_get_display().
99 Call to deprecated method assertIdentical() of class Drupal\KernelTests\KernelTestBase.

This is missing all the existing work in this issue and has some unrelated changes.

My bad, the patch didn't apply correctly

Adding the missing module dependency to kernel tests to the patch in #43, those were the only tests that failed on #43, so I think this is complete and ready.

I gave #48 a test run locally using Drupal 9.0.x HEAD. Initially, I got the same failure as we're seeing here. After patching Token with #3042568: Drupal 9 Deprecated Code Report for Token on @Berdir's recommendation, the failure disappeared.

For the record, we don't know why it passes with the Token patch, but it does, and Drupal is mysterious. So, in the name of getting this very important module ready for Drupal 9, I'm marking this RTBC.

Need to remove the dependency on the path_alias.manager service, which has been deprecated.

path.alias_manager is deprecated, not the same thing. This is passing on d9 now.

Ok definitely read that too fast path_alias.manager vs path.alias_manager.

Drupal 9 tests aren't passing yet, but that's because it is tested against token 8.x-1.6. Committed.

Thanks all for updating pathauto for D9!

Is there a schedule for the next release? It'd be great to have pathauto show as green in our Upgrade Status list :)
https://www.drupal.org/project/upgrade_status