Problem/Motivation

Report using 8.8.0-beta1.

The d6_action and d7_action migrations cause fatal errors preventing an automated migration from working. Since action usually runs first this causes a confusing error and the best work around is to remove and not run the migration at all. This is the second, completely different site I encountered this on so I decided to write it up as an issue. It could probably be very frustrating.

This might be an issue with action derivatives and migrations.

If I don't want to install a module on Drupal 8 or Drupal 9 because it was not necessary, then I should not have to install that module to migrate my Drupal 6 or Drupal 7 web site. For instance if I no longer need "comment" module, then I should not get an error like this:

 [error]  The "entity:publish_action:comment" plugin does not exist. Valid plugin IDs for Drupal\Core\Action\ActionManager are: node_assign_owner_action, node_delete_action, node_unpromote_action, node_promote_action, node_publish_action, node_save_action, node_make_sticky_action, node_unpublish_by_keyword_action, node_unpublish_action, node_make_unsticky_action, user_add_role_action, user_block_user_action, user_cancel_user_action, user_remove_role_action, user_unblock_user_action, action_send_email_action, action_goto_action, action_message_action, entity:publish_action:block_content, entity:publish_action:node, entity:publish_action:path_alias, entity:publish_action:taxonomy_term, entity:publish_action:menu_link_content, entity:save_action:block_content, entity:save_action:file, entity:save_action:node, entity:save_action:profile, entity:save_action:taxonomy_term, entity:save_action:user, entity:save_action:menu_link_content, entity:unpublish_action:block_content, entity:unpublish_action:node, entity:unpublish_action:path_alias, entity:unpublish_action:profile, entity:unpublish_action:taxonomy_term, entity:unpublish_action:menu_link_content (core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php:53)

Proposed resolution

  1. Add a "module_exists" process plugin to check whether a module exists by name.
  2. Adjusts d6 and d7 action migration templates to skip row when the module doesn't exist.

Remaining tasks

- Review

User interface changes

N/A

API changes

Adds a ModuleExists process plugin for all to use and enjoy.

Data model changes

N/A

Release notes snippet

CommentFileSizeAuthor
#14 interdiff_9_14.txt3.24 KBspokje
#14 drupal-3095427-missing-action-14.patch6 KBspokje
#9 interdiff-3095427-8-9.txt2.23 KBmradcliffe
#9 interdiff-3095427-4-9.txt3.42 KBmradcliffe
#9 drupal-3095427-missing-action-9.patch6.19 KBmradcliffe
#8 drupal-3095427-missing-action-8.patch6.19 KBmradcliffe
#8 interdiff-3095427-4-8.txt3.83 KBmradcliffe
#4 interdiff-3095427-test-only-2-4.txt3.16 KBmradcliffe
#4 drupal-3095427-missing-action-4.patch7.72 KBmradcliffe
#4 drupal-3095427-missing-action-test-only-4.patch4.28 KBmradcliffe
#2 drupal-3095427-missing-action-test-only-2.patch3.98 KBmradcliffe

Comments

mradcliffe created an issue. See original summary.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new drupal-3095427-missing-action-test-only-2.patch3.98 KB

Here's a test-only patch that blows up for me. Taking the existing MigrateActionsTest and removing the comment module as an installed module will cause the migration to fatal. It should not fatal. It should mark as skipped.

I'm not sure what the appropriate fix here is.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs review » Needs work 
+++ b/core/modules/action/tests/src/Kernel/Migrate/d6/MigrateActionsPartialTest.php
@@ -0,0 +1,58 @@
+namespace Drupal\Tests\action\Kernel\Migrate\d7;
...
+use Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase;
...
+class MigrateActionsPartialTest extends MigrateDrupal6TestBase {

Woops, copy paste fail here, but the D7 test should prove the issue.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new drupal-3095427-missing-action-test-only-4.patch4.28 KB
new drupal-3095427-missing-action-4.patch7.72 KB
new interdiff-3095427-test-only-2-4.txt3.16 KB

Here's a fixed test that should fail and a fix. I added a new process plugin called ModuleExists to check things like this situation where module may not exist in the destination (by intent) where it does in the source. It's pretty much EntityExists with module handler instead of entity storage.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Issue summary: View changes 
  1. +++ b/core/modules/migrate/src/Plugin/migrate/process/ModuleExists.php
@@ -0,0 +1,81 @@
+ * @MigrateProcessPlugin(
+ *   id = "module_exists"
+ * )

    Core plugins should be marked as @internal based on the backwards-compatibility policy. That would save work for someone down the line. 

  2. +++ b/core/modules/migrate/src/Plugin/migrate/process/ModuleExists.php
@@ -0,0 +1,81 @@
+    if (is_array($value)) {
+      $value = reset($value);
+    }

    I got this from EntityExists. I guess it might be possible that this is or is not necessary, but maybe safe to keep the same logic?

The last submitted patch, 4: drupal-3095427-missing-action-test-only-4.patch, failed testing. View results

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs review » Needs work

Yep, tests fail with test patch. Tests pass with full patch. :-)

line 24	Content missing for @var tag in member variable comment

Also need to fix this coding standard issue.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new interdiff-3095427-4-8.txt3.83 KB
new drupal-3095427-missing-action-8.patch6.19 KB

I added @internal to all the new classes because automated tests and plugins are @internal and so that it doesn't create more work for #2912642: Add @internal to automated tests classes.

I removed the protected variable that I added thinking I needed it, but forgot to remove in the patch in comment #4.

I removed the assertEntity methods because these are unused after I changed the test to only assert that the migration has skipped a row. That method could be considered for a test trait in a follow-up :-)

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new drupal-3095427-missing-action-9.patch6.19 KB
new interdiff-3095427-4-9.txt3.42 KB
new interdiff-3095427-8-9.txt2.23 KB

Hmm, I think I screwed something up by deleting the patch and re-uploading. Downloading from the links gets me the correct patch, but the one listed in the comment is wrong. So I'm uploading it again as #9.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

quietone’s picture

quietone commented
Issue tags: +migrate-d6-d8, +migrate-d7-d8
quietone’s picture

quietone commented
Component: action.module » migration system

Meant to change component too

spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new drupal-3095427-missing-action-14.patch6 KB
new interdiff_9_14.txt3.24 KB

- Re-rolled for 9.2.x-dev.
- Fixed deprecation warnings.
- Fixed 2 coding standards messages.

quietone’s picture

quietone commented
Status: Needs review » Needs work

If I understand this, to save an action entity the module providing the 'type' (from the source table) must be enabled on the destination. That is tricky from a migrate perspective because neither the source nor the process plugin should be peeking at the destination nor should a process plugin be gathering data from the source or destination site.

So, how to improve this? Let's have the source plugin peek at the destination and get the data needed in a well commented section in prepareRow(). Where prepareRow uses the module_handler that is injected into the source plugin. That will give the process plugins all the data needed to process the row. And it reduces this patch by removing a process plugin.

Then the first process in the pipeline can be a skip which tests if the destination_entity_type_exists. 

skip:
  - plugin: skip_on_empty
    source: destination_entity_type_exists

However, that raises a question for me. Will it be true that the 'type' in the source data corresponds to the entity name and module name on the destination site? I don't know the answer to that. If it is true, then checking for the module existence is sufficient.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

As I just ran into this issue: #3266996: Drupal\Component\Plugin\Exception\PluginNotFoundException: The "user_welcome_message_action" plugin does not exist. I thought about if it wouldn't be a good idea to fall back more gracefully if an action doesn't exist (anymore) than a WSOD. Just as side-note for core maintainers to decide. I understand that this would have further implications (relying on that action for example).

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Status: Needs work » Postponed

The Migrate Drupal Module was approved for removal in #3371229: [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3522602: [meta] Tasks to remove Migrate Drupal module and the removal work in #3522602: [meta] Tasks to remove Migrate Drupal module.

Migrate Drupal will not be moved to a contributed project. It will be removed from core after the Drupal 12.x branch is open.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

quietone commented
Status: Postponed » Closed (won't fix)

The Migrate Drupal Module and Migrate Drupal UI are deprecated and they are not in Drupal 12.0.0.

Issues for these modules should now be on the 11.x branch. And the changes are limited to critical and major bug fixes. Other changes are allowed at the discretion of the core Release Managers in consultation with the Migrate subsystem maintainers.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.