Move content_translation I18nQueryTrait to migrate module

I think this will be fixed by 📌 Move migration code from content_translation to migrate_drupal Postponed

Added a way to move this to migrate drupal.

This problem is about the source plugins, not migrations so I think we can just move the affected source plugins. I have not done the needed deprecations. I wanted to make sure there were no test failures.

Nice no test failures. Moving to NW for deprecations mentioned in #17

godotislate → changed the visibility of the branch 3258581-move-contenttranslation-i18nquerytrait to hidden.

This is a blocker for 📌 Convert MigrateSource plugin discovery to attributes Active , because instantiating reflection on classes that use I18nQueryTrait when content_translation is uninstalled cause a fatal error.

@godotislate, I see you are working on this issue. It is currently assigned to me and I have been working on it locally for a while. :-(

@quietone I completely missed the assigneed. Many apologies.

I've updated the title and unassigned.

Thank you.

I have added deprecations to the moved classes and traits. Do deprecations still need tests? Or is phpstan covering that now?

And again, I apologize to @quietone for missing that you already were working on this and I didn't notice.

@godotislate, no worries. It is just unfortunate that we both were doing the same thing. I think you got there first because I took a break to eat.

Updated per MR feedback.

Maybe it will be less disruptive to move file and provide alias via composer autoloader?

I'm not certain that we can do anything here, but we definitely can't do this. All of these sources being moved are scheduled to be deprecated and removed completely by d12 along with the migrate_drupal module, so we can't set a deprecation message telling people to use the migrate_drupal versions before d12 because those aren't going to exist anymore. I think at best we could introduce a copy of the trait to migrate_drupal and switch the sources over to it without doing any deprecation notices about moving things, and I'm not sure we should bother doing that at this point either.

Problem/Motivation

Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait is also used in multiple migrate source plugins of other modules (e.g. Drupal\block_content\Plugin\migrate\source\d7\BlockCustomTranslation). This may make sense as translations usually require to have content_translation enabled. However it prevents e.g. the type checking of such plugins without instantiating them, as done in https://www.drupal.org/project/entity_import → , Drupal\entity_import\EntityImportSourceManager::getDefinitions().

Steps to reproduce

• Install Drupal standard installation profile in English, make sure content_translation is not enabled
• Install migrate_drupal module
• Install https://www.drupal.org/project/entity_import →
• Login in and go to /admin/config/system/entity-importer/add (Content > Importers > entity importer creation page)

Instead of displaying a form to add an entity importer, the following fatal error occurs:

Trait 'Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait' not found in /var/www/html/web/core/modules/block_content/src/Plugin/migrate/source/d7/BlockCustomTranslation.php on line 22

Proposed resolution

I18nQueryTrait is used for source plugins that require migrate_drupal. So, move it to migrate_drupal. This is on MR 10426

User interface changes

None.

API changes

None.

Data model changes

None.

godotislate → changed the visibility of the branch 3258581-to-migrate-drupal to hidden.

I think you missed core/modules/menu_link_content/src/Plugin/migrate/source/d7/MenuLinkTranslation.php, so NW for that.

Can we really make the existing trait a wrapper for the new trait in the migrate_drupal module and not add any deprecation notice? Maybe we can, but it feels too easy. I guess the idea is that when we deprecate the new trait, that will automatically deprecate the existing one (the wrapper), and we have to do that before the next major version (D12).

The existing steps to reproduce (STR) involve a contrib module. That has two problems:

1. It is not clear from the STR that this is a bug in Drupal, rather than the contrib module.
2. Manual testing is complicated, since the fix is on a branch based on 11.x and the contrib module does not have a release compatible with Drupal 11.

I know, I can add the issue fork for the D11 compatibility issue as a package repository and install the contrib module that way, or I could use the lenient composer plugin. I said it is complicated, not impossible. ;)

We can solve both problems if we give different STR. Using drush php,

> \Drupal::service('plugin.manager.migration')->createInstance('d7_custom_block_translation')->getSourcePlugin();
PHP Fatal error:  Trait "Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait" not found in /var/www/html/core/modules/block_content/src/Plugin/migrate/source/d7/BlockCustomTranslation.php on line 22

Fatal error: Trait "Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait" not found in /var/www/html/core/modules/block_content/src/Plugin/migrate/source/d7/BlockCustomTranslation.php on line 22

Before running that code, I installed Drupal with the standard profile, enabled migrate_drupal and a custom module with a simplified version of d7_custom_block_translation:

id: d7_custom_block_translation
label: Content block translations
source:
  plugin: d7_block_custom_translation
process: {}
destination:
  plugin: null

The original version of d7_custom_block_translation is in the content_translation module.

Can we turn these STR into an automated test? I think so. I am adding the tag for tests.

We should update the issue summary with the improved STR. I can do that, but not now. I need some sleep. For now, another tag.

When I check out the feature branch from the MR, it fixes my manual test:

> \Drupal::service('plugin.manager.migration')->createInstance('d7_custom_block_translation')->getSourcePlugin();
= Drupal\block_content\Plugin\migrate\source\d7\BlockCustomTranslation {#7934}

It would be nice to get some manual testing of the original report: does the current MR fix the problem with the contrib entity_import module? Or does that require moving a lot more into the migrate_drupal module?

I think you missed core/modules/menu_link_content/src/Plugin/migrate/source/d7/MenuLinkTranslation.php, so NW for that.

Thanks for catching that. Rebased the MR and added the change.

Can we really make the existing trait a wrapper for the new trait in the migrate_drupal module and not add any deprecation notice?

Not sure what the appropriate deprecation message, given #29 🐛 Move content_translation I18nQueryTrait to migrate module Active and whatever is going to happen to migrate_drupal, but I think I agree with the idea that the deprecation message should be set when migrate drupal moves out of core.

Can we turn these STR into an automated test? I think so. I am adding the tag for tests.

Unfortunately, this is more difficult than you might think, because all classes are autoloaded when tests run. So the trait would never be missing, unless it just doesn't exist at all.

I did run the steps to reproduce with the Entity Import module. I had to

• Use the lenient composer plugin
• Edit entity_import.info.yml
• Add an inherited method return type.

I was able to install entity_import against 11.x and this MR's branch. Confirmed that the fatal error was reproduced on 11.x and gone after the trait was moved.

Moving back to Needs Review. Leaving at needs tests in case there is another idea of how to test this proposed.

I added a Functional test that shows the error locally. But when I run the test-only changes it doesn't fail as expected. Gotta run.

As it needs test, and IS needs updating and the added test doesn't fail on test only right now this is still needs work.

To figure this out I would like to see the browser output from the test. However, the artifacts for the test-only changes test run does not include the browser output. Is this by design? Is there something that can be changed in the template so the browser output is saved?

To get the browser output I made an MR with just the test changes. And looking at all the pages of output hasn't given me an idea as to why the fail test passes in gitlab but fails locally.

@quitone I check test case you create at your MR, and left my feedback there.
Short summary between difference running I18nQueryTraitTest On Gitlab CI and localhost:

• on 8 step Fatal error:is not presented (link)
• there is no 9 step presented at artifact

Found solution for checking fatal error at Drupal\FunctionalTests\Bootstrap\UncaughtExceptionTest.
Update quietone's test only MR 5316, and rebased and cherry picked changes to MR 10426.

Test is ready. Set status to NR.
Tests only MR 5316.
MR to review MR 10426