Integration with Redirect Module Not Working (or not present)

Yep makes sense. I've been looking into how we can re-work the module integration to help simplify things.

Frankly, I think we need to abstract the 'creating redirects when an URL alias in changed' just to redirect.module. And the best way to do that is to provide $path['original'] = $existing_alias; from within Pathauto.

That way, in redirect.module, we can do something like this:

function redirect_path_update($path) {
  if (!empty($path['original']['pid']) && $path['pid'] == $path['original']['pid'] && $path['alias'] != $path['original']['alias']) {
    $redirect = array(
      'source' => $path['original']['alias'],
      'redirect' => $path['source'],
    );
    redirect_save($redirect);
  }
}

Pathauto can easily add support for the $path['original'] property, but it would also really be nice to have it in core: #998256: Please let modules know about the original URL alias in hook_path_update().

Adding tag that this must need review from another of the co-maintainers

@bdunwood: Make sure to update to the latest 7.x-1.x-dev of Redirect. That functionality wasn't working until recently.

I feel like this still needs some official feedback from greggles or Freso, so I'm going to roll 7.x-1.0-beta1 without it.

This doesn't seem to work in my testing.

I'm fine with the philosophical change this represents.

Revised patch to fix #1040414: Notice: Undefined index: language in redirect_path_update() (line 316 of redirect.module

Now with some help text to encourage folks to install redirect module.

Now committed to 7.x http://drupal.org/cvs?commit=490046

I think we're ready for a 7.x-1.0 and plan to make that later today. Yipppeeee.

+++ pathauto.inc	26 Jan 2011 17:50:07 -0000
@@ -80,20 +72,7 @@ function _pathauto_alias_exists($alias, 
-  if (module_exists('path_redirect') && function_exists('path_redirect_delete_multiple')) {
-    // Delete from path_redirect the exact same alias to the same node.
-    path_redirect_delete_multiple(NULL, array('source' => $alias, 'redirect' => $source));
-
-    // If there still is this alias used in path_redirect, then create a different alias.
-    $redirects = path_redirect_load_multiple(NULL, array('source' => $alias));
-  }
-
-  if ($pid || !empty($redirects)) {
-    return TRUE;
-  }
-  else {
-    return FALSE;
-  }

I said before (well I thought I did, but apparently my comment never saved) I'm not sure how we deal with this code. There's nothing in redirect module that can handle this. We probably have to add a new hook_pathauto_alias_exists().

Powered by Dreditor.

We also need to update _pathauto_alias_exists to actually work.

I'm on it.

I get that this could be handy to have as a hook, but I don't think there's really going to be many modules that want to implement it. Redirect is the only one I can think of.

So, if you have a hook that only one module implements...

Brain dump from IRC:

(10:29:49 AM) davereid: I'm also a little unsure of what the proper thing is to do in that case. I wanted to re-investigate why that code is needed and what it has to do in order to work.
(10:32:07 AM) greggles: hmmm
(10:32:19 AM) greggles: the original code would delete a redirect if it were the exact same as the alias about to be created
(10:32:24 AM) greggles: and then look for the exact alias
(10:32:52 AM) greggles: I don't see a function in redirect.module to delete a redirect by path and entity
(10:33:21 AM) davereid: It's funny because the current code doesn't actually skip that if there is already an alias.
(10:33:27 AM) davereid: if $pid != FALSE
(10:34:38 AM) davereid: since it's used to find if it needs to create the unique suffix -N
(10:35:26 AM) davereid: or increment the suffix. At that point the alias has already been generated.
(10:36:18 AM) davereid: So maybe redirect module should implement hook_path_update() and delete any redirects that match the current alias that is being saved
(10:37:02 AM) davereid: And then hook_pathauto_alias_exists() is only a 'does a redirect already exist for this alias' so that it can be skipped and the next suffix used.
(10:37:19 AM) davereid: rather than a delete + check
(10:37:45 AM) davereid: that's what confused me before. why are we deleting stuff in a function that just wants to know if something already is assigned to this path

I agree it seems weird for something that just checks to also do a destructive action, but this is a private function and works for it's purpose. Maybe we should rename it to make it more clear?

Subscribing

Not only rename it (to signify that an action is happening)...
...also watch when you call it (i.e. only when you want that action to happen).

Currently, pathauto_create_alias() -> _pathauto_alias_exists() is called from pathauto_form_alter().
The effect: redirects to a node would disappear at the moment you load a node edit screen!

(I didn't come up with this, but spotted the issue when browsing the queue. This is actually still happening with the D6 version right now, where the delete statement hasn't been removed yet.
D6 issue at: #950042: _pathauto_alias_exists() deletes redirects on node edit (before save) for nodes with manually edited paths
I tested the scenario in the initial post of #9950042, with pathauto 6.x-1.5 and 7.x-1.x-dev. pathauto_persist.module should not be enabled, for the problem to occur.)