Problem/Motivation

A major use case for redirects is supporting legacy URLs after migrating content.

Proposed resolution

We can provide a handler for the Migrate module. This way, a developer can add a mapping with a simple one-line addition to their migrate class:

$this->addFieldMapping('migrate_redirects', 'old_legacy_path');

Remaining tasks

Patch reviews

User interface changes

none

API changes

The patch implements hook_migrate_api(), and adds a migrate.inc file containing the handler. The autoloader will make sure that migrate.inc is only loaded when needed.

CommentFileSizeAuthor
#128 interdiff-125-128.txt2.41 KBbenjifisher
#128 support_migrate_module-1116408-128.patch5.83 KBbenjifisher
#125 support_migrate_module-1116408-125.patch5.73 KBmikeryan
#124 redirect-support_migrate_module-1116408-124.patch5.33 KBtauno
#120 support_migrate_module-1116408-116.patch5.45 KBpatrick.thurmond@gmail.com
#118 1116408-118.diff4.69 KBpatrick.thurmond@gmail.com
#116 support_migrate_module-1116408-116.patch5.45 KBpatrick.thurmond@gmail.com
#114 support_migrate_module-1116408-114.patch5.36 KBpatrick.thurmond@gmail.com
#112 support_migrate_module-1116408-112.patch5.23 KBpatrick.thurmond@gmail.com
#109 support_migrate_module-1116408-109.patch4.29 KBgeerlingguy
#109 interdiff-101-109.txt411 bytesgeerlingguy
#101 redirect-migrate-support-1116408-101.patch4.65 KBhussainweb
#101 interdiff-96-101.txt3.13 KBhussainweb
#96 redirect-migrate-support-1116408-96.patch5.31 KBDarren Shelley
#92 interdiff.txt632 bytesrodrigoaguilera
#92 redirect-migrate-support-1116408-92.patch5.29 KBrodrigoaguilera
#88 interdiff.txt1.8 KBrodrigoaguilera
#87 redirect-migrate-support-1116408-87.patch5.29 KBrodrigoaguilera
#69 redirect-migrate-support-1116408-69.patch4.78 KBkostajh
#68 redirect-migrate-support-1116408-68.patch4.75 KBkostajh
#67 redirect-migrate-support-1116408-67.patch4.12 KBkostajh
#65 redirect-migrate-support-1116408-65.patch3.9 KBrvilar
#59 redirect-migrate-support-1116408-59.patch3.88 KBmikeker
#59 redirect-migrate-support-1116408.diff893 bytesmikeker
#53 redirect-migration-support-1116408-53.patch4.39 KBjamsilver
#52 migrate_redirect-1116408-52.patch3.84 KBbrockfanning
#45 migrate_redirect-1116408-44.patch3.71 KBmikeryan
#42 Screenshot.png206.58 KBCaptainSully
#40 migrate_redirect-1116408-40.patch3.63 KBrbayliss
#39 migrate_redirect-1116408-39.patch3.49 KBMatt V.
#34 migrate_redirect-1116408-34.patch3.49 KBmikeryan
#26 1116408_26_migrate_redirect.patch3.7 KBaidanlis
#22 1116408_22_migrate_redirect.patch3.29 KBmikeryan
#21 1116408_21_migrate_redirect.patch3.48 KBmikeryan
#19 1116408_19_migrate_redirect.patch3.63 KBmikeryan
#16 1116408_16_migrate_redirect.patch7.68 KBmarcusx
#14 1116408_14_migrate_redirect.patch7.82 KBmarcusx
#12 1116408_12_migrate_redirect_fix.patch1.27 KBmarcusx
#11 1116408_11_migrate_redirect_fix.patch819 bytesmarcusx
#9 1116408_9_migrate_redirect_fix.patch586 bytesmarcusx
#6 1116408_migrate_redirect.patch2.53 KBgrendzy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeryan’s picture

Project: Migrate » Migrate Extras
Component: Code » Miscellaneous
Category: support » feature

Support for contrib modules would go into the Migrate Extras module.

Dave Reid’s picture

I could add migration support to Redirect.module itself?

skitten’s picture

In the end I just wrote a small module to catch urls of the old format, look up the id in the migration db and redirect to the new node. But I'm not sure what the best way to write a generic migration/redirect mechanism would be.

mikeryan’s picture

Project: Migrate Extras » Redirect
Version: 7.x-2.x-dev » 7.x-1.x-dev
Component: Miscellaneous » Code

Yes, Dave, you certainly could:-).

mlncn’s picture

Not sure how this should manifest in either the Redirect or Migrate modules, if either, but here is the code i'm using in a custom migration module that extends Migrate of course and also makes use of Redirect.

In the include file for the class that extends the BaseMigration class, most of the action is in a class extending MigrateDestinationHandler, show second.

/**
 * Migrate tt news from Typo3 to Press release content type.
 */
class ExamplePressReleaseMigration extends BaseMigration {
  public function __construct() {
    parent::__construct();
// ... bunch of stuff not relevant here, but crucially including in the query the field that has the current path alias:
      ->fields('txalias', array('value_alias'))
// ... and more stuff, including this:
    $this->addFieldMapping('migrate_pathalias', 'value_alias');
// ... and in this case there was a need to construct the alias from possibly multiple fields:
  public function prepareRow($currentRow) {
    $currentRow->value_alias = example_migrate_news_path($currentRow->value_alias, 366);
  }
}

class ExampleMigratePathHandler extends MigrateDestinationHandler {
  public function __construct() {
    $this->registerTypes(array('node'));
  }

  public function fields() {
    return array(
      'migrate_pathalias' => t('Array of existing paths for the TYPO3 page.'),
    );
  }

  // Alternately, we could add redirects to the node array in the prepare
  // method, most likely, but going with the more aggressive way first.
  public function complete($entity, stdClass $row) {
    // If there's no alias in the source data, we have no choice but to bail.
    if (!isset($entity->migrate_pathalias)) {
      return;
    }
    if (!is_array($entity->migrate_pathalias)) {
      $entity->migrate_pathalias = array($entity->migrate_pathalias);
    }

    // Construct the destination ('redirect') path for nodes.
    if (isset($entity->nid)) {
      $destination = 'node/' . $entity->nid;
    }

    // If we cannot determine the path of the present entity, bail.
    if (!isset($destination)) {
      return;
    }

    // Establish defaults and make the redirects with the above values.
    $redirect_defaults = array(
      'uid' => 2,
      'status_code' => 301,
    );
    $redirect = new stdClass();
    redirect_object_prepare($redirect, $redirect_defaults);
    $redirect->redirect = $destination;
    foreach ($entity->migrate_pathalias as $path) {
      $redirect->source = $path;
      redirect_save($redirect);
    }
  }
}
grendzy’s picture

Status: Active » Needs review
FileSize
2.53 KB

To use this handler, simply add a mapping to your migrate class like so:

$this->addFieldMapping('migrate_redirects', 'old_legacy_path');
grendzy’s picture

Title: How to add redirected urls to migrated nodes » Support migrate module: redirect old urls to migrated nodes
Mark Theunissen’s picture

Awesome, this is exactly what we need. I'll be testing soon and will report back.

marcusx’s picture

Status: Needs review » Needs work
FileSize
586 bytes

Redirect is not saved. Because the entity_uri can not been determined due to wrong argument 'entity type'. Acctually this is not the entity type it is the entity_type_bundle_type or whatever it is called by the entity api. e.g. in a page node migration this is 'page' not 'node'. I have no idea if we can get the migrate destination type at this place to make this generic for nodes and users or whatever. For most cases 'node' would work fine.

Patch for the patched version enclosed.

  public function complete($entity, stdClass $row) {
     //wrong assumtion due to the variable name '$entity' $entity->type holds the nodetype, 'page' in my case
     // but we need to put 'node' or 'user' in the argument for entity_uri
    if (($destination = entity_uri('node', $entity)) && !empty($entity->migrate_redirects)) {
      if (!is_array($entity->migrate_redirects)) {
        $entity->migrate_redirects = array($entity->migrate_redirects);
      }

      foreach ($entity->migrate_redirects as $path) {
        $redirect_defaults = array(
          'uid' => $entity->uid,
          'status_code' => 301,
          'language' => $entity->language,
        );
        $redirect = new stdClass();
        redirect_object_prepare($redirect, $redirect_defaults);
        $redirect->redirect = $destination['path'];
        $parsed = redirect_parse_url($path);
        $redirect->source = $parsed['path'];
        $redirect->source_options['query'] = $parsed['query'];
        redirect_save($redirect);
      }
    }
  }
grendzy’s picture

Hm, that is a puzzle: the handler in #6 works, but only because $entity->uri was already set by the time entity_uri() was called. As a result the (incorrect) type parameter was ignored.
I'd like to avoid hard-coding "node", so that it could work for other entities. I'm not sure what's the best way to handle this. We could just refer to $entity->uri directly, but I don't know if that's guaranteed to be set.

marcusx’s picture

I totaly agree that hardcoding 'node' is a bad idea. I had no time to figure out how I can get this information from migrate yesterday and needed this working. I suggest this better solution.

...
  public function complete($entity, stdClass $row) {
  	
    //Looking up the destination entity type for the current Migration
    $migration = Migration::currentMigration();
    $destination = $migration->getDestination();
    $entity_type = $destination->getEntityType();
    
    if (($destination = entity_uri($entity_type, $entity)) && !empty($entity->migrate_redirects)) {
      if (!is_array($entity->migrate_redirects)) {
        $entity->migrate_redirects = array($entity->migrate_redirects);
      }
...

Patch against the patched version. Sorry for that, I know there is a way to create a follow up patch, I have to read git documentation about how this works again.

marcusx’s picture

Aehm maybe we should also rename the variable "$destination->$redirect_destination" to avoid confusion.


  public function complete($entity, stdClass $row) {
  	
    //Looking up the destination entity type for the current Migration
    $migration = Migration::currentMigration();
    $destination = $migration->getDestination();
    $entity_type = $destination->getEntityType();
    
    if (($redirect_destination = entity_uri($entity_type, $entity)) && !empty($entity->migrate_redirects)) {
      if (!is_array($entity->migrate_redirects)) {
        $entity->migrate_redirects = array($entity->migrate_redirects);
      }

      foreach ($entity->migrate_redirects as $path) {
        $redirect_defaults = array(
          'uid' => $entity->uid,
          'status_code' => 301,
          'language' => $entity->language,
        );
        $redirect = new stdClass();
        redirect_object_prepare($redirect, $redirect_defaults);
        $redirect->redirect = $redirect_destination['path'];
        $parsed = redirect_parse_url($path);
        $redirect->source = $parsed['path'];
        $redirect->source_options['query'] = $parsed['query'];
        redirect_save($redirect);
      }
    }
  }

grendzy’s picture

marcusx: oh, that's excellent. I'll work on a re-roll.

BTW - do you happen to know if $entity->uid and $entity->language are valid for other entity types? Or are those node properties?

marcusx’s picture

Status: Needs work » Needs review
FileSize
7.82 KB

Added validation before the redirect is saved because I got some database errors due to duplicate entries if the redirect was already existing. There is also a proper error message in the migration if this happens.

I created a complete reroll of #6 #12 and the additional changes for the validation.

This patch is against the latest dev from git.

grendzy’s picture

Status: Needs review » Needs work

Thanks for the re-roll. Can you please remove tabs and trailing whitespace?

marcusx’s picture

Status: Needs work » Needs review
FileSize
7.68 KB

Here we go... sorry for the inconvenience

marcusx’s picture

Status: Needs review » Needs work

Ahhh damn it! I can't set the current Migration here.

If I use

drush mfd MyMigration

I get an Fatal error.

Fatal error: Call to a member function getDestination() on a non-object in /mnt/hgfs/bk/htdocs/sites/all/modules/contrib/redirect/migrate.inc on line 19

What is quite clear because at this point there is no "current" Migration.

  public function __construct() {
    $this->registerTypes(array('entity'));

    //set current migration data that all other methods can use
    $this->migration = Migration::currentMigration();
    $this->destination = $this->migration->getDestination();  //Line 19
    $this->entity_type = $this->destination->getEntityType();
  }

I will rework this a soon as I can, but in case s.o. else has time I just want to document it.

Fidelix’s picture

This would be a much appreciated addition.
Thank you all for the excellent work!

mikeryan’s picture

Status: Needs work » Needs review
FileSize
3.63 KB

So close! Just needed to put off pulling stuff dependent on the active migration to complete() (note we now have a static message display function that will work anywhere without a Migration object).

mikeryan’s picture

Status: Needs review » Needs work

Oops! Caching the entity_type breaks when running a sequence of disparate migrations, since there's only one handler instance ever, not one per entity type... New patch coming.

mikeryan’s picture

Status: Needs work » Needs review
FileSize
3.48 KB
mikeryan’s picture

Some tweaks, in particular saving messages as informationals in the migrate message table rather than spewing them out.

bforchhammer’s picture

Status: Needs review » Reviewed & tested by the community

Patch #22 seems to be working quite well. Thanks for the great work!

mikeryan’s picture

Before committing, I just want to make sure it's properly credited - I just added the final tweaks, grendzy and marcusx did most of the work.

aidanlis’s picture

Should the file not be called redirect.migrate.inc for consistency with other projects?

aidanlis’s picture

Patch #22 is also missing the hook_migrate_api(). I've added that in and renamed the file "redirect.migrate.inc" to be consistant with every other module implementing migrate support.

I haven't contributed anything new to this patch, credit goes to marcusx and grendzy and others.

sirkitree’s picture

Sweet, so this is passing, but can I get a little hint on how to test it out?

I'm working on a migration currently, and one of my content types (let's say article) is migrating fine. I also have the:

$this->addFieldMapping('path', 'path');

I've applied the patch, and have it applied cleanly with patch p1 < *.patch.

What do I add to my current node migration?

Edit:
I'm migrating from path_redirect

grendzy’s picture

sirkitree: addFieldMapping() is all. The first parameter is literally 'migrate_redirects', and the second parameter is the name of a field in your source data.

sirkitree’s picture

This worked perfectly for me. Thank you, saved me so much time.

dillix’s picture

#26 doesn't work as expected, then enabled migrate extras and pathauto module. I use following code for migration:

//We generate new URL during migration
    $this->addFieldMapping('pathauto')
         ->defaultValue(1);
    //We save redirect from old legacy URL to new legacy URL
    $this->addFieldMapping('migrate_redirects', 'old_url');

But in redirect.migrate.inc in complete function I have entity:

stdClass Object
(
[title] => Nissan cars
...
[migrate_redirects] => ?go=obzor&amp;oid=7
[type] => articles
...
[path] => Array
(
[pathauto] => 1
[alias] => 
)
...

My old url - ?go=obzor&oid=7 and my new url should be /article/1 but I got /node/1. How can I tell to MigrateRedirectEntityHandler pathauto generated alias?

dillix’s picture

Priority: Normal » Critical
Status: Reviewed & tested by the community » Needs work

As I wrote, patches #22, #26 needs work to integrate with pathauto. When I go to url /?go=obzor&oid=7 then it 301 redirect to /node/1 and then it 301 redirect /article/1. I think its because pathauto runs after node creation and migrate redirect runs before.

bforchhammer’s picture

Priority: Critical » Normal
Status: Needs work » Reviewed & tested by the community

Pathauto generation does not have anything to do with this issue.

Besides, it should already work just fine if pathauto is configured correctly. In my case I was fine not adding anything to my migration classes, because the path is usually created automatically during node save...

When I go to url /?go=obzor&oid=7 then it 301 redirect to /node/1 and then it 301 redirect /article/1.

This sounds like it is working, no? What is it that you're expecting?

dillix’s picture

Status: Reviewed & tested by the community » Needs work

No, it should set redirect from /?go=obzor&oid=7 to /article/1 in redirect module, but I have /?go=obzor&oid=7 to /node/1 and in path auto /node/1 to /article/1. And when I run migrate, I have a lot of notices here http://site.local/#overlay=admin/content/migrate/messages/NodeArticles like this:
Undefined index: path File /Users/macspro/Sites/site.local/sites/all/modules/migrate_extras/redirect.migrate.inc, line 73(file: /Users/macspro/Sites/site.local/sites/all/modules/migrate_extras/redirect.migrate.inc, line 73)

It happens because pathauto runs after we generate redirect with redirect.migrate.inc. As you can see in #30 (I have following data in complete function in redirect.migrate.inc):

stdClass Object
(
[title] => Nissan cars
...
[migrate_redirects] => ?go=obzor&amp;oid=7
[type] => articles
...
[path] => Array
(
[pathauto] => 1
[alias] => 
)
...
mikeryan’s picture

Status: Needs work » Needs review
FileSize
3.49 KB

@dillix: I see no problem with the indirect 301 redirect, everything ends up as it should be. We don't know if and when pathauto will be creating an alias (and note that in migrations many people disable pathauto during migration and generate aliases in batch afterwards), so it's simplest and safest to just establish the redirect to the canonical address - it all works out in the end.

As for the notices, your paths don't have a 'path' component, just a query string - this updated patch should address it. Also note that I've moved hook_migrate_api() to the migrate.inc file, no need to have it in .module.

Status: Needs review » Needs work

The last submitted patch, migrate_redirect-1116408-34.patch, failed testing.

marvil07’s picture

Title: Support migrate module: redirect old urls to migrated nodes » Support migrate module: Destination handler class

I have just created another issue about migrate, which is not exactly the same of this at #1607038: Support migrate module: Individual destination class

As I mention there, I could merge the code here if maintainers thinks it's better.

dakhota’s picture

Status: Needs work » Needs review

#34: migrate_redirect-1116408-34.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, migrate_redirect-1116408-34.patch, failed testing.

Matt V.’s picture

Status: Needs work » Needs review
FileSize
3.49 KB

Here's an updated patch that is applying cleanly for me.

rbayliss’s picture

This works nicely, but as grendzy pointed out above, it requires you to map UID and language, which don't exist for all entities (taxonomy terms, for example). This patch adds isset() checks on these two properties.

Also, more generally, does it even make sense that the redirect would get the UID and language of the entity?

Status: Needs review » Needs work

The last submitted patch, migrate_redirect-1116408-40.patch, failed testing.

CaptainSully’s picture

FileSize
206.58 KB

I've tried the patches in #26 and #39, and in both cases I'm running into a problem where in the 'from' field of the redirect it parses the url such that there is a '/' in front. This is a problem since the beginning of the path is forced to be "http://localhost/drupal1/".

So if "http://localhost/drupal1/scholars/index" goes through the migration, I end up with
"http://localhost/drupal1//scholars/index" instead, and the // results in a page not found.

screenshot attached

elBradford’s picture

39 worked for me - helped create redirects for over 2000 nodes. Thanks!

eojthebrave’s picture

This worked great for me. Using something like the following called from prepareRow() to lookup path_redirect module redirects.

  /**
   * Return old redirect for given row.
   */
  public function getRedirect(&$row) {
    migrate_instrument_start('MyMigration: get redirect');
    $old_redirect = 'node/' . $row->nid;
    $results = $this->getConnection()
      ->select('path_redirect', 'pr')
      ->fields('pr', array('source'))
      ->condition('pr.redirect', $old_redirect)
      ->execute();
    if (!empty($results)) {
      foreach ($results as $result) {
        $row->redirect[] = $result->source;
      }
    }
    migrate_instrument_stop('MyMigration: get redirect');
  }
mikeryan’s picture

Status: Needs work » Needs review
FileSize
3.71 KB

For Migrate 2.5 and greater, handlers should be explicitly registered in hook_migrate_api().

brunodbo’s picture

Tested the patch in #45, works great. Just one question on the syntax: how would I write a field mapping rule to import multiple legacy redirects for 1 node?

brianfisher’s picture

#45 worked perfectly for me, thanks

mikeryan’s picture

@brunodbo - not a matter of syntax, what you need to do is populate the source field in your mapping with an array of the redirects you want (presumably in prepareRow).

brockfanning’s picture

#45 worked great for me, as well. I love that it implements the rollback too. Thanks everyone for putting this together!

Simon Georges’s picture

Status: Needs review » Reviewed & tested by the community

At this point, I think we can consider it reviewed and tested ;-)

brockfanning’s picture

Status: Reviewed & tested by the community » Needs work

I just noticed a minor issue when adding redirects for users (users which had in turn been created through another migration). The entity->language was apparently set as an empty string, so it was passing the "isset" conditional. Apparently redirects don't work if the language column is a blank string. It's an easy fix, just have to also test for blank strings. I will try to post my fix as a patch, just a temporary heads up until I get the time.

brockfanning’s picture

Status: Needs work » Needs review
FileSize
3.84 KB

Very sorry for the delay, here is that patch I promised above.

jamsilver’s picture

The patch in #52 did not work for me (using Migrate 2.5 - the latest stable version). Wrapping a line of the hook_migrate_api() implementation in an array did the trick. Turning:

+++ b/redirect.migrate.incundefined
@@ -0,0 +1,104 @@
+    'api' => 2,
+    'destination handlers' => 'MigrateRedirectEntityHandler',
+  );
+  return $api;
+}

Into:

+++ b/redirect.migrate.incundefined
@@ -0,0 +1,104 @@
+    'api' => 2,
+    'destination handlers' => array(
+      'MigrateRedirectEntityHandler',
+    ),
+  );
+  return $api;
+}

Patch rerolled and attached.

rvilar’s picture

Patch in #53 works for me.

DanChadwick’s picture

Patch 53 worked for me. Thank you very much.

JurriaanRoelofs’s picture

Status: Reviewed & tested by the community » Needs review

patch 53 works, thx. Tested on nodes and files.

JurriaanRoelofs’s picture

Status: Needs review » Reviewed & tested by the community
brant.darilek’s picture

Status: Needs review » Reviewed & tested by the community
mikeker’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
893 bytes
3.88 KB

The patch in #53 includes a leading slash for absolute URLs. For example, "http://example.com/foo" ends up saving a redirect of "/foo" instead of "foo". (Note: the UI prevents leading slashes from being added to redirects).

The attached patch is #53 plus an ltrim() on the parsed path. Interdiff attached, or @line 81:

$redirect->source = isset($parsed['path']) ? $parsed['path'] : '';

becomes

$redirect->source = isset($parsed['path']) ? ltrim($parsed['path'], '/') : '';
alforddm’s picture

#53 worked for me on nodes. Really appreciate the work on this.

jeffschuler’s picture

Status: Needs review » Needs work

#59 is working great for me.
Thanks!!

Very minor docs changes:

+++ b/redirect.migrate.incundefined
@@ -0,0 +1,106 @@
+   * Validate a redirect.

"summary lines must start with a third person singular present tense verb" ("Validates") and should have an extra newline.

+++ b/redirect.migrate.incundefined
@@ -0,0 +1,106 @@
+   * This function is simmilar to the validate function in the

"simmilar" should be "similar"

kwseldman’s picture

These patches don't appear to be working for me. I've tried both #53 and #59. I'm using Redirect 7.x-1.0-rc1. After migrating my Wordpress site to Drupal and having pathauto generate new aliases, when I point my browser to any of my old paths on my new Drupal site, it takes me to the front page of my blog rather than giving me a 301 HTTP code.

I noticed that when I migrated my content and chose the option "Have pathauto generate new aliases," I got this message: "You have the Redirect module enabled. To be able to generate redirects for your imported WordPress content, you need to patch Redirect." This was displaying after I'd patched Redirect. Does this message go away after patching, or no? I'm not getting an option to generate redirects. Does this indicate that the patch hasn't worked?

My husband (oseldman) has also suggested that due to the format of my Wordpress paths (for example: http://littleenglishgirl.com/?p=1171) this method of redirecting may not work at all - does anyone have similar paths, and if so, did this method work? If not, is there something else I can try? Thanks.

kwseldman’s picture

I realized I was patching 7.x-1.0-rc1 rather than the dev version. I patched the dev version with #59 and the option to generate redirects appeared.

kwseldman’s picture

Even with the patch seemingly working, redirects still aren't being generated. Any thoughts as to why that might be happening? Sorry for bombarding the issue queue. Thanks.

rvilar’s picture

Status: Needs work » Needs review
FileSize
3.9 KB

This is the patch in #59 with fixes from #61 applied and with coder review done. It works for me.

Bhanuji’s picture

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'xJMoIg4sq_0tnmtrrKRhOC6-2F1yga28A75jvr3tXCY' for key 'hash': INSERT INTO {redirect} (hash, type, source, source_options, redirect, redirect_options, language, status_code, count, access) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9); Array ( [:db_insert_placeholder_0] => xJMoIg4sq_0tnmtrrKRhOC6-2F1yga28A75jvr3tXCY [:db_insert_placeholder_1] => redirect [:db_insert_placeholder_2] => magazines/quiltsandmore/qam-table-contents-old_1.html [:db_insert_placeholder_3] => a:0:{} [:db_insert_placeholder_4] => node/32553 [:db_insert_placeholder_5] => a:0:{} [:db_insert_placeholder_6] => und [:db_insert_placeholder_7] => 0 [:db_insert_placeholder_8] => 0 [:db_insert_placeholder_9] => 0 ) in drupal_write_record() (line 7158 of /app/web/divinecaroline.com/includes/common.inc).

I guess, there might be multiple redirects in redirect table..

In migration, how to solve it.

As we are using Html (Static files) to Drupal migration.

kostajh’s picture

This version of the patch in #65 fixes some Coder complaints, and also allows for multiple redirects. Checking $entity for "migrate_redirects", at least in my environment, would only ever return one redirect to evaluate. Checking $row->migrate_redirects however returned an array of redirects defined in my mapping, which correctly saved multiple redirects for a given entity.

kostajh’s picture

Sorry, use this one instead. This works in my environment for cases with one or multiple redirects.

kostajh’s picture

Found another issue. Here's a new version of the patch.

kostajh’s picture

Issue summary: View changes

add summary

mariacha1’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

I ran this through a handful of redirect migrations using the structure:

 $this->addFieldMapping('migrate_redirects', 'old_path')
        ->defaultValue('MigrateRedirectEntity');

What I tested:
Redirects on Users
Redirects on Nodes
Redirects on Terms
Redirects using a separator (comma)
Redirects that produced the error "The source path is already being redirected."

Code looks solid as well. I think this is good to commit!

JeremyFrench’s picture

I have just used this patch for the second time now and it works fine.

Also think it is good to commit.

benjifisher’s picture

I am adding a related issue: #1607038: Support migrate module: Individual destination class.

The patch works as expected so far, updating migrated taxonomy terms.

kostajh’s picture

Any chance this could get committed?

joelstein’s picture

+1 from me. I'm using this patch for Migrate support, and I've had no problems. Works as advertised!

roberttstephens’s picture

I've used this several times as well without issues.

heddn’s picture

heddn’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/redirect.migrate.inc
@@ -0,0 +1,139 @@
+        $redirect->redirect = $redirect_destination['path'];
+        $parsed = redirect_parse_url($path);
+        $redirect->source = isset($parsed['path']) ? ltrim($parsed['path'], '/') : '';
+        if (!empty($parsed['query'])) {
+          $redirect->source_options['query'] = $parsed['query'];
+        }

Shouldn't there be a redirect_parse_url on the destination path? In case it has query string parameters?

heddn’s picture

benjifisher’s picture

I am adding #1607038: Support migrate module: Individual destination class as a related issue. I tried to do that in #72, but I guess it did not work.

apanag’s picture

Patch worked great. Thank you!

ckng’s picture

Path #69 works great for import. However, any suggestion on handling rollback?

mikeryan’s picture

Status: Needs work » Reviewed & tested by the community

Restoring the rtbc status.

@heddn: The destination of the field handler redirect is a newly-created entity - i.e., the destination path is always going to be something like node/123, user/456, etc., there will never be query string parameters.

Many people have been successfully using the patches from this issue for years - please, let's get it committed and address any narrow use cases missed in follow-up issues.

heddn’s picture

I'm fine with follow-ups to address outstanding issues.

re #82: In my use case, the redirects were from a CSV dump from an SAP-based website that was getting migrated to Drupal and pointed to off-site content. The anchor tags and query parameters were important to parse and migrate.

cthos’s picture

@heddn - Wouldn't that use case be better addressed in a migration using MigrateDestinationRedirect?

If they're not going to the node being created as part of a field handler, that doesn't seem like the proper use of ->addFieldMapping('migrate_redirects');?

DanChadwick’s picture

Re #82: +1.

rodrigoaguilera’s picture

Status: Reviewed & tested by the community » Needs work

I'm creating nodes with a language set but I want to redirect for all languages
example:

I create node/23 in Spanish with the redirect /myoldurl. The redirect will be created in Spanish so the redirect will happen on someone going to /es/myoldurl not /myoldurl as I wanted.

The language of the redirect depends on the language of the entity when it shouldn't be like that.

rodrigoaguilera’s picture

Patch rerolled to apply clean and support a language override with a new migrate field
migrate_redirects_override_language
#81
handles rollback already

rodrigoaguilera’s picture

FileSize
1.8 KB

forgot the interdiff

heddn’s picture

  1. +++ b/redirect.migrate.inc
    @@ -18,7 +21,10 @@ class MigrateRedirectEntityHandler extends MigrateDestinationHandler {
    +      'migrate_redirects_override_language' => t('Language for the path(s) to redirect from.'),
    

    Can't this just use 'language'? And again in line 111/112?

rodrigoaguilera’s picture

@heddn I don't get you.
What I this enables is the possibility for the redirects to be in a different language from the entity or other language things involved.

And maybe the type of the redirection should be something that can be overriden because is hardcoded

rodrigoaguilera’s picture

Status: Needs work » Needs review
rodrigoaguilera’s picture

mistake on line 111 using row instead of entity

heddn’s picture

Status: Needs review » Needs work
+++ b/redirect.migrate.inc
@@ -0,0 +1,153 @@
+          $redirect_defaults['language'] = $entity->migrate_redirects_override_language;

Is migrate_redirects_override_language a field/property on the entity? Can you explain more about it? It seems like a custom field, rather than the language property that is commonly available to entities. If it custom, this should be removed from the general use patch. It can be re-added to a custom extension of MigrateRedirectEntityHandler for your use case.

rodrigoaguilera’s picture

I think the language of the redirection should be handled in this issue other than setting the same language as the entity that has the redirection.
My patch is a suggestion about how to handle this case: I want my entity in one language and the redirection in another.

jeffam’s picture

Thanks for all the good work on adding support for redirects to migrations. Just yesterday I needed to create redirects during a migration and it was a ton of help to find this work.

Since it seems like folks are still hashing out some details, and I didn't want to patch my copy of redirect, I figured I'd share a solution that uses this code in one's custom migration module.

I essentially applied the patch in #92 to my custom migration module. I did it manually, by adding files[] = redirect.migrate.inc to the .info file and by creating the redirect.migrate.inc file with the most of the contents of the patch. Lastly, I updated my hook_migrate_api() settings to include the following in the returned array:

    'destination handlers' => array(
      'MigrateRedirectEntityHandler',
    ),

With that done, I can now use a mapping like this:

    // In the __construct() method of my migration class:

    // Add redirects to old URLs.
    $this->addFieldMapping('migrate_redirects', 'article_ID')
      ->callbacks(array($this, 'generateRedirectUrl'));

    // ...then later in my migration class:
    
    /**
     * Callback function to generate redirects for old-site-style URLs.
     */
    protected function generateRedirectUrl($article_id) {
      return sprintf('path/to/article/id-%d', $article_id);
    }

Just tested it and it works great. Thanks, all!

Darren Shelley’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.31 KB

If we look at the problem and motivation behind the patch we see that the intention is for "supporting legacy URLs after migrating content".
The most simple manifestation of this is where an entity has one to many urls associated with it and we have a need to redirect those from their previous location to a common path such as entitytype/entityid (Path module + global redirect can handle getting them to the pretty and often dynamic version)
Dave Reid early on highlighted Redirect as being the best location for this work.
For over 3 years the wordpress_migrate has been openly supporting patches from this issue queue and now actively supports patch #69.
I feel that it's important for the usability of the wordpress_migrate module to remove it's reliance on patching this module to perform what is a relatively common and important task to any migration.
Having reviewed the patches I feel that the patch in #69 best fits this need.

The later work by rodrigoaguilera whilst good hits a specific user need that the majority of migrations won't encounter so in keeping with agile methodology I see no reason to delay providing a solution in favour of waiting for agreement on how that additional functionality might manifest.

I would suggest that this multilingual request/work be moved to a separate issue queue.

I have attached a patch that applies cleanly against the latest 7.x-1.x-dev and have restored RTBC status.

I have tested this patch in a number of migrations successfully.

Thank you to all the developers for their hard work thus far on this matter.

osopolar’s picture

+1 for getting this in, and discuss additional stuff in follow up requests.

This works for me, although I am using the same method as jeffam describes above in #95 with a tiny difference: I renamed the class to avoid a possible redeclare class error (which may happens on updating redirect module after the patch got committed).

rodrigoaguilera’s picture

Ok, I agree that the language situation can be handled on other issue. I will open it and make the patch once this is committed so we don't make patches over patches

dillix’s picture

+1 for RTBC

jonathan_hunt’s picture

Patch in #95 works for me, thanks. FYI, I used $this->addFieldMapping('migrate_redirects', 'path'); in my extension of DrupalNode6Migration (from migrate_d2d).

hussainweb’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.13 KB
4.65 KB
  1. +++ b/redirect.migrate.inc
    @@ -0,0 +1,139 @@
    +    if (isset($row->migrate_redirects) && !empty($row->migrate_redirects)) {
    +      $migrate_redirects = $row->migrate_redirects;
    +    }
    

    The call to isset is redundant.

  2. +++ b/redirect.migrate.inc
    @@ -0,0 +1,139 @@
    +        if (isset($entity->language)) {
    +          if ($entity->language == '') {
    +            $redirect_defaults['language'] = LANGUAGE_NONE;
    +          }
    +          else {
    +            $redirect_defaults['language'] = $entity->language;
    +          }
    +        }
    

    This can be hugely simplified.

I have fixed the above and cleaned up the code in other places as seen by the interdiff.

rodrigoaguilera’s picture

Status: Needs review » Needs work
+++ b/redirect.info
@@ -2,6 +2,7 @@ name = Redirect
+files[] = redirect.migrate.inc

This is not needed, the file will be detected automatically http://cgit.drupalcode.org/migrate/tree/migrate_example/migrate_example....

heddn’s picture

Status: Needs work » Reviewed & tested by the community

I agree with #102, however that can be fixed on commit, unless someone gets to re-rolling this before then. Moving back to RTBC. The changes in #101 are sane.

hussainweb’s picture

@rodrigoaguilera, @heddn: Are you sure about that? I have had trouble with classes not found in the past, but they could just be with destinations, not handlers.

rodrigoaguilera’s picture

100% sure, never had a problem making migrate modules using that naming.
Also no problem with handlers #2512562: Migrate support

ohthehugemanatee’s picture

I've used this on a couple of projects now. RTBC for sure.

Summit’s picture

Hi,
What do you need to use this redirect.migrate.inc (https://www.drupal.org/node/1116408#comment-10275065)
Is this script in a bigger script enough?

$this->addFieldMapping('migrate_redirects', 'old_path')
        ->defaultValue('MigrateRedirectEntity');

EDIT, I see the MigrateRedirectEntityHandler handler within my Migrate Handlers.
But with adding this $this->addFieldMapping lines within my weblinks.migrate.inc script I do not see my redirects migrated from drupal 6 to drupal 7.
I see one redirect added and that is:
MigrateRedirectEntity node/1215
And furthermore a lot of mistakes like: The source path is already being redirected.

...What do I do wrong please?

Greetings, Martijn

emilorol’s picture

I was able to make it work and I posted my answer here:

http://drupal.stackexchange.com/questions/179083/drupal-6-to-drupal-7-mi...

Thank you for putting this together.

geerlingguy’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
411 bytes
4.29 KB

Just moving this along by uploading a patch with the change suggested by #102 and #104. Another +1 on getting this into the module!

geerlingguy’s picture

As an aside, if you're an oddball like me and you want to preserve all the existing redirects in an existing Drupal 7 site when doing a Migrate D2D migration of a content type, the simplest way to use this patch in your migration is to do the following:

Inside prepare():

  function prepare($entity, $row) {
    // Add in existing redirects.
    $row->migrate_redirects = $this->retrieveOldRedirects($row->nid);
  }

And then later in the migration class, define retrieveOldRedirects:

  protected function retrieveOldRedirects($nid) {
    $redirects = array();
    $node_path = 'node/' . $nid;

    // Switch to the source database.
    db_set_active('migration_source_database_name_here');

    // Get the URL alias for this node.
    $alias = db_query("SELECT alias FROM {url_alias} WHERE source = :source", array(':source' => $node_path))->fetchField();

    // Search for redirects by the found alias.
    if (!empty($alias)) {
      $redirects_by_alias = db_query("SELECT source FROM {redirect} WHERE redirect = :alias", array(':alias' => $alias))->fetchCol();
      foreach ($redirects_by_alias as $redirect) {
        $redirects[] = $redirect;
      }
    }

    // Search for redirects by the node path.
    $redirects_by_path = db_query("SELECT source FROM {redirect} WHERE redirect = :node_path", array(':node_path' => $node_path))->fetchCol();
    foreach ($redirects_by_path as $redirect) {
      $redirects[] = $redirect;
    }

    // Switch back to the main database.
    db_set_active();

    return $redirects;
  }

I'm certain there's a better/more efficient way to do this... but I was in a hurry and wanted to just grab all the redirects for a given source node, and this works great for me!

geerlingguy’s picture

Status: Needs review » Reviewed & tested by the community

(Setting back to RTBC as the patch in #109 was a one-liner to remove a file from the .info files array. No functional difference.)

patrick.thurmond@gmail.com’s picture

So I used this patch in my current project. However, I had to modify it because the project I am working on uses entity translations instead of node translations and I need to be able to apply the English and French versions of the product aliases to the same node (which translations the field entities) and the translation functionality included in the prior patch didn't account for that situation.

I also improved error messaging and slightly (but not completely) improved some of the code formatting.

My changes allow for the user to specify the language being set for the alias using "migrate_redirects_language" as a mapping point. I wanted to use the standard "FIELD_NAME:language", but I was having trouble understanding how that mechanism would work here. So if someone knows how to tackle that then by all means please do so.

Attached is my updated version of the patch file.

patrick.thurmond@gmail.com’s picture

We probably should also create a rollback option for this as well. I might try to tackle that soon.

patrick.thurmond@gmail.com’s picture

Ok, I have updated this patch again to make sure it checks the row AND the entity for the migrate_redirect_language. This fixes a problem I had and allows the mappings to passively set it instead of doing so in the prepareRow().

geerlingguy’s picture

Status: Reviewed & tested by the community » Needs review

Back to needs review since there are two new patches...

patrick.thurmond@gmail.com’s picture

One more update to this. I wasn't thinking and my error message was occurring in unrelated migrations. Fixed this by making sure that it found redirects specified in the current migration.

heddn’s picture

Can you attach some interdiffs? I was following this so I'd like to see what has changed. https://www.drupal.org/documentation/git/interdiff

patrick.thurmond@gmail.com’s picture

Here is the interdiff between the first patch I used and my most recent changes to it.

*** UPDATE: Here is a pastebin instead, so that this dumb forum system stops reporting the patch as broken. ***
http://pastebin.com/5hEPPWEq

Status: Needs review » Needs work

The last submitted patch, 118: 1116408-118.diff, failed testing.

patrick.thurmond@gmail.com’s picture

FileSize
5.45 KB

Just re-uploading my last actual patch since the stupid system won't let me delete that diff file I uploaded before.

mikeker’s picture

Status: Needs work » Needs review

@pthurmond, anything with a .diff or .patch extension will be run by the testbot. If you name your interdiff foo.interdiff.txt it won't run. Details here: https://www.drupal.org/node/332678

Not being able to delete files is by-design -- it keeps people from rewriting history. No matter how legitimate a rewrite it may be...

patrick.thurmond@gmail.com’s picture

So what does it take to get the contrib merged into the module at some point? Looks like everyone did the work for the module maintainer. He/she just needs to merge it in.

brockfanning’s picture

@pthurmond, it's now the community's job to review and test, so it can eventually be marked as "Reviewed & tested by the community". In the meantime, if the patch works for you, you're welcome to use it for your needs. Drupal 7 sites are likely going to employ more and more patches, as contrib maintainers have to spend more and more time on Drupal 8. (my day-job Drupal 7 site is using over 90 patches, and counting...)

tauno’s picture

Re-rolled the patch to create the new file in the correct directory when using drush. No other changes to the code.

mikeryan’s picture

Umm... The migrate.inc file got taken out of the .info file along the way, but it does need to be there. If it only contained hook_migrate_api() it would not be necessary, but because it contains classes it must be there for the core class registry to know where to find those classes on any request not invoking hook_migrate_api().

geerlingguy’s picture

Status: Needs review » Needs work
  1. +++ b/redirect.migrate.inc
    @@ -0,0 +1,178 @@
    +      'migrate_redirects_language' => t('The language this redirect applies to.')
    

    Technically should have a comma at end of line here.

  2. +++ b/redirect.migrate.inc
    @@ -0,0 +1,178 @@
    +   * This determines the language for the current redirect.
    

    Pedantic, but should read "Determine the language for the current redirect."

  3. +++ b/redirect.migrate.inc
    @@ -0,0 +1,178 @@
    +   * @param $entity
    

    I think we can drop the @param docs for these two params, and just add a note under the @return declaration like redirectValidate() has.

  4. +++ b/redirect.migrate.inc
    @@ -0,0 +1,178 @@
    +    //Defaults to the previous way.
    

    Needs a space between // and Defaults.

  5. +++ b/redirect.migrate.inc
    @@ -0,0 +1,178 @@
    +    /**
    ...
    +     */
    

    Inline comments need to be in // style, and in one paragraph. If necessary, maybe we pull this comment out into the main function docblock.

benjifisher’s picture

I am working on this at MidCamp.

benjifisher’s picture

I fixed a few other problems reported by CodeSniffer in addition to the ones mentioned in #126.

I rewrote the multiline comment and moved it to the doc block.

I rewrote the function: instead of setting $language to the default value and then looking for something better, I used a boring but consistent if .. elseif ... elseif .. else structure.

benjifisher’s picture

Status: Needs work » Needs review
Ron Collins’s picture

The patch at #128 worked for me. It's been 5 years in the making. Time to commit?

Ron Collins’s picture

Status: Needs review » Reviewed & tested by the community
pifagor’s picture

pifagor’s picture

RTBC

drupalfan2’s picture

Is this patch still necessary for latest Drupal 7 version?

  • pifagor committed 3930a06 on 7.x-1.x authored by benjifisher
    Issue #1116408 by benjifisher, pifagor: Support migrate module:...

  • pifagor committed 9b7ec00 on 7.x-2.x
    Issue #1116408 by benjifisher, pifagor: Support migrate module:...
pifagor’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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