Create a migration config and class for enable end user to migrate Blocks from Drupal 7 to Drupal 8

Contributors

People who helped in #2532550: Leading slashes not added to visibility paths, which was merged in patch #14: quietone, tim.plunkett

People who helped in #2372785: Error running d6_block migration, which was addressed in patch #22: eliza411, quietone, bdone

Suggested commit message:

git commit -m 'Issue #2408165 by phenaproxima, miguelc303, quietone, eliza411, tim.plunkett, bdone: Migration Files for Drupal 7 Blocks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjy’s picture

Status: Active » Postponed

There aren't any D7 migrations in core right now, they're in the sandbox: https://www.drupal.org/sandbox/chx/2105305

We need to get the sandbox re-rolled against HEAD and bring everything into core before we start with individual D7 issues.

miguelc303’s picture

I did a patch for enable end user to migrate Blocks from Drupal 7 to Drupal 8

hosef’s picture

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

Moving to the IMP issue queue.

miguelc303’s picture

benjy’s picture

miguelc303’s picture

Added organization support to Anexus IT

phenaproxima’s picture

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

It's time to get going on this. First step is tests for the patch in #4.

quietone’s picture

Started work on this, moved everything to their module. But currently fails tests because the blocks aren't migrated.

quietone’s picture

Issue tags: +Needs tests
FileSize
13.78 KB

Just moved files to block module.

quietone’s picture

Remove the Related issue that is already a child issue.

phenaproxima’s picture

Priority: Normal » Major
Issue tags: +Migrate critical

Escalating, because a site without its blocks is like...well, it's not complete, that's for sure. People will assume migration is broken is blocks do not migrate.

phenaproxima’s picture

phenaproxima’s picture

WIP patch. This generalizes a bunch of things, but I'm not sure it will pass the tests yet.

phenaproxima’s picture

Issue summary: View changes
FileSize
44.33 KB

Merged in #2532550: Leading slashes not added to visibility paths.

Credit from that issue carries over to @quietone and @tim.plunkett; IS updated accordingly.

quietone’s picture

Didn't get the opportunity to do much work on this. But here are a few namespace corrections.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
48.36 KB

Now with tests! Delicious, succulent, gluten-free tests.

Status: Needs review » Needs work

The last submitted patch, 16: 2408165-16.patch, failed testing.

hass’s picture

Why have you rolled back the latest changes where you splitted pages and roles? Now they are again merged. Larger patches are more difficult to review...

phenaproxima’s picture

Short answer: because, for reasons far beyond my control, they can't be split apart. I spent hours trying.

hass’s picture

Can you shed some light on these reasons, please? I need the same code for migration of roles and urls in #2531648: Migrate 'pages' and 'roles' settings to D8 and need some help, too.

The last submitted patch, 16: 2408165-16.patch, failed testing.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
47.32 KB
2.91 KB

Fixed a fatal and removed the update hook. This should pass the tests now.

Can you shed some light on these reasons, please?

Yes. It has to do with the way DefaultLazyPluginCollection works.

If visibility/user_roles is defined in the migration, for example, the resulting visibility plugin collection will look something like this if the visibility plugin fails:

array(
  'user_role' => array(),
)

The plugin collection will choke on this, throwing exceptions like Plugin ID "user_role" not found.

Ideally, if the visibility plugin failed, the plugin collection should not even have the user_role key. Unfortunately, because it is defined in the migration, it will be there come hell or high water. There's no way for the process plugin to say "I failed, so please unset my destination key." The only way to prevent the key from being defined is for all visibility stuff to be handled in a single plugin.

benjy’s picture

Looks ready for me, couple of things:

+++ b/core/modules/block/src/Plugin/migrate/process/BlockPluginId.php
@@ -86,10 +86,10 @@
+            if ($block_id) {
+              return 'block_content:' . $this->blockContentStorage->load($block_id)->uuid();
             }

Do we have tests for this? There is a separate issue for this very bug floating around somewhere. EDIT: here it is, #2372785: Error running d6_block migration

  1. +++ b/core/modules/block/src/Plugin/migrate/process/BlockVisibility.php
    @@ -0,0 +1,142 @@
    +        // If the PHP module is present, migrate the visibility code unaltered.
    +        if ($this->moduleHandler->moduleExists('php')) {
    

    How nice of you, i've have almost been tempted to leave this out entirely to discourage its usage.

  2. +++ b/core/modules/block/src/Plugin/migrate/process/BlockVisibility.php
    @@ -0,0 +1,142 @@
    +          'negate' => !$old_visibility,
    

    I notice this is "2" above, i'm guessing the other values are 0 or 1?

  3. +++ b/core/modules/block/src/Plugin/migrate/source/Block.php
    @@ -67,36 +88,54 @@ public function fields() {
    -          $item_count = $this->database->query('SELECT block FROM {aggregator_feed} WHERE fid = :fid', array(':fid' => $id))->fetchField();
    +          $item_count = $this->select('aggregator_feed', 'af')
    +            ->fields('af', ['block'])
    +            ->condition('fid', $id)
    +            ->execute()
    +            ->fetchField();
    

    Nice!

  4. +++ b/core/modules/block/src/Plugin/migrate/source/Block.php
    @@ -113,10 +152,10 @@ public function prepareRow(Row $row) {
    +          case 2: case 'new':
    ...
    +          case 3: case 'online':
    

    Lets put these on two lines?

phenaproxima’s picture

Title: Migration Files for Drupal 7 Blocks » Migration Files for Drupal 7 Blocks
Issue summary: View changes

Pulling in credit from #2372785: Error running d6_block migration, which is superseded by this patch.

Status: Needs review » Needs work

The last submitted patch, 22: 2408165-22.patch, failed testing.

phenaproxima queued 22: 2408165-22.patch for re-testing.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
48.7 KB
3.54 KB

Fixed #23.4 and added a test of the custom block migration.

mikeryan’s picture

Status: Needs review » Reviewed & tested by the community

The last submitted patch, 15: 2408165-15.patch, failed testing.

The last submitted patch, 13: 2408165-13.patch, failed testing.

The last submitted patch, 14: 2408165-14.patch, failed testing.

The last submitted patch, 2: migration_files_for-2408165-2.patch, failed testing.

The last submitted patch, 4: migration_files_for-2408165-4.patch, failed testing.

The last submitted patch, 13: 2408165-13.patch, failed testing.

The last submitted patch, 14: 2408165-14.patch, failed testing.

The last submitted patch, 15: 2408165-15.patch, failed testing.

The last submitted patch, 2: migration_files_for-2408165-2.patch, failed testing.

The last submitted patch, 4: migration_files_for-2408165-4.patch, failed testing.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Test:

- Fairly basic D7 test database, just a vocab, a menu link, maybe a piece of content.
- Added two blocks:
1) "Who's new" to the right sidebar, limited to authenticated users
2) "Who's online" to "tryptich middle" (which doesn't exist in D8), shown on front page only.

Migration appears to complete fine (using Migrate UI) but upon redirecting back to the front page I see the dread "The website encountered an unexpected error. Please try again later." error on every page. :(

Checking logs shows:

[Wed Sep 16 10:43:47.377629 2015] [fcgid:warn] [pid 70426:tid 3020025856] [client 127.0.0.1:59678] mod_fcgid: stderr: Uncaught PHP Exception InvalidArgumentException: "You must provide the context IDs in the @{service_id}:{unqualified_context_id} format." at /Users/webchick/Sites/8.x/core/lib/Drupal/Core/Plugin/Context/LazyContextRepository.php line 76, referer: http://8x.dd/batch?id=2&op=start

YOU'RE a lazy context repository! :P

drush cr gets my /admin back briefly, but Bartik is hosed. Incidentally, when running that command, I got the following notices in console:

The block bartik_system_powered-by was assigned to the invalid region[warning]
footer and has been disabled.
The block bartik_user_online was assigned to the invalid region      [warning]
triptych_middle and has been disabled.
The block seven_user_new was assigned to the invalid region          [warning]
dashboard_sidebar and has been disabled.
The block seven_search_form was assigned to the invalid region       [warning]
dashboard_sidebar and has been disabled.
The block seven_comment_recent was assigned to the invalid region    [warning]
dashboard_inactive and has been disabled.
The block seven_user_online was assigned to the invalid region       [warning]
dashboard_inactive and has been disabled.

Same error after drush cr tho.

Adam and I discussed; most likely this is happening because of the access-controlled block, which uses context, and this also means we need expanded test coverage.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
49.2 KB
2.68 KB

Fixed! And tests are updated!

Status: Needs review » Needs work

The last submitted patch, 40: 2408165-40.patch, failed testing.

phenaproxima’s picture

The last submitted patch, 40: 2408165-40.patch, failed testing.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

YAY! Restoring RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome!!

I had applied that fix locally earlier and the migration completed successfully, apart from #2569703: When migrating to the same theme with different region names, block will not migrate properly but that bug affects D6 as well.

Looked through the patch, no red flags. Let's DO THIS.

Committed and pushed to 8.0.x. YEAH!!

  • webchick committed 86cc10b on 8.0.x
    Issue #2408165 by phenaproxima, miguelc303, quietone, eliza411, tim....

Status: Fixed » Closed (fixed)

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