[IMP] ir.ui.menu: deleting a menu should orphan its children w/o corrupting the `parent_store` struct: test + fix

ir.ui.menu was recently changed to use _parent_store,
which precludes using ondelete=set null for the parent_id
column. We nevertheless need to be certain that menus
can always be deleted but *never* cascade-deleted,
due the possible presence of user-created menus.
Overriding menu.unlink() is therefore necessary,
and care must be taken to bypass the default menu
visibility (using the `ir.ui.menu.full_list` context
flag while doing so)

bzr revid: odo@openerp.com-20121213145821-u2ipdvffu00rsgdg

openerp/addons/base/ir/ir_ui_menu.py

@@ -168,9 +168,22 @@ class ir_ui_menu(osv.osv):
         self.clear_cache()
         return super(ir_ui_menu, self).write(*args, **kwargs)
 
-    def unlink(self, *args, **kwargs):
+    def unlink(self, cr, uid, ids, context=None):
+        # Detach children and promote them to top-level, because it would be unwise to
+        # cascade-delete submenus blindly. We also can't use ondelete=set null because
+        # that is not supported when _parent_store is used (would silently corrupt it).
+        # TODO: ideally we should move them under a generic "Orphans" menu somewhere?
+        if isinstance(ids, (int, long)):
+            ids = [ids]
+        local_context = dict(context or {})
+        local_context['ir.ui.menu.full_list'] = True
+        direct_children_ids = self.search(cr, uid, [('parent_id', 'in', ids)], context=local_context)
+        if direct_children_ids:
+            self.write(cr, uid, direct_children_ids, {'parent_id': False})
+
+        result = super(ir_ui_menu, self).unlink(cr, uid, ids, context=context)
         self.clear_cache()
-        return super(ir_ui_menu, self).unlink(*args, **kwargs)
+        return result
 
     def copy(self, cr, uid, id, default=None, context=None):
         ir_values_obj = self.pool.get('ir.values')

openerp/addons/base/tests/__init__.py

@@ -1,8 +1,9 @@
-import test_base, test_expression, test_search, test_ir_values
+import test_base, test_expression, test_search, test_ir_values, test_menu
 
 checks = [
     test_base,
     test_expression,
     test_search,
     test_ir_values,
+    test_menu,
 ]

openerp/addons/base/tests/test_menu.py

@@ -0,0 +1,34 @@
+import unittest2
+
+import openerp.tests.common as common
+
+class test_menu(common.TransactionCase):
+
+    def setUp(self):
+        super(test_menu,self).setUp()
+        self.Menus = self.registry('ir.ui.menu')
+
+    def test_00_menu_deletion(self):
+        """Verify that menu deletion works properly when there are child menus, and those
+           are indeed made orphans"""
+        cr, uid, Menus = self.cr, self.uid, self.Menus
+
+        # Generic trick necessary for search() calls to avoid hidden menus 
+        ctx = {'ir.ui.menu.full_list': True}
+
+        root_id = Menus.create(cr, uid, {'name': 'Test root'})
+        child1_id = Menus.create(cr, uid, {'name': 'Test child 1', 'parent_id': root_id})
+        child2_id = Menus.create(cr, uid, {'name': 'Test child 2', 'parent_id': root_id})
+        child21_id = Menus.create(cr, uid, {'name': 'Test child 2-1', 'parent_id': child2_id})
+
+        all_ids = [root_id, child1_id, child2_id, child21_id]
+
+        # delete and check that direct children are promoted to top-level
+        # cfr. explanation in menu.unlink()
+        Menus.unlink(cr, uid, [root_id])
+
+        remaining_ids = Menus.search(cr, uid, [('id', 'in', all_ids)], order="id", context=ctx)
+        self.assertEqual([child1_id, child2_id, child21_id], remaining_ids)
+
+        orphan_ids =  Menus.search(cr, uid, [('id', 'in', all_ids), ('parent_id', '=', False)], order="id", context=ctx)
+        self.assertEqual([child1_id, child2_id], orphan_ids)