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

By now I think you've figured this already but that's how I've solved this. I've created a custom process plugin where I query the migrate database for the pathauto_persist value for each node.
Here's an example of my migrate_plus.migration.d7_node_basic_page.yml for example:

...
source:
  plugin: d7_node
  node_type: basic_page
process:
  nid: tnid
  ....
  'path/pathauto':
     plugin: pathauto_generate
     source: tnid
  'path/alias':
    -
      plugin: url_alias_migration
      source: tnid

And then my process plugin src/Plugin/migrate/process/PathautoGenerate.php:

namespace Drupal\content_migration\Plugin\migrate\process;

/**
 * @file
 * Contains \Drupal\content_migration\Plugin\migrate\process\PathautoGenerate.
 */

use Drupal\Core\Database\Database;
use Drupal\migrate\Annotation\MigrateProcessPlugin;
use Drupal\migrate\MigrateExecutableInterface;
use Drupal\migrate\ProcessPluginBase;
use Drupal\migrate\Row;

/**
 * Process plugin to get the D7 pathauto setting to auto generate url.
 *
 * @MigrateProcessPlugin(
 *   id = "pathauto_generate",
 * )
 */
class PathautoGenerate extends ProcessPluginBase {

  /**
   * {@inheritdoc}
   */
  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
    // Query the db to get the pathauto_persist value.
    $db = Database::getConnection('default', 'migrate');
    $query = $db->select('pathauto_persist', 'p')
      ->fields('p', ['pathauto']);
    $query->condition('p.entity_id', $value)
      ->condition('p.entity_type', 'node');
    $data = $query->execute()->fetch();

    // Return boolean to indicate if url will be generated automatically or not.
    if (is_object($data)) {
      return $data->pathauto;
    }
    return 1;
  }

}

Important to notice that I'm also migrating the alias, but this will only take effect if 'path/pathauto' is set to 0 (do not auto generate url). Otherwise pathauto will override it.

I have based my example off this one https://drupal.stackexchange.com/questions/238393/migrate-duplicate-entr... although the issue was quite different.

@bbombachini's process plugin worked for me. Except that I had to change the D7 table name from pathauto_persist to pathauto_state.

One more problem I see is that the pathauto state had not been saved in Drupal 7 DB for some nodes and was auto-calculated. So, we have to somehow calculate the pathauto state of these nodes before or during or after the migration.

Tnx. This is correct only for Pathauto 7.x-1.5
https://www.drupal.org/node/1167612

FWIW you might be better off extending the source plugin to add a join to the query instead of performing an additional query for each row.

The patch I post here ensures that custom aliases aren't lost during the migration.

• The fix-only patch only contains the fix: basically, it saves the pathauto (path alias) state with a migrate event subscriber, modifies PathautoFieldItemList to check the state before blindly applying the default value, and flags migrated entities to suppress path alias generation for a migrated entity.
• The normal patch contains tests as well, but it requires #3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site and #3179865: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies.
• The combined patch contains the fix and also #3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site and #3179865: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies.

#3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site and #3179865: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies got newer patches.

1. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +/**
   + * A subscriber to migrate path alias states (pathauto states).
   + */
   +class ContentEntityMigration implements EventSubscriberInterface {
   

   🤔 The description makes much more sense than the classname IMHO.

   I think something like NodeAndTaxonomyPathAliasStateMigrator would make more sense as a name.

2. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +   * Constant to flags a new content entity.
   

   🤓 s/to flags/to flag/

3. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +    // "Flag" the entity to make PathautoGenerator skip creating an alias during
   +    // the migration process.
   +    // @see \Drupal\pathauto\PathautoGenerator::createEntityAlias()
   

   👏 Crucial comment!

4. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +    // Only nodes and terms may have pathauto alias state.
   

   🤔 Ah, so the Drupal 7 pathauto module literally only supports it for these entity types?

   If so, why are we then not moving this logic into ::isApplicable()?

5. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +    // Determine the right state table.
   +    $source = $migration->getSourcePlugin();
   +    $pathauto_schema = $source->getSystemData()['module']['pathauto']['schema_version'] ?? 0;
   +    $path_alias_state_table = $pathauto_schema >= 7006
   +      ? 'pathauto_state'
   +      : 'pathauto_persist';
   

   🤔 Hm, this means that we also support migrating from old pathauto modules on Drupal 7.

   Right?

   If so, let's document that explicitly in the class-level docblock.

6. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +    // Try to load the destination entity and check its alias current state.
   +    // This is required for e.g. node translation: multilingual node sources
   +    // have different IDs on the source site, but they will have the same node
   +    // ID on the destination site. If any of the translations (even the default
   +    // one) had a custom path alias, then we will set the state to
   +    // PathautoState::SKIP to prevent accidental data loss.
   +    // @see https://www.w3.org/Provider/Style/URI
   

   🤯🤩👏

7. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +    if ($path_alias_state_from_source === NULL && $path_alias_state_at_dest === NULL) {
   +      // No path alias status was found.
   +      return;
   +    }
   +    elseif ($path_alias_state_from_source === NULL && $path_alias_state_at_dest !== NULL) {
   +      // Source does not have path alias state for this entity row, but
   +      // destination does. This might happen with entity translations.
   +      $path_alias_state = $path_alias_state_at_dest;
   +    }
   +    elseif ($path_alias_state_from_source !== NULL && $path_alias_state_at_dest === NULL) {
   +      // Source does have state for this entity row, but destination does not.
   +      // This might be the row that contains the default translation.
   +      $path_alias_state = $path_alias_state_from_source;
   +    }
   +    else {
   

   👍 Nice, all four permutations: F && F, F && T, T && F, T && T.

8. +++ b/src/EventSubscriber/ContentEntityMigration.php
   @@ -0,0 +1,193 @@
   +    // Actually, I (huzooka) was not able to push the state value to
   

   Let's clean this comment up 😛 It seems like a debug leftover?

9. +++ b/src/PathautoFieldItemList.php
   @@ -32,9 +32,15 @@ class PathautoFieldItemList extends PathFieldItemList {
   +    // For a new entity, default to creating a new alias (unless it is a
   +    // migrated content entity).
   

   a migrated content entity → a content entity being migrated

10. +++ b/src/PathautoFieldItemList.php
    @@ -32,9 +32,15 @@ class PathautoFieldItemList extends PathFieldItemList {
    +      // Try to get the proper path alias state: This new entity could be a
    +      // migrated entity which already has a pathauto state.
    

    👍 I do not worry about the overhead on production sites because this code only runs if ($this→getEntity()→isNew()), which is slow already. One more key-value lookup won't make a material difference.

11. +++ b/src/PathautoFieldItemList.php
    @@ -32,9 +32,15 @@ class PathautoFieldItemList extends PathFieldItemList {
    +      $this->list[0]->set('pathauto', \Drupal::keyValue("pathauto_state.$entity_type_id")
    +        ->get(PathautoState::getPathautoStateKey($this->getEntity()->id()), PathautoState::CREATE));
    

    🤔 Let's first assign the key-value lookup in a variable. That will make this much easier to read.

12. +++ b/src/PathautoGenerator.php
    @@ -154,6 +155,20 @@ class PathautoGenerator implements PathautoGeneratorInterface {
    +      $reflection = new \ReflectionClass($entity);
    +      $value_ref_properties = $reflection->getProperty('values');
    +      $value_ref_properties->setAccessible(TRUE);
    

    🙏 Can you document why this needs reflection?

    Ideally we wouldn't have this, to avoid overhead on production sites.

13. +++ b/src/PathautoServiceProvider.php
    @@ -18,6 +20,15 @@ class PathautoServiceProvider extends ServiceProviderBase {
    +    $modules = $container->getParameter('container.modules');
    +    if (isset($modules['migrate'])) {
    

    Nice! Zero overhead on production sites! 👍

Re #15:

1. Well, this event subscriber will prevent automatic path alias generation for every content entity being migrated. Using name NodeAndTaxonomyPathAliasStateMigrator would be misleading imho.

   Instead of renaming the class, I kept the neutral ContentEntityMigration name, but added a long class description.

2. Fixed 🤦‍♂️.
3. 👍
4. Yes, exactly. Drupal 7 Pathauto only supports these entity types. But if I moved this into ::isApplicable(), I couldn’t prevent alias generation for users (or for other contrib entity types that might have an already migrated or preexisting pattern on the destination site).
5. 👍
6. Done.
7. 👍
8. 🙂
9. Well, this isn't a leftover. I hope that someone will find the solution.
10. Applied 🙂.
11. 👍
12. 11 This is also addressed 👍.
13. 12 Comment added.
14. 👍

Thanks, that addresses all of my concerns! 😊

The patch pathauto-prevent_losing_custom_aliases-3079275-17.patch depends on #3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site and #3179865: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies because of the tests.

Actually, this is a pretty major bug because it results in data loss.

Patch on top of #3179835-14: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site, #3179865-25: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies and #3182708-11: [PP-2] Migrate language-specific patterns.

Patch on top of combined patch #17.
The only change is that it checks whether $destination_entity_id is empty in ContentEntityMigration Event Subscriber, before setting the pathauto state, as in some cases it happens to be empty. Maybe it should not be empty and it is a stale content problem, but could we be more defensive at that point?

Above patch at #23 did not include added files. This is a fixed version of the #23 patch and an interdiff with #17 patch.

I was looking for a way to preserve pathauto state in the custom migration (where IDs are not preserved) and came across this issue.

Unfortunately, current patch doesn't cover my case, because these lines never returned destination IDs:

$destination_entity_key = $storage->getEntityType()->getKey('id');
// 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);

In my custom migration, IDs are not preserved, so the destination property is always missing.

I'm attaching a patch with the fix, where the piece of code related to pathauto state has been moved into MigrateEvents::POST_ROW_SAVE event handler. Normally, the destination entity should be already available at this stage and should cover both automatic and manual migrations.

For anyone reading at home, the sequence of necessary patches is:

• #3179835: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site
• #3179865: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies
• #3182708: [PP-2] Migrate language-specific patterns
• Patch #22

This is a combined patch that includes all of these changes in one.

Patch on top of #3179835-22: Migrate forum pattern to taxonomy term forums if forum is enabled on the source site, #3179865-25: [PP-1] Derive pathauto pattern migrations to solve inaccurate pattern migration dependencies and #3182708-12: [PP-2] Migrate language-specific patterns.

#27fixes my previous inaccuracy:
In the most recent PHP versions, getting an array offset of type null not just returns NULL but also throws an error.
This change ensures that the patch is compatible with PHP 7.4+.

This is an updated combined patch which includes the changes from #27.

Rerolled.

This should fix the mismatched testSource() method.

One of the test failures comes from this line:

      // Add the original simpletest prefix so SQLite can attach its database.
      // @see \Drupal\Core\Database\Driver\sqlite\Connection::init()
      $connection_info[$target]['prefix'][$value['prefix']['default']] = $value['prefix']['default'];

The odd part is that this class method doesn't exist, so why is this needed?

That part of the combined patch comes from #3179865, so I've crossposted my question there.

Hi,
I tried to apply the latest patch (pathauto-n3079275-32-combined.patch) to the latest tag 8.x-1.10, got the below error.

git apply pathauto-n3079275-32-combined.patch
pathauto-n3079275-32-combined.patch:1313: trailing whitespace.

error: patch failed: pathauto.module:185
error: pathauto.module: patch does not apply

I confirm that the patch fails to apply to the current release; it also fails against the dev branch.

Rerolled pathauto-n3079275-32-combined.patch based on the latest tag version 8.x-1.10.

pathauto-n3079275-38-combined.patch is not correct. Here is the correct one.

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.

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.

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.

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 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).

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.
  • EDIT: Maybe/probably the custom aliases are meant to come in via upgrade_d7_url_alias anyway, so this ^^ behavior is correct??
• 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?
EDIT: Apparently it's as simple as adding this to each node migration:

process:
  'path/pathauto':
    plugin: default_value
    default_value: 0

So please disregard my question!

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...

Rerolled patch #71

This patch is not applying on latest pathauto.

@anish.a: The MR at the top says MR !77 mergeable so it looks fine.

Is the patch in comment #75 of use, or should it be disregarded? 🤔

Thanks for sharing your observations in @alison #70! If you or anyone else feel like sharing examples on how to use features from the patch under the process section in migration files, it would be a great addition in the Issue Summary.

Some tests are failing.

Recategorizing this issue from "Bug report" to "Task".

This is related to Drupal 7 migration support, and reclassifying it as a task helps us better triage the issue queue by distinguishing migration-related work from actual bugs affecting current functionality.

No change in priority or scope — just a category adjustment for clarity.