[drupal-devel] [bug] Taxonomy_save_* return values

[chx]

Issue status update for 
http://drupal.org/node/29102
Post a follow up: 
http://drupal.org/project/comments/add/29102

 Project:      Drupal
 Version:      cvs
 Component:    taxonomy.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  chx
 Reported by:  chx
 Updated by:   chx
 Status:       patch (code needs review)
 Attachment:   http://drupal.org/files/issues/taxonomy_save_0.patch (6.17 KB)

this gets better.




chx



Previous comments:
------------------------------------------------------------------------

Thu, 18 Aug 2005 04:50:50 +0000 : chx

Attachment: http://drupal.org/files/issues/taxonomy_save_return.patch (4.08 KB)

What is described on http://drupal.org/node/22218 is nice. What is in
the core is super ugly. I surely was not paying attention when did
slipped in.




------------------------------------------------------------------------

Thu, 18 Aug 2005 06:09:06 +0000 : chx

Attachment: http://drupal.org/files/issues/taxonomy_save_return_0.patch (4.08 KB)




------------------------------------------------------------------------

Thu, 18 Aug 2005 09:37:27 +0000 : DriesK

Just before I read this issue, I updated http://drupal.org/node/22218 to
reflect what is currently in core.


Until about a month ago, taxonomy_save_* functions were as in your
current patch. However, several modules (such as forum, simplenews)
need vid to be returned by taxonomy_save_vocabulary (as it was before
the status messages were introduced). Therefore, Dries committed patch
#19621 to fix this. See http://drupal.org/node/19621 and
http://drupal.org/node/26421.




------------------------------------------------------------------------

Thu, 18 Aug 2005 10:25:22 +0000 : DriesK

I was too fast. Didn't see the & in taxonomy_save_vocabulary(&$edit).
This patch indeed solves the problem in a much cleaner way.




------------------------------------------------------------------------

Thu, 18 Aug 2005 12:09:33 +0000 : killes at www.drop.org

The patch looks excellent and removes some crufty code. :)
Unfortunately I cannot test it right now.




------------------------------------------------------------------------

Sat, 20 Aug 2005 04:37:45 +0000 : willmoy

When deleting a forum, i get two set_messages:


    * Deleted term My forum 12.
    * The forum 0 has been updated.


When deleting a forum container with one forum in it, I get 3 messages:


    * Deleted term cont.
    * Deleted term in.
    * The forum container 0 has been updated.


Perhaps there needs to be some method of suppressing the
taxonomy.module messages when the function is called from another
module.


Incidentally, if you click delete on a term or forum and then cancel,
you currently get redirected to admin/taxonomy or admin/forums. The
latter is a typo bug which should read admin/forum, but it would be
better UX in both cases if cancelling took you back to the page you
were on. The current behaviour feels like cancelling a dialog box in
Windows and having it show the desktop in response.


I'm not sure if you're taking responsibility for the forum.module cruft
as well as the taxonomy.module cruft: adding, editing and deleting terms
and vocabularies with various properties works fine in admin/taxonomy.
But as you can see above, forum.module doesn't even know there are
three SAVED_ possibilities so the logic is bad.


HTH,


willmoy




------------------------------------------------------------------------

Sat, 20 Aug 2005 22:07:32 +0000 : chx

Attachment: http://drupal.org/files/issues/taxonomy_save.patch (6.17 KB)

It seems forum.module was a bit broken before. Well, this patch cleans
up things a bit.