Geocoding from field should wait for complete entity validation

@mongolito404: thank you! I prefer this method as well. If you can reroll this commit to apply cleanly against the renamed module ill commit asap. Others seem to be arguing to keep both methods but i don't see much value in that (though i may be missing something).

Please do!!! That really bothered me with the module. We ended up having to write separate code to encode outside of the UI. Especially with google's API limits.

I went ahead and made a git-formatted patch from mongolito404's github commit. I only changed the names to geocoder, and some minor coding convention stuff… have not tested =)

Here is the patch from mongolito404's commit

Same patch with a fix for widget type name

Would this patch also fix the problem that nodes created programatically or saved via batch (vbo) don't get geocoded. As they do geocode when saving the node manually.

Patch didn't work for me though, I now get errors when saving manually:

Warning: Illegal offset type in isset or empty in field_info_field() (line 617 of modules/field/field.info.inc).
Warning: Illegal offset type in isset or empty in field_language() (line 294 of modules/field/field.multilingual.inc).
Warning: Illegal offset type in isset or empty in field_language() (line 294 of modules/field/field.multilingual.inc).
Notice: Array to string conversion in field_get_items() (line 972 of modules/field/field.module).
Notice: Undefined variable: values in geocoder_field_attach_submit() (line 330 of modules/contrib/geocoder/geocoder.module).
Warning: array_intersect_key(): Argument #1 is not an array in geocoder_field_attach_submit() (line 330 of modules/contrib/geocoder/geocoder.module).

rerolled this patch to patch cleanly against the latest. Additionally there were a few problems, the ones HnLn had spoke of, the fact that setting the form values in the hook_widget_validate() were being overwritten by the hook_field_presave in the geofield module. Which meant the 'geocode' => TRUE, would never stick.

~ line 61 - 62 in geofield.module fixes the issue (patch attached)

not sure of the implications of this, however so far i haven't noticed any adverse effects.

        $item += array('master_column' => $master_column);
        geofield_compute_values($item, $item['master_column']);
        $items[$delta] += $item; // changing this from $items[$delta] = $item to a `merge`

** note i've only tested this with the addressfield module, so it's the only one i've confirmed it working with.

was a bit hasty with the above patch, here's a new one.

Last one didn't work for me, errors are gone indeed but now I get 0,0 coordinates (and that's not where my address is :-).

This wasn't working for me for programmatic saving. I'm not sure I follow the logic that was in there for marking geocoded fields for encoding, it seems like we need to try to encode all properly configured fields in case data changed in other fields. This patch seems to be working for us and I think it should successfully apply on top of 7.x-1.x

Thanks for this! I'm going to see if I can get this implement and working correctly right now.

Commited! Thanks for the patch all!

Hope it's ok to reopen this.

@phayes, in #14 you say that the patch is committed, but I'm not seeing anything about presave in geocoder.module for 7.x-1.2

Am I looking in the wrong place?

Thanks.

When an adressfield is updated/changed, there is no update for the geocode field related.

I guess it should be updated automatically, without having to hook alter the update form and force a new geocode. Right ?

I thought it could be related with this patch, but I cannot apply #12 patch to current 7.x-1.2