Problem/Motivation
There is D6 fatal PHP code in this module; It calls taxonomy_node_get_terms(), which just flatly does not exist in D7.
To create the failure: Create a mapping which maps a taxonomy term from the feed definition node to a term field on the target node as in this screenshot:
Then run a feeds import, perhaps with "drush cron".
You end up with an error like
Error: Call to undefined function taxonomy_node_get_terms() in
sites/all/modules/feeds/mappers/taxonomy.inc, line 30
Proposed resolution
Several contributors have collaborated to provide a patch, rerolled in #82, and which includes a test.
Remaining tasks
Module maintainers and others should review and test the patch to make sure it does what it is supposed to and nothing else.
Original report by dasjo
taxonomy_feeds_get_source uses taxonomy_node_get_terms() which doesn't work anymore with drupal 7:
An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /batch?id=9&op=do StatusText: OK ResponseText: Fatal error: Call to undefined function taxonomy_node_get_terms() in /feeds/mappers/taxonomy.inc on line 31
this happens as taxonomies are stored in fields for drupal 7
http://drupal.org/node/909968
Comment | File | Size | Author |
---|---|---|---|
#82 | feeds.taxonomy_node_get_terms_fail_959984_82.patch | 8.14 KB | rfay |
#74 | mapping-node-processor-settings.png | 57.49 KB | hitfactory |
#67 | patch_rejects.txt | 3.15 KB | flailingmaster |
#64 | feeds__taxonomy_node_terms_fix_959984_64.patch | 8.14 KB | hitfactory |
#63 | feeds__taxonomy_node_terms_fix_959984_63.patch | 2.46 KB | hitfactory |
Comments
Comment #1
Rob T CreditAttribution: Rob T commentedI've smacked into the same issue. I'm unable to assign taxonomy terms from feeds to feed items.
This error occurs when attempting to import, after having mapped the correct term fields.
Comment #2
alex_b CreditAttribution: alex_b commentedNeeds fixing.
Comment #3
marcvangendSub. Function taxonomy_node_get_term simply doesn't exist anymore in D7, I don't know why.
Comment #4
marcvangendSee also #978242: Document the changes to taxonomy.module properly to avoid confusion about missing $node->taxonomy property.
Comment #5
meramo CreditAttribution: meramo commentedSubscribing. This is a critical issue, as it's one of the greatest feature of feeds.
Comment #6
meramo CreditAttribution: meramo commentedThe error still exists in Drupal 7 final, when trying to import items and automatically assign terms from the parent feed to them. It's easy to replicate. The error is:
An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows.
Comment #7
chegor CreditAttribution: chegor commentedSubscribing. Same problem.
Comment #8
niryariv CreditAttribution: niryariv commentedGetting the same issue... Subscribing.
Comment #9
todddevice CreditAttribution: todddevice commentedSubscribing.
Comment #10
XiaN Vizjereij CreditAttribution: XiaN Vizjereij commentedSubscribing
Comment #11
bricel CreditAttribution: bricel commentedSubscribing
Comment #12
desmondmorris CreditAttribution: desmondmorris commentedProbably not the best solution, but you can add the taxonomy_node_get_terms re-factored for D7 to a custom module or template.php. Again this is NOT the best solution, but it will get you by for now.
function taxonomy_node_get_terms($node, $key = 'tid') {
static $terms;
if (!isset($terms[$node->vid][$key])) {
$query = db_select('taxonomy_index', 'r');
$t_alias = $query->join('taxonomy_term_data', 't', 'r.tid = t.tid');
$v_alias = $query->join('taxonomy_vocabulary', 'v', 't.vid = v.vid');
$query->fields( $t_alias );
$query->condition("r.nid", $node->nid);
$result = $query->execute();
$terms[$node->vid][$key] = array();
foreach ($result as $term) {
$terms[$node->vid][$key][$term->$key] = $term;
}
}
return $terms[$node->vid][$key];
}
Comment #13
sovarn CreditAttribution: sovarn commentedThanks desmondmorris.
I managed to get the batch process to finish but when I check the node created by the feed, the taxonomy had not been updated. Did your new node manage to save the taxonomy term?
Comment #14
desmondmorris CreditAttribution: desmondmorris commentedsovarn, yes my new nodes did save the taxonomy term as expected. You must make sure that:
1. you have a taxonomy term associated to your feed
2. your feed item has a term_reference field
3. you have mapped your feed term to your feed item term in the node processor of your feed importer
Comment #15
sovarn CreditAttribution: sovarn commentedI got it working.
You have to have the feeder node to be published. This is because it will will only appear in the taxonomy-index table if the node is published.
Also putting the code in template.php didn't work as cron does not find the function if you want it to be updated.
Thanks again.
Comment #16
desmondmorris CreditAttribution: desmondmorris commentedsovarn, good catch about cron. glad I could help.
Comment #17
xmacex CreditAttribution: xmacex commentedSubscribing. I would love to see this fixed, though i do understand that Feeds is only alpha3 at this time.
Comment #18
sovarn CreditAttribution: sovarn commentedFor a quick hacked fix, put the function in #12 into the .module file or your own module and it will work fine.
Comment #19
desmondmorris CreditAttribution: desmondmorris commentedI have taken a shot at a patch for this issue. I renamed the function from #12 to "taxonomy_feeds_node_get_terms" and added it to feeds/mappers/taxonomy.inc. I then changed the call to taxonomy_node_get_terms to taxonomy_feeds_node_get_terms in the taxonomy_feeds_get_source function, also in taxonomy.inc.
Comment #20
desmondmorris CreditAttribution: desmondmorris commentedI forgot to change the status to "needs review"
Comment #21
simon_s CreditAttribution: simon_s commentedSubscribing
Comment #22
artatac CreditAttribution: artatac commentedsame here - sub
Comment #23
sw3b CreditAttribution: sw3b commentedSubscribing
Comment #24
sw3b CreditAttribution: sw3b commented#19 work great !
I notice that if the node feed is not publish the term is not transfer to the node feed item. It's not a big deal !
For now the issue look solve ! Maybe apply the fix to DEV version... Thanks !
Comment #26
Zoltán Balogh CreditAttribution: Zoltán Balogh commented#19 works fine.
Comment #27
newtoid+1 subscribing
Comment #28
trothwell CreditAttribution: trothwell commentedWhy would such a handy function be removed?
Comment #29
TimelessDomain CreditAttribution: TimelessDomain commentedcan we please commit this?
Comment #30
uxjam CreditAttribution: uxjam commentedsubscribing
Comment #31
jsdix CreditAttribution: jsdix commentedsubscribing
Comment #32
scott.whittaker CreditAttribution: scott.whittaker commentedCan anybody explain why taxonomy_node_get_terms() does not exist any more? Or why the taxonomy_index table only matches published nodes? It seems that the taxonomy functionality in D7 is vastly reduced from what it was in D6 or D5, but with no good reason. This is a definite WTF.
Comment #33
marcvangendScott: You can find an explanation for the changes in Drupal 7 in #978242: Document the changes to taxonomy.module properly to avoid confusion about missing $node->taxonomy property.
Comment #34
Dave ReidIf we are retrieving terms by a node's ID, but caching them by VID that could be confusing. Also, we should probably be only loading the tid column and then running taxonomy_term_load_multiple().
In addition, the taxonomy_index table is not maintained for any unpublished nodes. Will that become a problem here?
Powered by Dreditor.
Comment #35
Dave ReidComment #36
a.ross CreditAttribution: a.ross commentedsub
Comment #37
Dave ReidMarked #1140346: Taxonomy mapping not functioning in Feeds 7.x and #1190886: Possible to add taxonomy with node processor importing to an article? as duplicates of this issue.
Comment #38
jsdix CreditAttribution: jsdix commented#20 did work at one point. Anyone have any luck?
Comment #39
jsdix CreditAttribution: jsdix commented#20 did work at one point. Anyone have any luck?
Comment #40
a.ross CreditAttribution: a.ross commentedWhy did you change the priority and assignment?
Comment #41
Dave ReidComment #42
slashrsm CreditAttribution: slashrsm commentedThis patch follows guidelines from #34:
- nid is used as a key in static cache variable
- code that gets tids from all taxonomy fields is completely changed; it fixes issue with unpublished nodes now
Comment #43
hitfactory CreditAttribution: hitfactory commentedPreviously added tids with the same key get wiped here:
Same patch as in #42 attached but with above line replaced with:
Comment #44
Summit CreditAttribution: Summit commentedSubscribing coming from http://drupal.org/node/988856#comment-4731528
More reading gives me this quote: "Feeds already does a mapping and need to support mapping incoming terms to taxonomy reference fields". See: http://drupal.org/node/978242#comment-3748214 under feeds.
Term reference fields are supported by this patch, right?
greetings, Martijn
Comment #45
truls1502Subscribing. Same problem.
Comment #46
redndahead CreditAttribution: redndahead commentedI have tested patch #43 to work for this issue. Only issue I see is the extra whitespace that needs to be removed. But hopefully this can be done when committed.
Comment #47
Dave ReidThis still really could use a simpletest to confirm its behavior. :/
Comment #48
Anonymous (not verified) CreditAttribution: Anonymous commentedMaybe provide a
feeds_taxonomy_node_get_terms()
haha :-)
Comment #49
Swipular CreditAttribution: Swipular commentedSubscribing
Comment #50
John Bryan CreditAttribution: John Bryan commentedI'm also having this problem which is preventing me from re-implimenting D5.9 Feed Parser based solution as a "Feeds" solution on D7.8 using Feeds 7.x-2.0-alpha4 and CHAOS 7.x-1.0-rc1.
My "Feed" and "Feed Item" content types both have the same custom "Term reference" field type with identical title and field names. Every time a import attempt occurs the progress bar briefly appears saying initialising and then the above message appears.
About to try 7.x-2.x-dev 2011-Aug-14
Note: If you are trying to have RSS or XML (etc.) "Feed Item" nodes inherit a taxonomy term from the parent "Feed" importer node and seeing "An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows." then this Issue page appears to be the most relevant of several similar sounding Feeds project issues created ;¬)
Comment #51
John Bryan CreditAttribution: John Bryan commentedNo change with 7.x-2.x-dev 2011-Aug-14
Comment #52
TonyAngelo CreditAttribution: TonyAngelo commented#43 worked for me, thanks for that!
Comment #53
marcoscanosubscribe
Comment #54
arcane CreditAttribution: arcane commentedSubscribe
Comment #55
Yorgg CreditAttribution: Yorgg commentedsubscribing.
Comment #56
Jason Dean CreditAttribution: Jason Dean commentedSubscribing
Comment #57
Applebee CreditAttribution: Applebee commentedSubscribing.
Comment #58
marcvangend[off-topic] Applebee, you don't need to add a comment in order to subscribe anymore. There is now a "Follow" button at the top right of the page. [/off-topic]
Comment #59
brycesenz CreditAttribution: brycesenz commentedThe patch in #43 worked for me.
Comment #60
shenzhuxi CreditAttribution: shenzhuxi commented#43 works
Confirmed
Comment #61
NicoB CreditAttribution: NicoB commented#43 works
Thx
Comment #62
Dave ReidShould this be using drupal_static() in Drupal 7?
field_get_items() can return a non-array value. Is this condition covered here or will calling array_map() throw an error?
Trailing whitespace.
Trailing whitespace.
Comment #63
hitfactory CreditAttribution: hitfactory commentedRerolled patch from #43 with suggestions from #62.
Comment #64
hitfactory CreditAttribution: hitfactory commentedBetter still, rerolled patch from #43 with suggestions from #62 and a test.
Tweaked testInheritTaxonomy() in feeds_mapper_taxonomy.test to work for D7.
Commented out testRSSCategoriesToTaxonomy() as that also needs to be rewritten for D7.
Comment #65
flailingmaster CreditAttribution: flailingmaster commented#64 failed on a manual patch with this:
patch -p1 < feeds__taxonomy_node_terms_fix_959984_64.patch
patching file feeds.info
patching file mappers/taxonomy.inc
patching file tests/feeds_mapper_taxonomy.test
Hunk #2 FAILED at 118.
1 out of 3 hunks FAILED -- saving rejects to file tests/feeds_mapper_taxonomy.test.rej
first couple of lines of the rej file are:
*************** class FeedsMapperTaxonomyTestCase extends FeedsMapperTestCase {
*** 65,102 ****
* Test inheriting taxonomy from the feed node.
*/
Was I doing something wrong?
I reverted the changes and applied #63 successfully; testing it now.
Comment #66
hitfactory CreditAttribution: hitfactory commentedMake sure you are in the root of the module folder e.g. /feeds when you apply the patch.
Comment #67
flailingmaster CreditAttribution: flailingmaster commentedI think I'm in the correct folder; the first two files look to be patched correctly, it's the test/feeds_mapper_taxonomy.test that doesn't seem to be patching right. I'll attaching the rejects file as a txt file.
Comment #68
hitfactory CreditAttribution: hitfactory commentedAh. Looks like you're applying it to the 2.0-alpha4. You need to apply it to the latest 7.x-2.x-dev release.
Comment #69
lyy9981 CreditAttribution: lyy9981 commentedThis issue hasn't been solved after more than one year. Will a stable release of Feeds module finally fix this error? I really don't want to apply a patch.
Comment #70
dasjoit's still "needs work" as stated in #62, so you can help finalize it or just wait
Comment #71
maerys CreditAttribution: maerys commentedThe patch of #64 went through, but had no effect. I am still not able to inherit the tags from a feed to a feed item while the import of the feed items themself this time did not cause an error message.
Comment #72
hitfactory CreditAttribution: hitfactory commented@flailingmaster: Did the patch work for you?
@lyy9981: Please test the patch on a development site, if you can. It needs community feedback/review.
@maerys: Can you provide an export of your feed importer and/or provide details of your feed node -> feed node item set up?
Comment #73
maerys CreditAttribution: maerys commented@kidrobot oh, of course, sorry.
Feed importer
Both content types »feed item« and »feed« have just three fields: title, body (feed item)/newsfeed (feed) and the taxonomy field based on the same vocabulary (called »organisation« , which just has to options: »personal« or »institution«). Every feed item that is generated upon a feed should inherit the particular taxonomy tag. If a feed is tagged with organisation all the feed items automatically should have this tag as well.
Doesn't sound so difficult but it seems to be tricky. But before i patched there was an error. After patching I was able to import the feed items without error, but the tags do not come up.
Is there any other information you need?
Comment #74
hitfactory CreditAttribution: hitfactory commented@maerys, it looks like your Mapping for Node Processor settings are not right.
On admin/structure/feeds/feed/mapping you should not have any source mapping to the term reference field Organization because this will prevent the feed_item from inheriting the term from the feed node.
Can you remove that mapping so your settings look more like the attached screenshot? Then try the import again.
Comment #75
eloivaqueSubscribe
Comment #76
marcvangendeloiv, please see comment #58 and the follow button on top of this page.
Comment #77
sorensong CreditAttribution: sorensong commented+1
Comment #78
maerys CreditAttribution: maerys commentedHello @kidrobot. Thanks for answering.
Unfortunately there's a "but"... As I wrote import itself now works after patching, but the transfer of the taxonomy tag of the feed to the feed items still doesn't do its job.
Comment #79
hitfactory CreditAttribution: hitfactory commented@maerys, taxonomy terms will not be mapped from the feed node to the feed item until you remove this part of from your feed import as this makes the feed item inherit the terms from the source item instead of the feed node.
Can you remove the above from the code in your module or remove the mapping using the UI on the Mapping page?
Comment #80
maerys CreditAttribution: maerys commentedok. it's been removed, so »parent« means source and not feed node. but how do the feed item inherit the taxonomy of its feed node?
EDIT: ah, got it. there is this (new) option [http://cl.ly/3b2R1u0g3S3l0b3T382b] which I have never seen before at the mapping page and now everything works fine. thanks for your patience and great work!
Comment #81
twistor CreditAttribution: twistor commentedBumping, and tagging, as this is advertised functionality that doesn't work at all.
This just came on my radar, I'll give it a full review soon.
Will someone to update the issue summary?
Comment #82
rfayHere's #64 by @kidrobot rerolled for current code. Now I'll try it out.
Edit: I tried it out, nodes were created successfully with the taxonomy term I wanted to apply. I'm happy. I didn't look inside the patch.
Comment #83
rfayAnd updated the issue summary.
Comment #84
rfayAdding a graphic for the issue summary.
Comment #84.0
rfayAdded an issue summary.
Comment #85
emackn CreditAttribution: emackn commentedPatched #82 to dev and tests pass.
Comment #86
killtheliterate CreditAttribution: killtheliterate commentedPatch in 82 is failing against dev right now.
Comment #87
handsofaten CreditAttribution: handsofaten commentedI'm having trouble applying this patch. I did a fresh pull, and when I git apply --check on master, I get:
error: patch failed: feeds.info:23
error: feeds.info: patch does not apply
error: patch failed: mappers/taxonomy.inc:27
error: mappers/taxonomy.inc: patch does not apply
error: patch failed: tests/feeds_mapper_taxonomy.test:20
error: tests/feeds_mapper_taxonomy.test: patch does not apply
If I checkout 7.x-2.0-alpha4, I get:
error: patch failed: tests/feeds_mapper_taxonomy.test:65
error: tests/feeds_mapper_taxonomy.test: patch does not apply
Any hints?
Comment #88
hitfactory CreditAttribution: hitfactory commented@killtheliterate, @handsofaten: Try the 7.x-2.x branch. You should see something like this.
Comment #89
handsofaten CreditAttribution: handsofaten commentedThanks, kidrobot. Seems to be working.
Comment #90
marcvangendIMHO, taxonomy_feeds_node_get_terms() is something that all modules should be able to (re)use. Would there be a way to make it less feeds-specific? In an ideal world this would be in core, but I assume adding functions in core is not going to happen...
Comment #91
killtheliterate CreditAttribution: killtheliterate commentedThanks @Kidrobot. Now, I'ma see if this patched module will import into terms.
Comment #92
flashingcursor CreditAttribution: flashingcursor commentedPatch works beautifully! Thanks!
Comment #93
Liliplanet CreditAttribution: Liliplanet commentedHave applied the patch #64 several times manually, but unfortunately still receive error:
Parse error: syntax error, unexpected T_STRING in /home/public_html/sites/all/modules/feeds/mappers/taxonomy.inc on line 191
Any chance that the patch be rolled into the .dev version please?
Comment #94
emackn CreditAttribution: emackn commentedso is this RTBC?
Comment #95
rfayI think it's probably RTBC. The test request was taken care of, it's had a bit of review and a long run in the queue.
Marking #82 RTBC. It looks like it's mine, but it's really @kidrobot's, so I'll take the liberty. Again, I have not *reviewed* the patch, but I have it deployed in production (on a tiny site) and it has solved all the problems with no known side-effects.
Removing the "Needs tests" tag.
Comment #96
emackn CreditAttribution: emackn commentedcommitted to dev
Comment #98
Strompf CreditAttribution: Strompf commentedHow to apply this patch?
Trial 1
Following Kidrobot's example:
Subsequently, I copied file feeds.taxonomy_node_get_terms_fail_959984_82.patch to this freshly created directory /home/strompf/Dropbox/Projecten/Carbonbrushes/Patch/feeds. Followed by:
Trial 2
I've downloaded the latest developer's version of Feeds from the project page, and copied feeds.taxonomy_node_get_terms_fail_959984_82.patch to its root directory. The command
patch -p1 -i feeds.taxonomy_node_get_terms_fail_959984_82.patch
seems most promising, but now I get stuck on
Reversed (or previously applied) patch detected! Assume -R? [n]
After browsing patch's man pages:
After uploading, the error indeed didn't disapperar
Any help appreciated!
Comment #99
a.ross CreditAttribution: a.ross commentedErr, patch is already committed according to #96. Are you saying it isn't?
Comment #100
Strompf CreditAttribution: Strompf commentedThank you a.ross! I'm a bit new to the lingo, so I missed that comment. Actually, I did try the latest dev somewhere along the way without results.
Works now.Unfortunately, the newest dev breaks Feeds Tamper - http://drupal.org/node/1794482. Fortunately, got that fixed by using the latest Feeds Tamper dev.
Pff, still the same error. Maybe my problem isn't related to this issue - Error documentation
Comment #101
a.ross CreditAttribution: a.ross commentedStrompf, You're welcome. I'm Dutch, by the way, so I understand that page. One thing that might cause a 500 is when the import data is very large and importing it takes longer than the max execution time. Try increasing it in your php.ini file. Otherwise I would advise you to look around on the Feeds Tamper issue queue, and maybe create a new issue.
Edit: maybe this issue is related? #1425828: 2 plugins for one field trows exception
Comment #102
twistor CreditAttribution: twistor commenteduntagging
Comment #103
twistor CreditAttribution: twistor commentedarg
Comment #103.0
twistor CreditAttribution: twistor commentedUpdated issue summary.