Problem/Motivation

You can't programatically update books because some logic around original_bid is only implemented in form handlers.

// This is always true when programatically saving a node.
$new = empty($node->book['nid']) || empty($node->book['original_bid']);

Steps to Reproduce

  1. Enable the book module and then create a book page.
  2. In code, Node::load($nid)->save();
  3. See big fat error

Proposed resolution

Fix it.

Remaining tasks

Write the patch.

User interface changes

n/a

API changes

API will be fixed for working with books.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjy’s picture

Status: Active » Needs review
FileSize
581 bytes

Test to show the issue.

Status: Needs review » Needs work

The last submitted patch, 1: 2363647-1-FAIL.patch, failed testing.

martin107’s picture

I think there maybe a little confusion in testBookApi()

the call to createBookNode() with $parent, in effect, set to NULL means that

     $this->drupalPostForm('node/add/book', $edit, t('Save'));

will be called...when createBookNode exits $book->save() is called on an already saved node ?

This makes no sense to me

benjy’s picture

The bug is that you cannot save a book that already exists?

$book->save() is called on an already saved node ?

You're saying that you can't save any node twice? To clarify, this issue would exist even if you made a change to the node as well.

martin107’s picture

Ah good point

benjy’s picture

Issue summary: View changes
Berdir’s picture

What is critical about this?

Only fatal errors that can be triggered in the UI are critical, and actually, not even that is mentioned anymore on https://www.drupal.org/node/45111, and it also doesn't fall into any of the categories there?

benjy’s picture

Priority: Critical » Major

Sure, that's fine by me.

yukare’s picture

Just a note on this, may help to find a solution:

$node = Node::load(nid);
// The form code does something like this.
$node->book['original_bid'] = $node->book['bid'];
$node->save();

This works and any change on $node is save.

benjy’s picture

Status: Needs work » Needs review
FileSize
1.37 KB
822 bytes

I'm not sure if this is the right fix, but lets give it a try.

Status: Needs review » Needs work

The last submitted patch, 10: 2363647-10.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
FileSize
1.41 KB
578 bytes

I copied this logic from the form, it's not exactly pretty but it fixes the problem and tests should be green.

chx’s picture

Status: Needs review » Reviewed & tested by the community

book never promised to be pretty. I did try to play with the logic a bit but there isn't a lot you can do (I tried to replace $node->book['original_bid'] = $node->book['bid']; with simply $new = TRUE and adjust $new accordingly) but this fails. This patch works so let's do it.

alexpott’s picture

I wondered if could we remove the corresponding lines from book_node_prepare_form(). Yep this is ugly but this is not the worst thing in the Book module.

+++ b/core/modules/book/src/Tests/BookTest.php
@@ -65,6 +65,15 @@ protected function setUp() {
   /**
+   * Test that we can save a book programatically.
+   */
+  public function testBookApi() {
+    $this->drupalLogin($this->book_author);
+    $book = $this->createBookNode('new');
+    $book->save();
+  }

Let's not do a whole Drupal install just to test this. Perhaps we can add this to another test method. How about testBook() since that Tests book functionality through node interfaces.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

For #14

benjy’s picture

Status: Needs work » Needs review
FileSize
1.48 KB
1011 bytes

Done.

benjy’s picture

FileSize
174.37 KB

Missed the first part of #14, lets see the bot likes this.

Status: Needs review » Needs work

The last submitted patch, 17: 2363647-17.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
FileSize
782 bytes

Interdiff for #17

benjy’s picture

FileSize
2.24 KB

Re-roll. Interdiff in #19 is still accurate.

Status: Needs review » Needs work

The last submitted patch, 20: 2363647-19.patch, failed testing.

chx’s picture

Status: Needs work » Reviewed & tested by the community

OK that's enough of attempting this. #16 is RTBC. It's not the role of this issue to fix book.module.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed #16 a27d222 and pushed to 8.0.x. Thanks!

  • alexpott committed a27d222 on 8.0.x
    Issue #2363647 by benjy: Cannot programatically update books
    

Status: Fixed » Closed (fixed)

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