Custom aliases (which are not generated with the actual patterns) can be lost during the migration

FYI the migration plugin fails if you're doing a custom migration that does not include the nid or tid, i.e. the entity's ID is not migrated, failing with this error:

 [error]  SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'name' cannot be null: INSERT INTO "key_value" ("name", "collection", "value") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array
(
    [:db_insert_placeholder_0] => 
    [:db_insert_placeholder_1] => pathauto_state.node
    [:db_insert_placeholder_2] => i:0;
)
 (/var/www/html/web/core/modules/mysql/src/Driver/Database/mysql/ExceptionHandler.php:43) 

It tries to get the destination ID in this code:

    // The migration's row should be already processed at this point, this is a
    // MigrateEvents::PRE_ROW_SAVE subscriber.
    $destination_entity_id = $row->getDestinationProperty($destination_entity_key);

Should it not get the entity ID from the migration object instead of assuming the value was migrated?

Changing that line to the following makes it work, at least in my local testing where I'm running a migration update (drush migrate:import --update):

    $destination_entity_id = $row->getIdMap()['destid1'];

The question is - is that safe to use in all cases?

The trick from #41 doesn't work when migrations are new, so it might be best to add some logic to skip the record if $destination_entity_id is empty.

Rerolled everything...

This combines everything from the other three issues, adds #22 along with the fixes from #23, #25 and #27.

FYI #45 leads to bugs with PHP 8.1 as getPathautoStateKey() gets passed an empty variable when the node form is loaded.

Working on the PHP 8.1 compatibility fixes.

This resolves a PHP 8.1 bug that would trigger the following error when loading an form to create an entity which had a pathauto pattern defined:

Deprecated function: mb_check_encoding(): Calling mb_check_encoding() without argument is deprecated in Drupal\pathauto\PathautoState::getPathautoStateKey() (line 182 of modules/contrib/pathauto/src/PathautoState.php).

This fixes the error from #47.

The last submitted patch, 47: pathauto-n3079275-47-combined.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Working on the test regressions.

Looking at the test failures:

• This test fails:
  

      // There should be no Pathauto checkbox on user forms.
      $this->drupalGet('user/' . $this->adminUser->id() . '/edit');
      $this->assertSession()->fieldValueNotEquals('path[0][pathauto]', '');
  

  That suggests this introduces a UI change which adds the Pathauto checkbox to show on user forms.

Fixing a few of the smaller issues.

Sorry about that.

The last submitted patch, 52: pathauto-n3079275-52-combined.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Looking at the test failure of Drupal\Tests\pathauto\FunctionalJavascript\PathautoUiTest, it hits this error message:

Path pattern is using the following invalid tokens: [user:name], [term:name].

Working on it.

In local testing this fixes PathautoUiTest.

The last submitted patch, 55: pathauto-n3079275-55-combined.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

This should fix PathautoMigrateUiTest .

Incidentally, the changes in #57 stem from code like this:

    $this->assertEquals(NULL, $path_alias_repository->lookupBySystemPath('/node/11', 'en')['alias']);

The problem is that $path_alias_repository->lookupBySystemPath('/node/11', 'en') returns NULL if the value doesn't exist, so PHP yells about trying to access an array element on NULL. The solution here was to remove the array element on the assertions where it was checking for NULL.

The last submitted patch, 57: pathauto-n3079275-57-combined.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

The failures in PathautoBulkUpdateTest are because a URL is not automatically created for the admin user created in setUp(), which goes back to the problem identified in #54.

In MigratePathautoTest, for some reason the English version of the node results in a path alias, even though the comment says it shouldn't have an alias after the node is re-saved. Is the test wrong or is there a bug somewhere in the code?

This fixes PathautoSourceTest by simplifying/fixing some of the database prefix logic that was added at some point (I previously questioned this logic in #34).

The last submitted patch, 62: pathauto-n3079275-62-combined.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

FYI #62 includes the following dump() output, because I forgot to remove the lines prior to creating the patches:

array (
  id => "9"
  path => "/node/8"
  alias => "/blogs/admin/number-47"
  langcode => "en"
)
null
null

This is the output from MigratePathautoTest::testPathautoMigrations() and shows the output of the $path_alias_repository->lookupBySystemPath() calls - it's expected to show three "null" statements, but the first one shows an alias record that the test doesn't expect to find.

Leaving it for someone else to look at.

Unfortunately patch in #62 no longer applies to Pathauto 8.x-1.12.

#62-combined rerolled.

Thanks @DamienMcKenna, I am following your awesome upgrade from D7 to D10 videos, so thanks for documenting the process, as well clearing bumps in the road ahead for all us other upgraders :-)

Since Pathauto is used in most D7 projects, and the D7 EOL is getting closer, it would be awesome to land this before too long.

Rerolled.

Thank you for all the work on this issue!

I tried the patch on #69 on 1.x-dev this evening, and:

• It applied cleanly.
• When I ran node migrations with --update, nodes for which "Generate automatic URL alias" is disabled on my Drupal 7 source site now have this checkbox disabled on my Drupal 10 destination site 🎉
• When the source node had that ^^ box unchecked, and if I change the alias value (from the node form), and then re-run the node migration on the Drupal 10, so far, I'm not seeing the custom alias show in the URL alias field on that same node's node edit form in Drupal 10.
• I haven't tested any non-node migrations.

I'm not totally certain of the intended behavior, so I won't say if it's working or not working :) The issue summary might need an update, but that could just be my relative unfamiliarity with the issue.

I do have a question: Can the changes in this patch be used to disable "Generate automatic URL alias" during a node migration? Like, is there anything in the new/updated process plugin(s) that would let me set that field value in the process section of my node migration?

About me:
- Drupal 10.2.0
- Pathauto 1.x-dev (from this evening)
- Using "classic" node migrations, not "node complete"
- Preserving node IDs, not migrating revision history, not preserving VIDs.
- Not a multilingual site
- (Anything else?)

Patch from #69 is failing tests.

The node_type condition plugin was deprecated since Drupal 9.3 → in favor of a generalized entity_bundle:* condition plugin.

This pathauto issue → applies this change to 1.x branch.

The patch in this issue still references the removed node_type condition plugin and results in test failures.

The attached patch:

1. Replaces references to the node_type condition plugin with references to the generalized entity_bundle:node condition plugin.
2. Changes pathauto pattern label "Taxonomy term - Sujet de discussion" to "Taxonomy term - forums" as expected by latest D10.

I had to apply a couple other patches to apply this to 8.x-1.12 and I am setting a deprecation warning when on PHP8.3.

Deprecated function: Creation of dynamic property Drupal\pathauto\Plugin\migrate\PathautoPatternDeriver::$basePluginId is deprecated in Drupal\pathauto\Plugin\migrate\PathautoPatternDeriver->__construct() (line 41 of modules/contrib/pathauto/src/Plugin/migrate/PathautoPatternDeriver.php).

It looks like this requires defining the basePluginId variable.

I created a fork, applied the patch and updated the Deriver to fix the PHP 8.3 deprecation warning: https://git.drupalcode.org/issue/pathauto-3079275/-/commit/b93e5527a4b88...