Problem/Motivation

The Drupal 8 Forum module expect that the entity reference field used to link the forum nodes to the forum vocabulary is named 'taxonomy_forums'. But when we migrate Drupal 6 Forums to Drupal 8, this field is named whatever the Drupal 6 vocabulary name is. We then end up with two entity reference fields on forum nodes, with the right value being saved in the wrong field.

Proposed resolution

When we migrate vocabulary fields, check the Drupal 6 variable 'forum_nav_vocabulary' (where the forum vocabulary id is saved), and if it's the vocabulary used for forums, rename the field 'taxonomy_forums'.

Remaining tasks

Write a patch, Write tests, Review

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#63 2669030-63.patch17.04 KBmaxocub
#63 2669030-63.patch17.04 KBmaxocub
#58 2669030-53-for-8-5-x.patch17.98 KBmaxocub
#58 2669030-53-for-8-4-x.patch17.98 KBmaxocub
#58 2669030-53-for-8-3-x.patch17.98 KBmaxocub
#53 interdiff-49-53.txt3.13 KBmaxocub
#53 2669030-53.patch17.98 KBmaxocub
#49 2669030-49.patch16.06 KBmaxocub
#48 interdiff-46-48.txt694 bytesmaxocub
#48 2669030-48.patch16.06 KBmaxocub
#46 2669030-46.patch16.06 KBmaxocub
#44 interdiff-32-44.txt7.56 KBmaxocub
#44 2669030-44.patch15.57 KBmaxocub
#35 forum-upgrade.jpg24.69 KBsbogner@insightcp.com
#32 interdiff-29-32.txt2.39 KBmaxocub
#32 2669030-32.patch13.72 KBmaxocub
#32 2669030-32-test-only.patch8.61 KBmaxocub
#29 interdiff-27-29.txt8.23 KBmaxocub
#29 2669030-29.patch13.71 KBmaxocub
#29 2669030-29-test-only.patch8.61 KBmaxocub
#27 interdiff-26-27.txt2 KBmaxocub
#27 2669030-27.patch6.28 KBmaxocub
#26 2669030-26.patch5.04 KBmaxocub
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chegor created an issue. See original summary.

quietone’s picture

Issue tags: +migrate-d6-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.

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.

andrewmacpherson’s picture

I also ran into this problem when testing a migration, so it's an issue as of D8.2.1

We have forum module enabled, and two migrations to comment entity destinations.

  • One of these is for D6 comments attached to the forum node type, migrating to the comment_forum comment bundle provided by forum module.
  • The other comment migration is for D6 comments attached to a different node type, and has a different D8 comment bundle destination.

Interestingly, the error occurs during the migration of the NON-forum comments.

Line 261 is in hook_ENTITY_TYPE_update() for comment entities. It seems that $comment->getCommentedEntity() is returning null.

forum.module implements several hook_ENTITY_TYPE_foo() hooks for both Node and Comment entity types. The ones specific to Node entities include a check against the node type using \Drupal::service('forum_manager')->checkNodeType($node), however the entity hook implementations for Comment entities do not include a check for the target node type. I wonder if that would be worth doing.

My migrations use the d6_comment source, and have entity_id: nid processing, just like the migration templates from comment module.

I didn't solve the problem - it only happened on --update migrations, not on a fresh migration into a site with no content.

joel_guesclin’s picture

I am beginning to test migrations and have just tried a migration from D6 to D6 with a forum (just a single forum). On the D6 site, the forum was actually managed by the Advanced Forum module but I think that only handles displays by providing some extra Views (when I switch off the module on D6, the forum still displays as expected). At all events, here is what has happened.
As far as I can see, the actual content (the forum posts and their comments) has been correctly migrated. However:
1) If I go to forum/1056 (being the original number for the taxonomy term) the forum title is displayed, but no content.
2) On the other hand, if I display taxonomy/term/1056 then the forum content is properly listed as it would with a taxonomy term.
3) If I check the list of forums, there is indeed a forum called "General discussion" with an id of 1056
4) Whenever I access a forum post from the taxonomy listing I get a php error report with a location like this: http://d822.dd:8083/forum/1056/icc/3937/rules-forum and a referrer like this: http://d822.dd:8083/taxonomy/term/1056
and a VERY long error message of which I will post the beginning:
Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->getFromStaticCache() (line 139 of /Users/joelguesclin/Sites/d822/core/lib/Drupal/Core/Entity/EntityStorageBase.php) #0 /Users/joelguesclin/Sites/d822/core/includes/bootstrap.inc(548): _drupal_error_handler_real(2, 'array_flip(): C...', '/Users/joelgues...', 139, Array) #1 [internal function]: _drupal_error_handler(2, 'array_flip(): C...', '/Users/joelgues...', 139, Array) #2 /Users/joelguesclin/Sites/d822/core/lib/Drupal/Core/Entity/EntityStorageBase.php(139): array_flip(Array) #3 /Users/joelguesclin/Sites/d822/core/lib/Drupal/Core/Entity/EntityStorageBase.php(231): Drupal\Core\Entity\EntityStorageBase->getFromStaticCache(Array) #4 /Users/joelguesclin/Sites/d822/core/lib/Drupal/Core/Entity/EntityStorageBase.php(212): Drupal\Core\Entity\EntityStorageBase->loadMultiple(Array)

joel_guesclin’s picture

Version: 8.2.x-dev » 8.2.3
Priority: Normal » Critical

I changed this to "critical" because with 8.2.3 the migration of the forum just doesn't work. Here is what I did:
1) Created new Drupal 8.2.3 site
2) Enabled Forum and Book modules, plus the experimental migration modules
3) Kicked off the migration - content appears to have migrated successfully (at least I can see Book content and Forum content in the Contents list)
4) Navigate to forum/1056 (the id for the forum on the source site). It displays the headline ("General discussion forum") but nothing else.
5) Navigate to taxonomy/term/1056 - the content is listed.

I also notice that there are two Vocabularies both called "Forums", one of them empty and one containing the taxonomy term described above.

I notice sometimes ( but not systematically) when I look at a forum post, I get an enormous error message that starts with this:
Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->getFromStaticCache() (line 139 of /Users/joelguesclin/Sites/d823/core/lib/Drupal/Core/Entity/EntityStorageBase.php) #0 /Users/joelguesclin/Sites/d823/core/includes/bootstrap.inc(548): _drupal_error_handler_real(2, 'array_flip(): C...', '/Users/joelgues...', 139, Array) #1 [internal function]: _drupal_error_handler(2, 'array_flip(): C...', '/Users/joelgues...', 139, Array) #2 /Users/joelguesclin/Sites/d823/core/lib/Drupal/Core/Entity/EntityStorageBase.php(139): array_flip(Array) #3 /Users/joelguesclin/Sites/d823/core/lib/Drupal/Core/Entity/EntityStorageBase.php(231): Drupal\Core\Entity\EntityStorageBase->getFromStaticCache(Array) #4 /Users/joelguesclin/Sites/d823/core/lib/Drupal/Core/Entity/EntityStorageBase.php(212): Drupal\Core\Entity\EntityStorageBase->loadMultiple(Array) #5 /Users/joelguesclin/Sites/d823/core/modules/taxonomy/src/TermStorage.php(146): Drupal\Core\Entity\EntityStorageBase->load(NULL) #6 /Users/joelguesclin/Sites/d823/core/modules/forum/src/ForumManager.php(461): Drupal\taxonomy\TermStorage->loadAllParents(NULL) #7 /Users/joelguesclin/Sites/d823/core/modules/forum/src/Breadcrumb/ForumNodeBreadcrumbBuilder.php(29): Drupal\forum\ForumManager->getParents(NULL) #8 /Users/joelguesclin/Sites/d823/core/lib/Drupal/Core/Breadcrumb/BreadcrumbManager.php(83): Drupal\forum\Breadcrumb\ForumNodeBreadcrumbBuilder->build(Object(Drupal\Core\Routing\CurrentRouteMatch)) #9 /Users/joelguesclin/Sites/d823/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php(72): Drupal\Core\Breadcrumb\BreadcrumbManager->build(Object(Drupal\Core\Routing\CurrentRouteMatch)) #10 /Users/joelguesclin/Sites/d823/core/modules/block/src/BlockViewBuilder.php(203): Drupal\system\Plugin\Block\SystemBreadcrumbBlock->build() #11 [internal function]: Drupal\block\BlockViewBuilder::preRender(Array) #12 /Users/joelguesclin/Sites/d823/core/lib/Drupal/Core/Render/Renderer.php(376): call_user_func('Drupal\\block\\Bl...', Array) #13

cilefen’s picture

Version: 8.2.3 » 8.2.x-dev
Priority: Critical » Major

One broken aspect of an experimental module is not critical. See https://www.drupal.org/node/45111

mikeryan’s picture

Assigned: Unassigned » mikeryan

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.

swentel’s picture

hitting this one too .. will dig a bit as it's blocking my migration

Rodeo.be’s picture

have this issue as well :/

joel_guesclin’s picture

Should I go ahead and test again using 8.3.x-dev? Or has the bug not been addressed yet?

Drupalized’s picture

Same here. Any workaround for this?

NiklasBr’s picture

Encountered this error on a D6 site I tried to migrate. Have not found any workaround unfortunately.

robotjox’s picture

Me too - am I understanding this correctly: there is no way to migrate a D6 forum to D8 for now?

NiklasBr’s picture

I'm tempted to mark this as Migrate critical, anyone?

NiklasBr’s picture

Issue tags: +Migrate critical
mikeryan’s picture

Issue tags: +Baltimore2017

Tagging in case I don't get to it before DrupalCon...

maxocub’s picture

I'm testing this issue at DrupalCon Baltimore.
Here's what I have so far:

  • I haven't been able to reproduce the error from the issue summary.
  • The forums (taxonomy terms) are migrated.
  • The forum topics (nodes) are migrated, but don't seem to be link to the forums (taxonomy terms).
  • The comments seems to be migrated but are not attached to the forum topics (nodes).
  • When I look at a forum topic (node) I have this error (like in #6 and #7):
       Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->loadMultiple() (line 227 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).
       Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->getFromStaticCache() (line 139 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).
    
  • When I try to look at a comment, I get a white page and this error in the logs:
    PHP Fatal error: Call to a member function getSetting() on null in core/modules/comment/src/Controller/CommentController.php on line 125
maxocub’s picture

I see two things that seems to be wrong after a migration:

  1. The forum node type has two entity reference fields: taxonomy_forums & forums. The forum topic is migrated to the forums and it's the taxonomy_forums that seems to be used and is empty, hence the forum node is not linked to the forum topic taxonomy term.
  2. The comment field on the forum node type is named comment_forum but the forum module seems to be looking for a field named comments.
mikeryan’s picture

mikeryan’s picture

Assigned: mikeryan » Unassigned

@maxocub, are you actively looking at this?

maxocub’s picture

Assigned: Unassigned » maxocub

@mikeryan: Yes, I am.

sbogner@insightcp.com’s picture

@maxocub - I need this for a site conversion; I have a large forum with several containers so if you want help testing a patch let me know, I'm glad to help test it on my site.

maxocub’s picture

Assigned: maxocub » Unassigned
FileSize
5.04 KB

@sbogner: Thanks for the offer, it will be nice to test with an existing forum.

For now, the only thing I add time to do is to add some forum data in the fixtures and to build the base for the Kernel tests. I'll upload this WIP and keep working on a solution but I'll unassigned myself in case someone want to pick it up.

maxocub’s picture

Status: Active » Needs review
FileSize
6.28 KB
2 KB

Just adding a dedicated 'forums' vocabulary and a 'General discussion' forum to make it easier to test.
Putting it on NR just to see how many tests does that breaks.

Status: Needs review » Needs work

The last submitted patch, 27: 2669030-27.patch, failed testing.

maxocub’s picture

Here's the first part of the patch, that is the fix for the taxonomy.
There is still the comments to be fix.

The last submitted patch, 29: 2669030-29-test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 29: 2669030-29.patch, failed testing.

maxocub’s picture

Moving the 'forum_vocabulary' process plugin to the taxonomy module to fix the failing tests.

The last submitted patch, 32: 2669030-32-test-only.patch, failed testing.

maxocub’s picture

Status: Needs review » Needs work

Back to NW for the forum comments.

sbogner@insightcp.com’s picture

FileSize
24.69 KB

@maxocub - I applied patch 32 and ran the upgrade on my actual forum data, but still didn't seem to assign the correct taxonomy (see screen snap); comments came across fine.

maxocub’s picture

@sbogner: Thanks for testing the patch.
That is so weird, how come the comments could be OK since the patch don't do anything about them yet?
I also tested the patch manually and it worked for the taxonomy and not for the comments...

mikeryan’s picture

We have another issue for the comment type discrepancy at #2853872: Migration for forum and article comments: duplicate comment types and incorrect comment_entity_statistics - here, let's focus on the vocabulary discrepancy.

mikeryan’s picture

Assigned: Unassigned » mikeryan
Status: Needs work » Needs review

This is complete as far as the taxonomy goes, correct? So, unless there's a known gap here, I can review it this week...

The last submitted patch, 26: 2669030-26.patch, failed testing.

maxocub’s picture

I think it's complete for the taxonomy, at least with my manual testing and with the short test included in the patch.
But @sbogner reported in #35 that it didn't worked for him.

If we do the comment part in the other issue, the issue summary needs an update.

sbogner@insightcp.com’s picture

I started with a fresh install, applied 2669030-32.patch and still the same issue I reported in #35. Comments work, forum doesn't. It's really odd. I'm at Migrate version 8.3.2 if that matters.

maxocub’s picture

@sbogner: I'm trying to reproduce what you are reporting but the patch from #32 does not apply to the 8.3.2 version. How are you applying the patch? It should work with the 8.3.x branch.

phenaproxima’s picture

  1. +++ b/core/modules/forum/tests/src/Kernel/Migrate/d6/MigrateForumTest.php
    @@ -0,0 +1,56 @@
    +    $this->assertEquals([0 => ['target_id' => '7']], $node->get('taxonomy_forums')->getValue());
    

    This should be simplified -- $this->assertEquals(7, $node->taxonomy_forums->target_id)

  2. +++ b/core/modules/taxonomy/src/Plugin/migrate/process/ForumVocabulary.php
    @@ -0,0 +1,26 @@
    +class ForumVocabulary extends ProcessPluginBase {
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
    +    if ($row->getSourceProperty('forum_vocabulary')) {
    +      $value = 'taxonomy_forums';
    +    }
    +    return $value;
    +  }
    +
    +}
    

    Why do we need a process plugin for this? I think we could easily accomplish this with static_map.

maxocub’s picture

Here's adressing #43. I kept the process plugin because static_map won't work since we would need to get the value of a variable dynamically. I documented it instead, as discussed with @phenaproxima.

Status: Needs review » Needs work

The last submitted patch, 44: 2669030-44.patch, failed testing. View results

maxocub’s picture

Status: Needs work » Needs review
FileSize
16.06 KB

My last patch was against 8.4.x, so here's a reroll for 8.3.x.
I also added a link to this issue in the comments.

Status: Needs review » Needs work

The last submitted patch, 46: 2669030-46.patch, failed testing. View results

maxocub’s picture

Status: Needs work » Needs review
FileSize
16.06 KB
694 bytes

Just an entity count failure...

maxocub’s picture

There were new commits on 8.3.x, so this needed a re-roll, again.

maxocub’s picture

Title: Can't migrate forum from drupal 6 to drupal 8 » D6 Forum vocabulary is migrated to the wrong D8 field name
Assigned: mikeryan » Unassigned
Issue summary: View changes
Issue tags: -Needs issue summary update
Related issues: +#2887142: NodeType source plugin should include comment information, +#2853872: Migration for forum and article comments: duplicate comment types and incorrect comment_entity_statistics

As per #37, we decided to focus only on the taxonomy problem here, and deal with the comments problem in an other issue.
Updated the Issue title & summary accordingly.
Added the two related issues where the comments problem is being discussed.

quietone’s picture

Assigned: Unassigned » quietone

Assigning for review

quietone’s picture

Assigned: quietone » Unassigned
Status: Needs review » Needs work

This looks good, but I am not sure about the changes to the fixture and the tests.

I don't know much about d6 forum, so I made a d6 site with the test fixture from the patch in #49. I am surprised that it does not have any forums, but it does have forum content. The forum content can't be edited and saved because there are no forums. I think that needs to be changed so the test fixture is functional from the UI. And similarly, after migrating the fixture, the existing forum content can't be edited on the d8 site.

And the proposed resolution states that this patch will rename a field to taxonomy _forums. But I don't see a test for that.

maxocub’s picture

I had to do a re-roll in #46 because the fixtures were changed in another commit, I must have messed things there, now the forum in the fixtures should be usable.

I also added some tests for the four migrations that are affected by this patch, which are:

  • d6_vocabulary_entity_display.
  • d6_vocabulary_entity_form_display
  • d6_vocabulary_field
  • d6_vocabulary_field_instance
quietone’s picture

Status: Needs review » Reviewed & tested by the community

@maxocub, thanks it looks even better now. All my concerns addressed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 53: 2669030-53.patch, failed testing. View results

maxocub’s picture

Status: Needs work » Reviewed & tested by the community

CI error, batck to RTBC.

catch’s picture

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

That's not just an error, I did a retest against 8.5.x and the patch doesn't apply there, so needs a re-roll.

maxocub’s picture

Re-roll for 8.4.x and 8.5.x

maxocub’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

The changes in the re-rolls were just in context lines, so I think this can be back to RTBC.

catch’s picture

+++ b/core/modules/taxonomy/src/Plugin/migrate/source/d6/Vocabulary.php
@@ -69,6 +69,15 @@ public function prepareRow(Row $row) {
+    // See https://www.drupal.org/node/2669030 for more information.

We don't need an @see back to the issue, just git blame does enough. However it would be good to explain why the logic lives in taxonomy module and not forum module - I assume because there's no forum migration other than the variable?

maxocub’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll
maxocub’s picture

Hmm, with #2854878: Taxonomy vocabulary with name "Type" cannot migrate from d6 to d8 in, this issue won't work anymore.
The forum module expects it's taxonomy field name to be 'taxonomy_forums', but with the other issue, the field name will be prefixed with 'field_'.

maxocub’s picture

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

Here's a reroll. Sorry, I wasn't able to create an interdiff because the previous patch and this one don't apply to the same branches. I also tried with the program interdiff, but didn't worked. If someone know how, it would be appreciated.
I mostly just removed the @see and modified the process pipelines.

quietone’s picture

Status: Needs review » Reviewed & tested by the community

The @see has been removed from the comments as requested in #60.

  • catch committed 82284fa on 8.5.x
    Issue #2669030 by maxocub, sbogner: D6 Forum vocabulary is migrated to...

  • catch committed 31b67bb on 8.4.x
    Issue #2669030 by maxocub, sbogner: D6 Forum vocabulary is migrated to...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

catch’s picture

Status: Fixed » Closed (fixed)

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

dillix’s picture

Does this patch worked for D7 to D8 migrations?

maxocub’s picture

maxocub’s picture

maxocub’s picture

@dillix: The D7 to D8 migration doesn't have this problem since the D7 forum field for taxonomy terms is already named correctly, that is taxonomy_forums.