Follow-up from #2084421: Phase 2 - Decouple book module schema from menu links

Berdir points out there are a few menu_link-related references in the code including in book_uninstall()

Comments

pwolanin’s picture

Title: Remove remaining/sttray menu_link references from book module code » Remove remaining/stray menu_link references from book module code
Status: Active » Needs review
StatusFileSize
new1.72 KB
pwolanin’s picture

StatusFileSize
new3.15 KB
new4.87 KB

Hmm, still referencing a theme function from menu.inc too. The suggestion is wrong there too - shoudl be bid, not nid.

Status: Needs review » Needs work

The last submitted patch, 2: 2209035-2.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
StatusFileSize
new5.06 KB

Missed a pre-proccess function

berdir’s picture

$ grep -R menu_link core/modules/book/
core/modules/book/lib/Drupal/book/BookManager.php:        // After _menu_link_translate(), $item['title'] has the localized link title.
core/modules/book/lib/Drupal/book/BookManager.php:   *   fields from the {menu_links} table, and optionally additional information
core/modules/book/lib/Drupal/book/Form/BookAdminEditForm.php:use Drupal\menu_link\MenuLinkStorageControllerInterface;
core/modules/book/lib/Drupal/book/BookManagerInterface.php:   * Note: copied from _menu_link_translate() in menu.inc, but reduced to the
core/modules/book/book.module: * Implements hook_menu_link_defaults().
core/modules/book/book.module:function book_menu_link_defaults() {

- I think the first one should mention the bookLinkTranslate() method now?
- The second is part of the menu_tree_data method, and should probably be updated too, or was that left like that for a reason?
- The others are fine, the use is removed in #2209029: Follow-up: clean up use statements in book module code.

pwolanin’s picture

StatusFileSize
new848 bytes
new5.63 KB

For the comment referencing menu stuff I was going to tackle more completely at #2209025: Phase 3 - Make the BookManager interface more coherent, move more code out of book.module. Changling that 1 reference to the menu_links table doesn't make sense without re-writing the whole doc block

So, this patch adds a fix correcting the comment to reference bookLinkTranslate()

jhodgdon’s picture

Component: documentation » book.module

This is not a docs issue.

dawehner’s picture

  1. +++ b/core/modules/book/book.module
    @@ -715,6 +718,27 @@ function template_preprocess_book_node_export_html(&$variables) {
    +/**
    + * Returns HTML for a wrapper for a book sub-tree.
    + *
    + * @param $variables
    + *   An associative array containing:
    + *   - tree: An HTML string containing the tree's items.
    + *
    + * @see template_preprocess_menu_tree()
    + * @ingroup themeable
    + */
    +function theme_book_tree($variables) {
    +  return '<ul class="menu">' . $variables['tree'] . '</ul>';
    +}
    +
    

    Just wonder why we don't already create a twig template, which would open up another issue otherwise.

  2. +++ b/core/modules/book/lib/Drupal/book/BookManager.php
    @@ -921,7 +921,7 @@ protected function _menu_tree_check_access(&$tree) {
    +        // After bookLinkTranslate(), $item['title'] has the translated title.
    

    we could use static::bookLinkTranslate

pwolanin’s picture

StatusFileSize
new2.33 KB
new5.93 KB

Thanks - here's with a template.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

We could do more obviously, but let's stay with the these bits for now.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.