Problem/Motivation

Just adding this as a placeholder to decide if we want to provide an upgrade path. And if we do, perhaps use this issue to start the work on it. Linking to #2906878: [Meta] Support for D7 -> D9 contrib migrate

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 pathauto-migrate-2953000-12.patch9.42 KBslv_
#12 interdiff.txt632 bytesslv_
#9 Selection_034.png109.91 KBjuampynr
#9 pathauto-migrate-2953000-9.patch9.42 KBjuampynr
#9 interdiff.txt742 bytesjuampynr
#8 pathauto-migrate-2953000-8.patch9.4 KBjcnventura
#8 interdiff.txt1.02 KBjcnventura
#4 pathauto-migrate-2953000-4.patch9.32 KBjcnventura
#4 interdiff.txt5.25 KBjcnventura
#2 pathauto-migrate-2953000-2.patch8.95 KBjuampynr
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn created an issue. See original summary.

juampynr’s picture

juampynr CreditAttribution: juampynr at Lullabot for NBCUniversal commented
Status: Active » Needs review
FileSize
pathauto-migrate-2953000-2.patch8.95 KB

Here it is. This patch migrates module settings and patterns. I have seen some custom patterns that cannot be migrated automatically but this covers all of what comes with core such as content, users, and taxonomy patterns.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Needs work 
+++ b/pathauto.module
@@ -177,3 +180,23 @@ function pathauto_pattern_validate($element, FormStateInterface $form_state) {
+function pathauto_migrate_prepare_row(

I would really like to see this move into a source plugin. Could we extend the variable source plugin and do it all in a single class? Instead of spread across two places?

jcnventura’s picture

jcnventura CreditAttribution: jcnventura at Wunder commented
Status: Needs work » Needs review
FileSize
interdiff.txt5.25 KB
pathauto-migrate-2953000-4.patch9.32 KB

@heddn I don't think this can be in a single class, since all the settings are config, but the patterns are entities.

In any case, regarding #3, it doesn't need to be in a source plugin, as it is simply a long list of variables going into a single config array. The new patch makes that without requiring a prepareRow() function.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks for cleaning up the prepare row. This looks much more clean implementation.

This could at least use a unit test. See core/modules/color/tests/src/Kernel/Plugin/migrate/source/d7/ColorTest.php as an example to model.

jcnventura’s picture

jcnventura CreditAttribution: jcnventura at Wunder commented

Also, the migration yml files should be prefixed with d7_.

Even if the Drupal 6 migration is the same, the yml files should be cloned, as the current ones are tagged 'Drupal 7'.

jcnventura’s picture

jcnventura CreditAttribution: jcnventura at Wunder commented

Also, make this depend optionally on d7_node_type. If the pathauto migration is run before the d?_node_type, those paths depending on node type don't get imported.

jcnventura’s picture

jcnventura CreditAttribution: jcnventura at Wunder commented
FileSize
interdiff.txt1.02 KB
pathauto-migrate-2953000-8.patch9.4 KB

New patch, addressing my comments on #6 and #7.

juampynr’s picture

juampynr CreditAttribution: juampynr at Lullabot for NBCUniversal commented
Status: Needs work » Needs review
FileSize
interdiff.txt742 bytes
pathauto-migrate-2953000-9.patch9.42 KB
Selection_034.png109.91 KB

Here is a bug that I found while reviewing patterns. It was causing non-content patterns to contain invalid configuration.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

Tested this out on the Contrib Half Hour meeting this week and it worked as expected - the patterns were copied over from the D7 site and they worked as expected on new entities.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Needs work
Issue tags: +Novice

We could always use tests. But the basic code on here seems fine. No nits found. Actually just found 2. Bumping back to NW and tagging novice.

  1. +++ b/migrations/d7_pathauto_patterns.yml
@@ -0,0 +1,23 @@
+label: Pahauto patterns

    Label spelling. 

  2. +++ b/migrations/d7_pathauto_settings.yml
@@ -0,0 +1,95 @@
+label: Pahauto configuration

    Label spelling.

slv_’s picture

slv_ CreditAttribution: slv_ at Lullabot for NBCUniversal commented
Status: Needs work » Needs review
FileSize
interdiff.txt632 bytes
pathauto-migrate-2953000-12.patch9.42 KB

Re-rolled patch to fix the label spelling.

  • Berdir committed e293558 on 8.x-1.x authored by juampynr 
    Issue #2953000 by jcnventura, juampynr, slv_, heddn, DamienMcKenna:...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Title: Migrate configuration from Drupal 6/Drupal7 » Migrate configuration from Drupal7
Status: Needs review » Fixed

Thanks, looks like this has been sufficiently tested and reviewed, Tests would be nice, but I don't really see anyone going to do that.

I think this is just D7, so adjusting the title accordingly.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

I created follow-up issues for related items: #3045639: Migrate Pathauto configuration from Drupal 6 and #3045638: Add test coverage for Pathauto - Drupal 7 migration

Status: Fixed » Closed (fixed)

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