The list of alias types that generate the list are currently hard coded in _pathauto_objects_to_delete(). This is bad if some module implements pathauto for pages that don't fit any of those "objects". A simple hook to build the array of objects should be a simple fix. (Yes, I'm volunteering. :) )

CommentFileSizeAuthor
#6 pathauto_bulk_delete.patch3.26 KBIsland Usurper
#4 pathauto_bulk_delete.module.patch3.2 KBIsland Usurper
#2 pathauto_bulk_delete.module.patch985 bytesIsland Usurper

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Category: support » feature

Great! Go for it!

Island Usurper’s picture

Island Usurper commented
Status: Active » Needs review
StatusFileSize
new pathauto_bulk_delete.module.patch985 bytes

Hopefully I got the right version to roll the patch against. It creates a hook called hook_pathauto_objects() which should return an array similar to _pathauto_objects_to_delete().

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Needs work

This is great.

I think the hook should be hook_path_objects_to_delete or something similar. Just "pathauto_objects" doesn't make it clear what is happening. Also, perhaps some day this "bulk delete" belongs somewhere else at which point having the word pathauto in the hook won't be so good.

Can you add documentation for this to the API.txt file?

What do you think about making the array that you assigned to $objects into an implementation of this hook?

Using your hook name:

function pathauto_pathauto_objects() {
  return array('user/' => t('users'), 'node/' => t('content') , 'taxonomy/' => t('vocabularies and terms'), 'blog/' => t('user blogs'), 'user/%/track' => t('user trackers'));
}

Thanks very much!

Island Usurper’s picture

Island Usurper commented
Status: Needs work » Needs review
StatusFileSize
new pathauto_bulk_delete.module.patch3.2 KB

It's possible that this hook could be used for other reasons, so I named it hook_path_alias_type(). I don't believe anything like that is used in Path or Pathauto yet.

The patch also includes changes to API.txt.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Very cool.
Two more things then it looks good.

In API.txt you say for localization use % but it's not for localization, it's for a wildcard in the DB lookup.

And part of being really specific about what the % does is also in the naming of the hook. If it's just a list of objects then people are unlikely to get the alias part right. If the hook has the word "delete" in the name then people know "oh, when I implement this I better do it right so people don't accidentally delete something"

I purposefully leave it as review because I'd be ok with being overridden on these.

What do you think?

Island Usurper’s picture

Island Usurper commented
StatusFileSize
new pathauto_bulk_delete.patch3.26 KB

I didn't mean to connect the idea of localization with the % wildcard. I was referring to the path descriptions. But if I was that unclear, it needs adjusting.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Great - and thoughts on the hook name as "path_alias_types" vs. "path_alias_types_to_delete" ?

Island Usurper’s picture

Island Usurper commented

While it may never happen, this hook could be invoked at other times than just deleting the aliases. The hook itself doesn't have anything to do with deleting, it's just the only context it happens to be in.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

Visually this made sense to me, but I wanted to at least give it a quick test. Quick test done - works great.

Thanks Island Usurper! Committed to the DRUPAL-5--2 branch and HEAD.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.