Restore URL consistency for node types and menus

This is not robust, since we rely on each contrib module to implement this hook_requirements correctly?

It would be preferable to add an intermediate path, e.g.:

admin/content/node-type/[node-type] becomes: admin/build/types/edit/[node-type]

I would support this in very strong preference to trying to handle a blacklist by any of the mechanisms suggested. Note, this might mean we'd need to increase MENU_MAX_PARTS to 8, but as far as I know we have not seen any performance issues from that query to make me think that we could not risk increasing by up to 2x the number of strings we are matching against.

So, I like this better than the prior approach, but it still kind of breaks the usual Drupal pattern for path construction and it seems the patterns are different for node type vs. menu links. You need somethign for node types liej 'customize' here for menus.
For example:

admin/build/type/manage/[type-name]/delete 
admin/build/type/manage/[type-name]/export
admin/build/type/manage/[type-name] (as default tab)
admin/build/type/manage/[type-name]/edit (default local task)
admin/build/type/manage/[type-name]/fields (as a tab)
admin/build/type/manage/[type-name]/fivestar (as a tab)
admin/build/type/manage/[type-name]/template (as a tab)

Why did you make the change menu -> menus?

The common pattern is to use the singular 'node', 'user', 'comment'

subscribing.

Subscribing and bumping, let's do this for D7! Will review if rerolled.

I heartily endorse this effort.

• Fixing node type settings page paths to be below the node type listing page (admin/structure/types) can address breadcrumb issues as well (ie, on a node type settings page, there is no breadcrumb to go back to the listing of all node types)--partially dependent on 483614
• Inconsistent pluralization of menu paths make it harder to navigate via URL, which is a serious usability issue for developers and administrators (regardless of the many users who "don't see" the URL bar)

oops, knocked that D7DX tag off by accident.

New patch from scratch using '/manage' as the new, common pattern for both instances.

Renaming 'types' to 'type' is a different issue.

Likewise, using proper dynamic menu arguments for content types is also a different issue. The current node_menu() implementation registers all sub-items for every content type as separate menu router items/links - a total waste of resources.

Did a quick code review... pretty straightforward changes that, IMO, actually bring a little more sanity to the drupal URLs. One minor nitpick and the rest is RTBC.

+++ modules/node/content_types.inc	8 Sep 2009 22:27:08 -0000
@@ -446,5 +446,4 @@ function node_type_delete_confirm_submit
-  return;

I'm not sure I understand why we are removing this "return;"?

+++ modules/node/node.module	8 Sep 2009 22:27:22 -0000
@@ -92,13 +92,13 @@ function node_help($path, $arg) {
     case 'admin/structure/types/add':
       return '<p>' . t('Each piece of content is of a specific content type. Each <em>content type</em> can have different fields, behaviors, and permissions assigned to it.') . '</p>';
 
-    case 'admin/structure/node-type/' . $arg[3] . '/fields':
+    case 'admin/structure/types/manage/' . $arg[3] . '/fields':

I like this consistency with "/structure/types/[action]"

I'm on crack. Are you, too?

Thanks - that first hunk was caused by my initial pass, where I also renamed 'types' to 'type' for consistency, but then figured out it's total scope creep for this issue. And, btw, return; without anything is always the same as just skipping it -- the return value is NULL.

sun,

It applies to HEAD, the approach has been debated and I think you've found a sane way to handle it, and have removed at least one line from core that didn't need to be there.

RTBC.

Josh

Introducing a new tag for feature freeze: API clean-up.

is there an API change?

I'd say so, yes. If a contributed module registering menu router paths below those items is ported to D7 now, it will break after this patch lands.

Anyway, I thought about "feature freeze", "API clean-up", and some other issue tag terms and went with "API clean-up", lacking a better idea.

We can certainly rename that term. Has been applied to many clean-up issues in the queue now to provide a solid overview of things we really need to fix before releasing.

I'd like to see quicksketch give this a final thumbs-up, since this was his baby.

He can't RTBC his own patch, can he? Either way, I also say it's good to go.

The patch is based on the approach suggested by pwolanin in #6, which quicksketch already confirmed in #7.

Why was 'manage' chosen over 'edit'? It would seem to me that consistently using 'edit' is more predictable and hence easier to navigate the site using the address bar. Curious.

Thanks -- that makes sense. Committed to CVS HEAD.

nice patch. for remaining path issues check the SQL query in #220407: Fix collisions in admin menu

Heya!

Is it possible that we can also remove that menu name munging due to this clean-up now?

function menu_edit_menu_submit($form, &$form_state) {
  $menu = $form_state['values'];
  $path = 'admin/structure/menu/manage/';
  if ($form['#insert']) {
    // Add 'menu-' to the menu name to help avoid name-space conflicts.
    $menu['menu_name'] = 'menu-' . $menu['menu_name'];

Or what kind of name-space conflicts are meant here?