When migrating a site from D7 to D8, my custom block showed no content. I thought it was just a problem with displaying the content, but when I tried to edit the block content there was no body field.

It turns out the body field is just set to disabled on both "manage display" and "manage form display."

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

rocketeerbkw created an issue. See original summary.

rocketeerbkw’s picture

I added a new migration template to fix (I think) the form display issue.

rocketeerbkw’s picture

This patch adds a migration template to fix the manage display issue.

Status: Needs review » Needs work

The last submitted patch, 3: 2724903-3-migrate_block_body_disabled.patch, failed testing.

rocketeerbkw’s picture

Status: Needs work » Needs review
FileSize
2.81 KB

Fixing the tests.

Status: Needs review » Needs work

The last submitted patch, 5: 2724903-5-migrate_block_body_disabled.patch, failed testing.

rocketeerbkw’s picture

Status: Needs work » Needs review
FileSize
2.8 KB

Had a typo in the last patch.

mikeryan’s picture

Issue tags: +Needs tests
rocketeerbkw’s picture

I changed the "manage display" migration to hide the label by default and added tests for "manage display" and "manage form display."

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

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.

mikeryan’s picture

Version: 8.3.x-dev » 8.2.x-dev
Status: Needs review » Reviewed & tested by the community

Looks good!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2724903-9-migrate_block_body_disabled.patch, failed testing.

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.

mikeryan’s picture

Jo Fitzgerald’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.05 KB
6.32 KB

Updated the migration templates.

phenaproxima’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityDisplayTest.php
    @@ -0,0 +1,51 @@
    +/**
    + * Tests migration of block content body field display configuration.
    + *
    + * @group comment
    + */
    +class MigrateBlockContentEntityDisplayTest extends MigrateDrupal7TestBase {
    

    This test is in the wrong group.

  2. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityDisplayTest.php
    @@ -0,0 +1,51 @@
    +  public static $modules = ['block', 'block_content', 'filter', 'text'];
    

    Should have @inheritdoc comment.

  3. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityDisplayTest.php
    @@ -0,0 +1,51 @@
    +    $this->assertTrue(is_array($component));
    

    Let's use $this->assertInternalType('array', $component).

  4. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityDisplayTest.php
    @@ -0,0 +1,51 @@
    +    $this->assertIdentical('hidden', $component['label']);
    

    Should be assertSame().

  5. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityFormDisplayTest.php
    @@ -0,0 +1,51 @@
    +/**
    + * Tests migration of block content body field form display configuration.
    + *
    + * @group comment
    + */
    +class MigrateBlockContentEntityFormDisplayTest extends MigrateDrupal7TestBase {
    

    Wrong group, again.

  6. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityFormDisplayTest.php
    @@ -0,0 +1,51 @@
    +  public static $modules = ['block', 'block_content', 'filter', 'text'];
    

    Needs @inheritdoc.

  7. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityFormDisplayTest.php
    @@ -0,0 +1,51 @@
    +    $this->assertTrue(is_array($component));
    

    As before, should use assertInternalType().

  8. +++ b/core/modules/block_content/tests/src/Kernel/Migrate/MigrateBlockContentEntityFormDisplayTest.php
    @@ -0,0 +1,51 @@
    +    $this->assertIdentical('text_textarea_with_summary', $component['type']);
    

    Should be assertSame().

gaurav.kapoor’s picture

Assigned: Unassigned » gaurav.kapoor
gaurav.kapoor’s picture

Assigned: gaurav.kapoor » Unassigned
Status: Needs work » Needs review
FileSize
6.65 KB
9.66 KB

Status: Needs review » Needs work

The last submitted patch, 18: 2724903-18.patch, failed testing.

gaurav.kapoor’s picture

Status: Needs work » Needs review
FileSize
6.4 KB
2.26 KB
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

I'm trying to find objectionable things in this patch but I can't. Well done, all!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 20: 2724903-20.patch, failed testing.

gaurav.kapoor’s picture

Status: Needs work » Needs review

Looks like a random failure to me , should be moved back to RTBC.

quietone’s picture

Status: Needs review » Reviewed & tested by the community

@gaurav.kapoor, I agree. Back to RTBC.

alexpott’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

So these things are shipped with standard - what happens if this migration runs on standard? Do we get duplicates? Setting back to needs review to get an answer.

phenaproxima’s picture

Assigned: Unassigned » phenaproxima

Assigning to myself to discover the answer to @alexpott's question.

heddn’s picture

Issue tags: +Needs manual testing, +Novice

This could also be a fairly easy novice. Apply the patch and manually try to migrate a D7 site into D8 and see if duplicate block fields appear.

phenaproxima’s picture

Assigned: phenaproxima » Unassigned
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing, -Novice

@alexpott: The PerComponentEntityDisplay and PerComponentEntityFormDisplay destination plugins call entity_get_display() and entity_get_form_display(), respectively, so they should load the displays that ship with standard. Their shared base class then calls setComponent() on the display, so if the body field was already there, it will be overwritten with the incoming data rather than duplicated.

I'm not sure how else we could test this, beyond what we're already doing. So I think this can go back to RTBC.

alexpott’s picture

I would have expected a change to \Drupal\migrate_drupal_ui\Tests\d6\MigrateUpgrade6Test::getEntityCounts() and the d7 version too. I guess that is not happening because the form and view displays are being created anyway by the creation of the block type. This is one of the reasons why these secondary configuration writes are tricky to manage and should be on the UI level and not the API. Makes it much easier to spot things like this - not the fault of the migration system though. What I can't work out atm is what code is actually responsible for creating the form and view displays magically.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 20: 2724903-20.patch, failed testing.

phenaproxima’s picture

Status: Needs work » Reviewed & tested by the community

There have been no updates. Drupal CI is just mocking us.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 631e8d8 to 8.4.x and b9cdeaa to 8.3.x. Thanks!

As migrate is still experimental and this is an addition for something that is missing backporting to 8.3.x

  • alexpott committed 631e8d8 on 8.4.x
    Issue #2724903 by rocketeerbkw, gaurav.kapoor, Jo Fitzgerald,...

  • alexpott committed b9cdeaa on 8.3.x
    Issue #2724903 by rocketeerbkw, gaurav.kapoor, Jo Fitzgerald,...

Status: Fixed » Closed (fixed)

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