D7 Taxonomy Menu reworking for usability: tabs for vocabulary and term editing

Note: This note is not to my usual audience (ok, well it's still to myself...). Usually notes on Agaric are about working with Drupal, extending it, making modules or themes. This note is about developing and improving Drupal core itself. Although if you happen to be building any module for Drupal 7 right now, say because for instance you are insane, you would run into this issue also.

To those in IRC last night, I really wasn't crazy.

Nothing I could do through the user interface would touch the menu_router table.

This is developing in Drupal 7 - HEAD. (I do know I always forget to specify the version of Drupal when asking questions in IRC).

Dropping all tables and reinstalling Drupal did it. There has to be an easier way.

How does Drupal know which items in menu_router are defined by modules and which via the user interface by menu_link_save()?

Well let's create a menu item (I named him Fred) and find out!

Ah! The answer is, ALL the items in menu_router are defined by code. Poor Fred is stuck into menu_links.

OK, so now to find out what's wrong with the function that should be reloading menu_router.

That function is menu_rebuild(). Or rather, menu_rebuild() is the function called, which then calls menu_router_build(TRUE).

Patch includes cleanup to use load functions to get a terms object, move direct database calls in taxonomy_term_page into separate functions.

must replace all calls to admin/content/taxonomy/edit/vocabulary/%taxonomy_vocabulary with admin/content/taxonomy/%taxonomy_vocabulary/edit

This is so awesome. Local tasks that take different load functions for the arguments.

So for taxonomy/term/%/view (I hope to be just term/%/view soon, but that's another issue) we use the menu path taxonomy/term/%taxonomy_terms/view -- note the S.

That calls the taxonomy_terms_load() function which is just a wrapper for the tid string parsing function for now.

At the same root path we can call taxonomy/term/%taxonomy_term/edit -- with no s -- and it shows up as the sibling local task tab, but it calls a different function-- taxonomy_term_load().

And this bloody works! If the load function returns the drupal not found function, the menu item will not display! (Note that this doesn't work for the page callback function-- it can return drupal_not_found() but the menu will still display, and the user sees this error.

So this awful convoluted logic not only looks ugly, but it could not achieve the desired purpose of disabling the menu anyhow:

<?php
/**
 * Page to edit a vocabulary term.
 */
function taxonomy_admin_term_edit($terms) {
  if (isset($terms) && is_array($terms) && isset($terms['tids']) && count($terms['tids']) == 1) {
    $term = taxonomy_term_load($terms['tids'][0]);
    if (isset($term)) {
      return drupal_get_form('taxonomy_form_term', taxonomy_vocabulary_load($term->vid), (array)$term);
    }
  }
  return drupal_not_found();
}
?>

So back to beautiful, and we're moving itout of admin also, hence the name change:

<?php
/**
 * Page to edit a vocabulary term.
 */
function taxonomy_term_edit($term) {
  if (isset($term)) {
    return drupal_get_form('taxonomy_form_term', taxonomy_vocabulary_load($term->vid), (array)$term);
  }
  return drupal_not_found();
}
?>

Despite catch's faith, in all my experimenting I can't find a way to dynamically change the menu title based on the wildcard or the object loaded from the wildcard from within hook_menu().

'title callback' => 'taxonomy_term_edit_title_callback',
'title arguments' => '%taxonomy_term',

'title callback' => 'taxonomy_term_edit_title_callback',
'title arguments' => $taxonomy_term,

Oh. But we want the page title anyway. Nah. Same deal here. No way to mess with it either.

One more Never Mind! THAT'S the way!

Modeled exactly on page arguments, of course. Tricky, tricky, however it works, but very cool.

<?php
  $items['taxonomy/term/%taxonomy_term/edit'] = array(
    'title' => 'Edit term',
    'title callback' => 'taxonomy_term_edit_title_callback',
    'title arguments' => array(2),
    'page callback' => 'taxonomy_term_edit',
    'page arguments' => array(2),
    'access arguments' => array('administer taxonomy'),
    'type' => MENU_LOCAL_TASK,
    'weight' => 10,
  );
?>

<?php
function taxonomy_term_edit_title_callback($args = array()) {
  drupal_set_message('EEP <pre>'. var_export(get_defined_vars(),TRUE) .'</pre>');
}
?>

EEP
array (
  'args' =>
  stdClass::__set_state(array(
     'tid' => '1',
     'vid' => '1',
     'name' => 'New term',
     'description' => 'Hello world.',
     'weight' => '0',
  )),
)

So what I want:

But, indeed, changing the tab (menu) label is not what I want. I can do it, mostly how I want to, as shown here, in this slice of hook_menu():

<?php
  $items['admin/content/taxonomy/%taxonomy_vocabulary/edit'] = array(
    'title' => 'Edit vocabulary',
    'page callback' => 'taxonomy_admin_vocabulary_edit',
    'page arguments' => array(3),
    'access arguments' => array('administer taxonomy'),
    'title callback' => 'taxonomy_admin_vocabulary_edit_title_callback',
    'title arguments' => array(3),
    'type' => MENU_LOCAL_TASK,
    'weight' => -20,
  );
?>

and the title callback function:

function taxonomy_admin_vocabulary_edit_title_callback($vocabulary) {
return t('Edit vocabulary') .' '. $vocabulary->name .'';
}

Terms in TagsEdit vocabulary Tags

... so close, but no.

<?php
  $items['admin/content/taxonomy/%taxonomy_vocabulary'] = array(
    'title' => 'List terms',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('taxonomy_overview_terms', 3),
    'title callback' => 'taxonomy_admin_vocabulary_title_callback',
    'title arguments' => array(3, 4),
    'access arguments' => array('administer taxonomy'),
    'type' => MENU_CALLBACK,
  );
?>

That fourth parameter is never passed in, even on the list and edit pages. So the main menu / page title cannot be affected by what local task it is on.

Of course, this difficulty is because the title of a page SHOULDN'T change based on the local tasks. We'll refactor like that and see what people think.

What is the purpose / need for the 'parent' option in hook_menu()?

For test notes:
HOWTO: Write your own test - Drupal 6/7
http://drupal.org/node/273612

Resolution