Right now the pathauto_install and pathauto_uninstall() defy a lot of normal conventions.
1. pathauto_install() runs a lot of variable_sets() that do not need to happen as most of them use default values.
2. pathauto_uninstall() should run manual variable_del() on each variable instead of the blanket DELETE FROM {variables} SQL.
3. We should implement hook for when entity types (node type, vocabulary) are deleted, and delete the respective variables in those hooks.
4. Also we should make sure to implement hook_node_type_update() (or similar for other entities) for when user changes the machine name of an entity type, the variables also need to be renamed.

CommentFileSizeAuthor
#6 719906-pathauto-cleanup-install-D7.patch5.23 KBdave reid
#5 719906-pathauto-cleanup-install-D7.patch2.91 KBdave reid
#5 719906-pathauto-cleanup-install-D6.patch2.95 KBdave reid

Comments

greggles’s picture

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

1. was because install profiles in 5.x wouldn't work without those. I generally agree, but we should test this with an install profile that uses pathauto and creates some content.
2. Do you mean doing a select of variables like pathauto_% and then deleting them? If so, what's the difference. If not, it could get hard to be sure we're really "clean" of everything unless we change the pathauto variable to a single variable in a giant tree.
3. Sounds good.
4. Yep, sounds good.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

2. Would be instead of using the delete sql, manually removing all variables using variable_del().

Instead of:
db_query("DELETE FROM {variable} WHERE name LIKE 'pathauto_%%'");

Do:

variable_del(...);
variable_del(...);
variable_del(...);
...
greggles’s picture

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

I like that, but I think it will be really hard to get right given the dynamic nature of the pathauto variable system.

Moving to one giant tree has the additional benefit of making it easier to make pathauto settings "exportable."

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

My eventual roadmap is for the patterns to be stored in db. Most of the options about transliteration/removing words/casing would come with them into the db.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
StatusFileSize
new 719906-pathauto-cleanup-install-D6.patch2.95 KB
new 719906-pathauto-cleanup-install-D7.patch2.91 KB

Here's an initial cleanup of variables that are just setting their defaults and are not necessary. Also discovered two variables that are no longer even used by pathauto.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 719906-pathauto-cleanup-install-D7.patch5.23 KB
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Fixed

Committed to 7.x-1.x and 6.x-2.x:
http://drupal.org/cvs?commit=399730
http://drupal.org/cvs?commit=399732

Status: Fixed » Closed (fixed)

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