Symbioquine's 2.x Migration Testing Log

[Symbioquine]

Adding a little debugging to FarmLog.php seems to corroborate what I found above;

diff --git a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
index e5572601..e6ab86d8 100644
--- a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
+++ b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
@@ -155,7 +155,13 @@ class FarmLog extends Log {
     // Add the quantity IDs to the row for future processing.
     $row->setSourceProperty('log_quantities', $quantity_ids);
 
-    return parent::prepareRow($row);
+    $preparedRow = parent::prepareRow($row);
+
+    $field_farm_asset = $this->getFieldvalues('log', 'field_farm_asset', $id);
+
+    $this->idMap->saveMessage(['id' => $id], "is_movement = $is_movement movement_areas = " . print_r($movement_areas, TRUE) . " field_farm_asset = " . print_r($field_farm_asset, TRUE), MigrationInterface::MESSAGE_WARNING);
+
+    return $preparedRow;
   }
 
 }

docker-compose.yml

          chown www-data:www-data /opt/drupal/web/profiles/farm/modules/core/migrate/config/optional/migrate_plus.migration.farm_migrate_area_equipment_placeholder.yml

+         ln -sf /opt/testing_patched/FarmLog.php /opt/drupal/web/profiles/farm/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
        fi

        wait_db_ready

...

    volumes:
+     - '../../farmOS/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php:/opt/testing_patched/FarmLog.php:ro'

Re-running everything shows that the FarmLog plugin is setting is_movement = 1 and finding both the area tid and asset target_id for my asset;

$ docker-compose stop db2 www2 && docker-compose rm -f db2 www2 && sudo rm -rf db2 www2
$ docker-compose up -d
$ docker-compose exec -T www2 drush --root=/opt/drupal migrate-messages farm_migrate_log_activity | grep -A 17 8201
  8201           8201                2       is_movement = 1 movement_areas =  
                                             Array                             
                                             (                                 
                                                 [0] => Array                  
                                                     (                         
                                                         [tid] => 858          
                                                     )                         
                                                                               
                                             )                                 
                                              field_farm_asset = Array         
                                             (                                 
                                                 [0] => Array                  
                                                     (                         
                                                         [target_id] => 3358   
                                                     )                         
                                                                               
                                             )                                 

Next I guess I need to dig a bit deeper into how the migration field setting/lookups actually work…

[mstenta]

One outstanding question is why the migration said 2460 seed assets, but both farmOS 1.x and 2.x seem to think there are 2459 seed assets…

Hmm… did you find any more clues about this? Curious…

Re-running everything shows that the FarmLog plugin is setting is_movement = 1 and finding both the area tid and asset target_id for my asset;

That’s interesting… it does seem to be populating the $movement_areas. I’m curious if you could also print the contents of $this->getFieldvalues('log', 'field_farm_area', $id) - just to confirm that $movement_areas is in fact getting into field_farm_area. I don’t see why it wouldn’t… but worth confirming.

The “log group location field logic” you linked to (https://github.com/farmOS/farmOS/blob/3c7886dbc53b774fc799f974a5e3af68033db519/modules/core/migrate/config/install/migrate_plus.migration_group.farm_migrate_log.yml#L66-L73) is the next step after that… and it’s not very complicated, so I’d be surprised if that were to blame. And the fact that location IS being set for your other log imports suggests that it’s working properly.

Quick overview of how that works for reference:

    location:
      plugin: sub_process
      source: field_farm_area
      process:
        target_id:
          plugin: farm_migration_group_lookup
          migration_group: farm_migrate_area
          source: tid

This basically translates to:

In order to populate the log’s location field, look for values in the source log’s field_farm_area field. If it has values, then take the tid key for each, and use it to look up the new asset ID from all migrations that have already run in the farm_migrate_area group. If one is found, then set the destination log’s target_id key to reference it.

Those source => destination ID mappings are stored in database tables, so you could also check those to see if the mapping is set correctly. The table you want to look at specifically would be farm_migrate_area_equipment_placeholder.

[Symbioquine]

@mstenta

One outstanding question is why the migration said 2460 seed assets, but both farmOS 1.x and 2.x seem to think there are 2459 seed assets…

Hmm… did you find any more clues about this? Curious…

Nothing yet. I’ll probably try to tie up the loose ends and minor discrepancies once I’ve sorted out migrating the bulk of the data…

[Symbioquine]

Thanks for the tips @mstenta!

I’ve now added logging of the field values for field_farm_area - though I’m a bit puzzled about whether we’d actually expect that to be populated since I don’t know the connection between values set by $row->setSourceProperty and retrieved via $this->getFieldvalues;

diff --git a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
index e5572601..c05a5bb9 100644
--- a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
+++ b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
@@ -155,7 +155,19 @@ class FarmLog extends Log {
     // Add the quantity IDs to the row for future processing.
     $row->setSourceProperty('log_quantities', $quantity_ids);
 
-    return parent::prepareRow($row);
+    $preparedRow = parent::prepareRow($row);
+
+    $field_farm_asset = $this->getFieldvalues('log', 'field_farm_asset', $id);
+    $field_farm_area = $this->getFieldvalues('log', 'field_farm_area', $id);
+
+    $this->idMap->saveMessage(['id' => $id],
+      "is_movement = $is_movement" .
+      " movement_areas = " . print_r($movement_areas, TRUE) .
+      " field_farm_asset = " . print_r($field_farm_asset, TRUE) .
+      " field_farm_area = " . print_r($field_farm_area, TRUE),
+      MigrationInterface::MESSAGE_WARNING);
+
+    return $preparedRow;
   }
 
 }

Which results in the following log message; (Please excuse the icky awk )

$ docker-compose exec -T www2 drush migrate:messages farm_migrate_log_activity | awk 'flag{ if (/^\s*[0-9]+/){printf "%s", buf; flag=0; buf=""} else buf = buf $0 ORS}; /8201/{flag=1; print}'
  8201           8201                2       is_movement = 1 movement_areas =  
                                             Array                             
                                             (                                 
                                                 [0] => Array                  
                                                     (                         
                                                         [tid] => 858          
                                                     )                         
                                                                               
                                             )                                 
                                              field_farm_asset = Array         
                                             (                                 
                                                 [0] => Array                  
                                                     (                         
                                                         [target_id] => 3358   
                                                     )                         
                                                                               
                                             )                                 
                                              field_farm_area = Array          
                                             (                                 
                                             )                                 
 

Looking at the tables, we see pretty much the same thing as we’re seeing in farmOS 2.x after the migration. i.e. the asset, but not the location is being set on the log;

$ docker-compose exec db2 psql -U farm --command "SELECT * FROM log__asset WHERE entity_id = 8201;"
  bundle  | deleted | entity_id | revision_id | langcode | delta | asset_target_id 
----------+---------+-----------+-------------+----------+-------+-----------------
 activity |       0 |      8201 |         306 | en       |     0 |            3358
(1 row)

$ docker-compose exec db2 psql -U farm --command "SELECT * FROM log__location WHERE entity_id = 8201;"
 bundle | deleted | entity_id | revision_id | langcode | delta | location_target_id 
--------+---------+-----------+-------------+----------+-------+--------------------
(0 rows)

Interestingly, the mappings do seem to exist;

$ docker-compose exec db2 psql -U farm --command "SELECT * FROM migrate_map_farm_migrate_area_equipment_placeholder WHERE sourceid1 = 858;"
                         source_ids_hash                          | sourceid1 | destid1 | source_row_status | rollback_action | last_imported | hash 
------------------------------------------------------------------+-----------+---------+-------------------+-----------------+---------------+------
 bc51255c817fd08f3797fca75fd678a3b17566810c43eb1a0153ddb9edd23688 |       858 |    3593 |                 0 |               0 |             0 | 
(1 row)

Perhaps the next step is to add a bit of logging to FarmMigrationGroupLookup.php…

[Symbioquine]

Now with a bit more debug logging in place;

diff --git a/modules/core/migrate/src/Plugin/migrate/process/FarmMigrationGroupLookup.php b/modules/core/migrate/src/Plugin/migrate/process/FarmMigrationGroupLookup.php
index a3a107f0..4ac6e046 100644
--- a/modules/core/migrate/src/Plugin/migrate/process/FarmMigrationGroupLookup.php
+++ b/modules/core/migrate/src/Plugin/migrate/process/FarmMigrationGroupLookup.php
@@ -56,7 +56,11 @@ class FarmMigrationGroupLookup extends MigrationLookup {
     // Set the migration configuration and delegate processing to the parent
     // MigrationLookup::transform() method.
     $this->configuration['migration'] = $group_migrations;
-    return parent::transform($value, $migrate_executable, $row, $destination_property);
+    $transformed = parent::transform($value, $migrate_executable, $row, $destination_property);
+
+    $migrate_executable->saveMessage("value = " . print_r($value, TRUE) . " destination_property = " . print_r($destination_property, TRUE) . " group_migrations = " . print_r($group_migrations, TRUE) . " transformed = " . print_r($transformed, TRUE));
+
+    return $transformed;
   }
 
 }
diff --git a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
index e5572601..c05a5bb9 100644
--- a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
+++ b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
@@ -155,7 +155,19 @@ class FarmLog extends Log {
     // Add the quantity IDs to the row for future processing.
     $row->setSourceProperty('log_quantities', $quantity_ids);
 
-    return parent::prepareRow($row);
+    $preparedRow = parent::prepareRow($row);
+
+    $field_farm_asset = $this->getFieldvalues('log', 'field_farm_asset', $id);
+    $field_farm_area = $this->getFieldvalues('log', 'field_farm_area', $id);
+
+    $this->idMap->saveMessage(['id' => $id],
+      "is_movement = $is_movement" .
+      " movement_areas = " . print_r($movement_areas, TRUE) .
+      " field_farm_asset = " . print_r($field_farm_asset, TRUE) .
+      " field_farm_area = " . print_r($field_farm_area, TRUE),
+      MigrationInterface::MESSAGE_WARNING);
+
+    return $preparedRow;
   }
 
 }

We can see the lookup occurring for the seed asset reference, but not for the location reference;

$ docker-compose exec -T www2 drush migrate:messages farm_migrate_log_activity | awk 'flag{ if (/^\s*[0-9]+/){printf "%s", buf; flag=0; buf=""} else buf = buf $0 ORS}; /8201/{flag=1; print}'
  8201           8201                2       is_movement = 1 movement_areas =   
                                             Array                              
                                             (                                  
                                                 [0] => Array                   
                                                     (                          
                                                         [tid] => 858           
                                                     )                          
                                                                                
                                             )                                  
                                              field_farm_asset = Array          
                                             (                                  
                                                 [0] => Array                   
                                                     (                          
                                                         [target_id] => 3358    
                                                     )                          
                                                                                
                                             )                                  
                                              field_farm_area = Array           
                                             (                                  
                                             )                                  
                                                                                
  8201           8201                1       value = 3358 destination_property  
                                             = target_id group_migrations =     
                                             Array                              
                                             (                                  
                                                 [0] =>                         
                                             farm_migrate_asset_animal          
                                                 [1] =>                         
                                             farm_migrate_asset_equipment       
                                                 [2] =>                         
                                             farm_migrate_asset_plant           
                                                 [3] =>                         
                                             farm_migrate_asset_seed            
                                             )                                  
                                              transformed = 3358

[Symbioquine]

So it looks like the prepareRow function in the Drupal Log module is overwriting the field_farm_area source property.

As a demonstration of this we can set the property field_farm_area after that prepareRow call;

diff --git a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
index e5572601..532bdc3b 100644
--- a/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
+++ b/modules/core/migrate/src/Plugin/migrate/source/d7/FarmLog.php
@@ -123,13 +123,6 @@ class FarmLog extends Log {
         }
       }
 
-      // If the log has movement areas, copy them to the log itself.
-      // This will overwrite existing area references, but an exception should
-      // be thrown above unless overwriting is explicitly allowed.
-      if (!empty($movement_areas)) {
-        $row->setSourceProperty('field_farm_area', $movement_areas);
-      }
-
       // If the log has a movement geometry, copy it to the log itself.
       // This will overwrite an existing geometry, but an exception should
       // be thrown above unless overwriting is explicitly allowed.
@@ -138,9 +131,6 @@ class FarmLog extends Log {
       }
     }
 
-    // Set the "is_movement" property for use in migrations.
-    $row->setSourceProperty('is_movement', $is_movement);
-
     // Get quantity log field value.
     $quantity_values = $this->getFieldvalues('log', 'field_farm_quantity', $id);
 
@@ -155,7 +145,21 @@ class FarmLog extends Log {
     // Add the quantity IDs to the row for future processing.
     $row->setSourceProperty('log_quantities', $quantity_ids);
 
-    return parent::prepareRow($row);
+    $preparedRow = parent::prepareRow($row);
+
+    if (!empty($movement_value)) {
+      // If the log has movement areas, copy them to the log itself.
+      // This will overwrite existing area references, but an exception should
+      // be thrown above unless overwriting is explicitly allowed.
+      if (!empty($movement_areas)) {
+        $row->setSourceProperty('field_farm_area', $movement_areas);
+      }
+
+      // Set the "is_movement" property for use in migrations.
+      $row->setSourceProperty('is_movement', $is_movement);
+    }
+
+    return $preparedRow;
   }
 
 }

Which yields our expected log with a location set;

Unfortunately, it still isn’t setting the is_movement flag correctly so it didn’t affect the location of our seed asset;

This is getting a little hacky, but it’s starting to point at what might be going on…

[Symbioquine]

I’m not sure what this would break, but it looks like I can get the logs set to be movements with the following additional change;

diff --git a/modules/core/migrate/config/install/migrate_plus.migration_group.farm_migrate_log.yml b/modules/core/migrate/config/install/migrate_plus.migration_group.farm_migrate_log.yml
index 55973f9b..5d3b4239 100644
--- a/modules/core/migrate/config/install/migrate_plus.migration_group.farm_migrate_log.yml
+++ b/modules/core/migrate/config/install/migrate_plus.migration_group.farm_migrate_log.yml
@@ -87,7 +87,7 @@ shared_configuration:
       source: field_farm_geofield
     is_movement:
       plugin: get
-      source: movement
+      source: is_movement
     log_category:
       plugin: sub_process
       source: field_farm_log_category

[Symbioquine]

I guess I don’t have a deep enough understanding of what’s supposed to be happening with the field_farm_area/is_movement fields to tell whether I’m hitting a bug or some irregularity with my data… @mstenta can you shed any light on that?

[mstenta]

@Symbioquine

-      source: movement
+      source: is_movement

Ah ha - THIS is a bug. That should have been changed alongside this commit: Rename log movement field to is_movement. · farmOS/farmOS@3d6b8fa · GitHub

I fixed it: Fix is_movement mapping in Log migrations. · farmOS/farmOS@9fb8009 · GitHub

This is interesting though… I actually noticed this myself recently in my local testing environment: Animal and Equipment assets didn’t have any locations, but Plants did. That seemed odd to me, but I didn’t have a chance to dig in (and check the source data I was using), and just added it to my notes to follow up on. So this could be part of it!

But what I’m confused about is: why did Plants work?? I’m doing some testing right now, so I’ll let you know what I find…

[mstenta]

OK - I think I know what’s happening… and it is indeed a bug that went unnoticed!

TL;DR: $row->setSourceProperty('field_farm_area', $movement_areas); is not working as expected (in most cases…)

It IS working for Seeding and Transplanting logs, which are the main ones that I was testing against when I wrote it - and I assumed that it would work the same for other types, which was incorrect. This explains why Plant assets have locations, but other don’t: Plants typically receive their location from Seeding and Transplanting logs.

I think the fact that $row->setSourceProperty() is working for Seedings/Transplantings is just a happy accident, due to the fact that those logs don’t have field_farm_area fields. So for whatever reason, setSourceProperty() works in that case, but not in log types that do have a field_farm_area field. That’s just my current theory…

Now we just need to figure out why setSourceProperty() isn’t working to override the values…

Oh… and I discovered another minor bug in the process, fixed here: Fix queries in D7 FarmLog migration areas/geometry lookup to prevent … · farmOS/farmOS@063173d · GitHub

[Symbioquine]

@mstenta

I think the fact that $row->setSourceProperty() is working for Seedings/Transplantings is just a happy accident, due to the fact that those logs don’t have field_farm_area fields. So for whatever reason, setSourceProperty() works in that case, but not in log types that do have a field_farm_area field.

That would make sense…

[mstenta]

Ahhhhh! I figured it out.

The call to parent::prepareRow($row); overwrites field_farm_area. Of course!

I moved that up to the top of the method instead, and all my activity logs have locations now!

Fixed with this commit: Run parent::prepareRow($row) before performing our own modifications … · farmOS/farmOS@9023567 · GitHub

Try re-running your migrations and see if both the location and is_movement fields are correctly set now.

[Symbioquine]

Awesome! Pulling in the latest code now…

[Symbioquine]

Those fixes worked, thanks @mstenta!

So to summarize my current status;

• Almost all data migrated
• Need to look into scripting some sort of parity mechanism to validate what data is missing
• Need to expand seed asset fields and the associated migration logic
• Need to port some additional manual site customization and custom tools into 2.x modules
• Need to look into a few observed quirks;
  • Seed asset migration 2459 vs 2460 count discrepancy - Symbioquine's 2.x Migration Testing Log - #23 by Symbioquine
• It would be nice to change all uploaded files to be private as part of the migration

[mstenta]

Those fixes worked, thanks @mstenta!

Great!! Thank you for testing!

It would be nice to change all uploaded files to be private as part of the migration

Let me know if you need help figuring this out - hopefully not too tricky. farmOS 2.x makes the filesystem private by default, but if you are migrating public files then those will remain public (IIRC).

[Symbioquine]

@mstenta

It would be nice to change all uploaded files to be private as part of the migration

Let me know if you need help figuring this out - hopefully not too tricky. farmOS 2.x makes the filesystem private by default, but if you are migrating public files then those will remain public (IIRC).

Yeah, that matches what I’ve seen so far… planning to (re-)review what you wrote in File attachments are publicly accessible by default - if url is known · Issue #263 · farmOS/farmOS · GitHub and write up the steps I end up taking here.

[Symbioquine]

Another feature that seems like it should just work is groups. For my seed assets, groups are pretty critical since they are used to organize the overall seed inventory into collections. e.g. All the types of (common) beans are in “OISB Inventory: Beans”.

Here’s an example in farmOS 1.x where we can see the group set;

Unfortunately, in 2.x the group isn’t automatically coming through;

The first thing I notice is that the group module wasn’t automatically enabled, so let’s enable that;

docker-compose.yml

        su www-data -s /bin/bash <<'EOF'
          set -ex

+         drush --root=/opt/drupal pm-enable --yes farm_group
          drush --root=/opt/drupal pm-enable --yes farm_migrate
          drush --root=/opt/drupal migrate:import --group=farm_migrate

After re-running the migration, all the groups are showing up;

But unfortunately, it doesn’t look like there’s a concept of group membership log fields yet so that part is blocked… See https://www.drupal.org/project/farm/issues/3151233

[mstenta]

That’s correct - Group membership is not ported to 2.x yet. That’s one of the last remaining “data model” pieces, along with “Inventory” and “Data streams” (aka sensor data). Coming soon!