Port the location mapper from Feed Element Mapper.
Port tests for it.

I'm going to be frank: I am sceptical on including Location support in Feeds as the location mapper has caused a series of very confused and noisy threads on FeedAPI Mapper. I'm tired of accepting an initial patch and then somehow being responsible of keeping the functionality alive.

I can be persuaded. Alternatively, location mapper support could live in location module or in a separate location_feeds module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex_b’s picture

As an example for the API, take a look at mappers/content.inc should be used. For documenting the Mapping API, I opened an issue #623466: Document mapping API.

velosol’s picture

Is this solely about mapping to Location fields, or does it include (or is only) CCK-location fields? e.g. #416052: Mapper loses location targets

pbuyle’s picture

I need this for my project.

For this project I use a custom parser to read items from XML files. At the moment, this parser provides a single "location" source. For an item, this source's value is an associative array with all the location subfields (name, street, postal_code, etc.). So I've a simple mapper that simply assign the source's value to the target field. Of course, this is not useful to anybody outside my project since parsers usually output all the subfields as separate source.

If my code in #623444: Port FeedAPI Mappers: link (mostly my way to handle subfileds in targets) is Ok, I could start working on a re-usable mapper using separate targets for location subfields.

Summit’s picture

Yes please, a location mapper on feeds would be great!

Greetings, Martijn

BenK’s picture

Subscribing....

JayCally’s picture

mongolito404 - Could you show the code you used to map location cck fields? I'm trying to map location fields and it may be helpful to see how you solved it.

JayCally’s picture

FileSize
1.56 KB

mongolito404: I used your link mapper code to put together an email mapper quiet easily. I'm going to see if I can use it as a base for mapping location fields. Attached is the email mapper if anyone wants it. Unzip it and drag it to feeds/mappers/

Summit’s picture

Hi, any progress in location mapper for feeds please?
I need it to move my site from feedapi to feeds
Thanks a lot in advance for your effort!

Greetings, Martijn

pbuyle’s picture

Summit,

No progress for a proper Location mapper. I had to move to other works for the clients and don't have the time to update my working but specific code. Here is this specific code. As you can see, it expect to parser to provide the location as an structured array compatible with Location.


/**
 * Implementation of hook_feeds_node_processor_targets_alter().
 */
function location_feeds_node_processor_targets_alter($targets, $content_type) {
  $info = content_types($content_type);
  $fields = array();
  if (isset($info['fields']) && count($info['fields'])) {
    foreach ($info['fields'] as $field_name => $field) {
      if (in_array($field['type'], array('location'))) {
        $fields[$field_name] = isset($field['widget']['label']) ? $field['widget']['label'] : $field_name;
      }
    }
  }
  foreach ($fields as $k => $name) {
    $targets[$k] = array(
      'name' => $name,
      'callback' => 'location_feeds_set_target',
      'description' => t('The CCK !name location of the node (location).', array('!name' => $name)),
    );
  }
}

/**
 * Callback to set location field from Feeds source.
 * 
 * This should be handled by the Location or the Feeds module.
 * 
 * @see location_feeds_node_processor_targets_alter().
 */
function location_feeds_set_target($node, $target, $value) {
	if(is_array($value)) {
		$node->{$target}[0] = $value;
	} else if(is_numeric($value)) {
		$node->{$target}[0]['lid'] = location_load_location($value);
	}
}

Summit’s picture

Ok, thanks!
Can anyone step in to try to make a testing scenario? it would be great if location could be supported.
I noticed one thing, which is difficult with location, and that is that location itself want to change the latitude and longtitude if it is already provided in the feed. See this remark: http://groups.drupal.org/node/27864

Greetings,
Martijn

stefan81’s picture

would love to see this too.
As I need to import events and map with with cck node location.

@Summit: is there a code which could be used to test?
I am on a project where I could test it right away.
I just downloaded the module, so I also would appreciate some guidelines where to insert it. Thanks ;)

Summit’s picture

Hi,

You could try to enhance the code in #9 (http://drupal.org/node/623428#comment-2524878).
In feeds I do not know exactly, but I would expect you can insert it where also the taxonomy-mapping code is placed..please search yourself through the code, or ask alex.
I do not have time now to test.

Greetings,
Martijn

JayCally’s picture

I was able to get location cck fields to work but not to the "non-cck" location fields. This is fine but now is causing some issues with the openlayers module. Openlayers uses the non-cck fields to display points on a map but not the cck field.

stefan81’s picture

Hi JayCally, may I ask you to share your working mapper for cck location with us?

By the way, thank you very much for the email mapper posted in #7, it works fine :)

Summit’s picture

Hi Jay, I saw you posted an issue on Openlayers, great looking forward to feedback there:http://drupal.org/node/698004
But anyhow it would be great to have a regular location field mapper and a cck-location field mapper. Because different modules, uses different sorts.
greetings, Martijn

JayCally’s picture

FileSize
1.79 KB

yanku: Ya, I can upload what I've done but I've only written it to map to the fields I needed. But you should be able to add what you need to it. Its probably not the best solution but it works. Feel free to improve it.

Summit: I was able to map to cck location fields but OpenLayers wants to use the node location fields and I haven't been able to get that working. I agree, there needs to be one for the "standard" location fields and one for cck-location fields. More can be added to the cck-locations mapper I have but I'm not sure how to proceed on location field mapper.

alex_b’s picture

#16: http://drupal.org/patch/create :-) for adding new files, use cvsdo (Google)

JayCally’s picture

Sorry alex_b. I haven't made one yet but will once I get back from vacation. FIles are at work.

As a side note for getting the cck location fields to work with a view, the is a nice patch for locations on the below thread.
http://drupal.org/node/391160#comment-2333860

It works very well so you could just map location data to location cck fields. I'll try and get that patch made when I get back.

Anonymous’s picture

@JayCally: Thanks for making your work available!

I'm trying to use it to upload a lat/long into a CCK location field but your code only does street, city, state, zip?

Strangely enough the code comments say:

* The below functions allow lat and lon to go into location_cck - they are uncommented here
* and the related lines in customxml_parser.inc.

I don't see any lat/lon code at all though, and I can't find a customxml_parser.inc file either. Did I miss something?

Assuming all I need was some extra code I copy/pasted what you had and I added lines for latitute and longitude in both functions. Unfortunately the lat/long was not loaded.

To test that your code works I tried to load just the city information, but that did not import. I looked at the DB and the field_cache_location_lid column in my custom content type is always 0 and no new location entries appear in the location table.

I'm using the latest dev version of location and Feeds

Summit’s picture

Hi,
Any progress on this field please? I would very much like to port my feeds from feedapi+location mapper or cck location mapper to feeds, but am not able to do this yet.

Thanks a lot in advance for your reply!

greetings, Martjin

ekes’s picture

Hi, I've just been looking at this for a while. In part because of #348230: Parse GEO ical and VVENUE fields but also for other tasks. (By the way if anyone wants some code to map long/lat points to location node (not cck) there's one in the patch on comment 13 on the issue).

Anyway I'm throwing this in here for responses and thoughts:-

What I want to ask is if this can't be broadened a bit. There are a growing number of geo related modules so it would be good to make it easy to map to any of them. Openlayers CCK (v2) seems to have a mapper with it already for example http://drupalcode.org/viewvc/drupal/contributions/modules/openlayers/mod... However as I read that I'm going to have to pass it WKT. Where as for the location field I have to pass an array. Then there will be geo module and any number of others.

At its simplest this could just be making all the mappers accept an array for a point. This doesn't over complicate things, and will work for most of the feeds out in the wild. It will mean repeating the creation of wkt for a point within a few different mappers, but it's not exactly complicated. It will however restrict to only mapping points and not work for the wide variety of other geometry types that can, though less common, be in rss.

The more complicated, but extendible solution, would be to make a (set of) simple classe(s) to carry point, linestring, polygon etc. That each mapper could then get their wkt or co-ordinates from. Note: This is the second time I've come across this as a possible 'need' #737076: Document extendable clases comment #3

Somehow location would also still have to be able to handle the postal addresses too.

alex_b’s picture

#21: I'm sceptic whether different architectures (like one geo module requires WKT, the other an array, the next supports postal addresses, the other one doesn't) will be reconcilable.

But most importantly, there is a very practical problem:

Most people use one or the other geo module which leaves you with very little incentive to create a common solution. Right now, we don't even have enough demand to find a developer bringing the locations mapper patch home, how are we going to work on a common solution for a whole series of location modules?

I may be missing something, feel free to grab me on IRC to discuss this.

ekes’s picture

No probably not, I was thinking it might be overcomplicating things. I am myself more interested in spending time on a mapper to work well for geo (wkt) cck and openlayers wkt cck than location.

alex_b’s picture

#23: I'd focus on that, then.

Summit’s picture

Still +1 for location support for feeds!!

Summit’s picture

Still +1 for location support for feeds!!
There are New developers Bussy to bring location up to date.
And to repair all issues.
Greetings, Martijn

Summit’s picture

Sorry for double post. Something went wrong blogging From iPhone:(

webwriter’s picture

Subscribe. Really need to be able to map my csv files to the basic location fields (adddress, city, state, etc.)

vvackie’s picture

It would really help to have a csv map the location fields (address, city, state, zip). If anyone figures something out please post!

vlooivlerke’s picture

How can i map non cck location fields, like the standard fields that come with the location module?

Summit’s picture

Yes, I am searching for this also from november.
I can;t do it myself. I tried, but am not a good enough programmer.
Please bring location up to speed with feeds, so locative information can be used as was possible with feedapi.
greetings, Martijn

mirzu’s picture

subscribing

pdrake’s picture

FileSize
1015 bytes

#16 didn't work for me but was a good start for this modified version which works for address line 1, city, state and zip.

webwriter’s picture

Very cool, pdrake. Pardon my slowness.. but how do I implement this? Does it go in the feeds folder? I'd love to test it!

EDIT- never mind, I think it goes in the mappers folder under feeds. However, it doesn't seem to work with the Location module.

Summit’s picture

Hi,

As stated in #16, it's about cck location. But I am still very, very interested in a location module option also!

greetings, Martijn

clemens.tolboom’s picture

Status: Active » Needs review
FileSize
4.42 KB

I've created a patch based on the work from #16 but split into two inc files.
- location.inc
- location_cck.inc

The location latitudes/longitudes work for my test feed http://xml.weather.yahoo.com/forecastrss?p=USCA1116 which has location.

I tested location.inc better than location_cck.inc.

There's a caching problem when trying to test. I'm still not sure how to shake this cache. empty cache is not working always. Editing the file resets the importer fields.

I need some feeds with multiple locations, location names, city, etc. Please supply these feeds

clemens.tolboom’s picture

FileSize
4.95 KB

This patch has country included for location.module.

clemens.tolboom’s picture

FileSize
3.64 KB
4.45 KB

I merged both files back to just location.inc ... the code was almost similar.

My test case is a csv file containing 20 records with country _code_ , province name which I import
- country names are not working ... just use the iso country codes!
- province names are chopped to 1 character

I have tested location.module a little. My CCK Location gives errors and stay empty :(

I've attached the location.inc_.txt file for simple test. Place this file into feeds/mappers directory.

clemens.tolboom’s picture

Category: task » feature
webwriter’s picture

I can see the targets!! I think it works! Pulling together an import now- will post back results. Very exciting!

Works like a charm- clemens.tolboom you are my hero...

alex_b’s picture

Status: Needs review » Needs work

Great. From my perspective as maintainer, in order to get this committed two things are missing:

- Please observe coding standards (I see a lot of tabs instead of spaces in the patch)
- Needs tests http://drupal.org/simpletest

webwriter’s picture

Just a quick FYI... when I do an import, everything works beautifully. Then I do my next import of the same content type and it works beautifully... but all of my previously imported content for that type is deleted. I don't know it that's a location thing or maybe I'm doing something wrong... from one to the next is totally different stuff, different taxonomy, titles, locations, etc. but the same content type and mapping settings.

clemens.tolboom’s picture

FileSize
4.13 KB

I fixed the tabs ... sorry about that.

Writing test? I'd love to but
- location_cck.module is way too buggy (not working) on my site :( so I do not use that one.
- location.module: Hope someone could prepare one that create the content type with location variables min/max set.

I do understand the feeds content test so reading some csv files would be no problem :)

(thanks for the feeds module)

clemens.tolboom’s picture

Status: Needs work » Needs review

Please add some tests while reviewing.

alex_b’s picture

Status: Needs review » Needs work

#43 looks good. array('!label' => $sub_field) - needs filtering (use @label).

Tests that assert the most important mapping operations with location are needed. Setting to NW therefore.

dadderley’s picture

@clemens.tolboom

I used your patch (http://drupal.org/node/623428#comment-2896694) on feeds-6.x-1.0-alpha14.
I successfully imported 350 nodes with full location data into one of our sites.

Interestingly enough the data came from a D5 site that I upgraded to D6.
I then used views_bonus to create the CSV file that I imported.
Jeeze, what a process.

Drupal complained a bit and produced the error messages below, but happily imported all the data.

Thanks, this is much needed functionality.

Warning: Call-time pass-by-reference has been deprecated; If you would like to pass it by reference, modify the declaration of [runtime function name](). If you would like to enable call-time pass-by-reference, you can set allow_call_time_pass_reference to true in your INI file in /home/vvvv/vvvvv.org/sites/all/modules/feeds/mappers/location.inc on line 38

Warning: Call-time pass-by-reference has been deprecated; If you would like to pass it by reference, modify the declaration of [runtime function name](). If you would like to enable call-time pass-by-reference, you can set allow_call_time_pass_reference to true in your INI file in /home/vvvv/vvvvv.org/sites/all/modules/feeds/mappers/location.inc on line 49
clemens.tolboom’s picture

FileSize
4.09 KB

@dougzilla: thanks for testing ... that ref shouldn't be there :(

We still need some tests.

dadderley’s picture

It was a test, but in a real world situation.
It was very necessary to import all of the nodes with the location data.

I should do the patch and try it again in a test environment.
Thanks for your work.

clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
4.08 KB

Hold your horses ... the previous patch had no changes.

This patch removes the & ref call (#46) and changed the t-function params as per #45

I don't have time left for adding test cases ... hope someone else will jump in.

@dougzilla: in #46 you mentioned full location data. Could you attach test data to this issue. That would make writing test a little easier :)

alex_b’s picture

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

Allow me to set this to 'NW' as long as there are no tests...

elliotttf’s picture

FileSize
2.33 KB

I've updated this patch to support user locations as well. Will try to write tests this week.

elliotttf’s picture

FileSize
8.33 KB

Whoops, forgot to add the location mapper. Use this patch instead.

clemens.tolboom’s picture

Nice addition ... but there is a lot of DRY (don't repeat yourself) in the code now.

Could you split of the common code in location_feeds_user_processor_targets_alter and location_feeds_user_processor_targets_alter
Same goes for location_feeds_set_target and location_feeds_set_user_target

I'll try to test your patch after the test are added ... I'm curious about those too :)

elliotttf’s picture

FileSize
14.98 KB

Ok, here's another patch with a baseline of the test for nodes and some of the refactoring requested from above. I need to do another pass at the refactoring but don't have time to continue tonight so if someone else wants to pick up where I left off...

I had issues running the test (I think a problem with the location module: #724700: Fatal Error 'WithinMarginExpectation') but this is my first time doing anything with tests anyway so if someone wants to give it a try and let me know if it even works we can go from there.

DanielJohnston’s picture

Subscribing. Keen on this getting across as Feeds appears to allow imports by non-administrators.

elliotttf’s picture

FileSize
13.16 KB

Here's the rest of the refactoring.

To get past this location test error (#724700: Fatal Error 'WithinMarginExpectation') you can comment out the offending class and calls to it in the location module's test files.

The groundwork for the test is laid but either the content type isn't be created correctly or the additional location fields aren't being made available in the test. Once that's sorted I think we'll be good to go on this. If someone wants to take a look at the test and let me know where I went wrong that will expedite this whole process.

elliotttf’s picture

FileSize
13.08 KB

In my own use, I found a problem with the CCK targets. If the location had a label this would be used on the node object. Rather than try add a bunch of code to handle the CCK case I'm just printing the field name. Here's a quick fix to a problem with the CCK targets.

elliotttf’s picture

Status: Needs work » Needs review
FileSize
20.32 KB

Finally had time to get the tests working.

Here's the whole shebang.

clemens.tolboom’s picture

@alex_b : I feel indeed somewhat uncomfortable regarding maintaining this patch ... maybe the location module should get this patch instead of the feeds module.

@elliottf : My two cents (no time to apply and test :( )

The test data is too limited. We need more fields like latitude and longitude.

If time permit I'll create a test file of http://earthquake.usgs.gov/earthquakes/catalogs/1day-M2.5.xml which is working for me when importing.

We need to tell what version of location module we use. I use ie location-6.x-3.1-rc1

alex_b’s picture

maybe the location module should get this patch instead of the feeds module.

This is a possibility. It's what I am doing with geotaxonomy module.

elliotttf’s picture

Yeah, as this patch has gotten more complex I've thought making it part of location might make more sense too.

The piece to allow user field mappers would still (obviously) need to be part of feeds, but assuming the location maintainers agree this is important it probably should go there.

I suppose worst case this would be it's own module but that seems kind of silly since it's so small.

As for making the imports more comprehensive, it should be as simple as adding fields to the csv and mappers and updating the code on what's expected, nothing crazy.

elliotttf’s picture

Actually, on second thought, maybe making this its own module (like Alex originally suggested...) would make sense. This relies about equally on location and feeds so putting the functionality into either one seems like it would require the maintainer of that module to be overly committed to the other module's changes (also like Alex originally suggested).

elliotttf’s picture

FileSize
2.31 KB

Based on this discussion I've gone ahead and created the location_feeds module from the most recent location mapper patch. In order to support user locations feeds will still need the following patch.

alex_b’s picture

Title: Port FeedAPI Mappers: Location » Provide hook_feeds_user_processor_targets_alter()

Great. Adjusting title.

alex_b’s picture

Status: Needs review » Closed (duplicate)
alex_b’s picture

Issue tags: -Needs tests

Remove tag.