As mappers become more sophisticated and cover more cases, it is becoming necessary to allow per-importer, per-field configuration of each mapper. For example:
* For date mapper, define the expected input format
* For node reference mapper, define the field used for matching

Is such a feature on the roadmap? If not, what would be the best approach to implement it? Initially, I would think that mappers should be rewritten as plugins - does that make sense?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Hanno’s picture

Are the field settings not already managed in the node type?

Hanno’s picture

infojunkie’s picture

I meant the field import settings, as in the examples above.

TonyK’s picture

It definitely should be there. One possible use is to have a setting for cck field mappers to add another value to multi-value field rather than rewrite it. Currently you have to write additional mapper for each field type to accomplish it.

Hanno’s picture

I see, makes sense. But what about the settings in node type fields:
- default values
- allowed values
- input format
- required

Do we want to have two places to define field handling?
Can we extend the manage fields interface in the node type interface?
Or can we copy some relevant configuration values to the Feeds mapper?

@TonyK in that specific usecase ideally the mapper should check which values are set by this mapper itself in a former session and changed in the source. In that case the value should get replaced, otherwise it should get added.

alex_b’s picture

infojunkie - I have no plans to implement a 'heavier' mapping API.

This would be quite some work :-) Do you have a need/the time for tackling it?

infojunkie’s picture

@alex_b: I don't have the immediate need right now. But given that most of my projects involve Feeds, I wouldn't be surprised that one will require it soon.

alex_b’s picture

Let's talk when the need comes up.

TonyK’s picture

@Hanno, no I didn't mean two sessions of the same mapper. Imagine a feed with two links in each item and a content type with multi-value CCK link field. When I assign two mappers to one field the latter replaces the former (they are executed in the order they are listed on processor settings page so an ability to reorder them would also be nice). I would like to be able to set the second mapper to add another value to the field, not replace it. It will allow users to make more complex logic without writing their own processors.

Hanno’s picture

@TonyK I see, but that should become the actual behavior. I think the date mapper handles this a simular way (not tested though). When the time element is in another field as the date, the mapper combines the date and time for the date field.
In the same way, multiple values should be stacked and returned as an array when there are multiple source fields for a target and the target field accept multiple values. No configuration needed.

alex_b’s picture

#9 - that behavior should be possible with the current API. It's just the mappers that replace values rather than appending to existing values.

twistor’s picture

Version: 6.x-1.x-dev » 7.x-2.x-dev

Adding configuration to mappers is an interesting idea. I think it would definitely be useful, but a large undertaking. I'd be willing to put in some work if we can come up with a design. It would simplify some of my other projects quite a bit I think.

#10, I agree, it should be the default behavior to append fields rather than overwrite them.

eiriksm’s picture

Any solutions to this?
i have a feed with multiple values i would want in a multiple value field.
ie.

 <item>
  <somefield>
    <value1>12-12-12</value1>
    <value2>12-11-10</value2>
  </somefield>
 </item>

value1 and value2 should both be added to a field named "somefield", which is a multi-value field. as of now only one value is added. for the record, this is a datefield and it is configured so that the feed matches the field format.

any ideas?

EDIT: I just tried to map to a plain text field, and this works perfectly. all values are added to seperate field values of one multifield. so i am wondering if the issue is in the date module? or is it the date handling in feeds?

NealB-1’s picture

It seems to me that configuration for mappers is desirable and will be necessary to do a lot of field types properly.

Ex: There is no single canonical way to import an international phone number. You could decode the country code from the digits, you could infer a country based on a default, or you could get the country some other way. This should be configurable.

I have a need for handling of multi-value fields that's different from the one implemented in Feeds. Changing the default behavior is currently difficult.

Some things that currently require Feeds Tamper, such as mapping taxonomy terms that don't perfectly match the local versions, could be part of the mapper config.

eiriksm’s picture

Although i thought my problem (#13) was related i found out this was related to dates as arrays. If you have the same problem: dig in half a day (like me) and code it yourself or simply see this (duh, it was already an issue)

#697842: Support array of values for dates

Niklas Fiekas’s picture

Subscribe.
This would be really useful.

Niklas Fiekas’s picture

Assigned: Unassigned » Niklas Fiekas
Status: Active » Needs review
FileSize
12.02 KB

Now, that I'll need it, I'll spend some time working on it.

The attached patch works already (at least for me), but it is WIP. Please review.

Changes to feeds.api.php aren't included in this version. Basically it adds form_callback and summary_callback to the targets array, so that modules can define their setting forms. The settings will be passes as a fifth parameter to the mapping callback. All the changes should be backwards compatible.

Niklas Fiekas’s picture

Missed a whitespace error.

twistor’s picture

Awesome! Will be checking out shortly.

colin_young’s picture

I installed the patch last night (against the dev branch from git). Everything looks like it went okay. I replaced the "supported" feeds with the patched version and ran update, which also appears to have been successful. Feeds is still working, but I don't see anything different in the mappings. I'm assuming I need to write a plugin or something? Is there an example to look at?

Thanks.

Colin

Niklas Fiekas’s picture

Yes, it is mostly for other modules to extend.

You can see one thing, because I replaced the unique flags with settings forms like that. Both as a demo, because it feels right and because there has to be enough room for the setting forms. Screenshots attached.

colin_young’s picture

Thanks. I'll continue over in #1262732: Map to existing media? then.

colin_young’s picture

Do you have any details available on implementing your own mappers? I've got some custom taxonomies that have additional fields. I'd like to create a mapper that automatically picks the vocabulary based on the one specified by the node target, and allows you to specify which field in that vocabulary you want to use for the mapping. e.g. I have vocab Stone with Name = 'Diamond', Description = '', Code (custom) = '01' and my incoming feed needs to map '01' to that term.

Thanks.

Niklas Fiekas’s picture

To solve this on a mapping level you can implement hook_feeds_processor_targets_alter and expose custom mappers. See feeds/mappers/ for examples.

My patch adds a form callback and summary callback property to the processor targets, that can be defined.

Despite that you could - of course - use those properties, this is slightly off topic. Where could something like this be discussed best? groups.drupal.org? Forum? Private mail?

colin_young’s picture

I'm trying to figure out how to implement the form callback so I can provide a form like you've done with the #1262732: Map to existing media?. I was really trying to get some documentation for this patch started, but seeing as the Feeds documentation is a bit of a WIP, expecting to start some documentation for a WIP patch to feeds might be getting a bit ahead of things.

Niklas Fiekas’s picture

Ah, ok. The attached patch adds an example to feeds.api.php. Probably we should discuss the API anyway, before reviewing the giant patch for the corresponding UI.

Thank you for trying this and please let me know if it works or how the API could be changed to make this more useful.

colin_young’s picture

Just wanted to check in and let you know I haven't forgotten about this item. I've been prettying up the site and dealing with requirement changes, but I'm back to the feeds now. I'm just doing a quick and dirty implementation with feeds_tamper, but I'm trying to make it modular enough that it will be easier to port later :)

I've been thinking though that there is potentially some overlap between this patch and feeds_tamper. At the moment, the biggest difference seems to be that feeds_tamper easily allows attaching multiple plugins to each mapping, but at the cost of a less integrated workflow. I wonder if it wouldn't be possible to leverage the feeds_tamper plugins and provide some additional UI and API enhancement to allow attaching multiple existing feeds_tamper plugins to each mapping.

In essence, I'm suggesting that per-field mapping configuration be promoted to a first-class member of feeds, and effectively converting feeds_tamper to a collection of useful plugins for modifying individual mappings.

twistor’s picture

Reviewed it and love it!
The first use case that comes to mind is taking something like #724536: Mapper for nodereference field in Drupal 6 where you have a target for GUID, URL, title, nid and making those config options instead.

@Niklas, one critique that I have, albeit minor, is that the form shouldn't be optional_unique or custom, but add the custom form to the optional_unique form when mappings are marked as optionally unique.

@Colin, converting feeds_tamper plugins to use the mapping config would be difficult simply on the basis that there's not enough room. Another problem would be that there's no nice way to form_alter all of the custom forms that would exist. I will be thinking about this much more however.
As far as integration goes, the next version of Feeds will allow a tighter-feel thanks to #1235394: Use more Drupal-y paths.. Feeds Tamper will become a tab on the importer configuration page.

I love that it's backwards compatible, provides enhancement, without needing any changes to supporting modules if they don't care.

Here's hoping this issue goes somewhere!

franz’s picture

Assigned: Unassigned » Niklas Fiekas

This is a wonderful feature that will aid us in #1107522: Framework for expected behavior when importing empty/blank values + text field fix. With this, all mapper could include a setting for the empty/blank behavior.

madeby’s picture

@franz - this could really help a great lot and make the module so much more useful.

What is the status of all this?

franz’s picture

@madeby, we just need some more reviewing and testing.

Niklas Fiekas’s picture

Oh ... sorry. I lost track of this. When talking to twistor in October the plan was that I'd implement the feedback in #28 real quick, but I forgot it and it never happend.

Here's a reroll, solving a merge conflict in the API docs and doing that: The optional unique form will now be merged in, instead of overriden by custom mapping config.

madeby’s picture

Applied the patch but I can't seem to find a way to configure each mapping? How to I access the new features?

franz’s picture

By default, I think the only possible configuration available is the "unique" one for 'guid' and other targets. Have you tried to use this mapper?

madeby’s picture

But wasn't that already there? Nothing new to see. I can set GUID to a unique target just as before.

franz’s picture

@madeby, that setting was just converted to the new system, which provides an API for other settings as well, and a new UI, similar do Field UI display settings.

franz’s picture

BTW, I think we need tests for the API. Some common form elements and such.

Niklas Fiekas’s picture

Assigned: Niklas Fiekas » Unassigned

Yeah. Tests would be awesome. Not for fully automated testing on d.o, until the Feeds testsuite passes, but to locally check everything is fine. Media Feeds (a module that I maintain) uses the upcoming API, too, prematurely. That works.

That said: I am not going to write the tests soon. Unassigning, in case someone else wants to try.

madeby’s picture

I really really need the feature were I can control what happens with empty entries. I would love to test that but lack the coding skills to code it myself.

franz’s picture

Assigned: Unassigned » franz

I'll work on a test for this.

franz’s picture

As I'm working on a API testing for this, we also need to fix the tests that use the 'unique' value. Also, it's not clear to me what will happen to existing importers when Feeds is updated, we need an update hook for this, or maybe something even sturdier to be backward compatible.

Niklas Fiekas’s picture

Thanks for attacking this. No update hook required, all the changes are backwards compatible. See #28.

But yes: The UI changed, so some existing tests that use the UI might need to be adjusted.

franz’s picture

Sorry, bad attention on it. I already have a feeds_test.module making use of the API, just need to add a test case now.

franz’s picture

Assigned: franz » Unassigned
FileSize
17.7 KB

Ok, I got some tests. There is an error when using textarea field, not sure if the value is being trimmed or what. Also, I get a lot of undefined index warnings for the "unique" field, this is worth fixing.

Unassiging to get others to review this.

franz’s picture

Assigned: Niklas Fiekas » Unassigned
Priority: Normal » Major

Many features are now depending on this, so I'll increase priority.

twistor’s picture

Re-roll.

twistor’s picture

Status: Needs review » Fixed

Alright, I committed this beasty.

http://drupalcode.org/project/feeds.git/commit/17269f88adb6f12075eca2bc9...

Please create new issues for followups. Thanks to all.

Status: Fixed » Closed (fixed)

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