Problem/Motivation

Per #2533886: [meta] Move module-specific migration support into the particular modules supported, move support for user migration into the user module.

Proposed resolution

For the most part, this is a matter of moving files around and updating namespaces. Also, explicit destination module dependencies can be removed.

Migration templates to be moved:

  • d6_profile_values
  • d6_user
  • d6_user_contact_settings
  • d6_user_mail
  • d6_user_picture_entity_display
  • d6_user_picture_entity_form_display
  • d6_user_picture_field
  • d6_user_picture_field_instance
  • d6_user_picture_file
  • d6_user_profile_entity_display
  • d6_user_profile_entity_form_display
  • d6_user_profile_field
  • d6_user_profile_field_instance
  • d6_user_role
  • d6_user_settings

Remaining tasks

Submit a patch.

User interface changes

N/A

API changes

Namespaces of affected migration plugins will change.

Data model changes

N/A

Comments

mikeryan’s picture

Status: Active » Postponed
Related issues: +#2534158: MigrateFullDrupalTestBase must use dynamic test discovery
StatusFileSize
new43.81 KB

A little trickier than the others thanks to the password service, but the patch is here. Postponed on #2534158: MigrateFullDrupalTestBase must use dynamic test discovery.

phenaproxima’s picture

Status: Postponed » Needs review

Unblocked.

Status: Needs review » Needs work

The last submitted patch, 1: move_module_specific-2534042-1.patch, failed testing.

mikeryan’s picture

Status: Needs work » Postponed
Related issues: +#2540594: MigratePassword service applied globally
mikeryan’s picture

Status: Postponed » Needs review
StatusFileSize
new44.04 KB
new361 bytes

OK, the MigratePassword service wasn't as broken as I had thought at first, it just needed to implement getCountLog2() - I do think it's rather hacky and it's worth looking for a cleaner method, but it shouldn't hold up this patch.

phenaproxima’s picture

Priority: Normal » Major
Issue tags: +blocker
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

WOW. That patch is like half of Migrate freakin' Drupal! Major kudos to @mikeryan for taking it on.

44.04 KB of very slow scrolling later, I think this looks pretty good. Git 'er done!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Needs a reroll.

mikeryan’s picture

Status: Needs work » Needs review
StatusFileSize
new44.1 KB
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

It continues to please the eye and tickle the brain. (If by "tickle", you mean "smash with a sledgehammer".)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Needs reroll.

mikeryan’s picture

Status: Needs work » Needs review
StatusFileSize
new44.12 KB
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

@mikeryan says this was just a re-roll, and no functionality changed. If it passes the tests, I'm satisfied.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

@mikeryan and @phenaproxima walked me through this patch.

Even though this patch is quite large, the vast majority of this is just moving stuff around, so that's pretty straight-forward. A couple of weird things stuck out:

-migrate.destination.entity:user:
-  type: migrate_destination
-  label: 'User'
-  mapping:
-    md5_passwords:
-      type: boolean
-      label: 'Passwords'
-
 migrate.destination.entity:file:
   type: migrate_destination
   label: 'Picture'

Question: Why are we not moving the user picture at the same time we're moving the rest of the User stuff? Answer: Because that entry is mis-labeled. It has nothing to do with user pictures, but is rather about file entities, so will be addressed in #2534012: Move module-specific migration support into the file module.

+  /**
+   * {@inheritdoc}
+   */
+  public function getCountLog2($setting) {
+    return $this->originalPassword->getCountLog2($setting);
+  }
+

Uh... what the heck is this? :) Unlike other parts of this patch, this one has no corresponding - lines. Also, @inheritdoc is a misnomer; there's nothing in the parent class that defines this method; it actually happens in PhpassHashedPassword.

I asked for an updated comment here, and mikeryan posted:

/**
 * Implements the getCountLog2() method expected by tests assuming the PhpassHashedPassword service.
 * @todo: Revisit this whole alternate password service: https://www.drupal.org/node/2540594.
 */

I shortened this to:

  /**
   * Implements the PhpassHashedPassword::getCountLog2() method.
   *
   * @todo: Revisit this whole alternate password service:
   *   https://www.drupal.org/node/2540594.
   */

...and fixed on commit.

Committed and pushed to 8.0.x. Onwards with the great migration migration! ;)

  • webchick committed 246ec16 on 8.0.x
    Issue #2534042 by mikeryan, phenaproxima: Move module-specific migration...

Status: Fixed » Needs work

The last submitted patch, 12: move_module_specific-2534042-12.patch, failed testing.

webchick’s picture

Status: Needs work » Fixed

Yes, yes.

Status: Fixed » Closed (fixed)

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