Closed (fixed)
Project:
Geofield
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
8 Nov 2016 at 07:03 UTC
Updated:
15 Jan 2020 at 20:13 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
sam152 commentedHere 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.
Comment #3
sam152 commentedGot some help in making the cck migration pull out the latlong field values, so the process plugin did not have to change at all.
Comment #4
sam152 commentedFixed a comment.
Comment #5
sam152 commentedComment #6
hussainwebIt 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.
Comment #7
plopescHi 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!
Comment #8
basvredelingI'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]
Comment #9
itamair commentedComment #10
basvredelingIn addition to patch #6, the annotation of the migrate plugin file GeofieldItem.php lacks the source_module and destination_module declarations. It should read:
Comment #11
itamair commentedDo we reach a well tested and working patch on this?
Comment #12
basvredeling@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.
Comment #13
itamair commentedOk ... 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 ...
Comment #14
artatac commentedcould I ask where this patch goes and also how the /upgrade is made aware of its existance? Sorry if this is Drupal 101
Comment #15
artatac commentedI 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?
Comment #16
super_romeo commentedShould be removed:
because of:
and "geofield_bounds" widget exists and not need to be replaced.
Comment #17
timwoodEdited patch #6 for comments in #10 and #16.
Comment #19
timwoodSorry, the last patch was bad. This one applies cleanly and isn't missing a closing }
Comment #20
timwoodI'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:
Replace with:
Comment #21
chriscalip commentedThis patch is reference by community documentation for migrate and geofield integration at :
https://www.drupal.org/node/2798907
Please update community documentation accordingly.
Comment #22
itamair commentedMight 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.
Comment #23
timwood@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. :-(
Comment #24
itamair commentedok 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.
Comment #25
itamair commentedComment #26
itamair commentedComment #27
chriscalip commented@timwood
Debug via
`drupal debug:plugin migrate.process`
or
`drush php; \Drupal::service('plugin.manager.migrate.process')->getDefinitions();`
Comment #28
timwood@chriscalip, thanks for the tip. I've used that drupal console command, which without the patch shows:
And when running
../vendor/bin/drupal --uri=local.manufacturingusa.test debug:plugin migrate.process geofield_latlonIs that what you mean or am I missing something? Thanks!
Comment #29
chriscalip commented@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.
Comment #30
quietone commentedCame across this issue today.
The CckFieldPluginBase is deprecated instead use FieldPluginInterface. See Migration field plugins and classes renamed
This is now sub_process. See Renamed Migrate iterator process plugin to sub_process
Comment #31
mpp commentedComment #32
itamair commentedComment #33
2phaHmm, 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.
Comment #34
2phaReopening 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.
Comment #35
2phaFix for the above test failing (because the test itself is broken).
https://www.drupal.org/project/geofield/issues/3097292
Comment #37
itamair commentedthanks thanks @2pha, #34 patch committed into dev, will be part of the next release ...
Comment #39
roderik de langen commentedremoved comment since it was working in the end!