Design flaw: manual aliases won't work without storing auto alias setting in the DB

I'm having the same issue. If i try to revert content on the page I have linked to the homepage it breaks my site because the url is being erroneously auto created even though I have this option unchecked.

I don't think this is only with reverting revisions.

I have a similar problem with the Publish Content-module. When I create a custom alias and I afterwards press on the "Publish" or "Unpublish"-buttons, the alias gets changed to the automatic alias.

I've done some digging and found that this piece of code in pathauto_nodeapi() is causing the problem:

        // Only create an alias if the checkbox was not provided or if the checkbox was provided and is checked
        if (!isset($node->pathauto_perform_alias) || $node->pathauto_perform_alias) {
          $placeholders = pathauto_get_placeholders('node', $node);
          $src = "node/$node->nid";
          $node->path = pathauto_create_alias('node', $op, $placeholders, $src, $node->nid, $node->type, $node->language);
        }

Since the Publish Content-module only changes $node->status, it doesn't display the checkbox so Pathauto sets the automatic path. The same thing happens when you publish a node through /admin/content/node.

I'm guessing this is also why it happens when you revert to an older revision.

It should check if there isn't already an alias if the checkbox wasn't provided.

made a patch for it.
At presave, it stores what the auto-path would be in $node->autopath and at update, it'll check if there is no path yet (so a new node) or if the old path is the autopath when the checkbox wasn't given.

ok, I think it's because I didn't check wetter there was a pattern and because it shouldn't try to make an autopath if the node does not exist yet.

Subscribing.

This error is quite serious. Manual path aliases break when nodes are being processed programmatically, which happens quite often (Workflow, Rules, moderation modules etc).

As a possible quick solution, we could set "pathauto_perform_alias" property during "load" operation of hook_nodeapi() to ensure that it exists in all cases.

@crea, instead of bumping the priority perhaps you could review the patch in #6?

Here's one review: $node->autopath should be $node->pathauto or $node->pathauto_something since nowhere in the module do we use "autopath"

@greggles
I didn't understand what was the logic behind the patch. Basically, manual aliases break because $node->pathauto_perform_alias property exists only for nodes processed inside node forms, and Pathauto uses the property in all cases (including outside the forms processing). The patch didn't fix the faulty logics.

If it doesn't fix your case at all then that's useful information as well and should be stated. Thanks for stating it.

I'll try to explain the logic behind the patch as best as possible:
the bug is because the module checks if it needs to set an autopath everytime the node gets updated in one way or the other.
to see if it needs to set an autopath, it checks if the checkbox for autopath either was checked or was missing. But when the node gets updated in another way than with the edit-form, there is no checkbox, wich is one of the conditions to set the autopath.

what my patch does, is at presave store in $node->autopath what would be the autopath before the node got updated. Then where the non-patched module would only check if there is a checked checkbox, this patch also checks if the old path (if it existed) matches the old autopath ($node->autopath) and if so, update the autopath.

So with other words, my patch makes the module only overwrite a manual path if you explicetly told it to do this (by checking the checkbox on the edit-form).

There could be better ways to do this, but this is the best I could come up with.

@greggles
$node->autopath would indeed probably be better as $node->pathauto_old or something

So far, it works fine on my site, but because there weren't any replies and I can't make heads or tail from the testresults, I can't do much more about it.

subscribe

>> // Only create an alias if the checkbox was not provided or if the checkbox was provided and is checked

Can someone explain this ? Why do we need to create an alias if the checkbox was not provided ? That is the source of the problem.

The checkbox is not provided in situations where the person editing the content is not allowed to create url aliases so they don't see the text area nor checkbox on the node form.

Ok. Let's try this one.

Hmm this could lead to unnecessary operations if we don't want to update the node.

This is one of the reasons I created http://drupal.org/project/pathauto_persist and also am looking at #936222: Merge in pathauto_persist module functionality to prevent losing manual aliases with node_save() calls...

"The checkbox is not provided in situations where the person editing the content is not allowed to create url aliases so they don't see the text area nor checkbox on the node form."

Note that the checkbox code actually does exist in code even if the user doesn't have access to it since we use the #access FAPI property. So if the user submits the node form that doesn't have access to the URL alias field or checkbox, the proper value should still be submitted.

Note that the checkbox code actually does exist in code even if the user doesn't have access to it since we use the #access FAPI property. So if the user submits the node form that doesn't have access to the URL alias field or checkbox, the proper value should still be submitted.

Ok. Do we need to process automated aliases outside of node forms ? If not, then simplest fix would be to change this to "Only create an alias if the checkbox was provided and is checked".
I also suggest to stop thinking in terms of form processing and switch to nodeapi event terms.

Pathauto calculates "generate auto alias using a pattern" flag based on "existing alias == auto alias" condition. But auto alias can be generated based on node properties which can change during the update. Let's say, path alias pattern contains node title. This leads to following consequences:

• If the node title was changed prior to node being edited, this is a problem

This means checkbox will be auto-checked on the form, and user must be somehow aware that manual alias will be overwritten. If the user is not aware that node title was changed before his edit, and he does not notice checkbox checked, he is in trouble. Note that described situation is probably common in any environment where multiple editors work on same content or content is updated in multiple different ways.

• If the node title changes during node update event outside of the node form, this is a problem too

This means we will have 2 choices:

• Calculate $node->pathauto_perform_alias old way, which (because of the changed node title) will end up "auto alias disabled" and will lead to auto pattern not working.
• Force auto-alias update, because path changed and we must update auto-alias according to the pattern. This is what Pathauto does now, breaking manual aliases.

Now you must see, both choices are bad, because they are based on wrong assumptions and lack of information.

I see the current situation as a major design flaw in Pathauto. Why not merge http://drupal.org/project/pathauto_persist into the module ? Manual aliases will not work without storing "auto alias" setting in the database.

I agree with crea, this needs to go into core pathauto. I'm seeing similar symptoms on a client's site and will see if pathauto_persist does the trick.

Agree with #23, for me the problem happens like this...

On a multilingual site certain cck fields are to automatically update the translation nodes upon change so that upon any adjustments to the original language the changes are propagated through other languages, this automatic update however causes pathauto to rewrite the paths of the translations despite the fact that they were manually set and the automatic checkbox was turned off, very frustrating!

Installing http://drupal.org/project/pathauto_persist does indeed do the trick for now (and also works with 6.x-1.x-*) and the proper way is to merge it into pathauto (6.x-2.x and 7.x-1.x) for the future.

(as seen here http://drupal.org/node/936222)

pathauto_persist does indeed store the setting in the DB so solves it like described above and also seems like the only sane way to do this without breaking any other modules.

Best Wishes,

Fabian (LionsAd)

...subscribing

#936222: Merge in pathauto_persist module functionality to prevent losing manual aliases with node_save() calls.