I'm working on a patch to build in compatibility with the schedule module. Nodes with a publishing schedule will not create a url alias when the are created, but will instead, url aliases will be created when the node is published. This issue will also incorporate url alias deleting so that when a node is deleted or unpublished, the url alias will also be deleted.
This issue will NOT address unpublished nodes creating url aliases via bulkupdates. That will be addressed in a separate patch.
--Bill
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | pathauto_13.patch | 2.98 KB | csevb10 |
Comments
Comment #1
csevb10 commentedThis patch will address issues from this earlier issue:
http://drupal.org/node/61505
Comment #2
csevb10 commentedThis patch should allow administrative control of unpublished node url aliasing.
You can choose between the following 2 options for Unpublished Nodes:
- Delete/Do not create aliases
This will not create aliases until a node is published. Likewise, when a node is unpublished or deleted, url aliases for nodes will be deleted.
- Create Aliases
This will create aliases as soon as a node is created or when a node is updated. At present, url aliases will not be deleted to keep it completely inline with the current functionality.
This is patched against the current 5.0 version of pathauto.
Comment #3
csevb10 commentedForgot to change the status.
Comment #4
gregglescsevb10 - here's some initial feedback on this patch. I haven't tried it, just read the code.
Settings - I think the dropdown should read "Do not create aliases/Delete aliases". The primary and chronologically first action is the "do not create" part so that makes more sense to me to come first (but I'd love to hear other opinions).
When you do the variable get, I'd say either do it once outside of the switch case so that we only make that call once, OR just include the variable get right into the if statements. I did a quick "grep -R variable_get * | grep if" and it appears that core uses variable_get inside of if() statements so it's a reasonable way to go.
You've made the delete conditional on the setting which seems like the wrong case to me. If we delete a node from the site it's totally gone and we should get rid of the alias always. Right?
Finally - this code doesn't handle the case where someone already has a site and they have unpublished nodes that have aliases. That's fine - I don't think the code should handle that, but it would be nice to provide a SQL statement that would delete aliases for unpublished nodes. If you want me to create that I'd be happy to.
In general - thanks for getting a start on this! I think it's really close.
Comment #5
csevb10 commentedI've been swamped recently, so I'm behind where I'd like to be, but I'll make those revisions, and post a new patch as soon as I'm able.
I agree that logically, at least, it doesn't make sense to make the delete conditional on the setting, but I simply did that for backward compatibility in case someone really liked the way it functioned now and didn't want it to change.
I'll look into the revisions and get back as soon as I have some breathing room!
Comment #6
gregglesI checked a little while ago because I was really curious about this and it turns out that the deletion of the node path aliases happens in path.module so we don't need to worry about it here.
path.module around line 248 shows it
Comment #7
csevb10 commentedPerfect. That makes complete sense.
Comment #8
gregglesDid you get any more round tuits recently, Bill?
Also moving to a better version where it won't get lost.
Comment #9
gregglesThinking more about this, it seems like it's not really that important of a feature (I guess the fact that neither you nor anyone else has implemented it yet is a testament to that). Also, since this adds more to the UI which is already too complex, I think that we should leave this out. if there is some really good reason to implement this please correct me and mark it back to CNW.
Comment #10
dkruglyak commentedHow is this feature not important? Keeping your path structure is critical for SEO. Polluting the path table by unpublished nodes that might have to be re-timed later is a bad idea. Paths should be generated on publishing.
Why not just implement some modification of @csevb10's patch?
Comment #11
gregglesYou think it's important but not important enough to actually re-roll the patch?
Also, the patch needs more work: we are not adding anything to the UI of the module because it is already too complex. So, it needs work to
1) decide what the default behavior should be
2) expose some means (a hidden variable or a hook) to control the behavior
Marking won't fix until and unless someone fixes this and the previous bits of feedback on the issue.
Comment #12
jelo commentedWhy can't this be done with rules? Rules already has an integration with path aliases. I am currently struggling to get it to work, but a triggered rule on event "Node has been unpublished" should work. With conditions this could be limited to variations of use cases.
I am using action "Create or delete target revision of operation's URL alias" which seems to be what can handle deletions. However, as mentioned it does not work currently for me.