D6->D8 Profile field (list selection)

I'm hoping to start helping out with Migrate stuff. Where would I find the file to play with to get allowed values to come across in a d6->d8 migration for a list profile field for user migration? I'm trying to get to know where things are to help with this issue. Also I setup PhpStorm to debug even when running the migration from drush, but I'm not finding a good place to put a breakpoint, any suggestions so I can get the hang of how migrate is setup. thanks!

James,

Finding the right file is half the battle. Have you reviewed the Migrate API docs yet (https://drupal.org/node/2127611)? These are invaluable.

I'm thinking that you'll first have to determine if the allowed values are set in the base field or the field instance (I _think_ they are set in the base). All migrations are configured in the .yml files found in drupal8/core/modules/migrate_drupal/config/install/

Depending on the migration, various source, process, and destination plugins are called.

Let me know if this is enough to get you going. Feel free to ping me on IRC.

Thanks,
-mike

I did read the docs and they were helpful, I'm still getting lost in the maze. I'll ping you on IRC. Thanks!

The problem was similar to #2281627: Freeform text profile field does not migrate properly from D6 to D7 in that allowed values wasn't been turned into an array and it also just wasn't mapped. I've added it to the yaml since it always exists on the D6 side that should be fine.

I've used the regex from #2281627: Freeform text profile field does not migrate properly from D6 to D7 in prepareRow() again but I'm not really sure that is the best approach. It feels like something we should move into a process plugin?

Benjy and I worked on this one a bit and we think there's more work to do here. A quick summary:

1. The "Allowed values" are migrated properly and assigned proper integers for their keys.
2. For individual profiles, selected values are displayed on the profile view page.
3. For individual profiles, selected values are not displayed on the profile edit page.
4. While debugging we discovered that the ProfileField->prepareRow() function was being run 4 times for each profile field - not sure why this is happening.
5. The ProfileField->prepareRow() function was missing a return - updated patch attached.

Thanks,
-mike

I made a tiny bit of process on item 3 from comment 8 above:

For individual profiles, selected values are not displayed on the profile edit page.

The reason selected values are not displayed on the profile edit page for selection widgets is that because in D6, profile selection widgets don't use key|label - just the label. When the field is migrated, each of the allowed values is provided with a key (starting from 0, 1, 2, etc...), but when the field value is migrated, it is just the value - no key.

I think the solution is to somehow figure out what key was assigned to the incoming value and set it appropriately.

I've updated the patch from comment 8 with part of the code we'll need, but I'm going to need some guidance on how to figure out the key.

I didn't look into the fact that ProfileField->prepareRow() is being run multiple times...

Thanks,
-mike

Ok - made some good progress on this one.

Benjy - I saw that you and chx were chatting about the fact that ProfileField->prepareRow() was running multiple times, so I'm leaving that to you (you're welcome).

I figured out how to grab the proper list item key and use that instead of the label - this fixes the problem of the value not displaying properly on the profile edit page. I'm not 100% sure I went about it the proper way, so I'd love some feedback. Here's the code I used:

// Grab the key and use that, not that label.
$field_config = \Drupal::entityManager()->getStorage('field_config')->load('user.' . $profile_value['name']);
$profile_value_key = array_search($profile_value['value'], $field_config->settings['allowed_values']);
$row->setSourceProperty($profile_value['name'], array($profile_value_key));

I also found that the test we were using for this was missing the "allowed_values" settings, so it was ignoring keys and just comparing labels. I updated the MigrateProfileValuesTest to hopefully fix that as well.

Updated patch and interdiff attached.

Thanks,
-mike

Turns out that the fix for #2293159: D6->D8 Profile field on/off checkbox error involves much of the same code as this issue, so I'm going to suggest that we combine them. I'm attaching a patch with the 3 additional lines of code that fix #2293159: D6->D8 Profile field on/off checkbox error. When the patch for this issue gets in, both issues should be fixed.

Thanks,
-mike

Fixed the unit tests and tested this patch manually. The profile selection fields are migrated as expected.

Just going to test the checkbox issues now we're combining the issues and then i'll take a look at the issue with prepareRow() running four times.

OK, checkboxes are migrating fine as well.

The reason we go into ProfileField::prepareRow() four times is because our manifest file has these four migrations which all use d6_profile_field as their source.

d6_user_profile_entity_display
d6_user_profile_entity_form_display
d6_user_profile_field
d6_user_profile_field_instance

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/ProfileFieldValues.php
@@ -55,6 +55,12 @@ public function prepareRow(Row $row) {
+        $profile_value_key = array_search($profile_value['value'], $field_config->settings['allowed_values']);

D6 allows the same value twice in the list and this won't work for that. I don't think we need to handle that case though since it makes little sense.

+1 for RTBC from me.

chx reviewed this (via IRC) and suggested that we inject this:

$field_config = \Drupal::entityManager()->getStorage('field_config')->load('user.' . $profile_value['name']);

The attached patch makes this change.

Thanks,
-mike

OK, I spoke with chx about this and we discussed that the source querying D8 for the field keys wasn't right.

Changes:

1. We now use the value as the key in the ProfileField source
2. I've changed the regex to only match newlines.
3. The query is now distinct so we have no duplicate entries in ProfileField.

OK, another change to query for the profile field values from the profile_values table since chx pointed out that values in profile_fields can actually change after a user has selected a value on their user page. So, I had to include the Drupal6User dump which had the profile_values table into the four migrations which used the ProfileField source.

This is getting there, thanks!. The distinct() in ProfileField::query() isnt needed; that was a misunderstanding.

Two, the migrate source unit tests use the in memory db driver which doesnt support query just select. It would be good to convert the new query into a select and test it.

Removed the un-needed distinct and added a comment.

Will do the unit test stuff later if someone doesn't beat me to it.

Fixed the tests.

1. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/ProfileField.php
   @@ -46,6 +47,24 @@ public function query() {
   +      $options = $this->getDatabase()->query('SELECT DISTINCT value FROM {profile_values} WHERE fid = :fid', array(':fid' => $row->getSourceProperty('fid')))->fetchCol();
   +      $row->setSourceProperty('options', array_combine($options, $options));
   

   Perhaps I missing something here, but is this going to get all of them? What if the D6 field has 20 "allowed options", but only 12 of them have ever been selected. Won't 8 be left behind and not migrated?

   I understand that this is getting all of the profile values that have ever been selected (even if the allowed values have changed), but I'm worried that we're leaving behind allowed values that have never been selected. I'm pretty sure I verified this with a quick manual test.

2. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/ProfileFieldValues.php
   @@ -7,9 +7,12 @@
   +use Drupal\migrate\Entity\MigrationInterface;
   ...
   +use Symfony\Component\DependencyInjection\ContainerInterface;
   
   @@ -23,6 +26,11 @@
   +   * @var \Drupal\field\FieldConfigStorage
   +   */
   +  protected $fieldConfigStorage;
   +
   +  /**
   

   We don't need this stuff anymore (since we're not injecting), correct?

-mike

Both of your points are valid, we are no longer injecting (sorry I have not caught this properly before) and also options need to be added. If we are using options, we need to agree on delimiters. I had reservation about the comma delimiter and https://api.drupal.org/api/drupal/modules%21profile%21profile.module/fun... $lines = preg_split("/[\n\r]/", $field->options); there is no comma anywhere. Care to explain where did that come from?

The phantom comma originally came from the fact that D6 "freeform list" profile fields allow for the use of commas (in addition to \r or \n) to separate list items. Somewhere along the way, the comma delimiter (incorrectly) made its way over to this "list selection" profile field migration as well. I think it is safe to say that we don't need to include a comma as a delimiter here.

-mike

Agreed.

Updated patch attached that accounts for the current D6 "allowed options" as well as any that may have been removed. This patch also removes some left-over dependency injection stuff that is no longer needed.

The only oddness that remains for this is that after migration, the D8 "allowed values" may contain previously-deleted D6 items (if any users still have their profile field set to deleted allowed values). This will be an unexpected result as far as a site admin is concerned. I guess the question is now which is more undesirable?

1. Throw out user profile data (list items that no longer exist).
2. Have previously removed allowed value items reappear in D8 post-migration.

-mike

Ah you don't need to care about duplicate options due to + $options = array_merge($current_options, $options); because array_combine($options, $options) will make short work of any duplicates. Nifty. Perhaps a comment to note about this?

A question, $row->setSourceProperty('options', array(NULL, NULL)); wtf is NULL, NULL?

@chx - comments added for the array_merge and array(NULL, NULL) stuff.

Should we be thinking about putting some/all of the prepareRow() stuff in the new ProfileFieldSettings that is part of #2260241: D6->D8 Profile field (date)?

Thanks,
-mike

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/ProfileField.php
@@ -54,11 +54,14 @@ public function prepareRow(Row $row) {
+      // D6 profile checkboxes values are always 0 or 1 (with no labels), so we need to create two

more than 80 chars ;)

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/ProfileField.php
@@ -54,11 +54,14 @@ public function prepareRow(Row $row) {
+      // array_merge() takes care of any duplicates options.

array_combine() is what takes care of the duplicates?

@benjy,

I just set my text editor to show me the 80-column mark for comments ('bout time, I know).

I fixed the "duplicates" comment as well. To explain, since we're getting options from both the current "allowed values" as well as values set in user profiles (that may have been since removed from "allowed values", there could be some overlap. The array_combine() function will remove any duplicates.

Patch and interdiff attached.

Thanks,
-mike

OK, I gave this a good manual test with a list field that had lots of values, users that selected values both in the list and items that had since been removed from the list. Everything came across to D8 and the users had the correct items selected.

I think we should probably document the fact that your "allowed values" in D8 will be a combination of all selected user values and the current allowed values in D6 and not just the allowed values in D6.

I also tested a user checkbox field manually and everything worked as expected.

I noticed a missing full-stop but hardly worth re-rolling the patch for unless something else comes up.

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/ProfileField.php
@@ -46,6 +47,31 @@ public function query() {
+      // Get the current options

Added to https://www.drupal.org/node/2167633 .

I had a few minutes to spare so I re-rolled the patch with the full stop.

Thanks,
-mike

Committed 753f225 and pushed to 8.x. Thanks!