Migrated URL aliases not working until nodes are re-saved

I've committed a basic work-around to the ui_poc branch:

  langcode:
    -
      plugin: get
      source: language
    -
      plugin: default_value
      default_value: und

This probably won't help the OP, but will help UI testing for now with sites where the language is empty.

A general fix should probably go into the UrlAlias destination - if the incoming langcode is empty, use the site's default, if it's present on the D8 site, pass it through, if it's present in the source but not on D8 - I guess use the D8 site's default. But, that's not granular enough - node aliases should follow the specific language of the node. Should we follow the D7 practice, and migrate entity paths as part of the entity migrations themselves? We'll then need to figure out how to identify and handle any random manual aliases not covered by those.

@mikeryan: This is probably a no-go because it uses CONCAT (I'm guessing that is bad for DB abstraction?) but here is a patch to maybe get the ball rolling.

@paulihuhtiniemi and/or @brockfanning,

What happens if (without the patch) after the migration is run the missing language is then enabled on D8? Do the aliases start working?

I was just chatting with Benjy about this on IRC and he suggested "if that worked, we could just throw some warnings to tell the user to enable it".

If we need to create a patch to fix it, we should start by creating a failing test. Either of you comfortable doing that?

Thanks,
-mike

OK I added this to the known issues here: https://www.drupal.org/node/2167633

If something want's to roll a simple patch that logs a message when the language of the alias we're importing is not enabled, that would be great.

I had a different, but I think closely related experience migrating a monolingual English site.

Behavior

• URL alisases do migrate
• Nodes migrate
• Nodes are correctly associated with their alias
• Nodes don't load by their alias (but do load by id)

Steps to reproduce: Language not enabled

1. I didn't even think to enable the Language module the first time and ran the migration
2. When the nodes didn't load, I looked at the URL alias table and noticed no language was set
3. There was no option to set a language because I hadn't enabled the module and still didn't know I should
4. I investigated in the database and directly updated the langcode in the url_alias table to en
5. Nodes loaded

Steps to reproduce: Language enabled

1. Enable Language module and run the migration
2. Nodes don't load, url_alias langcode isn't set
3. I can resave the node or the url_alias and the language is set to None.
4. I directly updated the langcode in the url_alias table to en because it was faster than resaving things
5. Nodes loaded

FWIW, the attached patch resolved my problem. In both cases (Language enabled prior to the migration and Language NOT enabled before or after the migration) the aliases were set to English and worked

I'm leaving as Needs work, though based on the author's comments in #6

I just re-rolled this patch to get things moving on this issue - it still needs work. Some notes/thoughts:

1. The test was failing because there was no source "node" table provided by the test. This is easily fixed by extending "MigrateNodeTestBase" instead of "MigrateDrupalTestBase".
2. I'm pretty sure that language logic should go into a separate process class, not the UrlAlias destination (maybe even the configuration - I haven't looked that closely at it yet).
3. I'd take a stab changing the CONCAT to something else, but I'm pretty sure chx would just correct me :p
4. If we are going to move to get this patch in, the tests should probably be bolstered a bit to test a different language or two.
5. As Benjy mentioned in comment 11, a warning message would be great as well.

Did I miss anything here?

-mike

While CONCAT is not cross sql Drupal drivers add functions to their respective databases which is supported everywhere. So in the Drupal universe using CONCAT is better than || as adding an operator is hard/impossible.

I don't think this will work for sites with entity translation for example, where the node may have multiple languages, so assuming the node language should be THE path language is a no-go. (Same in Drupal 8 actually). Also a path alias without a language code is a *feature* since then it applies to all languages. If you modify that to only apply to certain languages, you are actually loosing possibly intentional setup.

When testing #2504513: Migrate URL aliases - this is still a thing, we need to fix it. I would say, given that my vanilla D6 site loses its aliases migrating to D8, this is migrate-critical.

So, should we just use Language::LANGCODE_NOT_SPECIFIED when the incoming language is empty, rather than try to guess what the most appropriate language might be?

Implemented mikeryan's suggestion of "using Language::LANGCODE_NOT_SPECIFIED when the incoming language is empty". For testing I added 2 url aliases on my test D6 site, one with an empty language and one with 'af' as the language.

Then, without enabling language module, ran a migration. Post migration the url alias for the alias that didn't original have a language set worked without having to save the node. Good. Now for the other one. I enabled the language, changed my language to 'af', and tried that alias. And got 'Page not found'. After saving the node the alias worked.

The ran a migration with all the multilingual modules installed and Afrikaans installed. The results were the same as when the language modules weren't installed.

The attached patch also has changes to MigrateUrlAliasTest because it was testing for a NotNull path when, in fact, the service returns FALSE if there is no path. Also added tests for 3 different languages instead of 1.

Without the patch bad things happen to the url_alias table. After doing the save node thing, the language field for the 'af' url alias was overwritten with 'und'.

Nice the patch didn't fail, but still need to fix the original problem.

Actually, think this is working correctly. it was me not setting up to use the second language correctly. Some more testing has convinced me this is working.

Can someone familiar with multi-lingual confirm this?

Behaviour in HEAD isn't consistent at the moment on new sites either.

Not sure if this is helpful, here, but I was having a similar (404 error) problem, which had nothing to do with the language settings. For me, the imported path alias had a trailing slash.

I'm working on an upgrade migration from Drupal 6 to Drupall 8.0.2 using Migrate Upgrade and I have hit the issue outlined in the original post.
All the path aliases are apparently migrated correctly, but the aliases do not work and visiting a node from the /admin/content page displays the internal Drupal path like node/321.
Editing and saving the node fixes the url alias.
A quick look in the url_alias table shows that the langcode column is empty. Editing and saving the node changes this to 'und'.

I've tested the patch in #19 and re-run the upgrade migration on a fresh install, and the problem is resolved.

So the patch in #19 works for me!
- Drupal 8.0.2
- Migrate Upgrade 8.x-1.x-dev ( 2015-Dec-05)

+++ b/core/modules/path/src/Plugin/migrate/process/d6/UrlAlias.php
@@ -0,0 +1,33 @@
+ *   id = "d6_url_alias"
...
+class UrlAlias extends ProcessPluginBase {

Is this plugin named correctly? It seems all it does it calculate the alias language?

Good point. Changed to d6_url_alias_language. Will that do or did you have something else in mind?

That works for me, it could be just url_alias_language since i'm presuming this will work exactly the same for D7? Although, i've not confirmed that.

I was thinking the same thing. I opted to stick with D6 until such time it was confirmed this was needed for D7.

OK, RTBC.

Not sure why there is a failure.

Ran into this issue. The patch in #26 looks to be working just fine.

Source (D6)

mysql> SELECT src,dst from url_alias WHERE src = 'node/12';
+---------+----------------------------+
| src     | dst                        |
+---------+----------------------------+
| node/12 | documentation/mypath |
| node/12 | mypath                      |
+---------+----------------------------+
2 rows in set (0.01 sec)

Both paths were failing with a 404 on D8 until the node got re-saved. With the patch, both paths now work as expected. Looks RTBC to me.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x

I added paulihuhtiniemi, eliza411, benjy and mikeryan to issue credit but not to the commit message.

+++ b/core/modules/path/migration_templates/d6_url_alias.yml
@@ -17,6 +17,9 @@ process:
+  langcode:
+    plugin: d6_url_alias_language
+    source:
+      - language

+++ b/core/modules/path/src/Plugin/migrate/process/d6/UrlAliasLanguage.php
@@ -0,0 +1,33 @@
+  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
+    $language = reset($value);

Oops! anavarre picked this up, there's no reason for the source value to be an array... we should be able to specify the YAML as

source: language

but the reset() complains.

@mikeryan would you mind opening a quick follow-up issue for that patch? Just keeps the commit history more obvious when there's 1 commit per issue. Sorry I missed that before committing.

@catch: #2671946: d6_url_alias_language should accept language as a scalar opened, thanks.