Deletion of terms fails in current HEAD (taxonomy.admin.inc,v 1.36 2008/11/09 00:58:03) because taxonomy_term_confirm_delete_submit() calls the non-existing function taxonomy_del_term() (taxonomy.admin.inc line 840). This function was renamed with #314147: DX: Standardise taxonomy load/save/delete functions on objects to taxonnomy_term_delete().

Unfortunately I can't create patches with cvs diff from here, so here is one taken with diff -uP from within directory modules/taxonomy.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

swentel’s picture

Status: Active » Needs review
FileSize
1.15 KB

Can confirm this, patch attached with diff from drupal root.

Dries’s picture

Looks like there is more untested code ...

./modules/forum/forum.admin.inc:  taxonomy_del_term($form_state['values']['tid']);
./modules/taxonomy/taxonomy.admin.inc:  taxonomy_del_term($form_state['values']['tid']);
catch’s picture

FileSize
3.22 KB

Now 50% less untested. The other 50% will need to go in forum.test, but not by me tonight at least.

swentel’s picture

Status: Needs review » Needs work

Lot's of notices pop up when trying to write a test to delete a forum:
if you go to forum/notexistingtid, following pops up:

* Notice: Trying to get property of non-object in taxonomy_get_parents_all() (line 773 of /var/www/drupal/drupal-cvs/modules/taxonomy/taxonomy.module).
* Notice: Trying to get property of non-object in template_preprocess_forums() (line 717 of /var/www/drupal/drupal-cvs/modules/forum/forum.module).
* Notice: Trying to get property of non-object in template_preprocess_forums() (line 721 of /var/www/drupal/drupal-cvs/modules/forum/forum.module).
* Notice: Trying to get property of non-object in template_preprocess_forums() (line 721 of /var/www/drupal/drupal-cvs/modules/forum/forum.module).

swentel’s picture

Also notices on install / reinstall of forum - not sure if this should be fixed in this patch, but just mentioning it.

* Notice: Trying to get property of non-object in taxonomy_vocabulary_save() (line 209 of /var/www/drupal/drupal-cvs/modules/taxonomy/taxonomy.module).
* Warning: Attempt to assign property of non-object in taxonomy_vocabulary_save() (line 209 of /var/www/drupal/drupal-cvs/modules/taxonomy/taxonomy.module).
* Notice: Trying to get property of non-object in taxonomy_vocabulary_save() (line 218 of /var/www/drupal/drupal-cvs/modules/taxonomy/taxonomy.module).
* Notice: Trying to get property of non-object in taxonomy_vocabulary_save() (line 219 of /var/www/drupal/drupal-cvs/modules/taxonomy/taxonomy.module).
* Warning: Invalid argument supplied for foreach() in taxonomy_vocabulary_save() (line 219 of /var/www/drupal/drupal-cvs/modules/taxonomy/taxonomy.module).

catch’s picture

I tracked down the bug in taxonomy_get_parents_all() and uploaded a patch over at #351669: taxonomy_get_parents_all() is not E_ALL compliant - since this affects both Drupal 6 and 7. Can't reproduce the install/uninstall notices though.

@swentel - can you try your test with the other patch applied?

swentel’s picture

@catch if I apply the patch from #217676: taxonomy_term_load_parents_all() doesn't work correctly with multiple hierarchy terms on the latest head, going to forum/# simply does nothing, apache cpu goes nuts on it ... looking into it why that happens on my setup, not sure if you can reproduce that behavior.

swentel’s picture

FileSize
4.98 KB

Patch with removing forum test also included. Still marking CNW since #351669: taxonomy_get_parents_all() is not E_ALL compliant needs to get in first.

catch’s picture

Status: Needs work » Postponed

Marking postponed until the other patch gets in.

catch’s picture

Priority: Normal » Critical
Status: Postponed » Needs review
Issue tags: +Quick fix, +Needs tests
FileSize
5.46 KB

Well that didn't work. Here's a patch combining the two.

webchick’s picture

Issue tags: -Quick fix

This exceeds the brain requirements for a quick fix. ;) I'll take a look tomorrow.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

Status: Needs work » Needs review

Test bot not happy.

webchick’s picture

Status: Needs review » Fixed

Looks good to me. :) Committed to HEAD. Thanks, guys!!

Status: Fixed » Closed (fixed)
Issue tags: -Needs tests

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