- 🇺🇸United States m.stenta
I don't mean to revive an old issue, but I just looked into using this plugin and noticed that it isn't actually doing anything.
The
GeofieldWKT
class pulls in thegeofield.wkt_generator
service via dependency injection but doesn't use it anywhere.The
transform()
method just returns the value, so it is essentially the same as theget
plugin.It sounds like @stopopol originally recommended performing some WKT validation:
then in the transform function you could take the $value and check if it's valid WKT (e.g. https://stackoverflow.com/questions/9985484/php-validate-wkt-values) and if yes, just return $this->$value.
Notably, if you set
validate: true
in the migration'sdestination
config,$entity->validate()
will be called and invalid WKT will throw an error. This is the proper way to handle validation, I think.With all that in mind, I don't think this
geofield_wkt
plugin provides any additional value over the coreget
plugin. Should we consider deprecating it and removing it in the next major release? - 🇮🇹Italy itamair
thanks @m.stenta
definitely right you are ...
geofield.wkt_generator service is needed in the geofield_latlon @MigrateProcessPlugin but not in the geofield_wkt one.The following (just performed) commit accomplishes what you suggest in the 8.x-1.x branch:
https://git.drupalcode.org/project/geofield/-/commit/77e70c1b0ba74478984...It will be part of the next Geofield 8.x release.
- 🇺🇸United States m.stenta
Makes sense thanks for responding so quickly @itamair. If you would like any help with deprecating it I'd be happy to start a separate issue for that. Also fine to leave as is. :-)
- 🇮🇹Italy itamair
Hi @m.stenta … I cannot (I am not sure I can) get what you mean in “deprecating” it …
The geofield.wkt_generator service still is used (neede) elsewhere.What is further needed on this? (also with an additional issue …?)
- 🇺🇸United States m.stenta
Oh sorry for the misunderstanding.
I meant we could consider deprecating the
geofield_wkt
migrate process plugin. It doesn't perform any transformation of source values. It just takes the value and returns it.In other words, these are equivalent:
process: my_geofield: plugin: geofield_wkt source: my_wkt_source
process: my_geofield: my_wkt_source
The latter uses Drupal core's
get
plugin, which just takes the source value as-is and inserts it into the field. So thegeofield_wkt
plugin is redundant.But there's also no harm in keeping it as a placeholder, in case any additional logic need to be added in the future.
- 🇮🇹Italy itamair
Ah! got it @m.stenta ...
Thanks for the detailed explanation. Sometime I loose detailed track of what it is going on all over these Geofield stack of modules.What you say makes perfect sense, but let's keep it as example @MigrateProcessPlugin and not harmful placeholder for Geofield.
I provided this new commit:
https://git.drupalcode.org/project/geofield/-/commit/483958ae703f02f46cb...
where I simply added all your warning into the @MigrateProcessPlugin description itself.
- 🇺🇸United States m.stenta
That's great - thanks @itamair for your dedicated maintenance!