On the /project/issues and /issues/search pages, the breadcrumbs are incomplete. This patch fixes the problem.

Comments

dww’s picture

Status: Needs review » Closed (duplicate)

thanks for the flurry of patches on this. ;) however:

1) this is duplicate with some older issues:
http://drupal.org/node/58630
http://drupal.org/node/76030
http://drupal.org/node/97207
it's great that you're solving the problems, but it'd be nice if you were also helping to tame the wild west known as the project* issue queue while you were at it. ;)

2) this code is coming up a lot:

+  
+  // Breadcrumb navigation
+  $breadcrmb = array();
+  $breadcrumb[] = l($node->title, 'node/'. $node->nid);
+  project_project_set_breadcrumb($node, $breadcrumb); 

a) there's a bug ("breadcrmb" vs. "breadcrumb" so the attempt to avoid the PHP warning would fail.

b) this cut and paste error is a perfect example of why this code should just be shared. ;) for example, perhaps if you call project_project_set_breadcrumb() with a magic 2nd arg, it should do the above behavior itself. the function could see if the 2nd arg is an array, and if so, do the existing behavior, and if the 2nd arg is defined but not an array, do this as the default (or something).

thanks,
-derek

dww’s picture

to be really clear, this particular issue is duplicate with http://drupal.org/node/58630

aclight’s picture

Regarding #1, I used the Advanced Search to search the Project, Project_Issue, and CVSlog issue queues for the term "breadcrumb" and got no results in any of those searches. Given that d.o is running extremely slow today, I decided that this was a thorough enough search and submitted the issues. I'm not sure why none of the related issues you pointed out came up--maybe the search timed out or something.

As for #2, is this similar to what you are thinking?

function project_project_set_breadcrumb($node = NULL, $extra = NULL) {
  global $_menu;

  $breadcrumb = array();
  $breadcrumb[] = l(t('Home'), NULL);

  // Find out if the site has created a menu name for /project and use that
  $pid = $_menu['path index']['project'];
  $name = $_menu['items'][$pid]['title'];
  $breadcrumb[] = l($name, 'project', array('title' => t('Browse projects')));

  if (!empty($node) && project_use_taxonomy()) {
    $result = db_query(db_rewrite_sql('SELECT t.tid, t.* FROM {term_data} t INNER JOIN {term_hierarchy} h ON t.tid = h.tid INNER JOIN {term_node} r ON t.tid = r.tid WHERE h.parent = 0 AND t.vid = %d AND r.nid = %d', 't', 'tid'), _project_get_vid(), $node->nid);
    $term = db_fetch_object($result);
    $breadcrumb[] = l($term->name, 'project/'. $term->name);
  }
  
  if (is_array($extra)) {
    $breadcrumb = array_merge($breadcrumb, $extra);
  }
  elseif (isset($extra) && isset($extra->nid) && isset($extra->title)) {
    $breadcrumb[] = l($extra->title, 'node/'. $extra->nid);
  }
 
  drupal_set_breadcrumb($breadcrumb);
}

It works, but it's kind of odd when the same variable (the $node object in this case) is passed as both parameters of the function.

If this is what you want, let me know and I'll reroll the patches again.

AC

dww’s picture

Yeah, advanced search is fubar, since it relies on core's search index, which i believe to be totally unreliable. :(

re: $extra, i was just thinking of an array vs. TRUE or something, instead of passing in $node twice (especially since $node will hopefully be an array soon). haven't had time to look too closely at the usage to see what would be best, but i agree what you posted above is yucky and i wasn't thinking of going that route.

thanks!
-derek

aclight’s picture

Ok, so something more like this:

function project_project_set_breadcrumb($node = NULL, $extra = NULL) {
  global $_menu;

  $breadcrumb = array();
  $breadcrumb[] = l(t('Home'), NULL);

  // Find out if the site has created a menu name for /project and use that
  $pid = $_menu['path index']['project'];
  $name = $_menu['items'][$pid]['title'];
  $breadcrumb[] = l($name, 'project', array('title' => t('Browse projects')));

  if (!empty($node) && project_use_taxonomy()) {
    $result = db_query(db_rewrite_sql('SELECT t.tid, t.* FROM {term_data} t INNER JOIN {term_hierarchy} h ON t.tid = h.tid INNER JOIN {term_node} r ON t.tid = r.tid WHERE h.parent = 0 AND t.vid = %d AND r.nid = %d', 't', 'tid'), _project_get_vid(), $node->nid);
    $term = db_fetch_object($result);
    $breadcrumb[] = l($term->name, 'project/'. $term->name);
  }
  
  if (is_array($extra)) {
    $breadcrumb = array_merge($breadcrumb, $extra);
  }
  elseif ($extra && !empty($node)) {
    $breadcrumb[] = l($node->title, 'node/'. $node->nid);
  }
 
  drupal_set_breadcrumb($breadcrumb);
}

AC

dww’s picture

yup, that's basically what i had in mind... thanks.

when you roll your new patch, please post it to http://drupal.org/node/58630 so we can let this issue sleep quietly... ;)

cheers,
-derek

aclight’s picture

Ok.. New patch has been submitted to http://drupal.org/node/58630

aclight’s picture

Status: Closed (duplicate) » Needs review
StatusFileSize
new963 bytes

Actually, this issue isn't a duplicate of http://drupal.org/node/58630 because that issue pertains to the Project module, whereas this specific issue and patch pertains to the Project Issue Tracking module.

I'm sorry this has been a little confusing--with 3 very similar issues and 3 similar project names, I got a little confused.

Attached is the patch for the Project Issue Tracking module. This patch relies on the patch submitted in http://drupal.org/node/58630 to fix the problem.

I think I've got everything straight now :)

AC

dww’s picture

Status: Needs review » Fixed

Reviewed, tested, and committed to HEAD, DRUPAL-4-7--2 and DRUPAL-4-7. Thanks!

Anonymous’s picture

Status: Fixed » Closed (fixed)