Closed (duplicate)
Project:
Drupal core
Version:
6.x-dev
Component:
path.module
Priority:
Major
Category:
Bug report
Assigned:
Reporter:
Created:
16 Nov 2009 at 10:39 UTC
Updated:
18 Oct 2012 at 16:04 UTC
The path id isn't set on the node object when a node is loaded which results in errors when saving loaded nodes. This is a big problem when using synchronize translations from the i18n module since it saves the translated node.
To replicate the issue:
- Active locale and path.
- Active two languages.
- Create two nodes, one in each language.
- Give them both the same path and make sure the language is set for each path.
- In a test module try to load and save one of the nodes. Since the pid isn't set on the load the path module will try to execute an UPDATE query which would result in duplicate entries.
The id of the loaded alias should be set on the node so that update queries for this node will update the correct path alias.
| Comment | File | Size | Author |
|---|---|---|---|
| path.patch | 526 bytes | jax |
Comments
Comment #1
jax commentedNeeds review.
Comment #2
jax commentedIn D7 the path is no longer set on node_load (there is no implementation hook_node_load()). So I'm not sure if the same issue will arise in D7, this needs to be investigated.
Comment #3
jody lynnYeah, that's pretty fubar that update checks for $node->pid which is never set anywhere.
Comment #4
rolodmonkey commentedMan, I wish the 'major' priority was rolled out.
I am setting this to critical, but I won't be offended if someone sets it back.
We found this patch just before we were about to write the same code.
We have reviewed this patch and it solved some major problems we were having where url_alias.dst was the same for different languages. Without the pid, path_set_alias() was rewriting all of the records to have the same src, dst and language!
So, that is one vote for 'reviewed & tested by the community'. If someone with a little more experience could look at this, I think it could be added to the next release.
Comment #5
rolodmonkey commentedI'm still a little new to this. What settings do I need in order to get this reviewed and into the next release?
Comment #6
jax commentedWell, if the patch works for you and you have technically verified that patch the patch does what it claims you can set the status to "reviewed and tested by the community". Then we hope that the branch maintainer accepts the change and commits it.
But, since Drupal 7 is being developed we should verify if this issue still exists in D7 and if it does also provide a patch for it.
Comment #7
rolodmonkey commentedComment #8
rolodmonkey commentedComment #9
gábor hojtsyDrupal 7 applicability is not yet checked. This looks like a pretty major issue, so I'm quite puzzled if/why it was not found before?!
Comment #10
jax commentedThe situation in D7 is as follows:
There is an implementation of hook_node_insert() and hook_node_save() but the path no longer is set on hook_node_load() which means that by default it will also not get saved since it doesn't get added to the node object. Doing the steps in the description no longer results in an error when loading and saving a node.
The first question is, should the path be loaded on node_load()? Maybe the answer is in the patch which ports path.module to D7.
How the path is actually saved when submitting a node is still a mystery to me. The path_form_alter adds the path and suddenly it's available in the node object. I'll need to look at the new form API in more detail to understand this.
To be continued.
Comment #11
savedario commentedsubscribe
Comment #12
dave reidD7 works just fine. It sounds like #269877: path_set_alias() doesn't account for same alias in different languages would fix this issue. Can anyone confirm that?
Comment #13
rolodmonkey commentedYes. It looks like #269877: path_set_alias() doesn't account for same alias in different languages fixes this issue. I am closing this one as a duplicate.