Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
- Enable the book module and then create a book page.
- In code, Node::load($nid)->save();
- 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.
Comment | File | Size | Author |
---|---|---|---|
#16 | 2363647-16.patch | 1.48 KB | benjy |
#12 | interdiff.txt | 578 bytes | benjy |
#12 | 2363647-12.patch | 1.41 KB | benjy |
#10 | interdiff.txt | 822 bytes | benjy |
#10 | 2363647-10.patch | 1.37 KB | benjy |
Comments
Comment #1
benjy CreditAttribution: benjy commentedTest to show the issue.
Comment #3
martin107 CreditAttribution: martin107 commentedI think there maybe a little confusion in testBookApi()
the call to createBookNode() with $parent, in effect, set to NULL means that
will be called...when createBookNode exits $book->save() is called on an already saved node ?
This makes no sense to me
Comment #4
benjy CreditAttribution: benjy commentedThe bug is that you cannot save a book that already exists?
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.
Comment #5
martin107 CreditAttribution: martin107 commentedAh good point
Comment #6
benjy CreditAttribution: benjy commentedComment #7
BerdirWhat 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?
Comment #8
benjy CreditAttribution: benjy commentedSure, that's fine by me.
Comment #9
yukare CreditAttribution: yukare commentedJust a note on this, may help to find a solution:
This works and any change on $node is save.
Comment #10
benjy CreditAttribution: benjy commentedI'm not sure if this is the right fix, but lets give it a try.
Comment #12
benjy CreditAttribution: benjy commentedI copied this logic from the form, it's not exactly pretty but it fixes the problem and tests should be green.
Comment #13
chx CreditAttribution: chx commentedbook 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.
Comment #14
alexpottI 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.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 thatTests book functionality through node interfaces.
Comment #15
alexpottFor #14
Comment #16
benjy CreditAttribution: benjy commentedDone.
Comment #17
benjy CreditAttribution: benjy commentedMissed the first part of #14, lets see the bot likes this.
Comment #19
benjy CreditAttribution: benjy commentedInterdiff for #17
Comment #20
benjy CreditAttribution: benjy commentedRe-roll. Interdiff in #19 is still accurate.
Comment #22
chx CreditAttribution: chx commentedOK that's enough of attempting this. #16 is RTBC. It's not the role of this issue to fix book.module.
Comment #23
alexpottThis 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!