Add support for Money as a Feeds mapping target
smokris - November 10, 2009 - 04:55
| Project: | Money CCK field |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Description
Attached is a patch that provides the Money CCK field as a mapping target for http://drupal.org/project/feeds (the successor to FeedAPI).
Caveat: only the 'amount' value is currently exposed; for this to work, a default currency needs to be specified for the content type.
| Attachment | Size |
|---|---|
| money-feeds-00.diff | 1.68 KB |
#1
CCK fields may have several columns, and I think the feeds mapper should support that, either allowing the user to choose from different sources, or allowing them specify defaults. So when money_feeds_set_target() is invoked. the currency should be provided in $value argument.
Other than that, I would happily commit this. One thing I would "touch" tough is that it should be implemented on a separate file, just like diff integration in money module.
#2
Ah, good point. I moved it to a separate file (of course, rename it to take off the ".txt" extension and remove the funky underscores drupal.org added).
Since we have two columns, we need two mappings (one field for amount, one field for currency). I've modified the code to do this. How does this look?
#3
Looks better. Thanks
There are a few things that concern me. I looked briefly at feeds module code, and it seems to me the node is not being validated after the mappers have populated content, and so the feed could inject data that is not valid. This is specially important with CCK fields, or any other data that limits allowed values to a set, or that have other specific validators. For example:
1) Money fields allow users specify allowed list of currencies, so we should not accept a currency that is not allowed.
2) Users should be able to map the amount and the currency. Both are tied together. Money field does not allow amounts without currency.
3) CCK fields could be configured to be required, so this check should be enforced here somehow.
4) CCK field may or may not accept multiple values, or a fixed number of multiple values. This check should also be enforced here somehow.
Before committing support for feeds to money module, I would appreciate some feedback on that. Otherwise, I may end up getting support request and bug reports for something that I have no control, and lack of time too.
Keeping the patch here still allows users to test. And once this is considered stable, then I would happily commit. :)
#4
This patch needs work, but I think Feeds module integration with CCK also needs work. Otherwise, it's a potential source of inconsistencies, hence unexpected behavior. If invalid data is saved with the node, then there may be modules that do not work as expected. Feeds module should validate the node before saving.