Now that pathauto uses path redirect to create a lot of redirects, I got to thinking about the delete case. Currently when a node (or user or term) is deleted pathauto will try to delete any aliases for that object. It does not handle the case of deleting the path redirects that may be associated with the node.
Is there a good API for this? Perhaps path_redirect should have a cron job which deletes redirects that point to aliases that no longer exist? Thoughts?
I've filed this as support because I'm requesting guidance - I'm not sure that any code is needed in path redirect to help with this change.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | path_redirect-5.x-1.x_delete_5.patch | 2.9 KB | HorsePunchKid |
Comments
Comment #1
HorsePunchKid commentedAn optional clean-up cron job might be a good idea, particularly now that auto-generating redirects opens up the possibility of easily ending up with lots of redirects. Would we just loop over all of the destination paths (excluding external links somehow) and check the return value of
drupal_lookup_path?Alternatively (or in addition), we could use
hook_nodeapilike path.module does and delete redirects to nodes when said nodes are deleted. It wouldn't cover all the cases (i.e. redirects pointing to things other than nodes), but it would probably cover the most common ones.The API for deleting redirects is pretty minimal, currently comprising simply
path_redirect_delete_do($rid). I think you'll rarely have the$ridwhen you're wanting to delete a redirect outside of the context of the path_redirect module. I can imagine wanting to delete based on the source or the destination; what is the best way to provide that as an API?I notice path.module has this functionality; perhaps for the sake of consistency, we could model it on that. On the other hand, combining creation and deletion into one function as
path_set_aliasdoes isn't exactly intuitive.Comment #2
gregglesdrupal_lookup_path only tells you if an alias exists, not if it's a valid path. There is no way in Drupal4.7 or 5 to know if an alias exists other than to perform a drupal_http_request (which is painfully slow!). For Drupal6 it would be nice to have this feature which would use http://api.drupal.org/api/function/menu_valid_path/6
I think using hook_nodeapi (and hook_user and hook_taxonomy) would be a decent idea, but it has the problem that you have to parse apart your redirects trying to make sure that you delete the right one (i.e. that when node 1 is delete you delete redirects that point to "node/1" and "node/1/feed" etc. but don't delete redirects that point to "node/12"). Also, what about the scenario "node/1" which is aliased as "about" and has a redirect from "home" that points to "about". The code would get complex pretty quickly.
So...I think the best thing is to make the deletion api really easy to use. I agree that I probably shouldn't have to know the rid. I further agree that the path_set_alias interface is not intuitive. How about path_redirect_delete($from = NULL, $to = NULL) which delete redirects for those from/to values (and do nothing if both are null). I'm changing this to a feature request to reflect that desire.
Comment #3
HorsePunchKid commentedIndeed, this does sound more complex than it did at first glance. But the delete function does sound like a good idea in terms of simply exposing the functionality so others can deal with the complexity.
:)The function
path_redirect_deleteis currently what implements the confirmation form, if I recall correctly. I'd be fine with repurposing that function, since it seems unlikely that any external code would be dependent on it doing what it currently does.Comment #4
gregglesThe function name isn't a big deal to me - I'd even go for path_redirect_api_delete or something so that it's clear that it is an API function that other modules can/should use.
Comment #5
HorsePunchKid commentedIn this patch, I opted to:
path_redirect_deletewith the signature suggested in #2, though with the additional option of passing a$ridas the final (optional) argument, andpath_redirect_delete_confirm.Is the comment excessive? I figured better too much than too little when documenting something exposed like this.
Comment #6
HorsePunchKid commentedThis has been working fine for me on MySQL and Postgres, so I went ahead and committed it. It doesn't address the "cleaning up old redirects" idea, but I think that could be a separate issue if there is demand for it.
Comment #7
gregglesThanks very much HorsePunchKid.
I'll implement this into pathauto shortly.
Comment #8
HorsePunchKid commentedGood deal. I have also ported this to D6, though I had already done some of the function naming cleanup. No patch for 4.x yet.
Comment #9
(not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #10
promesI see one maybe problem by automatic deletion, or I miss the point.
Sometimes I delete a node because this node is replaced by something else. This can be another node, a view or whatever. In this case I like to change my redirects to the new path.
With the current proposal I have to update the redirects before deleting the old node, view, ....
I see 3 options:
- a "delete redirects OK? yes/no"
- a warning, with a list of the deleted redirects
- watchdog warning with a list of the deleted redirects
I think the 3th option is the best, because when another user is deleting a node, every administrator is able to see this.
Thanks for a great module. It is one of the first modules I always implement in my websites.