#550254: menu links reparenting. From: Damien Tournoud --- menu.inc | 110 +++++++++++++++-------- simpletest/tests/menu.test | 212 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 286 insertions(+), 36 deletions(-) diff --git includes/menu.inc includes/menu.inc index 4ed155f..9b645a8 100644 --- includes/menu.inc +++ includes/menu.inc @@ -2890,42 +2890,8 @@ function menu_link_save(&$item) { } } - // If we have a parent link ID, we use it to inherit 'menu_name' and 'depth'. - if (isset($item['plid'])) { - if ($item['plid']) { - $parent = db_query("SELECT * FROM {menu_links} WHERE mlid = :mlid", array(':mlid' => $item['plid']))->fetchAssoc(); - } - // If the parent link ID is zero, then this link lives at the top-level. - else { - $parent = FALSE; - } - } - // Otherwise, try to find a valid parent link for this link. - else { - $query = db_select('menu_links'); - // Only links derived from router items should have module == 'system', and - // we want to find the parent even if it's in a different menu. - if ($item['module'] == 'system') { - $query->condition('module', 'system'); - } - // We always respect the link's 'menu_name'; inheritance for router items is - // ensured in _menu_router_build(). - $query->condition('menu_name', $item['menu_name']); - - // Find the parent - it must be unique. - $parent_path = $item['link_path']; - do { - $parent = FALSE; - $parent_path = substr($parent_path, 0, strrpos($parent_path, '/')); - $new_query = clone $query; - $new_query->condition('link_path', $parent_path); - // Only valid if we get a unique result. - if ($new_query->countQuery()->execute()->fetchField() == 1) { - $parent = $new_query->fields('menu_links')->execute()->fetchAssoc(); - } - } while ($parent === FALSE && $parent_path); - } - // If a parent link was found, assign it and derive its menu. + // Try to find a parent link. If found, assign it and derive its menu. + $parent = _menu_link_find_parent($item); if (!empty($parent['mlid'])) { $item['plid'] = $parent['mlid']; $item['menu_name'] = $parent['menu_name']; @@ -3047,6 +3013,78 @@ function menu_link_save(&$item) { } /** + * Find a possible parent for a given menu link. + * + * Because the parent of a given link might not exist anymore in the database, + * we apply a set of heuristics to determine a proper parent: + * + * - use the passed parent link if specified and existing. + * - else, use the first existing link down the previous link hierarchy + * - else, for system menu links (derived from hook_menu()), reparent + * based on the path hierarchy. + * + * @param $menu_link + * A menu link. + * @return + * A menu link structure of the possible parent or FALSE if no valid parent + * has been found. + */ +function _menu_link_find_parent($menu_link) { + $parent = FALSE; + + // This item is explicitely top-level, skip the rest of the parenting. + if (isset($menu_link['plid']) && empty($menu_link['plid'])) { + return $parent; + } + + // If we have a parent link ID, try to use that. + $candidates = array(); + if (isset($menu_link['plid'])) { + $candidates[] = $menu_link['plid']; + } + + // Else, if we have a link hierarchy try to find a valid parent in there. + if (!empty($menu_link['depth']) && $menu_link['depth'] > 1) { + for ($depth = $menu_link['depth'] - 1; $depth >= 1; $depth--) { + $candidates[] = $menu_link['p' . $depth]; + } + } + + foreach ($candidates as $mlid) { + $parent = db_query("SELECT * FROM {menu_links} WHERE mlid = :mlid", array(':mlid' => $mlid))->fetchAssoc(); + if ($parent) { + return $parent; + } + } + + // If everything else failed, try to derive the parent from the path + // hierarchy. This only makes sense for links derived from menu router + // items (ie. from hook_menu()). + if ($menu_link['module'] == 'system') { + $query = db_select('menu_links'); + $query->condition('module', 'system'); + // We always respect the link's 'menu_name'; inheritance for router items is + // ensured in _menu_router_build(). + $query->condition('menu_name', $menu_link['menu_name']); + + // Find the parent - it must be unique. + $parent_path = $menu_link['link_path']; + do { + $parent = FALSE; + $parent_path = substr($parent_path, 0, strrpos($parent_path, '/')); + $new_query = clone $query; + $new_query->condition('link_path', $parent_path); + // Only valid if we get a unique result. + if ($new_query->countQuery()->execute()->fetchField() == 1) { + $parent = $new_query->fields('menu_links')->execute()->fetchAssoc(); + } + } while ($parent === FALSE && $parent_path); + } + + return $parent; +} + +/** * Helper function to clear the page and block caches at most twice per page load. */ function _menu_clear_page_cache() { diff --git modules/simpletest/tests/menu.test modules/simpletest/tests/menu.test index 22f7146..79cc61e 100644 --- modules/simpletest/tests/menu.test +++ modules/simpletest/tests/menu.test @@ -434,6 +434,218 @@ class MenuRouterTestCase extends DrupalWebTestCase { } /** + * Tests for menu links. + */ +class MenuLinksUnitTestCase extends DrupalWebTestCase { + // Use the lightweight testing profile for this test. + protected $profile = 'testing'; + + public static function getInfo() { + return array( + 'name' => 'Menu links', + 'description' => 'Test handling of menu links hierarchies.', + 'group' => 'Menu', + ); + } + + /** + * Create a simple hierarchy of links. + */ + function createLinkHierarchy($module = 'menu_test') { + // First remove all the menu links. + db_truncate('menu_links')->execute(); + + // Then create a simple link hierarchy: + // - $parent + // - $child-1 + // - $child-1-1 + // - $child-1-2 + // - $child-2 + $base_options = array( + 'link_title' => 'Menu link test', + 'module' => $module, + 'menu_name' => 'menu_test', + ); + + $links['parent'] = $base_options + array( + 'link_path' => 'menu-test/parent', + ); + menu_link_save($links['parent']); + + $links['child-1'] = $base_options + array( + 'link_path' => 'menu-test/parent/child-1', + 'plid' => $links['parent']['mlid'], + ); + menu_link_save($links['child-1']); + + $links['child-1-1'] = $base_options + array( + 'link_path' => 'menu-test/parent/child-1/child-1-1', + 'plid' => $links['child-1']['mlid'], + ); + menu_link_save($links['child-1-1']); + + $links['child-1-2'] = $base_options + array( + 'link_path' => 'menu-test/parent/child-1/child-1-2', + 'plid' => $links['child-1']['mlid'], + ); + menu_link_save($links['child-1-2']); + + $links['child-2'] = $base_options + array( + 'link_path' => 'menu-test/parent/child-2', + 'plid' => $links['parent']['mlid'], + ); + menu_link_save($links['child-2']); + + return $links; + } + + /** + * Assert that at set of links is properly parented. + */ + function assertMenuLinkParents($links, $expected_hierarchy) { + foreach ($expected_hierarchy as $child => $parent) { + $mlid = $links[$child]['mlid']; + $plid = $parent ? $links[$parent]['mlid'] : 0; + + $menu_link = menu_link_load($mlid); + menu_link_save($menu_link); + $this->assertEqual($menu_link['plid'], $plid, t('Menu link %mlid has parent of %plid, expected %expected_plid.', array('%mlid' => $mlid, '%plid' => $menu_link['plid'], '%expected_plid' => $plid))); + } + } + + /** + * Test automatic reparenting of menu links. + */ + function testMenuLinkReparenting($module = 'menu_test') { + // Check the initial hierarchy. + $links = $this->createLinkHierarchy($module); + + $expected_hierarchy = array( + 'parent' => FALSE, + 'child-1' => 'parent', + 'child-1-1' => 'child-1', + 'child-1-2' => 'child-1', + 'child-2' => 'parent', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + + // Start over, and move child-1 under child-2, and check that all the + // childs of child-1 have been moved too. + $links = $this->createLinkHierarchy($module); + $links['child-1']['plid'] = $links['child-2']['mlid']; + menu_link_save($links['child-1']); + + $expected_hierarchy = array( + 'parent' => FALSE, + 'child-1' => 'child-2', + 'child-1-1' => 'child-1', + 'child-1-2' => 'child-1', + 'child-2' => 'parent', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + + // Start over, and delete child-1, and check that the children of child-1 + // have been reassigned to the parent. menu_link_delete() will cowardly + // refuse to delete a menu link defined by the system module, so skip the + // test in that case. + if ($module != 'system') { + $links = $this->createLinkHierarchy($module); + menu_link_delete($links['child-1']['mlid']); + + $expected_hierarchy = array( + 'parent' => FALSE, + 'child-1-1' => 'parent', + 'child-1-2' => 'parent', + 'child-2' => 'parent', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + } + + // Start over, forcefully delete child-1 from the database, and check + // that the children of child-1 have been reassigned to the parent, + // going up on the old path hierarchy stored in each of the links. + $links = $this->createLinkHierarchy($module); + db_delete('menu_links') + ->condition('mlid', $links['child-1']['mlid']) + ->execute(); + + $expected_hierarchy = array( + 'parent' => FALSE, + 'child-1-1' => 'parent', + 'child-1-2' => 'parent', + 'child-2' => 'parent', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + + // Start over, forcefully delete the parent from the database, and check + // that the children of parent are now top-level. + $links = $this->createLinkHierarchy($module); + db_delete('menu_links') + ->condition('mlid', $links['parent']['mlid']) + ->execute(); + + $expected_hierarchy = array( + 'child-1-1' => 'child-1', + 'child-1-2' => 'child-1', + 'child-2' => FALSE, + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + } + + /** + * Test automatic reparenting of menu links derived from menu routers. + */ + function testMenuLinkRouterReparenting() { + // Run all the standard parenting tests on menu links derived from + // menu routers. + $this->testMenuLinkReparenting('system'); + + // Additionnaly, test reparenting based on path. + $links = $this->createLinkHierarchy('system'); + + // Move child-1-2 has a child of child-2, making the link hierarchy + // inconsistent with the path hierarchy. + $links['child-1-2']['plid'] = $links['child-2']['mlid']; + menu_link_save($links['child-1-2']); + + // Check the new hierarchy. + $expected_hierarchy = array( + 'parent' => FALSE, + 'child-1' => 'parent', + 'child-1-1' => 'child-1', + 'child-2' => 'parent', + 'child-1-2' => 'child-2', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + + // Now delete 'parent': 'child-1' and 'child-2' should get moved to the + // top-level. + db_delete('menu_links') + ->condition('mlid', $links['parent']['mlid']) + ->execute(); + $expected_hierarchy = array( + 'child-1' => FALSE, + 'child-1-1' => 'child-1', + 'child-2' => FALSE, + 'child-1-2' => 'child-2', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + + // Now delete 'child-2', 'child-1-2' will get reparented under 'child-1' + // based on its path. + db_delete('menu_links') + ->condition('mlid', $links['child-2']['mlid']) + ->execute(); + $expected_hierarchy = array( + 'child-1' => FALSE, + 'child-1-1' => 'child-1', + 'child-1-2' => 'child-1', + ); + $this->assertMenuLinkParents($links, $expected_hierarchy); + } +} + +/** * Tests rebuilding the menu by setting 'menu_rebuild_needed.' */ class MenuRebuildTestCase extends DrupalWebTestCase {