The book navigation provides an "Add child page" link, which passes the parent ID to the node/add/book form via a GET parameter.

Currently this is broken: the parameter is passed correctly, but it looks as though book_link_load(), which calls entity_load('menu_link', $mlid), is not returning everything book_node_prepare() expects.

This results in the book outline not being filled out as expected.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mcjim’s picture

Specifically, book_node_prepare() expects bid and access which are both missing from the menu_link object returned by book_link_load().

Previously, book_link_load() did this:

<?php
function book_link_load($mlid) {
  if ($item = db_query("SELECT * FROM {menu_links} ml INNER JOIN {book} b ON b.mlid = ml.mlid LEFT JOIN {menu_router} m ON m.path = ml.router_path WHERE ml.mlid = :mlid", array(
      ':mlid' => $mlid,
    ))->fetchAssoc()) {
          var_dump($item);
    _menu_link_translate($item);
    return $item;
  }

  return FALSE;
}
?>

Now it just does this:

<?php
function book_link_load($mlid) {
  return entity_load('menu_link', $mlid);
}
?>
mcjim’s picture

Component: entity system » book.module
mcjim’s picture

Some sample code, which fixes the issue. Whether it's the right approach is another story.

I'm not yet sure how we're checking menu_link access in D8, so just threw in TRUE for now.

<?php
function book_link_load($mlid) {
  if ($item = db_query("SELECT * FROM {book} b WHERE b.mlid = :mlid", array(
      ':mlid' => $mlid,
    ))->fetchAssoc()) {
    $book_link = entity_load('menu_link', $mlid);
    $book_link->bid = $item['bid'];
    $book_link->access = TRUE; // @todo Check access! Access was checked by _menu_link_translate() in D7.
    return $book_link;
  }
  return FALSE;
}
?>
larowlan’s picture

Issue tags: +Needs tests

Tagging

Ivan Zugec’s picture

Status: Active » Needs review
FileSize
1.96 KB

Here's an initial working patch and test for the issue.

But it'll be great if someone who knows the menu_link module can help us.

dawehner’s picture

Issue summary: View changes
Issue tags: -Needs tests
FileSize
1.9 KB

This fix is fine. Here is a quick reroll.

pwolanin’s picture

Status: Needs review » Needs work

Given that we are decoupling from menu links, I'm not sure the change is right here. In any case, the bug seems to be that the access check wasn't run or failed?

dawehner’s picture

Status: Needs work » Needs review
FileSize
807 bytes
2.03 KB

Pwolanin mentioned that the access call was dropped, let's replace that by a direct entity access call.

Status: Needs review » Needs work

The last submitted patch, 8: book-2012920.patch, failed testing.

pwolanin’s picture

FileSize
1.91 KB

That overwrote the $node variable. how about this instead?

pwolanin’s picture

Status: Needs work » Needs review

needs to test

pwolanin’s picture

FileSize
1.91 KB

re-posting. to get it tested.

The last submitted patch, 10: book-2012920-10.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: book-2012920-10.patch, failed testing.

mcjim’s picture