Separating each 'book' allows for richer feature set

looks interesting. i've got some ideas about improving book, too (http://drupal.org/node/61397#comment-116556), and perhaps those would be more easily accomplished built on top of your proposed changes.

a *very* quick skim of your patch revealed this in the db update hook:

+      $ret[] = update_sql("CREATE TABLE `wechange_book` (
...
+      $ret[] = update_sql("ALTER TABLE `wechange_book_pages` ADD `bid` INT( 10 ) UNSIGNED NOT NULL AFTER `nid` ;");
+      $ret[] = update_sql("ALTER TABLE `wechange_book_pages` ADD INDEX ( `bid` );");

what's up with "webchange_book"? looks like the rest of your patch assumes (as it should) that this table is just called "book"...

also, this isn't acceptable in core:

 case 'pgsql':
+        //TODO

;) i feel your pain, but you'll need to find a pgsql person to help you get this (at least) pgsql compatible before any of the core developers will seriously consider it. of course, you're probably waiting to see if it's just going to be shot down from the start before you pour much more time into it, which is probably a good move on your part...

anyway, without looking in detail at the code, and without having thought too much about it yet, i think this is a good move in principle. +1 to the idea, code definitely needs work.

thanks for pushing this forward, beginner!

-derek

Fantastic!
I've thought this is one of the major issues that needs to be addressed. It's ludicrous to have drop-down boxes that are 1000 lines long. Thank you very, very much. You can change your username to "expert"!

The concept sounds great to me.

One question- how do you determine the author/maintainer of a "book"? Is this just the author of the first node? Or, is this a node-independent piece of data? Can a book have multiple authors?

I'm not convinced this is the right direction ...
One immediate consequence is that the pull-down menu can rapidly become unwieldy because all nodes are listed within.

This is actually a great feature, as pages can move from one book to another.

Permissions (access/create/edit/relocate) can be granted individually for each book without the need for contrib access module.

Here you are assuming that each book has exactly one maintainer/author, not?

Better caching possibility of the book outline, because it can be done more easily per book (improving performance)

The book outline shouldn't be cached. Instead, we should switch to a different algorithm to retrieve the book outline. In particular, we should use the comment.module's approach which allows the entire thread to be retrieved with a single SQL query.

beginner: One immediate consequence is that the pull-down menu can rapidly become unwieldy because all nodes are listed within.

dries: This is actually a great feature, as pages can move from one book to another.

dries, you must admit that while it's definitely a good feature to be able to move pages between books, the current UI is a nightmare. i don't have a great solution in mind, but i think what we've got now needs help. maybe we need an active select, and have 2 drop-downs: 1 for the book this page belongs in, and the other for what page in that book should be the parent of this page. then, if you change the option in the "book" dropdown, the active-select kicks in and changes the options available in the "page" dropdown...

Agreed, regarding moving pages. I've posted handbook entries and spent more time trying to scroll to the correct parent page than I did writing the entry itself. It's good to be able to move things around at will, but finding something in a 400 item select box offsets that ability.

@dww- I agree.

How about this idea: in the drop down, only list the top-level book name for all books except the current one where you see the whole hierarchy. If you move a page to a different book, it gets put somewhere by default. The advantage is that it avoids any dependence on JS.

This is a little suboptimal, since you then have to go and put that page where you want it within the other heirarchy. But, in terms of actual useability, how often are pages moved between books?

Another thing to think about, how and where do you control whether I can move my pages into your book, if this module is going to lend itself to per-book editing permissions.

Sounds excellent. I will try to test the patch this weekend.

Ok, trying the patch out now. I like the way it's headed and here's some feedback:

The option to create a new book doesn't appear at /node/add I suppose you can argue it shouldn't, since it's not a node. Maybe a better approach would be to make each book a node? Similar to the way each group is a node in the organic groups module? In fact, the more I think about it the more important this seems. If it's not a node, then it becomes much harder to associate taxonomy terms, use access control modules, associate it with organic groups, etc.

When I'm adding a book page from the create content menu, I'd suggest re-using the same form with the drop-down as when moving a book page to a different book. I think it's a little confusing to have two different interfaces for these operations. In particular, it's confusing now when I'm adding a new book page and I'm asked to select from the list of books at /book, the "New book" tab is visible which makes me think that I can add the page to a new book by clicking this.

Right now you apparently cannot delete a book? I suppose to allow deletion there needs to be a mechanism in place for making the book pages within that book orphans?

Right now the new code doesn't seem to work well for non-book-page nodes that have been included in a book hierarchy. For example, on those pages if I click the "outline" tab I get the previous type of book drop-down showing all the book pages . It seems like the UI here needs some thought. Maybe do away with the "outline" tab, and only have a a tab that says either "add to book" or "move to book". The "move to book" tab would be the place to include the "Remove from book" type of button that currently appears on the "outline" tab for non-book-page nodes. I think for pages already in a book it would make sense to inject the book-outline drop-down into the editing form rather than having the "outline" tab.

Right now the "move to book" tab does appear for non-book-page nodes, but going through the process here doesn't actually seem to move the node into the book.

This is more in terms of improving the module in general, and not a comment on your patch. On the /node/x/edit for a book page (or ideally any page in a book following my suggestion above), I think the book terminology should be clarified. The cuurent heading for the drop down is "Parent". And the help text below is "The parent that this page belongs in.". I think the ideal would be to replace "Parent' with something like "Position in book: %s" with %s getting replaced with the name of the book it belongs to. The text underneath could be improved to something like "Select the page within this book that should be the parent of this page".

A minor point- the meaning of <top-level> has changed, which is fine but which probably should be documented clearly since it's likely to cause some initial confusion.

In terms of making a node type, the easiest would be just to start from "story". Also brainstorming additional name possibilities to "book" or "book cover" node, how about something like a "book binding", or "book spine"?

You could also follow the current OG model where any node type can be a group. However, I think it may be better to avoid that level of complexity.

having a book as a node is a good idea. however, i think "book" is the best term for this. "spine", "binding", etc all have other possible meanings, which could add confusion. you wouldn't want the UI to say "move this page to another book binding". just call it a "book". then we have "books" and "book pages", which seems utterly natural to me. ;)

in terms of moving pages, i agree that avoiding JS where unnecessary is a good idea. however, i don't like the idea of moving a page to a new book and having it just show up randomly in the new book (or worse yet, at the top-level of the new book). how about this -- when you move a page to a new book, the page is just stuffed into the top-level of the new book (as proposed) but is automatically unpublished. when you hit [submit] for the move operation, it could bring you back to the edit tab again (instead of the node view tab) and the drupal_set_message() text should clearly state something like:

This page has been moved to the %title book, but is now unpublished. You can put it into the book outline by selecting an appropriate parent page, and making it visible again by selecting the "Published" checkbox under the "Publishing options" set of fields.

obviously, that's a little bit wordy and we could probably do better, but you get the basic idea. i suppose we could even keep the default behavior of going back to the node view tab after the edit, in which case the drupal_set_message() text would be roughly the same, but would be re-phrased about "you can now edit the page to put it into the book outline..." and could perhaps include a link to the edit tab.

still no time for a more thorough review or testing. but i'm following the discussion closely. more later.

Ok, just a brain storm. I'm perfectly happy using "book" and "book page".

If I recall correctly, the version above does force you to an edit screen after you move the book page to a new book. Thus, you can immediately choose a place for it in the hierarchy. I think the default (where it starts) is at the top level, which seems the most reasonable thing to do.

i'm just brain-storming, too. :)

the important point (maybe lost among everything else) in my previous comment is that when you move a page to a new book, it should be unpublished, so that the author/editor must select a reasonable place to display the page in the new book before it becomes visible to the world.

either that, or we just use an active select. ;)

Since you are trying to solve something I'd like to use on an upcoming site of mine, I hereby sign up for the job as guinea pig. Here are results from diff6:

* update.php did not automatically detect version to upgrade from. If I've understood correctly, this is not you patch'es fault but something may have gone wrong in the previous upgrade (to 4.7.1)?

* It seems that now you take each top level book page and make that into a separate book. Assuming that your patch gets accepted into Drupal core, I think it is wrong to assume that, even though it might be correct in a majority of cases. It would be better that those who think they only have one book do so also after your upgrade, and those that have multiple books have to go through some upgrading procedure which could even be manually moving top-level pages to new books of their own.

* On a correlating note, now each top level page creates a new book of the same name. So when you view the first page in a book the breadcrumbs say something like "Home > books > book about history > book about history". This is another reason why old top level book pages should not automatically become books.

* Go to: create content > book page > [select one of the books]
It would be nice to show somewhere which book a page is being added to

* There is a new menu item "books". I can disable it in administer > menus, but I'd like to be able to do that from administer > access control so that it would be visible to some and not to others.

* Following Drupal style, "Move to book", "Add page", "New page"... should not be capitalised. (It seems you have them all capitalised)

I guess if you are going to make "book" a node type, it is not a good time to start using this patch just yet on a production system? I can take a small hit for being a happy guinea pig, but if there are still major rewrites ahead maybe it's wiser for me to wait.

Then the real question: it sounds like you already have an idea on implementing per-book access control? Care to share about that?

Readers of this thread might be interested to know, that I developed a simple version of the book.module that allows you to define protected books.

The Book 6 patch does not apply properly to 4.7.2

nikdotca modules $ patch -p 0 <book.patch 
can't find file to patch at input line 8
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: database/database.4.0.mysql
|===================================================================
|RCS file: /cvs/drupal/drupal/database/database.4.0.mysql,v
|retrieving revision 1.4
|diff -u -p -r1.4 database.4.0.mysql
|--- database/database.4.0.mysql        26 May 2006 09:21:10 -0000      1.4
|+++ database/database.4.0.mysql        27 May 2006 16:55:52 -0000
--------------------------
File to patch: book.module
patching file book.module
Hunk #1 FAILED at 133.
1 out of 1 hunk FAILED -- saving rejects to file book.module.rej
can't find file to patch at input line 42

I haven't been following this thread extensively, but on the subject of multiple nodes from one module, it is actually fairly straightforward in 4.7.

See: http://api.drupal.org/api/4.7/function/hook_node_info

If you want to define a book node and a bookcover node, you'd have something like this:

function book_node_info() { 
   return array( 
     'book' => array('name' => t('book page'), 'base' => 'book'), 
     'bookcover' => array('name' => t('book cover'), 'base' => 'bookcover') 
   ); 
 }

Then all of your node hooks for the book node have a prefix of "book_", while those for the bookcover node have "bookcover_".

Personally, I like to then separate my node functions out into a separate file, mymodule_node_mynodename.inc, and require_once() it. It makes no difference to Drupal currently, but I find it better code organization to split the module up into each node, then the non-node-hooks in the mymodule.module file. (I'd actually love .node files, but that's another topic. :-))

HTH

Yeah, nodes are not fully separated from modules, which I agree is bad. I believe you have to also add a menu entry for each one in hook_menu().

Feel free to contact me off-thread for more, so that we don't clutter up this thread anymore than we need to. :-) (Or there should be a handbook page somewhere...)

Well, is there any reason not to break this into two separate module files? I think the OG module could provide some interesting cross-pollination of ideas for this. For example, one can choose which node types are groups, and which node types can be added to a group's content. A default "group" type of node module is included, but any other node type can be used instead or in addition (incl. flexinode, etc).

OG also has functionality where the any user designated a "manager" of the group can edit any content posted to that group.

Allowing site admins the ability to choose other/custom node types as books (essentially the cover page and/or TOC of the book's content) seems like it would enhance the flexibility and extensibility.

The node relativity module also seems to be related, at least in some respects http://drupal.org/node/17004

I'm very excited by these developments, as my site would be greatly enhanced by distinct books. I'm trying to give each Organic Group its own book. Is this going to be possible with these modifications?

I tried the patch and hunks #5, 9 and 13 failed at lines 235, 438, 582 respectively. I also ran update.php and no changes were made to the database. Any thoughts? Should I wait until this patch is further along.

did you try applyying the patch to 4.7 or to HEAD? I'd wait anyhow until beginner produces the next iteration.

I'll try to test this- I assume this requires a HEAD install, not 4.7?

Won't the proposed changes to phptemplate allow for per-node substitution of a different page.tpl.php? http://drupal.org/node/65706

I realize an automatic system would be easier for some users, but maybe not by enough to be worth the effort of getting it in?

Trying to test this- last big patch seems to have broken some of the menu paths:

patching file modules/book/book.module
Hunk #4 FAILED at 131.

Hmmm, install doesn't seem to have worked either:

user warning: Table 'dp1.book' doesn't exist

Ok, an initial review now that it's working. Overall, I think this is going in the right direction, so I hope you'll see the below as constructive criticism:

The 2-step outline procedure for non-book nodes seems pretty smooth and reasonable.

On a book_cover page: "Table of Content" should be "Table of Contents"

The setup at /node/add for book pages is counter-intuitive now and a little hard to understand. I'd suggest forcing all book pages to go under a book_cover. If no book_cover nodes have been created, an error message and link to create book_cover would be better. Allowing the independent, top-level book page thing seems confusing. I'd suggest essentially that creating a book page from /node/add should essentially follow the 2-step process that's used for moving a book page to a within a different book_cover. First you have to choose a book_cover to go within, and then 2nd you can edit the node and position it within the pages contained in the book_cover.

I'm not sure then how to deal with the deletion of a book_cover. You could simply not allow it until all contained pages have been moved or deleted, though I'm not sure if this is possible within the current API. Maybe some sneaky use of hook_form_alter. Or (especially if the trashbin patch gets in) deleting the book_cover could also delete all associated nodes.

The outline feature at /admin/content/book seems not to work, or at least not in the way I'm expecting.

When adding (editing?) a book page, it would be nice if the heading at the top of the page should indicate which book_cover it will be under. i.e. book cover: page title, since there still seem to be some breadcrumb problems, and the "Parent:" drop-down doesn't indicate which book_cover this page belongs to. I guess this is an alternative- label this drop-down with the book_cover title.

If I have no book_cover nodes defined, the "outline" tab still appears. I'd disable it as per requiring all book-related nodes to fall within a book_cover.

If I delete or have no book_cover nodes, the book shelf shows all the top-level book pages.

Thinking about how to prevent book_cover deletion when there are child pages:

How about using (node_access('delete', $node)?

I guess the problem with this is that any user with "adminiser nodes" will be able to delete without a check by the book.module.

I sort of don't like to suggest it, but how about automatic creation of a book_cover called "orphans" where orphaned book pages are relocated?

Ok, more thinking- just send book pages whose book_cover is deleted to the orphaned pages limbo:
/admin/content/book/orphan

You could surely set a message upon book_cover deletion informing trhe user that their book pages can now be found there?

I'm sure the recent changes to HEAD have made more work necessary to move this patch along.

Let me know if there are some TODO items with the code that you'd like my assistance with.

In regard to your comments above, I don't feel strongly about having book_pages outside a book- just seems like a cleaner interface to me to avoid it.

As far as hard-coded HTML, looks like one easy change might include a couple like this:

    $output .= '<h2>' . t('Independent top level book pages') . '</h2>';
    $output .= theme('item_list', $books);

to this:

 $output .= theme('item_list', $books,t('Independent top level book pages'));

Alsp,

_book_cover_toc($bid) 

should probably just do the query/array building and then call a theme function (also needs rewrite)

function _book_cover_toc($bid) {
  $result = db_query(db_rewrite_sql('SELECT n.nid, n.title, bp.parent, bp.weight FROM {node} n INNER JOIN {book_pages} bp ON n.vid = bp.vid WHERE n.status = 1 AND bp.bid = %d ORDER BY bp.weight, n.title'), $bid);

  while ($node = db_fetch_object($result)) {
    if (!$children[$node->parent]) {
      $children[$node->parent] = array();
    }
    $children[$node->parent][] = $node;
  }

  return theme('book_toc',$children);
}

and then anyone who wanted to could totally change the theme function, including using a different function to walk through the array:

function theme_book_toc ($children) {
  $toc = '<hr /><div class="book_toc"><h3>' . t('Table of contents') . '</h3>';
  $toc .= build_book_toc($children);
  $toc .= '</div><hr />';
  return $toc;
}

function build_book_toc($children, $nid = 0) {
  if ($children[$nid]) {
    $toc = '<ol>';
    foreach ($children[$nid] as $foo => $node) {
      $toc .= '<li>'. l($node->title , 'node/'.$node->nid). '</li>';
      $toc .= build_book_toc($children,$node->nid);
    }
    $toc .= '</ol>';
  }
  return $toc;
}

Are you trying to get the extra fields to load from the db, or to display during node_view?

I am trying to review this patch in detail and maybe make some modest changes- I'll assume you're not making more changes now?

In particular, I want to unify the UI for adding/moving pages to books.

also, why not put an "outline" tab on all pages? I think this may be simpler than the current system- for the book cover, the outline tab could bring you conveniently to the outline for that book.

I'm trying to review this in detail and maybe make some modest changes- I'll assume you're not changing it any more and try to post a revised patch later this evening or tomorrow.

Well, I'm making some progress but it's not fully backed yet. I'll let you look at my concept. I'm trying for maximum reuse of code and interface. I'm trying to make "outline" look the same for book_pages and non-book_pages with the exception that non-book_pages have an extra option to remove from the book outline. Both have a "move to another book" option. the outline tab on a book (cover) redirects to the outline admin page for that book. Maybe it should show as a local task, rather than redirecting.

Also, there are a couple issues that seem more fundamental

the book_page table should not use vid since there is not proper revisionsing for non-book_page nodes. Also, if you move a book_page to another book, there would be odd side effect of loading a revision that's back in the other book/with parents in another book. The OG module does not try to track the vid, only the nid of nodes within a group and I think that makes sense here. I'm not sure how to do the SQL for the upgrade path- basicially all the entries in the book table for the non-current vid should be discarded/not copied to a new table.

Second, the book table should include vid and use this as the primary key, since the data (license, author, etc) may be different per revision.

also, i think the node type should be named book and book_page to match the table names.

Please take a look at the attached patch (doesn't include node module patch). I was already well down the path of merging the outline tabs, and perhaps it will grow on you when you see it. I realize there is some cleanup that is needed in terms of the form appearance, but it seems to be functional.

The update part of the install file is still non-working. Also, the permissions may need work. Found several bugs that are present in the 4.7 module, though I'm not sure I'll remember them to try to fix them (one's in 4.7's book_submit).

Also- a side benefit of this approach to the outlining is that the permissions to outline a node can be totally separated from the permission to edit the node. i.e. someone who cannot edit a book_page can still move it to a different position in the book or move it to a different book.

I was hoping that the hard-coded book node type would actually be removed and this module would use only nodeapi now that we have configurable content types.

I'll review this again if it gets to 'ready to be committed' (same for all features).

Oops, one feature was broken- I got hung up on the new node URLs: node/add/book-page vs. node/add/book_page. Fixed in the attached patch.

As far as making the book_page type optional or redundent: I think this patch is pretty close to this. The essential difference is that the edit form for a book node may include the select form to choose the level in the book and the weight, while for other node types this is only accessible via the "outline" tab.

Also, the book_page type serves as a default for the "add child page" and related functionality. I'm really not sure how you could do that with a defined node type.

Make a prefilled book node type, like page and story when the book mopdule is installed (first enabled). Also prefill the settings to make that node type function as a book page.

@drumm- Well, I can imagine how much of the functionality could be accomplished that way. However, I'm not sure how one could do the kind of two-step process that kicks in now at node/add/book_page. if you click "book_page" under create content, the code above redirects to a form to choose which book to put the page in, before sending you to the actual node form. I suppose that this could be considered dispensible??

setting back to needs work - attached patch is (I hope) continuing to move in the right direction, but is not complete.

I changed book_page_form further so that the "special" paths are like node/add/book-page/$bid/$parent. This simplifies the function so only one check is needed. Need to think further about how to deal with the case of a manually entered path where the user doesn't have access to view the book. I think if they don't have access to the parent (or the parent doesn't exist), then the select just defaults to .

Tried to rename some functions to distinguish book/book_page

Also worked on the display function for the book_page menu, but it may need more. There is some very odd logic the way it works now.

I'm also thinking that like OG, the bid should be the nid of the book node? This would probably simplfy some of the SQL and logic. The only downside is that bid values will be arbitraty and potentially large numbers rather than in order.

Upgrade path is still undefined. I've been thinking about this and still wonder if converting all the top-level book pages to book nodes might not be the best solution???

trying to work more kinks out and improve the overall code.

Diff attached, but this is almost certainly broken by recent changes to HEAD (FAPI and t()). I will endeavor to update this to work with HEAD.

I think this is fixed now to work with HEAD and incorporate recent patches.

some areas that still need work:

function book_help
function book_view

also, the orphan pages page is a little odd (I think it's the same in 4.7 though), since the nodes have to be edited to re-attach them. Maybe a link to the "outline" page for that node should be included also as an operation.

found a couple more mistakes (including using nid rather than vid for joins between the book and node tables, and a broken book_update()). Also, made an (untested) stab at the update function.
Try this instead of #78

attached script is hacked from the devel module- iit will quickly create book and book_page nodes for testing purposes. Create a few users and enable the path module before running. Note- put this script in the Drupal root directory and expect it to take a minute or two.

Running this script and then looking at the book TOCs, it seems that there are problems with the book navigation code when you are at a node with no children.

Also, there is a problem with the book block: for pages associated with a book (bid != 0), the block still only shows the nodes up to the immediate parent that has parent == 0 , and titles the block that parent. Thus, the block only shows a "chapter" rather than the whole "book".

caught a bug where user_access was called instead of node_access. Caused ugly SQL errors, etc.

Oh, right- I must have been working off an earlier version of the install file. See attached.

also, there's some problem in the genration script I posted with the assignment of parents. I'll try to fix and re-post.

fixed permission checks for links

better generation script attached- #of nodes desired can now be set at the top. Also, before I was forgetting to empty the array of parents when moving to the next book.

Ok, put back node type checking for the outline tab as per e-mail.

The outline function on the admin pages is horribly inefficient and needs some love (using the same tree function as generates the selects, etc). I think it's also displaying in the wrong order (look at the weights).

Also, attach patch fixes the book navigation block. If bid != 0, title is the book title, all pages displayed. If bid ==0, title is the title of the top level book that's a direct parent, and only the pages under that parent are displayed.

oops- block code still wasn't quite right.

script attached to create lots of 4.7-style book nodes to test the update code.

changed the update script- book nodes need to be renamed to book_page

yes the link is fixed for child page.

a couple outstanind isssue if you want to try tackling them:

the help text and links need work, since the book/book_page change hasn't really been delt with there.

Also, there is no printer-friendly link on books (covers) - I'm not sure what need to be done to make that work.

Ok, the conversion frmo a separate counter from bid to useing the book nid as bid is nearly done. The book_form now includes a select for the weight (missing before) and a select for the *new* toc_depth field.

Also, I took out the <ol> function for making the book TOC and used the same function that generates the block. The reason: the toc_depth field for each book that allows you to choose to what depth the TOC should be expanded on the book page. However, this only makes sense when using the expanded/leaf attributes so that you can see which items have children.

Attached is a version of the original function that can be plugged into your theme folder. I'm not sure if something like this could be included- maybe just put it as a theme snippet.

Ok, I haven't found any bugs in the last 10 minutes, so mayeb it's ready to review. Helper node-generating script to follow.

Also, beware this block bug: http://drupal.org/node/80150

script to generate book and book_page nodes that match schema from #96

this patch: http://drupal.org/node/4074
was just applied which seems to break #96. will try to re-work.

Added equivalent change to the above-referenced patch. Also fixed up book_help.

Note- look at function book_submit(). I fixed a bug(?) there so now if a user edits a book page they become its author. This doesn't happen in 4.7 because of an incorrect reference to $book rather than $node.

Don't change past updates (such as system #1000). Make new updates.

I misread that. This would be our first non-system core update hook.

This is still not ready to commit since it is a huge change only reviewed by two people. This must be okay for the documentation team for Drupal.org since we have to deal with this module too.

I'd like to reiterate that I think we should be removing hard-coded content types instead of adding them.

@drumm, I have been thinking about the use of non-hard-coded types. I certainly think we could get there if it's really important to you for this cycle, but there are two problems:
1) time before Sept 1st.
2) is it possible now with the forms API tp do multi-step forms (via form alter)?

For #2 I got no answer on the devel list,though maybe i'll try again. The problem is with the book_page type. We want to be able to go to a form to select a book if the user chooses that type from the create content page. The current solution uses a redirect within book_page_form().

What if you selected, in settings, which node type (or types) to be a book page? This leaves us without a book page, but still gives you a method to deal with nodes that are meant to be in a book.

@Boris - I think that could indeeed be a next step, but would be enough of a change that it may need to be for a subsequent release (see below).

@drumm - I'm on the documentation list and have permissions to update Drupal handbook pages, so I can be involved in that effort.

Also, as food for thought, this is were I see that this module *could* go:

1) All types are DB- defined, user configurable (as per Neil's idea).

2) Admins can choose one or more node types to be "book" types (like OG groups)

3) For each book, the admin can choose a (different) default type for the child pages.

4) Clean up lots of inefficient code (reduce # of queries) for the book navigation and others. (note that this work could be considered bug fixes, done after Sept 1, and maybe backported to 4.7)

I'm very reluctant to try implement 1-3 all in the next week, though I think 1-3 are actually not that hard. Partly, I'm a little worried about trying to keep up with the prospective changes in the content types code . I also don't want to put that level of effort in without some real assurance that the patch is likely to go into HEAD.

In concept this looks to expand an already powerful feature of Drupal's. Seperating books and adding descrete permissions.

I myself don't mind the long move pages tree and have often switched pages between books but can see how some wouldn't like the current UI. The intial use case as to why one wouldn't was to limited. I can think of several alternative use cases but am not bothered by it to make a long discussion.

So... what's in the patch? Needs a summary seperate from the original issue in May and the misc other wrap ups since. I agree with Drumm that I would prefer less hard coded content types as we're going that way with core but am not married to it right now. In any case I understand the reluctance to go if not sure of the reception. Otherwise I like what I've been able to parse regarding the increased functionality and I suspect it will be a popular addition. I am moved enough that if I can this weekend I will even try and test it with HEAD.

The patch in #105 doesn't apply cleanly for me, so I've attached another with the same bug fix, plus a reworking of one utility function (book_update_children_bid) - no UI or functional change.

Let's think about moving the issue, since some of the initial description does not really apply (including the schema and some of the permissions). This patch does not currently try to provide per-book permissions.

Wow, this is awesome. This makes books actually useful! I might have to rethink books with this one, I could actually use them in some cases. This would be fantastic for educational based distros.

I certainly understand if you're not interested in pouring more energy into major changes, but #3 that you mentioned -- "For each book, the admin can choose a (different) default type for the child pages" -- would make this tremendously useful as a tool for online webcomics and other such structured sites. Right now, it's a bit of a pain managing those things, and these changes would make it a good deal simpler.

Ok, I'll take a look at how to implement #3. Does any one have sugesstions as far as the data structure and UI? My initial thought it to make this part of the {book} table and to present the option as part of the book create/update form. Should the visibility of this option be based on an existing permission (e.g. "administer nodes")?

beginner makes an excellent point. adding node type configurability may be best left to subsequent patches, as nice as the feature is. This patch is already pretty heavy, energy should probably go to making it commit-ready rather than adding a few more features to the checklist.

O Godiva,

this would be better as an issue against the documentation project or on the documentation mailling list. This issue is for a patch for the next version of Drupal. No need to reply to this.

I think this would be a great addition and I need functionality like this in an upcoming project, so I'm about to test this. I couldn't get the book.install part of the patch to apply, but the patch to book.module worked fine.

I absolutely need the ability to make both the book and book_page configurable types since I need to be able to add other fields to them (using cck). This should actually reduce the size of the code by letting other modules handle table and field maintenance and eliminate possibly unnecessary fields (i.e. not everyone has any need to track 'license' for each book, but may have other fields they need.)

I guess I agree (reluctantly) not to slow things down by adding that capability in now but would like to see it be a top priority.

Anyway, this looks like a wonderful and much needed effort.

right now any node type *can* be added to a book, so the reason to make other node types than book_page the default child page is for an easier workflow.

If this patch were to get in within the next day or so, I'll try to supply additional patches to add at least part of the other functionality I proposed. So, please test and provide you feedback on this patch!

attached is another patch against the most recent HEAD. The functionality of this patch is the same as the previous but includes some clean-up and reformatting of comments to try to better conform to http://drupal.org/node/1354, plus added validation of the book ID ($node->bid) when setting the $node->bid for a new node based upon the URL. To facilitate this, the book/parent info is now added to the URL as a query string, rather than as part of the path (before: node/add/book_page/1/1, now: node/add/book_page?book=1/1).

for those who want to test the patch- test site with dummy content:
http://book.wolanin.net

This patch conflicts somewhat with my view of the book module. I want the book module to become a generic tool to outline/structure nodes in hiearchies and to provide the nagivational elements for that. I would also like to get rid of the "book page node type" and just use a CCK node types (or simple stories) to populate the book hierarchy.

In a way, I want the book module to be less of a book module. This patch makes the book module more of a book module (eg. it adds license information, book covers, book authors, etc).

Coding style needs work. For example, we use dashes, not underscores to name CSS elements.

I tried to follow this whole long thread to see if this was discussed earlier, so may have missed it. Does it make sense to make this a contributed module instead of a patch to core? I think this kind of functionality (once the ability to use configurable content types for book and page are added) is going to be valuable to many, but will be overkill for others. To me, this is sort of a 'book plus' module that not everyone will need. Why not just make it a contributed module, and let the people who need it work the bugs out of it without affecting people who are perfectly happy with the current book module. Like other contributed modules, if it is useful enough and used by enough people, it might get rolled into core in the future.

The only problem with that approach that I can see is the naming of the module since you'll need a name other than book.

@Dries, I agree that the function of the current book module is more navigational than book-like and that there needs to be a module that provides that simple navigational functionality. I think the confusion is that it is called 'book' when it is not exactly that, and people who really need the ability to create real books think they will get it and do not. (I found that confusing, too at first). I think making this kind of book functionality available in another module would be big win for everyone. Too bad the names are going to be confusing :-)

I'll just throw this out, knowing that it would probably create too much havoc to consider. I'd like to see the core book module re-named to 'outline', which is much more descriptive of its actual use, then set this module up to be 'book', for people who need to create things that are really books.

I'm in full areement with Dries at to what the end result should look like. It's really a question of the best tatics - I'd like to see a version of this patch go in, and then have additional patches as incremental steps towards the goal.

@KarenS - I don't think the functionality of this vs. the existing book module is different enough to warrant a split. I'd rather see a core module that's much more flexible, but that woks similarly to the current book module by default.

Attached patch cleans up the CSS classes per Dries' comment and gets rid of hard-coded <hr/> elements in favor of adding borders with CSS. Also caught a minor bug and tried to improve the code a little by reducing queries.

As far as I'm concerned, this patch is RTBC and would be a fine addition to 4.8. From here I will start a new issue and try to modify this in stages as follows:

1. drop the license and author fields from the book table
2. add a field to the book table for a selectable default child page type
3. allow other node types to be "books"
4. make the "book" and "book_page" types DB-defined

Please direct follow on to: http://drupal.org/node/81226