| Issue 23: | delete node destroys the tree | |
| 4 people starred this issue and may be notified of changes. | Back to list |
1. I make a tree having the following structure test --test1 ----test1_1 ------test1_1_1 ----test1_2 --test2 ----test2_1 --test3 2. I call the delete method on test1 3. If I display the tree after deletion I don't see test2_1 What is the expected output? What do you see instead? I am using django mptt 0.2.1. I also tested it with the trunk version and still the same problem. I attached my test file.
Jun 2, 2008
I can acknowledge this bug. The problem is that django will call pre_delete method on all objects, that are being deleted. In above case, this means that pre_delete will be called for: - test1, test1_1, test1_1_1, test1_2 The pre_delete method calls the managers _close_gaps which update the lft and rght fields of all rows. This update should only happen *once* for the entire delete opertaion! But at the moment the lft and rght fields are updated for each object being deleted, thus in above case the update of lft/rght fields are updated four times.
Jun 2, 2008
This is an example of the SQL queries being executed when you try to delete an node with children in it. # Originating from pre_delete on node being deleted UPDATE `menus_menuitem` SET `lft` = CASE WHEN `lft` > 6 THEN `lft` + -4 ELSE `lft` END, `rght` = CASE WHEN `rght` > 6 THEN `rght` + -4 ELSE `rght` END WHERE `tree_id` = 1 AND (`lft` > 6 OR `rght` > 6) # Originating from pre_delete on the childs node also being deleted UPDATE `menus_menuitem` SET `lft` = CASE WHEN `lft` > 5 THEN `lft` + -2 ELSE `lft` END, `rght` = CASE WHEN `rght` > 5 THEN `rght` + -2 ELSE `rght` END WHERE `tree_id` = 1 AND (`lft` > 5 OR `rght` > 5) # Originating from Django when deleting an object with foreign keys UPDATE `menus_menuitem` SET `parent_id`=NULL WHERE `id` IN (215,216) DELETE FROM `menus_menuitem` WHERE `id` IN (216,215) it ought to look like this, to not destroy the tree: UPDATE `menus_menuitem` SET `lft` = CASE WHEN `lft` > 6 THEN `lft` + -4 ELSE `lft` END, `rght` = CASE WHEN `rght` > 6 THEN `rght` + -4 ELSE `rght` END WHERE `tree_id` = 1 AND (`lft` > 6 OR `rght` > 6) UPDATE `menus_menuitem` SET `parent_id`=NULL WHERE `id` IN (215,216) DELETE FROM `menus_menuitem` WHERE `id` IN (216,215
Jun 2, 2008
A fix could be implemented by overriding the save method. The attached patch is a simple way to do this.
Jun 3, 2008
I've tested above patch and it works. All tests passes including my own tests on my menu system. Thanks :-)
Jul 24, 2008
Works for me pretty well! Although there surely is a more elegant way without monkey-patching the save-method?
Sep 22, 2008
Doesn't the patch just do the same thing as using a post_delete signal instead of a pre_delete one? This is an important bug as it messes up the MPTT structure whenever we try and delete objects in the admin.
Oct 12, 2008
Crud - for some reason I assumed pre_delete wouldn't get called for every model - the curren
Status:
Accepted
Owner: jonathan.buchanan Labels: -Priority-Medium Priority-Critical
Oct 13, 2008
Fixed in revision 117, with a fix similar to jeroen.vloothuis's patch but using a simple decorator and wraps() instead. Thanks for the test cases and patches. This includes an in-progress, but working, refactor of the tests which includes a regression test.
Status:
Fixed
|
2.5 KB View Download