Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This module + geocode is working great for individual items.
However, I'm running into an issue with Geocode when importing lists of addresses through feeds. Everything imports correctly and is available, but it isn't geocoded. If I individually visit & save each node, they are geocoded just fine.
I'm not sure of the exact Feeds to Addressfield to Geofield (to Geocode) cycle. Any thoughts or pointers about where to jump into that process?
Comment | File | Size | Author |
---|---|---|---|
#69 | geocoder-feeds_addressfield-1265036-69.patch | 4.72 KB | NWOM |
Comments
Comment #1
phayes CreditAttribution: phayes commentedThe reason for this is that geocode works at the form level, whereas feeds works as the node_save level.
I wonder if feeds have a hook that we can add to geofield to let it do it's work?
Comment #2
avr CreditAttribution: avr commentedThanks for the response. I wondered if this was the case. However, even through views_bulk_operations "node_save" isn't invoking the widget's validate function.
I found a thread & related snippet to use via VBO: http://drupal.org/node/905814#comment-4931016
Here it is again for reference (although it belongs more with Geocode).
I'll look into Feeds' hooks - because I'd like to have this work without VBO for the client.
Comment #3
phayes CreditAttribution: phayes commentedThe other alternative is to add a setting in the widget for geocode to use node_save updates instead of form updates. That's likely a good idea, since it takes care of the VBO issue too.
Comment #4
avr CreditAttribution: avr commentedphayes - adding an option in the widget sounds amazing.
i actually am running into this same issue with an event registration system through commerce. the work around for now is to invoke an arbitrary order save during my checkout process.
i'm not familiar enough with the process and distinctions for node/widget validation to know where to start. if you can provide any direction, i'd love to try to help.
Comment #5
Brandonian CreditAttribution: Brandonian commentedphayes, Is there any advantage to have the ability to geocode on both the form submit and node_save? Seems like keeping both options might cause some unneeded complexity.
IMHO, geocoding should happen with a hook callback on entity_presave. Happens right before the node is saved to the db, while still supporting all fieldable entities.
Comment #6
wesnick CreditAttribution: wesnick commentedI have found that when you geocode to geofield from another field, which is usually addressfield, that any ajax events cause the geocode plugins to fire multiple times before you even submit the whole form. So say you are using the country selector in addressfield, and select one country, then change it, the form submission causes the geocode to run as many times as you change the select box. The geocoding event should only take place on crud actions, or as an alternative, they could be indexed on crud actions to run later in bulk during cron.
I personally think, the geocoding should move to hook_entity_insert() and hook_entity_update(), which could be the same function, this way it will always get run no matter what type of entity it is attached to or in what context: form update, bulk processes like VBO or Feeds.
I am actually here because I was looking around for metadata and property callbacks issues, as I found geofield to be tough to use with entity wrappers, so instead of using $wrapper in bulk operations, if find my self having to do:
Whereas if the field were set to automatically geocode from another field and was trigger by entity_update, then I wouldn't even have to bother with this. I could roll a patch for something like this if there is interest.
Comment #7
philippejadin CreditAttribution: philippejadin commentedThose two looks like good candidates :
http://api.drupal.org/api/drupal/modules--system--system.api.php/functio...
http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...
+1 imho for doing the geocoding at this stage.
Comment #8
wesnick CreditAttribution: wesnick commentedA patch for geocode that encodes on hook_entity_presave(). Strategy was to append some info to entity property info, then use this value on entity_presave to avoid expensive lookups on every entity_save. This patch allows you to choose either form submission encoding or save encoding. We could conceivably add more granular control over how geocoding gets done.
So technically this patch is against geocode, and not geofield, but what is the status on getting D7 version of geocode up on DO? Should post this patch to github?
Comment #9
wesnick CreditAttribution: wesnick commentedPrevious patch gives errors when adding a new field. Apparently, you can't do field_info_field within hook_entity_info_alter. Made a little db query, but it doesn't feel like the right solution. At any rate, attached patch.
Comment #10
avr CreditAttribution: avr commented@wesnick,
I haven't had a chance to test this myself, but it looks like your function names are off. In
hook_entity_info_alter
, you havegeocode_get_active_geofields
, but the function itself is calledgeocode_get_geofields
.I'll try to test and let you know how it goes - I've solved the original question I had so I'll have to work on getting my specific use case back and testing from there.
Comment #11
wesnick CreditAttribution: wesnick commented@avr,
Right you are, and there were also some extra variables in there not doing anything. Reroll with fixed function names.
Comment #12
philippejadin CreditAttribution: philippejadin commentedJust an idea :
Geocode could be an action (drupal action) that would be triggered by the rule "when node of type xxx is saved"
Comment #13
dasjoComment #14
Brandonian CreditAttribution: Brandonian commentedMoving to Geocoder issue queue. Patch will most likely need to be rerolled.
Comment #15
michaelfavia CreditAttribution: michaelfavia commentedRerolled to match new filenames and fixed assumed patch typo in widget_info() (said gecode not geocode).
Entity_info_alter() also implemented a
$info[$entity]['geocoder'] so i temporarily renamed it to geocoderr to preserve uniqueness.
Rerolled patch applies but doesnt work as intended with my tests.
Is there a reason not to ALWAYS save on entity save and not through the form system? We are increasingly trying to make an api out of drupal on the core side is there a reason not to leverage that so ANY entity can use this?
Comment #16
wesnick CreditAttribution: wesnick commentedCool, glad to see geocode become a full-fledged project.
I can think of a few reasons why you would not always want to geocode on save, so I think we should preserve options.
I like options. I think geocode should provide options to use it's current widget, which would encode on validation. It probably should be moved from validation to submission, however. I also think that there should be an ajax widget. Just a simple js script that will can do encoding client side and either submit the values to geocoder callback, or fill in some hidden form values.
The entity_presave option is nice for bulk import or other contexts.
My goal is to minimize the impact of the average 400ms wait for geocoding, which is why I will most likely use a js script for actual form submissions, but the presave is nice for import.
Comment #17
phayes CreditAttribution: phayes commentedI agree with wesnick - make the hook_presave optional.
I also like the idea of creating a javascript file that can be included on any page that basically just allows the "geocoder" function to be called via JavaScript (via ajax). We could then create a separate widget that can translate between address-field -> geofield on the client-side in realtime.
Comment #18
phayes CreditAttribution: phayes commented-- duplicate --
Comment #19
esclapes CreditAttribution: esclapes commentedHi there,
Found myself in this use-case. I am importing an addressfield through feeds and used the patch on #15 to test geocoding on save.
I did check the option "geocode on save operations" in the widget settings.
The importing works fine (once you apply this other patch: #1139676) but the nodes are not geocoded. No warning or anything after the import.
However, when I try to edit and save one of the imported nodes I get this warning:
Warning: Invalid argument supplied for foreach() in geocoder_entity_presave() (line 425 of home/quickstart/websites/geofield.dev/sites/all/modules/geocoder/geocoder.module)
Comment #20
dasjothere's a patch for geofield #1303472: Compute values in hook_field_presave
Comment #21
wesnick CreditAttribution: wesnick commented@ciberligre,
I never actually tested the patch, just wrote it as a proof-of-concept. I went ahead and made the conversion from geocode -> geocoder and rerolled the patch, and fixed a few bugs and tested it with simple encoding from an addressfield -> geofield and it seems to be working ok.
I am not sure why patch from #1139676 is necessary.
@dasjo,
I think geofield should stick to generic field handlers, and do it's thing on validate, submit, process, etc. Geocode only implements an action that executes an entity-level action, so it has to do it's thing in entity_presave.
For this matter, we should also provide a 'No Geocode on submit or save' options so we can provide a basic rules condition, like 'source_field value changed' for entity save events, and a basic action for each geotype: geocode point from source_field, geocode, polygon from taxonomy, etc
Comment #22
tglynn CreditAttribution: tglynn commentedThanks so much for this patch! You guys rock! It worked for me when I imported nodes with a normal textfield of "Country Code".
Comment #23
philippejadin CreditAttribution: philippejadin commentedThe last path (from #21) applies cleanly but doesn't work for me.
The checkbox appears in the field configuration option. If I left it unchecked, Iwhen I submit a node edit form, the field is geocoded.
Either checked / unchecked, nothing happens when the nodes are imported using feeds, and nothing happens when I use the "View bulk operation" node save action.
Comment #24
mike_yang CreditAttribution: mike_yang commentedI am trying to use patch (from #21) to automatically generate geocodes for geofields for different nodes within my project. I can never get them all to work at the same time. Tracing the code turned up the root of the problem:
In the
geocoder_get_active_geofields()
function, the $query->execute()->fetchAllAssoc('entity_type')
will NOT return all geofields, since the returned value is keyed by the'entity_type'
, which in this case will be'node'
for all the fields. In other words, all the different field information will get aliases into a single entry by thefetchAllAssoc
call.The following change fixed the problem for me:
Comment #25
userofdrupal CreditAttribution: userofdrupal commentedNewbie here with exact same issue as OP. I have a Drupal 7.10 installation with Feeds 7.x-2.x-dev and AddressField 7.x-1.0-beta2.
Is this fixed? If so, how/where to get/use the fix?
Comment #26
userofdrupal CreditAttribution: userofdrupal commented-- duplicate post --
Comment #27
mcarbone CreditAttribution: mcarbone commentedMake sure to do a cache clear after applying the patch -- that got it working for me.
Comment #28
joemoraca CreditAttribution: joemoraca commentedSince this issue got a little mixed - can i use the latest versions of feeds / addressfield / geofield / geocoder and import addresses and have them geocoded after the import finishes? Yes is awesome..
Is the answer install the patch from #21.
EDIT -- I used the latest versions of all modules and everything worked great (using yahoo geocoding)
Comment #29
avr CreditAttribution: avr commentedI've rerolled the patch from #21 to include the update in #24.
For me, the attached patch working exactly as expected. I've had some issues with using Google to geocode - with a number of items not getting geocoded. When I've used Yahoo's Placefinder, all records were geocoded.
Comment #30
phayes CreditAttribution: phayes commentedI *think* this has been fixed in the latest refactoring. We are using hook_field_attach_presave. Please re-open this issue if the problem still exists.
Comment #31
rerooting CreditAttribution: rerooting commentedConfirmed! Just used VBO 'node save' option and I geocoded 88 nodes in about 10 seconds! Superb! Just made my evening.
Comment #33
rolkos CreditAttribution: rolkos commentedProblem is still there, I have postal address and geofield on taxonomy. Geocode works OK if I save term manualy if I use VBO + Rules geofield persist empty. I was also using VBO to copy data from multiple fields to address field (previously I used multiple fields to store addresses).
I tryied Geocoder 1.2 and current devel version 7.x-1.2+4-dev.
Thanks
Pawel
Comment #34
TBarina CreditAttribution: TBarina commentedSubscribe #33
Comment #35
aanjaneyam CreditAttribution: aanjaneyam commentedThe problem is still there. It geocodes only on manual save. I thought that as per #30 and #31 above the problem is fixed in the module itself and we don't need to apply the patch. Do still need to apply the patch- if yes then what was fixed as per #30.
Comment #36
aanjaneyam CreditAttribution: aanjaneyam commentedAnyways the patch does not apply to geocoder-7.x-1.x-dev dated 30 September 2013 - 7.x-1.2+9-dev. After manually looking at the geocoder.module file I cannot find relevant lines mentioned in the patch.
Comment #37
BoySherman CreditAttribution: BoySherman commentedI've rerolled this patch for the 7.x-2.x (dated Aug 27, 2012), and it it now working for me by using VBO. There was a tweak needed to '$source_field = $settings['geocoder_field'];' on geocoder_entity_presave() in order to make it work.
Comment #38
BoySherman CreditAttribution: BoySherman commentedComment #39
kopeboy CreditAttribution: kopeboy commentedPlease include it in a release!
Comment #40
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthis introduced a small error
Undefined index: geocoder_presave in geocoder_entity_presave() (line 349 of sites/all/modules/all/geocoder/geocoder.module
change L 349
to
Comment #41
NWOM CreditAttribution: NWOM commented#37 combined with the fix in #40 fixed the issue I was having with Feeds.
My Setup:
In my case, only 25% were geocoded, and the other 75% (give or take), could only be geocoded after manually saving the entities via the edit form.
It seemed to be sporadic, since I imported numerous times with different results. Different entities were geocoded each time.
Comment #42
discipolo CreditAttribution: discipolo commentedreroll including change from #40
Comment #43
joelpittetShould this take into account geofield data coming in and if it's empty or not or would that be out of scope?
So if geofield source are not empty there may be no need to geocode but if they are it would be nice to geocode.
The use-case is new data coming in on a feed, some of it has lon/lat other it doesn't. Filling in the gaps by geocoding the addressfield if we don't have geocoded data could help with some of the service limits on the geocoding services like I think google is at 2000 api requests/day from the same IP.
Comment #44
discipolo CreditAttribution: discipolo commentedi am not sure about taking into account the data coming in
but in the sense of #2306341: Geocoder only if required. we could add an option to only geocode empty fields (for when we are geocoding from another field and dont expect addresses to change)
Comment #45
discipolo CreditAttribution: discipolo commentedComment #46
discipolo CreditAttribution: discipolo commentedComment #47
discipolo CreditAttribution: discipolo commentedComment #48
discipolo CreditAttribution: discipolo commentedsorry, will stop now
Comment #49
PolHi,
What's the status of this patch ?
Comment #50
joelpittet@Pol it was "needs review". Now it's "needs work" because it doesn't apply due to recent changes.
Comment #51
joelpittetHere's a re-roll.
Comment #52
PolThanks!
Are these drush_print_r() necessary ?
Comment #53
PolComment #54
joelpittetThey are not:) I've removed those and re-rolled again with latest dev.
This hunk didn't exist anymore.
Comment #55
joelpittetHere's something of an interdiff as it was a re-roll + changes the interdiff was kinda worthless.
Comment #56
NWOM CreditAttribution: NWOM commented#54 applied cleanly to the newest dev and works as expected. Thank you!
Comment #57
PolComment #58
NWOM CreditAttribution: NWOM commented#54 is still in use on a live environment. I don't believe this is outdated at all. Does it perhaps need a reroll or was it included in the module in another way?
Comment #59
NWOM CreditAttribution: NWOM commentedComment #60
joelpittetHere's a re-roll.
Comment #61
minorOffense CreditAttribution: minorOffense at Coldfront Labs Inc. commentedReroll for latest dev. Avoid duplicate declaration.
Comment #62
AnybodyWell the same will be needed for Drupal 8 as follow-up...
Comment #63
gturnbull CreditAttribution: gturnbull as a volunteer and commentedHi Guys,
Here is an updated version of this patch for the latest stable release of the Geocoder module, version 7.x-1.4.
If you have a moment, please test this patch.
Thanks,
Gordon
Comment #64
NWOM CreditAttribution: NWOM commentedPatches must be instead made for dev version in order to get committed. Please don't update a thread with a stable patch when there is already one for the dev release. #61 is still the active patch in this issue thread. If the patch no longer applies on the newest dev, please reroll.
Comment #65
gturnbull CreditAttribution: gturnbull as a volunteer and commentedHi NWOM,
Thanks for your feedback. I'll try out the latest dev release, test the current patch, and reroll the patch if needed.
Its been a while since I last contributed a patch. Thanks for your notes.
Thanks,
Gordon
Comment #66
gturnbull CreditAttribution: gturnbull as a volunteer and commentedI'm getting an Undefined index: title error when attempting to access the admin/people/permissions page of my site after applying the feeds_addressfield-1265036-61.patch to the latest Geocoder dev release.
The error appears to be caused by the geocoder_reverse_handler permission set on line 243. This permission is being set to an empty array, which does not contain a title element. The error is occurring when the foreach loop on line 247 fires, and attempts to set the title associated with the geocoder_reverse_handler permission.
Patch to follow.
Thank you,
Gordon
Comment #67
gturnbull CreditAttribution: gturnbull as a volunteer and commentedReroll for latest dev.
Comment #68
MariaIoann CreditAttribution: MariaIoann commentedI am attaching a patch containing 2 minor fixes based on #67:
1) Check if isset
$settings['geocoder_skip_existing_targets']
in geocoder_entity_presave()2) Commented out the unnecessary
$title = $field_value['organisation_name']
in exception catchComment #69
NWOM CreditAttribution: NWOM commentedHere is a reroll against the newest dev, since #68 failed to apply. Please review.
Comment #70
FiNeX CreditAttribution: FiNeX as a volunteer commentedHi, the patch works fine, thankyou :-)