Comments

Sam152 created an issue. See original summary.

sam152’s picture

Status: Active » Needs review
StatusFileSize
new146.63 KB

Here is the plugin and test. The process plugin had to be updated because it expected a pair of lat/lon values, whereas the source data is the D7 field schema. If we were maintaining backwards compatibility, we can keep it the way it is, otherwise the process plugin could only be used in source data that resembled a keyed array, like the D7 field.

sam152’s picture

StatusFileSize
new3.03 KB
new144.38 KB

Got some help in making the cck migration pull out the latlong field values, so the process plugin did not have to change at all.

sam152’s picture

StatusFileSize
new497 bytes
new144.38 KB

Fixed a comment.

sam152’s picture

Title: Implement a migrate cck field plugin for migrating from Drupal 7 to 8 » Implement a migrate cck field plugin for migrating from Drupal 7 to 8 and test the existing process plugin
hussainweb’s picture

StatusFileSize
new2.1 KB

It seems like tests are not being run here. I basically wrote this same thing (for the most part) and I found two migration issues. Since the other one breaks backward compatibility, I am inclined towards this one.

Although, I mean to ask why should we rely on this service to translate latlong when we essentially have all the data? I have attached the patch I prepared for review. I'm testing it at the moment and will report back on how effective this was. If this works, we could add tests to this as well.

plopesc’s picture

Hi Sam152,

I'm not very familiar with the migrate topic, so I don't know well how could I QA this.

In any case, these patches would need to be updated after the last changes and the creation of alpha4 release.

Thank you!

basvredeling’s picture

I've tried both #4 and #6. Both don't seem to migrate a WKT text value correctly. The patch from #4 automatically sets the widget to a lat/lon and doesn't import any value. #6 messes up the text value. It looks like a character set conversion issue.

[update] Issue in #6 looks like a problem on my part converting utf8mb4 to non-utf8mb4. [/update]

itamair’s picture

Status: Needs review » Needs work
basvredeling’s picture

In addition to patch #6, the annotation of the migrate plugin file GeofieldItem.php lacks the source_module and destination_module declarations. It should read:

/**
 * @MigrateCckField(
 *   id = "geofield",
 *   core = {7},
 *   source_module = "geofield",
 *   destination_module = "geofield"
 * )
 */
itamair’s picture

Do we reach a well tested and working patch on this?

basvredeling’s picture

@itamair it is certainly an improvement over what is currently available. I can add my small change to a new patch. It is not well tested though.

itamair’s picture

Ok ... well, if not properly tested and reviewed it means that this still needs work.
So as @plopesc I am not a great expert of geofield migrations too, so I wouldn't know how to QA this.

Would be nice to have (necessary) a working patch, that applies to a tagged version of Geofield (more than the dev, so to bypass the need to re-roll it all the time ...), that might be already somehow tested and available for further testing and review from the community.
Otherwise we risk to go on as a "pour parler" indefinetly with this ...

artatac’s picture

could I ask where this patch goes and also how the /upgrade is made aware of its existance? Sorry if this is Drupal 101

artatac’s picture

I have updated as per #6 and also #10 but when I then go back to /upgrade and link to the D7 database I still get "geofield not upgraded". Am I missing a step please?

super_romeo’s picture

Should be removed:

'geofield_bounds' => 'geofield_Bounds',

because of:

* @FieldWidget(
 *   id = "geofield_bounds",

and "geofield_bounds" widget exists and not need to be replaced.

timwood’s picture

Status: Needs work » Needs review
StatusFileSize
new2.13 KB

Edited patch #6 for comments in #10 and #16.

Status: Needs review » Needs work

The last submitted patch, 17: implement_a_migrate_cck-2825635-17.patch, failed testing. View results

timwood’s picture

Status: Needs work » Needs review
StatusFileSize
new2.13 KB

Sorry, the last patch was bad. This one applies cleanly and isn't missing a closing }

timwood’s picture

I'm getting the error below (which I wasn't before) after applying the patch and modifying my node type field mapping like so. Is this how the CCK field plugin is supposed to be used (using migrate plus)? Not sure why the error is related to geocoder...

Remove:

  field_mfg_geofield: field_mfg_geofield

Replace with:

  field_mfg_geofield:
    plugin: geofield_latlon
    source: field_mfg_geofield
 [error]  The "geocoder" plugin does not exist. (/var/www/mfgov-d8/docroot/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php:52) 
chriscalip’s picture

This patch is reference by community documentation for migrate and geofield integration at :
https://www.drupal.org/node/2798907

Please update community documentation accordingly.

itamair’s picture

Might the #19 patch be considered definitive and fixing/implementing this issue?
Does it apply cleanly to the actual Geofield dev version?
Does it solve this issue?

Again, me and @plopesc are not expert of geofield migrations so wouldn't know how to QA this.
Please review asap ... and let us know, when ready to commit into dev.

The community documentation would be updated subsequently.

timwood’s picture

@chriscalip, the documentation page only references this issue, not a specific patch, so I'm unsure what needs to be updated in the documentation at this time. -- Disregard, I see you updated the documentation to link to this issue. Apologies.

@itamair, I did NOT test the patch against geofield dev, only last released version (8.x-1.0-beta4) where it applies cleanly. I'm still struggling to figure out how to configure my migrations to use what is provided by the patch. Also, I still get the "geocoder plugin" error I mentioned in comment #20 above, which only appears after applying the patch. Data is still not migrated, in my testing. :-(

itamair’s picture

ok thanks for the feedback @timwood,
but your review simply means that the latest #19 patch doesn't solve this issue, yet,
so cannot be committed right now, right?
I update this issue version to 8.x-1.0-beta4, so you won't care about further re-rolling of the patch evolution with module dev.
Just focus on optimize and make it definitive over the 8.x-1.0-beta4 if you further can, and let us know.
So that we will be able to commit it ...

Forget now about the community documentation page: it is now correctly referring this issue.
Will be updated (by me ...) consequently when this issue will be solved/fixed.

itamair’s picture

Status: Needs review » Needs work
itamair’s picture

Version: 8.x-1.x-dev » 8.x-1.0-beta4
chriscalip’s picture

@timwood
Debug via
`drupal debug:plugin migrate.process`
or
`drush php; \Drupal::service('plugin.manager.migrate.process')->getDefinitions();`

timwood’s picture

@chriscalip, thanks for the tip. I've used that drupal console command, which without the patch shows:

  geofield_latlon                     Drupal\geofield\Plugin\migrate\process\GeofieldLatLon                  

And when running ../vendor/bin/drupal --uri=local.manufacturingusa.test debug:plugin migrate.process geofield_latlon

 ------------------ ------------------------------------------------------- 
  Key                Value                                                  
 ------------------ ------------------------------------------------------- 
  class              Drupal\geofield\Plugin\migrate\process\GeofieldLatLon  
  handle_multiples   FALSE                                                  
  id                 geofield_latlon                                        
  provider           geofield                                               
 ------------------ -------------------------------------------------------

Is that what you mean or am I missing something? Thanks!

chriscalip’s picture

@timwood
Just double checking some of the variables in play.
To be on the same page, geocoder is contrib module that is often used together with this module.

It also provides its own plugin types and plugins.
I believe the error you are getting is about geocoder; that just gets exposed as you used the ecosystem more thoroughly.

quietone’s picture

Came across this issue today.

+++ b/src/Plugin/migrate/cckfield/GeofieldItem.php
@@ -0,0 +1,72 @@
+use Drupal\migrate_drupal\Plugin\migrate\cckfield\CckFieldPluginBase;

The CckFieldPluginBase is deprecated instead use FieldPluginInterface. See Migration field plugins and classes renamed

+++ b/src/Plugin/migrate/cckfield/GeofieldItem.php
@@ -0,0 +1,72 @@
+      'plugin' => 'iterator',

This is now sub_process. See Renamed Migrate iterator process plugin to sub_process

mpp’s picture

itamair’s picture

Status: Needs work » Closed (duplicate)
2pha’s picture

Hmm, I am not sure why this was closed, although there are duplicates... they are all closed and marked as a duplicate.

The related issue in #31, although related is not the same, it is for a process plugin, not a field migration.

It seems to me that there is still no field migration..
I tried upgrading a very simple D7 site today, after installing the Migrate and Migrate Drupal UI modules, I navigated to "/upgrade" only to find the geofield was listed under "modules that will not be upgraded".
I ran the upgrade anyway, and was saddened by the fact that everything migrated perfectly except for the geofields :(

Maybe I am just doing something wrong, I have not experience with D7 to D8 migrations, if so, please guide me in the right direction.

2pha’s picture

Version: 8.x-1.0-beta4 » 8.x-1.x-dev
Status: Closed (duplicate) » Needs review
StatusFileSize
new2.13 KB

Reopening and adding a patch.
After adding this, the geofields on my site (2000+ of them) migrated fine with the migrate_drupal and migrate_drupal_ui modules "/upgrade".

This patch adds the migration field plugin.... not to be confused with the migration process plugin.

A note on testing this...
When you navigate to the "/upgrade" form, the drupal_migrate module creates tables in the DB of the migrations that it can generate automatically, these DB tables are not removed when uninstalling the migrate_drupal module. So if you have tried the "/upgrade" before this patch, and then apply this patch, the upgrade form will still report the geofields as not being able to be migrated.
To get around this problem before applying this patch, all DB tables that have the prefix "migrate_map" and "migrate_message" must be deleted before navigating to the "/upgrade" page again.

2pha’s picture

Fix for the above test failing (because the test itself is broken).
https://www.drupal.org/project/geofield/issues/3097292

  • itamair committed 84a5579 on 8.x-1.x authored by 2pha
    Issue #2825635 by Sam152, timwood, 2pha, hussainweb: Implement a migrate...
itamair’s picture

Status: Needs review » Fixed

thanks thanks @2pha, #34 patch committed into dev, will be part of the next release ...

Status: Fixed » Closed (fixed)

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

roderik de langen’s picture

removed comment since it was working in the end!