Allow Feeds to use existing Data table [#982242]

Would it be feasible for Feeds to map sources to the fields of an existing Data table? AFAICS currently the table needs to be created in the process with the names of the resulting database columns depending on the source name. This can result in quite meaningless names (see #982102: Labels/field names become meaningless with Data Processor).

The mapping with the Node Processor is much nicer, having node and CCK fields available as targets with their names intact, not dependent on the name of the source.

So, one could choose one from all available Data tables on the Data Processor Settings screen, then the fields of that table would be available in the target dropdown on the mappings screen.

Any interest?

Comment	File	Size	Author
#9	create_new_feeds_table-982242-9.patch	6.7 KB	nicolash
#5	feeds_data_select_table_2.patch	1.98 KB	jcmarco
#3	feeds_data_select_table.patch	1.88 KB	jcmarco

Comments

Comment #1

imclean commented 10 January 2011 at 00:53

Sounds like a very useful feature. Being able to name the target independently of the source on the mappings screen could be handy as well.

I think you can get around this by altering a data table after it has been created by the Feeds module. Any new columns added should appear in the target dropdown on the mappings screen.

Comment #2

nicolash commented 25 January 2011 at 09:04

Yeah...I discovered that bit by bit....then rebuilt all my data tables for feeds, thanks for the tip, though. Having tons of columns in the database named xpathparserN was just wrong.

Glad it's possible already in this roundabout way. But I think it supports this feature request, since feeds and data mapping is obviously working fine already, it's just making the initial connection between the two that is somewhat confusing.

If anybody with commit rights would indicate that they'd support this, I'd look into it.

Comment #3

jcmarco commented 24 February 2011 at 22:38

Status:

Active

» Needs review

Status	File	Size
new	feeds_data_select_table.patch	1.88 KB

This patch add the option to select the data table to use with the data processor.
It respects existing default settings.

Comment #4

nicolash commented 26 February 2011 at 06:19

Nice work! Functionally working for me...used the Planet Drupal RSS which was easy to map to a pre-made data table. Cool that it took so little coding to achieve it...

One thing, I ran the test "RSS import to data records" and it had fails with the patch applied. Haven't looked into it in more detail so far...

Comment #5

jcmarco commented 26 February 2011 at 11:10

Status	File	Size
new	feeds_data_select_table_2.patch	1.98 KB

Problem solved.
Now it respects existing feeds with default values.
The problem was that the new table_name variable only is set when saving the settings form.
Now all tests pass fine and any existing defined feed stand working as previously.

Please test it again.

Comment #6

nicolash commented 26 February 2011 at 14:50

Awesome, tests running fine. Noticed one more thing, though....when trying to delete all items in the feed, it doesn't work. This may well be the same problem.

Thanks again for your work on this.

Comment #7

nicolash commented 28 February 2011 at 10:59

Ok, feeds expects the feed_nid column to be in the table if items are to be deleted (the query generated in the delete() method in FeedsDataHandler.inc won't work, otherwise). That makes sense, since you might have several feeds being connected to the same table...and you'd only want to clear the items of the particular feed.

That also showed the issue that there needs to be a timestamp column for the expiry process to still to work.

So the process of claiming an existing table is a bit more complex. Feeds needs to add these 2 fields when hooked up to a table.

In fact, it should add the baseSchema() to what's already there...need to look at it some more, but I guess that would happen in the table() method.

Comment #8

jcmarco commented 28 February 2011 at 13:02

Good analysis, I was checking the tableName function and I didn't find anything wrong to not allow deleting content from any customized table.
Probably your last finding it is a new issue.
Let's see if the module maintainer accept in this new feature (define custom tables for data table) and later we could try to fix any other issue with the Data table features.

Comment #9

nicolash commented 2 March 2011 at 10:50

Status:

Needs review

» Needs work

Status	File	Size
new	create_new_feeds_table-982242-9.patch	6.7 KB

Yeah, but feeds really needs those fields in the table to manage it properly. It could otherwise never delete existing items and/or expire them. In case it's not clear what I mean, go to import/[YOUR FEED]/delete-items for the deletion part. That's a core part of feeds functionality...we have to somehow solve it.

What needs to happen is to add these fields when a data table gets hooked up to feeds (when the config form gets submitted). I've attached a patch that takes the schema of the existing data table, then clones it and adds the base schema of feed_nid, id, timestamp.

A few issues to consider:

Should the old data table be dropped?
Not possible to have feeds contributing to an existing table that already has data
Possible clashes between schemas?

This would definitely need more work, I'm just throwing the idea out there....let me know what you think. Also, I just started toying around with the new Git way of doing things...hope that patch applies ok :)