CVS edit link for mcjim

The core Book module provides one book navigation block, which is very useful, but occasionally you may need a book navigation block on specific non-book pages or in an additional region (e.g. in the footer).

The module I have written lists the books you have created on your site. You can select which ones you would like to create book navigation blocks for and they will then be available to you on the blocks administration page, where you can control which pages they appear on and in which region.

This solves the problem of wanting a certain book navigation menu to persist in a certain section of the site, regardless of whether the actual page being viewed is a child of that book.

For example, you may have set up a site for a client using the core Book module as the client finds it very easy to that way create a hierarchical structure for certain areas of their site and doesn't have to mess with menus. If you have set up the core book navigation block to show only on book pages, this can work really well for the client. However, as soon as they ask for certain book hierarchies to appear in the footer, for example, you'll find yourself needing to write some custom code (snippets can be found at http://drupal.org/node/44648 and http://drupal.org/node/209336). This situation cropped up in the development of http://carrotpharma.co.uk, where we used custom code to display certain book navigation menus in the footer.

Another example is where you are using Book module, and some of your book pages list or link to other pages within that section that you don't want to (or can't) actually include in that book. One of your child pages may have a View Reference field, perhaps, that lists dozens of nodes that you don't want included in the actual book hierarchy. In that case, you would lose the book navigation block for those pages when you clicked through. Using this module and page specific visibility settings or, better still, Context module, you can keep that book navigation persistent.

I couldn't find any other modules providing this functionality, either via Google, drupal.org or drupalmodules.com

CommentFileSizeAuthor
#12 book_block-v2.tgz1.78 KBmcjim
#9 book_block.tgz1.77 KBmcjim
#6 book_blocks.tgz1.83 KBmcjim
#1 booknavigationblocks.tgz1.84 KBmcjim

Comments

mcjim’s picture

mcjim commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new booknavigationblocks.tgz1.84 KB

Latest version of booknavigationblocks module attached.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Looks good. However, I'd recommend to use an internal module name that adheres to our common naming standards, i.e., separate words with underscores, describe a thing, not multiple things.

Thus, for example, book_block would be a good match.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: +Module review

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, pointing out what needs to be changed.

I am not sure I understood correctly the purpose of the module, and the differences with the block provided from the Drupal module. May you report the differences between the proposed module, and the existing one?

mcjim’s picture

mcjim commented

Thanks for looking at this, @kiamlaluno, apologies if my explanation is not clear.

It's best explained by looking at the site which inspired the creation of this module, http://carrotpharma.co.uk

"About Us", "Candidates" and "Clients" are all books, and can be accessed via the primary links. The core book navigation block is used to provide the navigation for those books and is shown in the left sidebar when viewing those books. All functionality provided by core so far.

However, the client also wanted the navigation for just "Candidates" and "Clients" to appear in the footer on every page. We wrote custom code to do this at the time, but I have since written this module. The core Book module cannot provide these extra blocks, but this module can. (When set to Show only on book pages, the core Book module only shows the book menu that applies to the node being viewed. When set to Show block on all pages, it displays menus for all of the site's books on every page defined in the block visibility settings. And it only provides the one block. We could duplicate the block using multiblock, but we'd still not have full control over which book menus we wanted to display.)

We wanted to make full use of the Book module's "magic" menu generation, rather than manually add menus, as extra menus would be an unnecessary complication.

Also, I am currently working on another site which uses the Book module. One of the books (let's call it "Work") has a few child pages and one of these child pages has a View Reference field which links to lots of other nodes. We don't want to include these other nodes in the book as it would make the navigation messy, but would like the book navigation block for the "Work" book to be visible on these other nodes so that context is retained. This module solves that issue, also, and allows us to make the most of the core Book module's menu-building.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

Thanks for the reply; it was not the description that was not understandable, but it was me to not understand it.

mcjim’s picture

mcjim commented
StatusFileSize
new book_blocks.tgz1.83 KB

Module re-rolled to match naming standards as per @sun's comment.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

I'd recommend to use a singular name, i.e., book_block.module.

You can (and need to) remove the following lines from hook_uninstall():

  // Delete the blocks we created.
  db_query("DELETE FROM {blocks} WHERE module = 'book_blocks'");
  menu_cache_clear_all();

Aside from that, this looks good to go.

mcjim’s picture

mcjim commented

@sun Many thanks for your review.
I'll change the name and re-roll.
Is it bad practice to remove the blocks on uninstall?

mcjim’s picture

mcjim commented
StatusFileSize
new book_block.tgz1.77 KB

Re-rolled with name change and removal of code from hook_uninstall as suggested by @sun.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

The module builds the menu tree of the book structure without first check if the node is accessible from the current logged in user; node_load() does not verify that before to return the node object.

mcjim’s picture

mcjim commented
Status: Needs work » Needs review
StatusFileSize
new book_block-v2.tgz1.78 KB

Thanks, @kiamlaluno. Well spotted.
Have added a node_access() check following the node_load().
Tested with private module and blocks are now shown only if user has access to the parent book node.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno

I am assigning the review to myself, as I will review it in the next 12 hours. Basing on my previous review, it's probable I will approve this application.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Fixed 
        $node = node_load($delta);
        if (node_access('view', $node) && $node->book) {

The code should be changed to avoid a warning about trying to access a not existing object property (what happens when node_load() returns FALSE, or $node->book is not defined?).

I am going to approve this application.

Thank you for your contribution! I am going to update your account.
These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.

mcjim’s picture

mcjim commented

Many thanks for your time, @kiamlaluno.
I'll get that tidied up, read through the CVS docs and get it released shortly!

Status: Fixed » Closed (fixed)
Issue tags: -Module review

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

Issue tags: +Module review

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: Miscellaneous » new project application
Issue summary: View changes