Move I18nQueryTrait from content_translation to migrate_drupal

Created on 14 January 2022, almost 3 years ago
Updated 3 June 2024, 6 months ago

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 and the source plugins to migrate_drupal. This is on MR 7616

I18nQueryTrait is used in the context of migrations and the migrate module already contains Drupal\migrate\Plugin\migrate\source\DummyQueryTrait. Hence it seems reasonable to move Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait to Drupal\migrate\Plugin\migrate\source\I18nQueryTrait in order to avoid unnecessary dependencies.

User interface changes

None.

API changes

None.

Data model changes

None.

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
MigrationΒ  β†’

Last updated 1 day ago

Created by

πŸ‡¨πŸ‡­Switzerland boromino

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡ΏNew Zealand quietone

    I think this will be fixed by πŸ“Œ Move migration code from content_translation to migrate_drupal Postponed

  • Merge request !7616move to migrate_drupal β†’ (Open) created by quietone
  • Status changed to Needs review 7 months ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    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.

  • Status changed to Needs work 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    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 Needs work , because instantiating reflection on classes that use I18nQueryTrait when content_translation is uninstalled cause a fatal error.

  • Pipeline finished with Failed
    7 months ago
    #163170
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    @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.

  • Issue was unassigned.
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    I've updated the title and unassigned.

  • πŸ‡³πŸ‡ΏNew Zealand quietone

    Thank you.

  • Pipeline finished with Success
    7 months ago
    Total: 587s
    #163175
  • Status changed to Needs review 7 months ago
  • 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.

  • Status changed to Needs work 7 months ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    @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.

  • Pipeline finished with Running
    7 months ago
    #163345
  • Pipeline finished with Failed
    7 months ago
    Total: 194s
    #163354
  • Pipeline finished with Success
    7 months ago
    Total: 644s
    #163361
  • Status changed to Needs review 7 months ago
  • πŸ‡«πŸ‡·France andypost

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

  • Status changed to Needs work 6 months ago
  • πŸ‡ΊπŸ‡ΈUnited States mikelutz Michigan, USA

    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.

Production build 0.71.5 2024