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?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phayes’s picture

The 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?

avr’s picture

Thanks 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).

$test = node_load($entity->nid);
module_load_include('inc', 'node', 'node.pages');
$test_state['values']['op'] = t('Save');
drupal_form_submit('church_node_form', $test_state, $test);

I'll look into Feeds' hooks - because I'd like to have this work without VBO for the client.

phayes’s picture

The 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.

avr’s picture

phayes - 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.

Brandonian’s picture

phayes, 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.

wesnick’s picture

I 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:

      // Make the geofield data
      $geo = array();
      $geo['lat'] = $sg_result['address']['geometry']['coordinates'][1];
      $geo['lon'] = $sg_result['address']['geometry']['coordinates'][0];
      geofield_compute_values($geo, 'latlon');
      $venue->field_venue_geolocation['und'][0] = $geo;

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.

philippejadin’s picture

wesnick’s picture

Category: support » feature
Status: Active » Needs review
FileSize
3.65 KB

A 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?

wesnick’s picture

Previous 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.

avr’s picture

@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 have geocode_get_active_geofields, but the function itself is called geocode_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.

wesnick’s picture

@avr,

Right you are, and there were also some extra variables in there not doing anything. Reroll with fixed function names.

philippejadin’s picture

Just an idea :

Geocode could be an action (drupal action) that would be triggered by the rule "when node of type xxx is saved"

dasjo’s picture

Brandonian’s picture

Project: Geofield » Geocoder
Version: 7.x-1.0-alpha4 » 7.x-1.x-dev

Moving to Geocoder issue queue. Patch will most likely need to be rerolled.

michaelfavia’s picture

Rerolled 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?

wesnick’s picture

Cool, 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.

phayes’s picture

I 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.

phayes’s picture

-- duplicate --

esclapes’s picture

Hi 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)

dasjo’s picture

wesnick’s picture

@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

tglynn’s picture

Thanks so much for this patch! You guys rock! It worked for me when I imported nodes with a normal textfield of "Country Code".

philippejadin’s picture

The 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.

mike_yang’s picture

I 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 the fetchAllAssoc call.

The following change fixed the problem for me:

/**
 * Implements hook_entity_info_alter().
 */
function geocoder_entity_info_alter(&$info) {

  $fields = geocoder_get_active_geofields();

  if ($fields) {
    foreach($fields as $field_info) {
      $entity = $field_info['entity_type'];
      $bundle = $field_info['bundle'];
      $field_name = $field_info['field_name'];
      $info[$entity]['geocoderr'][$bundle] = $field_name;
    }
  }
}


function geocoder_get_active_geofields() {

  static $info;

  if (!isset($info)) {
    $query = db_select('field_config', 'fc', array('fetch' => PDO::FETCH_ASSOC));
    $query->join('field_config_instance', 'fci', 'fc.id = fci.field_id');
    $query->fields('fci', array('field_name', 'entity_type', 'bundle'));
    $query->condition('fc.type', 'geofield', '=');
    $query->condition('fc.active', 1);
    $info = $query->execute()->fetchAll();
  }
  return $info;
}
userofdrupal’s picture

Newbie 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?

userofdrupal’s picture

-- duplicate post --

mcarbone’s picture

Make sure to do a cache clear after applying the patch -- that got it working for me.

joemoraca’s picture

Status: Fixed » Needs review

Since 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)

avr’s picture

I'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.

phayes’s picture

Status: Needs review » Fixed

I *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.

rerooting’s picture

Confirmed! Just used VBO 'node save' option and I geocoded 88 nodes in about 10 seconds! Superb! Just made my evening.

Status: Needs review » Closed (fixed)

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

rolkos’s picture

Status: Closed (fixed) » Needs work

Problem 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

TBarina’s picture

Issue summary: View changes

Subscribe #33

aanjaneyam’s picture

The 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.

aanjaneyam’s picture

Anyways 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.

BoySherman’s picture

I'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.

BoySherman’s picture

Status: Needs work » Needs review
kopeboy’s picture

Please include it in a release!

SocialNicheGuru’s picture

this 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

 if ($settings['geocoder_presave']) {

to

 if (isset($settings['geocoder_presave']) ){
NWOM’s picture

Status: Needs review » Needs work

#37 combined with the fix in #40 fixed the issue I was having with Feeds.

My Setup:

  • Organization Entities via Redhen Module
  • 8600 Entities
  • Import with Feeds via Redhen Feeds module
  • Geocoded with Bing
  • Geocoded using an Addressfield that includes Country

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.

discipolo’s picture

Status: Needs work » Needs review
FileSize
3.47 KB

reroll including change from #40

joelpittet’s picture

Should 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.

discipolo’s picture

i 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)

discipolo’s picture

FileSize
6.54 KB
discipolo’s picture

FileSize
6.67 KB
discipolo’s picture

discipolo’s picture

sorry, will stop now

Pol’s picture

Hi,

What's the status of this patch ?

joelpittet’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

@Pol it was "needs review". Now it's "needs work" because it doesn't apply due to recent changes.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
6.58 KB

Here's a re-roll.

Pol’s picture

Thanks!

Are these drush_print_r() necessary ?

Pol’s picture

Status: Needs review » Needs work
joelpittet’s picture

Status: Needs work » Needs review
FileSize
5.47 KB

They are not:) I've removed those and re-rolled again with latest dev.

+++ b/geocoder.widget.inc
@@ -248,7 +268,7 @@ function geocoder_widget_get_field_value($entity_type, $field_instance, $entity
-      if (!empty($entity->original)) {
+      if (!empty($entity->original) && !empty($entity->original->$field_instance['field_name'])) {

This hunk didn't exist anymore.

joelpittet’s picture

Here's something of an interdiff as it was a re-roll + changes the interdiff was kinda worthless.

NWOM’s picture

#54 applied cleanly to the newest dev and works as expected. Thank you!

Pol’s picture

Status: Needs review » Closed (outdated)
NWOM’s picture

Status: Closed (outdated) » Active

#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?

NWOM’s picture

Status: Active » Needs review
joelpittet’s picture

Here's a re-roll.

minorOffense’s picture

Reroll for latest dev. Avoid duplicate declaration.

Anybody’s picture

Well the same will be needed for Drupal 8 as follow-up...

gturnbull’s picture

Version: 7.x-1.x-dev » 7.x-1.4
FileSize
4.67 KB

Hi 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

NWOM’s picture

Version: 7.x-1.4 » 7.x-1.x-dev

Patches 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.

gturnbull’s picture

Hi 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

gturnbull’s picture

I'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

gturnbull’s picture

Reroll for latest dev.

MariaIoann’s picture

I 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 catch

FiNeX’s picture

Hi, the patch works fine, thankyou :-)