Problem/Motivation

Contact module is being deprecated. Usages is tests not in Contact needs to removed and test coverage moved to the Contact module.

Steps to reproduce

Proposed resolution

Add migration test coverage from migrate_drupal_ui to contact
And adjust User module kernel tests.

  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php - Remove load of contact module and the 'contact-category' migration and adjust test. No need to move testing to the contact module as it is testing user roles, which is not the responsibility of the contact migrations.
  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserContactSettingsTest.php - Remove load of contact module, it is not needed.

No change needed on these

  • d6_user_role and d7_user_role - The contact_category migration from the contact module is an optional migration_dependency. Therefor, no change needed to those.
  • d6_user_contact_setting - No change. The process plugin checks if the user data has a contact index.
  • core/modules/user/tests/src/Kernel/Plugin/migrate/source/d7/UserTest.php - No change
  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php - Remove load of contact module and the 'contact-category' migration and adjust test. No need to move testing to the contact module as it is testing user roles, which is not the responsibility of the contact migrations.
  • core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php - No change

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3551595

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

quietone created an issue. See original summary.

quietone opened merge request !13462

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs work
andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3535775: Remove Contact module from the Standard profile

I see it has conflicts with #3535775: Remove Contact module from the Standard profile

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
Issue tags: +Vienna2025

Hope I fixed tests

quietone’s picture

quietone commented

@andypost, thanks for the fixes.

Previously, I moved the needed test files to the module being deprecated, so let's try that.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +11.3.0 release priority

it will help to split it to contib without dated drupal_migrate_ui

andypost’s picture

andypost
he/him
Primary language Russian
commented

rebased after previous removals

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Reviewed & tested by the community » Needs work

somehow it started to fail

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

hope fixed

quietone’s picture

quietone commented

@andypost, thanks for updating the tests.

I reviewed the changes to the tests and I agree that they are correct.

dcam’s picture

dcam commented
Status: Needs review » Needs work

I tried to be thorough about checking for other contact-related tests. There's probably a more efficient way to do it, but I used this grep command:
grep -ir --exclude-dir=contact --exclude-dir=migrate_drupal_ui --exclude-dir=phpstan-tmp --exclude-dir=fixtures --exclude-dir=lib --exclude=.phpstan-baseline.php "contact" ./. Here is what I found:

  • core/tests/Drupal/KernelTests/Core/Entity/ContentEntityNullStorageTest.php - This isn't specifically contact-related, but contact is the only thing tested by it. It probably doesn't need to be moved, but I wanted to mention it.
  • core/modules/config_translation/tests/src/Functional/ConfigTranslationUiModulesTest.php - It looks like testContactConfigEntityTranslation() needs to be copied.
  • core/modules/config_translation/tests/src/Functional/ConfigTranslationListUiTest.php - It looks like doContactFormsListTest() needs to be copied.
  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php and core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php - These test migrating permissions, including contact's. Does that need to be preserved?
  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserContactSettingsTest.php - Test migrating user contact settings from D6.
  • core/modules/user/tests/src/Kernel/Plugin/migrate/source/d7/UserTest.php - I may be mistaken, but I think this tests migrating user contact settings from D7.

Are these in-scope or is this issue limited to only migrate_drupal_ui tests?

quietone’s picture

quietone commented
Issue summary: View changes

@dcam, thanks for searching for usages of contact.

Answering your last question first, this can be expanded to handle the migrate kernel tests as well. That might make it easier to ensure the test fixture has everything needed. so, for now, let's investigate those tests. I can probably do that tomorrow.

core/tests/Drupal/KernelTests/Core/Entity/ContentEntityNullStorageTest.php - is mentioned at #3520460-12: [meta] Tasks to deprecate the Contact module. That still needs discussion on the meta and/or a child issue. I haven't looked at it myself.

core/modules/config_translation/tests/src/Functional/ConfigTranslationUiModulesTest.php and core/modules/config_translation/tests/src/Functional/ConfigTranslationListUiTest.php are part of #3551599: Remove use of Contact from config_translation tests.

quietone’s picture

quietone commented
Issue summary: View changes

For the user module,

  • d6_user_role and d7_user_role - The contact_category migration from the contact module is an optional migration_dependency. Therefor, no change needed to those.
  • d6_user_contact_setting - No change. The process plugin checks if the user data has a contact index.
  • core/modules/user/tests/src/Kernel/Plugin/migrate/source/d7/UserTest.php - No change
  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserRoleTest.php - Remove load of contact module and the 'contact-category' migration and adjust test. No need to move testing to the contact module as it is testing user roles, which is not the responsibility of the contact migrations.
  • core/modules/user/tests/src/Kernel/Migrate/d7/MigrateUserRoleTest.php - No change
  • core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserContactSettingsTest.php - Remove load of contact module, it is not needed.
quietone’s picture

quietone commented
Status: Needs work » Needs review
quietone’s picture

quietone commented
Issue summary: View changes

Rebased and changed the Kernel tests to use the new fixture in the contact module.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Trying to draw up memory from when we did this for book.
Copying of the fixtures into contact looks correct and inline with what was done then too.
All references of contact remaining in migrate seems to be addressed (minus the change here).

believe this one is good, not sure what else to review so will go on a limb.

Saved credit for everyone on this ticket (not myself).

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 586641adfee to 11.x and f0638f8f3dc to 11.3.x. Thanks!

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.

  • alexpott committed f0638f8f on 11.3.x 
    Issue #3551595 by quietone, andypost, dcam: Handle migration test...

  • alexpott committed 586641ad on 11.x 
    Issue #3551595 by quietone, andypost, dcam: Handle migration test...

andypost closed merge request !13462

Status: Fixed » Closed (fixed)

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