Problem/Motivation

We need an upgrade path from D7 for the core Book module.

Proposed Resolution

This is blocked by #2382985: Migration Files for Drupal 7 Menu & Menu Entries and #2423103: Migration Files for Drupal 7 Content.

Once those two land, this migration path needs two parts:

  1. Book's settings need to be moved into configuration (all set as of #6)
  2. Book hierarchies must be migrated -- i.e., the {book} table needs to be handled

Remaining Tasks

  1. Update the patch to match HEAD
  2. Migrate the {book} table
  3. Write tests
  4. Review
  5. Commit!
CommentFileSizeAuthor
#75 2409435-75.patch16.51 KBedysmp
#75 interdiff-74-75.txt777 bytesedysmp
#64 2409435-64.patch16.56 KBJo Fitzgerald
#64 interdiff-61-64.txt3.91 KBJo Fitzgerald
#61 2409435-61.patch12.33 KBJo Fitzgerald
#58 2409435-58.patch13.29 KBJo Fitzgerald
#58 interdiff-56-58.txt774 bytesJo Fitzgerald
#56 2409435-56.patch11.58 KBJo Fitzgerald
#54 2409435-54.patch11.26 KBJo Fitzgerald
#54 interdiff-43-54.txt1.41 KBJo Fitzgerald
#43 2409435-43.patch11.5 KBJo Fitzgerald
#43 interdiff-39-43.txt752 bytesJo Fitzgerald
#40 2409435-40.patch10.86 KBsrjosh
#39 2409435-39.patch10.77 KBsrjosh
#37 2409435-37.patch10.77 KBJo Fitzgerald
#37 interdiff-35-37.txt461 bytesJo Fitzgerald
#35 2409435-35.patch10.75 KBJo Fitzgerald
#35 interdiff-33-35.txt1.96 KBJo Fitzgerald
#33 2409435-33.patch10.8 KBJo Fitzgerald
#33 interdiff-31-33.txt7.58 KBJo Fitzgerald
#31 2409435-31.patch8.33 KBJo Fitzgerald
#27 2409435-27.patch8.4 KBJo Fitzgerald
#27 interdiff-25-27.txt1.39 KBJo Fitzgerald
#25 2409435-25.patch7.94 KBJo Fitzgerald
#15 book_7_upgrade_reroll-2409435.patch5.08 KBKarenS
#11 upgrade_path_for_book_7_x-2409435-11.patch14.75 KBsvendecabooter
#6 upgrade_path_for_book_7_x-2409435-6.patch2.45 KBbdimaggio
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 97,250 pass(es). View
#1 book_settings-2409435-1.patch732 byteshosef
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

hosef’s picture

Status: Active » Needs work
Issue tags: +Needs tests
FileSize
732 bytes

Attached is the YAML for this issue, from the patch in #2382117: Migration Files for Drupal 7 Variables.
Test(s) (and maybe a dump file) still need to be written.

miguelc303’s picture

Added organization support to Anexus IT

jcost’s picture

Project: IMP » Drupal core
Version: » 8.0.x-dev
Component: Code » migration system

Will need to be submitted again to Core since moving from sandbox.

phenaproxima’s picture

Title: Variable to config: book.settings [d7] » Upgrade path for Book 7.x
Priority: Normal » Minor
Issue summary: View changes
Parent issue: #2181257: [meta] Variables to config migration [d7] » #2456259: [META] Drupal 7 to Drupal 8 Migration path

Changed the issue summary.

bdimaggio’s picture

Assigned: Unassigned » bdimaggio

I'll get a test in for this within the next few days.

bdimaggio’s picture

Status: Needs work » Needs review
FileSize
2.45 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 97,250 pass(es). View

Added a test and made a tweak to migrate.migrations.d7_books_settings.yml to account for migration_groups becoming migration_tags.

phenaproxima’s picture

Issue summary: View changes

Postponing on #2382985: Migration Files for Drupal 7 Menu & Menu Entries and #2423103: Migration Files for Drupal 7 Content.

#6 is a great start, but I should have done my research before posting this issue -- because we don't only need to migrate book settings, but book hierarchies (i.e., the {book} table). Books depend on nodes and menu links being migrated first, so those need to be in before we can proceed on this.

phenaproxima’s picture

Status: Needs review » Postponed

Erm -- yeah, status change. :)

mgifford’s picture

Status: Postponed » Needs work

Ok, that's in.

svendecabooter’s picture

Assigned: bdimaggio » svendecabooter
svendecabooter’s picture

Attached patch is my work in progress.
Still todo:
* continue updating D7 MigrateBookTest (blocked by missing helper functions in MigrateDrupal7TestBase currently)
* check if the config migration code is still correct

svendecabooter’s picture

Status: Needs work » Postponed
KarenS’s picture

Status: Postponed » Needs work

I would argue this is at least medium priority, book migrations are meaningless without the hierarchy, and book is pretty widely used (for instance on D.O.). I also don't think this is postponed on anything now.

KarenS’s picture

Status: Needs work » Postponed

OK I guess it is still postponed on https://www.drupal.org/node/2578263.

KarenS’s picture

Priority: Minor » Normal
FileSize
5.08 KB

I couldn't get the previous patch to apply so I did a reroll and it worked great to migrate the book heirarchy for a D7 site with a fairly complex book structure. I don't have a D6 site to test so hopefully someone else can confirm that part. Then we just need the tests finalized.

EDIT: I finally got the original patch to apply, so the reroll may not be needed.

KarenS’s picture

Issue summary: View changes
mgifford’s picture

quietone’s picture

Issue tags: +migrate-d7-d8

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

svendecabooter’s picture

Assigned: svendecabooter » Unassigned

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

DamienMcKenna’s picture

Status: Postponed » Active

Now that #2382985 and #2423103 have landed, this can be worked on again.

almaudoh’s picture

Issue tags: +Needs reroll

I guess the patches here won't apply anymore.

Jo Fitzgerald’s picture

Status: Active » Needs review
Issue tags: -Needs reroll
FileSize
7.94 KB

Re-rolled.

D7 tests are a work in progress.

Jo Fitzgerald’s picture

Status: Needs review » Needs work

Setting back to NW for D7 tests (and probably other things too!)

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
1.39 KB
8.4 KB

Completed the tests for d7/MigrateBookConfigsTest, but I'm tying myself in knots with d7/MigrateBookTest so I'll leave that for someone else.

Jo Fitzgerald’s picture

Status: Needs review » Needs work

Setting back to NW for someone (else!) to work on d7/MigrateBookTest.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rakesh.gectcr’s picture

Assigned: Unassigned » rakesh.gectcr
Jo Fitzgerald’s picture

Assigned: rakesh.gectcr » Jo Fitzgerald
Status: Needs work » Needs review
FileSize
8.33 KB

Patch in #27 no longer applies. Re-rolled prior to taking another run at d7/MigrateBookTest.

Status: Needs review » Needs work

The last submitted patch, 31: 2409435-31.patch, failed testing. View results

Jo Fitzgerald’s picture

Version: 8.4.x-dev » 8.5.x-dev
Assigned: Jo Fitzgerald » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
7.58 KB
10.8 KB

Wrote d7/MigrateBookTest.

Status: Needs review » Needs work

The last submitted patch, 33: 2409435-33.patch, failed testing. View results

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
1.96 KB
10.75 KB

Moved new migration templates to the correct directory.

Status: Needs review » Needs work

The last submitted patch, 35: 2409435-35.patch, failed testing. View results

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
461 bytes
10.77 KB

Added source_module to d7_book_settings.

Status: Needs review » Needs work

The last submitted patch, 37: 2409435-37.patch, failed testing. View results

srjosh’s picture

Status: Needs work » Needs review
FileSize
10.77 KB
srjosh’s picture

So, that patch applies to 8.5, but doesn't work for 8.4. Here's one for 8.4.

The last submitted patch, 39: 2409435-39.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 40: 2409435-40.patch, failed testing. View results

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
752 bytes
11.5 KB

Fix the test failures.

The last submitted patch, 37: 2409435-37.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 43: 2409435-43.patch, failed testing. View results

Jo Fitzgerald’s picture

Status: Needs work » Needs review

Retest has passed, returning to NR.

The last submitted patch, 11: upgrade_path_for_book_7_x-2409435-11.patch, failed testing. View results

The last submitted patch, 15: book_7_upgrade_reroll-2409435.patch, failed testing. View results

The last submitted patch, 1: book_settings-2409435-1.patch, failed testing. View results

srjosh’s picture

Having trouble actually using this patch. Following the migration template, I get hundreds of these errors:

Missing bundle for entity type node (/var/www/build/html/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:73)                                          
Missing bundle for entity type node (/var/www/build/html/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:73)                                          
Missing bundle for entity type node (/var/www/build/html/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:73)                                          

That's running it stand-alone, after the migration for the book content type has been run.

Or, if I substitute 'book' as the source/destination plugin for the book content type, adding in the fields from the book migration template, that doesn't work either. I get these errors:
Missing bundle for entity type node (/var/www/build/html/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:73)

How is this supposed to be used?

anpolimus’s picture

Issue tags: +CSKyiv18

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

phenaproxima’s picture

Status: Needs review » Needs work

I see very little to object to here. Once these minor fixes are made, let's get this in.

  1. +++ b/core/modules/book/migrations/d7_book.yml
    @@ -0,0 +1,24 @@
    +    -
    +      plugin: migration
    +      migration: d7_book
    

    plugin should be migration_lookup.

  2. +++ b/core/modules/book/tests/src/Kernel/Migrate/d7/MigrateBookConfigsTest.php
    @@ -0,0 +1,41 @@
    +/**
    + * Upgrade variables to book.settings.yml.
    

    Should be "Tests the migration of Book settings."

  3. +++ b/core/modules/book/tests/src/Kernel/Migrate/d7/MigrateBookTest.php
    @@ -0,0 +1,70 @@
    +/**
    + * Upgrade book structure.
    

    Should be "Tests migration of book structures from Drupal 7."

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
1.41 KB
11.26 KB

Made changes recommended by @phenaproxima in #53.

Status: Needs review » Needs work

The last submitted patch, 54: 2409435-54.patch, failed testing. View results

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
11.58 KB

Gah! I managed to not move a couple of files.

Status: Needs review » Needs work

The last submitted patch, 56: 2409435-56.patch, failed testing. View results

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
774 bytes
13.29 KB

Correct test failure.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

We're done here. Thanks, @Jo Fitzgerald et. al!

larowlan’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7Test.php
@@ -154,7 +155,6 @@ protected function getAvailablePaths() {
diff --git a/patches/interdiff-56-58.txt b/patches/interdiff-56-58.txt

diff --git a/patches/interdiff-56-58.txt b/patches/interdiff-56-58.txt
new file mode 100644

new file mode 100644
index 0000000..0416e00

index 0000000..0416e00
--- /dev/null

--- /dev/null
+++ b/patches/interdiff-56-58.txt

+++ b/patches/interdiff-56-58.txt
+++ b/patches/interdiff-56-58.txt
@@ -0,0 +1,20 @@

@@ -0,0 +1,20 @@
+diff --git a/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7ReviewPageTest.php b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7ReviewPageTest.php
+index b8ea52b..be048f9 100644
+--- a/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7ReviewPageTest.php
++++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MigrateUpgrade7ReviewPageTest.php
+@@ -34,6 +34,7 @@ protected function getAvailablePaths() {
+     return [
+       'aggregator',
+       'block',
++      'book',
+       'comment',
+       'contact',
+       'date',
+@@ -115,7 +116,6 @@ protected function getAvailablePaths() {
+    */
+   protected function getMissingPaths() {
+     return [
+-      'book',
+       'color',
+       'rdf',
+       'views',

some patch noise here?

Other than that, looks good

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
12.33 KB

Remove noise.

heddn’s picture

Status: Needs review » Reviewed & tested by the community

And noise is now silent.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/book/migrations/d7_book_settings.yml
similarity index 95%
rename from core/modules/book/src/Plugin/migrate/source/d6/Book.php

rename from core/modules/book/src/Plugin/migrate/source/d6/Book.php
rename to core/modules/book/src/Plugin/migrate/source/Book.php

Migrate API is in beta soon to be stable. I don't think we should be removing plugins because it will break existing migrations. Let's leave the old plugin in place but deprecate it and use the new plugin for both d6 and d7.

+++ b/core/modules/book/migrations/d6_book.yml
@@ -21,3 +21,4 @@ destination:
+    - d6_menu_links

Why this change to the D6 migration and how come no test changes because of it?

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
3.91 KB
16.56 KB
  • Deprecated d6/Book.
  • There appears to be no need for the addition of d6_menu_links as a required migration dependancy of d6_book so removed that (and the equivalent in d7_book).
alexpott’s picture

The menu links dependency was added in #11 without explanation. It'd be good to try and reason why.

heddn’s picture

Re #65: It was probably added because of the comment in #7. @phenaproxima can you recall what caused you to state that?

heddn’s picture

Status: Needs review » Needs work

Needs work to address the feedback in #65.

phenaproxima’s picture

It was 3+ years ago, so I can't say I remember my reasoning. I say, let's try removing the menu links dependency, and if tests fail we can add it back again and do a little more detective work.

Jo Fitzgerald’s picture

Status: Needs work » Needs review

The latest patch removed the menu_links dependency (and passed the tests) so are we happy for this to be RTBC?

heddn’s picture

Status: Needs review » Needs work

Nice work. We're migrating the book settings and book content. We have tests. Wonderful! This would be ready. Except, this needs a partial re-roll to account for https://www.drupal.org/node/2944527

heddn’s picture

Issue tags: +Novice

Adding those tags seem easy enough. Tagging novice. The d6/d7_book yml files are content stuff. While d7_book_settings.yml is configuration.

heddn’s picture

Title: Upgrade path for Book 7.x » Upgrade path for Book 6.x and 7.x
Issue tags: +migrate-d6-d8

Re-titling, since we seem to be doing both d6 and d7 in the same patch.

heddn’s picture

Does this need a change record or any release notes mention?

edysmp’s picture

Assigned: Unassigned » edysmp

Working on this.

edysmp’s picture

Assigned: edysmp » Unassigned
Status: Needs work » Needs review
FileSize
777 bytes
16.51 KB

rerolled and applied the migration tags.

Status: Needs review » Needs work

The last submitted patch, 75: 2409435-75.patch, failed testing. View results

heddn’s picture

Probably a namespace thing. Check to make sure it includes the correct name to match the file system path of the class. Plus, it looks like we missed d6_book.yml as well.