During writing my test for Drupaltest I saw that we don't have error messages in taxonomy module, so you can add taxonomy with empty name!! :)
I've added something which prevent from this

CommentFileSizeAuthor
#4 taxonomy.module.patch.txt7.04 KBkuba.zygmunt
#1 taxonomy_10.patch5.24 KBkuba.zygmunt
taxonomy_3.module52.92 KBkuba.zygmunt

Comments

kuba.zygmunt’s picture

kuba.zygmunt commented
StatusFileSize
new taxonomy_10.patch5.24 KB

I added diff patch

dries’s picture

dries commented

Please resubmit without tabs and proper coding conventions. Thanks.

Steven’s picture

Steven commented

Putting all these tests into separate functions is a bit too much IMO.

kuba.zygmunt’s picture

kuba.zygmunt commented
StatusFileSize
new taxonomy.module.patch.txt7.04 KB

Ok, I've updated these functions, improved my style and changed a little code which prevents from deleting voc. and terms after changing name to 0.

1.
Changing name to '0' (single number) causes that the condition:

if ($edit['vid'] && $edit['name'])

will fail.

2. There was an error in taxonomy_del_term() function which gives error/warning message.

drumm’s picture

drumm
he/him
Location NY, US
commented

After deleting a forum (which is internally stored as a term) I get two messages:

* Deleted term fdgfd.
* The forum 0 has been updated.

The forum was named "fdgfd." I think that deleting terms needs to return the same status and name array as the other operations and not call drupal_set_message(). This is needed so modules built on taxonomy such as forum and image can fully override the terminology used. And this also separates the model and controller layers.

This becomes problematic quickly since the delete function is recursive and the return type does not currently have room to store information about multiple deleted terms. I think deleting terms should not be recursive and any child terms should have their parent set to the deleted term's parent. I think this would be easier to code and makes this action more forgiving for users.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

forgot to change the status

And two small indentation problems:

<code>
+        taxonomy_vocabulary_validate($edit);
+          if (!form_get_errors()) {

and

<code>
-  $form .= form_textarea(t('Synonyms'), 'synonyms', implode("\n", taxonomy_get_synonyms($edit['tid'])), 60, 5, t('<a href="%help-url">Synonyms</a> of this term, one synonym per line.', array('%help-url' => url('admin/help/taxonomy', NULL, NULL, 'synonyms'))));
+	$synonyms = (isset($edit['synonyms'])) ? $edit['synonyms'] : implode("\n",taxonomy_get_synonyms($edit['tid']));
+  $form .= form_textarea(t('Synonyms'), 'synonyms', $synonyms, 60, 5, t('<a href="%help-url">Synonyms</a> of this term, one synonym per line.', array('%help-url' => url('admin/help/taxonomy', NULL, NULL, 'synonyms'))));

moshe weitzman’s picture

moshe weitzman commented
Status: Needs work » Closed (fixed)

the form API renders this patch badly out of date.