Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
I want to manually assign taxonomy terms to a feed. So I have created a vocab, lets say one of the terms is news. I have set-up a feed from a news site and set the feed to use the news term. I thought this would then assign that term to all the imported items, but it doesn't. Is there any chance this can be added as a feature?
Nick
Comment | File | Size | Author |
---|---|---|---|
#205 | 632920-205_inherit.patch | 50.06 KB | alex_b |
#204 | 632920-204_inherit.patch | 49.48 KB | alex_b |
#192 | 632920_192.patch | 5.72 KB | David Goode |
#191 | 632920_190.patch | 5.33 KB | David Goode |
#190 | 632920_189.patch | 5.3 KB | David Goode |
Comments
Comment #1
alex_b CreditAttribution: alex_b commentedThis feature is called taxonomy inheritance in FeedAPI, for clarity I'm going to call it Taxonomy inheritance here, too.
This feature can be easily added by extending FeedsNodeProcessor. As for actually making this part of Feeds at this point, I don't have any plans for that. I would accept a solidly reviewed patch with a test though.
Comment #2
nickbits CreditAttribution: nickbits commentedHi Alex,
Thanks for making that clear. I will revert back to FeedAPI for the moment then, but if I get time, will look at seeing if I can put a patch together for Feeds, but wont be any time soon.
Cheers,
Nick
Comment #3
nickbits CreditAttribution: nickbits commentedHi alex_b,
I have made an attempt to get this to work and have attached the associated patch. It seems to work on my system but needs thorough testing. To be honest, I ahve taken some code from the old FeedAPI module, updated and added a few bits. Anyway, the idea is:
1. You create a feed.
2. In that feed you assign it a term(s).
3. When nodes are imported for that feed, they inherit the term(s) applied to the feed created above.
It maybe that an option needs to be added to turn this feature on and off.
Anyway people, let me know what you think and any issues.
Nick
Comment #4
nickbits CreditAttribution: nickbits commentedHi,
Has anyone else tested this yet? Is it a feature people want?
Nick
Comment #5
willmoy CreditAttribution: willmoy commentedNo (sorry), yes (please). :)
Comment #6
tobedeleted CreditAttribution: tobedeleted commentedVery keen to see this type of feature included. For me it is essential that I can tag new content coming in to the site so that is can then be displayed in the relevant place/s.
Comment #7
alex_b CreditAttribution: alex_b commented#3:
- I see tabs as spaces http://drupal.org/coding-standards
- Let's make this a setting in the FeedsNodeProcessor
- Use
node_load()
to retrieve taxonomy of feed node, copying taxonomy from the feed node to the item node should then be very simple (sth. like$node->taxonomy = $feed_node->taxonomy
)- Before committing, this patch will need a test.
Comment #8
nickbits CreditAttribution: nickbits commented#7:
- Tabs changed to spaces as per standards
- Option now included - seems to work on mine but needs testing
- Code changed to simple version, didn't see this simple method when I was coding initially.
- testing, what testing is it? I know there are several modules and ways to do this. I ahvn't looked at this for feeds yet. Can you point me in teh right direction please?
Final point, was not sure which patch file to attach so have two.
1. Taxonomy Inheritance Patch V2 (Against Last) - patch against output of my previous patch
2. Against normal unaltered file
Nick
Comment #9
alex_b CreditAttribution: alex_b commentedGreat.
- There are still some indentation issues. Please also have a look at standards for comments and control structures http://drupal.org/coding-standards
- Always patch against the current branch (right now there is only one branch which is head).
- Feeds ships with 1000+ assertions testing its existing functionality (see feeds/tests). We should also include a test for taxonomy inheritance. Refer to http://drupal.org/project/simpletest for learning how to run tests and how to write them (tests look funky, but they're *really* easy to pick up).
Comment #10
nickbits CreditAttribution: nickbits commentedAlex,
Will sort out the remaining issues on Thursday. Simpletest is what I needed to know. As for the coding standards, this is what I hate, going between different packages and systems and trying to remember which one I am in and the standard to use. Anyway will correct.
Nick
Comment #11
alex_b CreditAttribution: alex_b commentedYeah, you gotta speak the local dialect here ;-)
Comment #12
nickbits CreditAttribution: nickbits commentedIt's just a case of remembering where "here" is! Catches me out every time!
Comment #13
nickbits CreditAttribution: nickbits commentedHi,
I have corrected, with any luck, the issues in the patch file. I ahve also started work on a test, also attached. However I need a bit of help on this. I understand the principles, but combining SimpleTest and Feeds is getting my head in a spin.
On the test what I think I have, please correct me if I have totally messed the code up, is:
1. Created a config for a feed
2. Have ensured Taxonomy Inheritance is disabled
3. Created the feed nodes.
I have attempted to put this together by reading the other test, and documentation, albeit rather quickly. I know this is far from complete but I like to write a bit and test what I have so far. At this point I get and error associated with:
Can someone tell me if I am on the right tracks or gone completly wrong? I will finish coding the test as soon as I have this issue resolved.
Regards,
Nick
Comment #14
nickbits CreditAttribution: nickbits commentedComment #15
paganwinter CreditAttribution: paganwinter commentedSubscribing...
And what about any other CCK field, like how the FeedAPI Field Inherit module did?
Comment #16
robertDouglass CreditAttribution: robertDouglass commentedI think the patch represents the wrong approach. I'd like to be able to register "inheritors" for a lack of a better name, and specify exactly which bits of the parent node get inherited. The current patch is a one-off solution for taxonomy.
Here's a chat I had with alex_b where he shared some pointers for the approach:
[1/26/10 9:48:40 PM] Alex Barth: I would make it a method FeedsNodeProcessor::inherit($feed_node, $node)
[1/26/10 9:49:05 PM] Alex Barth: that I'd invoke from FeedsNodeProcessor::process() just before node_save($node);
[1/26/10 9:49:41 PM] Alex Barth: You can load the feed node in process() with
[1/26/10 9:50:08 PM] Alex Barth: if ($source->feed_nid) {
$feed_node = node_load($source->feed_nid);
}
[1/26/10 9:50:30 PM] Alex Barth: keep in mind that there may be no feed node in cases of direct imports from the stand alone form
[1/26/10 9:50:32 PM] Robert Douglass: And how to register inheritance rules?
[1/26/10 9:50:46 PM] Robert Douglass: Seems like it needs to be another h tab for CTools
[1/26/10 9:51:18 PM] Alex Barth: that can be a setting of the FeedsNodeProcessor that is exposed in FeedsNodeProcessor::configForm() IF the domain access module is present
(Note I was asking specifically about inheriting domain module settings from the parent node).
Sorry to nickbits for hijacking the issue =) It'll be better this way, though.
Comment #17
robertDouglass CreditAttribution: robertDouglass commentedAs a temporary stopgap to this issue I wrote a small custom module that implements hook_nodeapi and successfully "inherited" my domain module settings to the imported nodes like this:
One could do the same with taxonomy or whatever. I'm not suggesting this is a long term solution... but if anyone else has a similar need and can't wait, I thought the code might be useful.
Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedSo I thought that this thread was going in the direction that I was hoping, but maybe I missed something.
Here's my situation:
I am importing multiple calendars from our association's Google Apps account. I have created a feed for each one, "Symposiums", "Events", "7 Pillars". What I can't seem to do is find a way to sort by the feed. For example, I want to show the first event associated with "Symposiums", the first one association with "Events", and the first one associated with "7 Pillars".
Is there a way to have a particular Taxonomy term, like "Symposiums", attached to the feed that will then be automagically added to all the nodes created from that feed? I could filter each block by the term that way...
I have learned a heck of a lot about using Drupal, but unfortunately, I can't help much with building/developing patches. I welcome testing new developments, lol.
I have the development site at www.thecfaa.com/indev
Comment #19
paganwinter CreditAttribution: paganwinter commentedSubscribing...
Comment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedWhat part of this would you change to inherit the taxonomy? I really want to be able to apply taxonomy terms to the imported nodes so that I can sort the different types of events by the taxonomy. Any help on what you wrote would be appreciated, and I understand it would only be a temporary fix...
Thanks
Comment #21
paganwinter CreditAttribution: paganwinter commentedCreate a node-.tpl.php file (by copying your theme's node.tpl.php file and renaming it accordingly) for your Feeds config node as well as your feed item node, and add this at the bottom of those files:
This will cause all data associated with your node for that node type, to be displayed when you view the respective nodes.
This will list out all taxonomy related data for that node.
Eg.
Feeds Config -> node-feeds_config.tpl.php
Feeds Item -> node-feeds_item.tpl.php
From this you can figure out how the taxonomy terms are stored in your Feeds Config type nodes.
Then accordingly modify the above code:
I think this should do it.
(Boy, am I bad at explaining stuff...)
Comment #22
skizzo CreditAttribution: skizzo commented@cfaadev: As I understand it, that's what nickbits's patch Taxonomy Inheritance Patch is supposed to do...
Comment #23
skizzo CreditAttribution: skizzo commented@robertDouglass Re:#17
Indeed I do have a similar need (actually I need to inherith both domain and taxomomy).
- created a custom module mymodule
- in your code: renamed function roller_nodeapi into mymodule_roller_nodeapi
- in your code: renamed 'video_external' into my feed item's content type name
- enabled mymodule
Now, clicking on the feed Import tab the feed item nodes are created, but the they are
all assigned to "current domain" (as per default Domain configuration).
Any other setting I am missing?
What makes MyModule override Domain Access behaviour?
Thank you...
Comment #24
drewish CreditAttribution: drewish commentedsubscribing.
Comment #25
lennart CreditAttribution: lennart commentedA general solution to inheritance is a good idea. Feed items inheriting the node author from the feed node is another example that comes to mind alongside taxonomy inheritance.
Comment #26
AntiNSA CreditAttribution: AntiNSA commentedThis is not happening in feeds right now? Then how can you say feeds is better than feedapi? Sure I need less server load... but I newad to have basic features.
Comment #27
AntiNSA CreditAttribution: AntiNSA commentedCan someone give a status update on this issue? At least the option of inheriting taxonomy?
Comment #28
AntiNSA CreditAttribution: AntiNSA commentedWhen patching withthe latesto option I get this error... here is a copy from the rejected file with cygwin
***************
*** 37,43 ****
$node->nid = $nid;
$node->vid = db_result(db_query('SELECT vid FROM {node} WHERE nid = %d', $nid));
}
-
// Save the node.
node_save($node);
--- 37,51 ----
$node->nid = $nid;
$node->vid = db_result(db_query('SELECT vid FROM {node} WHERE nid = %d', $nid));
}
+
+ if ($this->config['taxonomy_inheritance'] && module_exists('taxonomy')) {
+ // Find out what term(s) the parent node uses
+ // Use node_load to get the parent information (see http://api.drupal.org/api/function/node_load/6).
+ $feed_node = node_load($source->feed_nid);
+ // Set node to use the same taxonomy term(s) as it's parent
+ $node->taxonomy = $feed_node->taxonomy;
+ }
+
// Save the node.
node_save($node);
Comment #29
AntiNSA CreditAttribution: AntiNSA commentedComment #30
alex_b CreditAttribution: alex_b commented#27 - To my knowledge, nobody is actively working on this feature at the moment. robertDouglass posted a good stop gap in #17 and constitutes in IMO a good practice.
Issue is not critical (Feeds uses 'critical' only for issues that need to be solved for upcoming releases).
Comment #31
AntiNSA CreditAttribution: AntiNSA commentedok I applied his patch, however since there ios now a new dev release , will I have to patch it with the old fix , and patch every release?
Comment #32
AntiNSA CreditAttribution: AntiNSA commentedCan you please tell me how to alter this information in order to get the audience settings for organic groups to be inherited? And where do I put this information at?
Comment #33
jboeger CreditAttribution: jboeger commentedDefinitely a feature I need... subscribing....
Comment #34
Dmxr100 CreditAttribution: Dmxr100 commentedInheritance of ImageField CCK would also be of great use, so that a feed could perhaps have the source's logo associated with it, and then displayed alongside all posts from that feed.
Comment #35
drupallogic CreditAttribution: drupallogic commentedwithout mapping terms to feed items, I can't use Feeds and need to stay with FeedAPI...
hope it's added.
Comment #36
AntiNSA CreditAttribution: AntiNSA commentedSeems like this is a very important feature, and popular amongst all users. I cant see the logic if you were going to create a method to create new nodes automatically from feeds why would you prevent them from being categorized? PLease accept this ongoing petition from feeds users and feedapi converts. We need this! Power to the people.
Comment #37
alex_b CreditAttribution: alex_b commentedThere is a fundamental misunderstanding: Nobody is here to fulfill other people's feature requests.
Contributing to open source is not petitioning feature requests, it's writing patches, helping out with documentation and filing solid issue reports.
Comment #38
alex_b CreditAttribution: alex_b commented#17 is a good stop gap solution.
#13 is very close.
Comment #39
alex_b CreditAttribution: alex_b commented#36/#37 - To make this very clear: there is nothing wrong with feature requests, in fact I appreciate them. But we can't petition each other to implement them. If you want to see something happening, you need to step up.
Comment #40
AntiNSA CreditAttribution: AntiNSA commented:) I am trying so hard to step up every way I can and in every way... but I dont have that ability to make the patch. You know what it takes.. you have the skills.. you have the power and my sincere hope that you will grant us nonables your skills in completing this feature request
Comment #41
gravelpot CreditAttribution: gravelpot commentedsubscribing
Comment #42
squinternata CreditAttribution: squinternata commentedexcuse me maybe i didn't understand well..but it means that it s not there feature about inherit taxonomy language and whatever?
thank you
A
Comment #43
AntiNSA CreditAttribution: AntiNSA commentedNo, it seems that it is not a priority at all for feeds. I hope it will be though!!!1
Comment #44
squinternata CreditAttribution: squinternata commentedok i ll wait for news about it..in the meanwhile i come back to feedapi..
cause for me this feature is necessary!
thank you
A
Comment #45
AntiNSA CreditAttribution: AntiNSA commented44 replies and no solution? Come on Drupal!!!!! Go Go Go! Where is the solution? We need this!!!!
Comment #46
skizzo CreditAttribution: skizzo commenteduhm... 10 out of 45 come from the same user :-)
Comment #47
bflora CreditAttribution: bflora commentedSubscribing...
Comment #48
pdcarto CreditAttribution: pdcarto commentedOn the project page, it says:
* Feeds is the successor module of FeedAPI and Feed Element Mapper.
I appreciate the advances in usability from Feeds, but this is not going to be a successor to FeedAPI and FEM until this feature is implemented. The authors have two choices: 1. Change this issue to a bug report and assign someone to fix it, or 2. Remove the "successor to FeedAPI..." claim from the project page.
Comment #49
pdcarto CreditAttribution: pdcarto commentedI'm wondering how hard it would be (for someone who knows what he or she is doing) to adapt the feedapi inherit module to work with Feeds?
Comment #50
AntiNSA CreditAttribution: AntiNSA commentedI completely agree with #48, and would love someone to do what 49 says. Right now I am creating tons of content types.... which is a huge timewaster instead of being able to inherit taxonomy....
Comment #51
jtr CreditAttribution: jtr commentedFor some reason, setting the uid of the feed item did not work for me using the trick in #17 or #21. Here is my code that now works:
I also had to comment out the line in FeedsNodeProcessor.inc that does this:
I can't help but think that my code is pretty inefficient, but it seemed that with just the insert nodeapi hook, somehow i was still getting anonymous authors.
Comment #52
pdcarto CreditAttribution: pdcarto commentedW/respect to #49...
The more I think about it, the more I think a separate feeds_inherit module would be the right way to go. It could be bundled with Feeds, or made a separate project, depending primarily upon the ownership/maintainer situation.
Which brings me to my second thought, which is that if each person who chimed in requesting this feature (just on this issue) were to ante up some money, we might be able to get this on the fast track. I count 17 individuals expressing a desire for this feature on this thread; if just half of them were willing to put down some cash, we could do this. I'm thinking alex_b would be the obvious first-choice for such a bounty, since he has been so closely involved in both feeds and feedapi (and feedapi inherit).
So, Alex, what kind of bounty would it take to get you to take this on (porting feedapi_inherit to feeds_inherit)?
Thanks, Pat
Comment #53
alex_b CreditAttribution: alex_b commentedPat - I'd actually like to see this feature in Feeds, my problem is not money but time. I'd love to see a developer step up and go for the whole nine yards: hammer out basic architectural decisions (I can be of help for that), write patches with tests and respond to feedback on issue. #13 was a good start but we need an approach that scales beyond taxonomy inheritance. If somebody's seriously interested in nailing this feature, please contact me on IRC, I'll be really happy to help out.
Comment #54
AntiNSA CreditAttribution: AntiNSA commentedI am a poor teacher in China I would help if I could but I cant afford it and have no credit card :(
Comment #55
pdcarto CreditAttribution: pdcarto commentedRe #53, Alex is saying that he would prefer embedding inheritance into FeedsNodeProcessor.inc, rather than having a separate "feeds inherit" module. At the same time, he recommends making inheritance scalable, which I take to mean that one could plug in new types of inheritance that are not provided in the basic Feeds module. I have no idea how one would go about doing that.
Re #16, Robert "hijacked" the issue saying it'll be "better this way", but I'm not sure what "way" that is. The conclusion from his IRC chat with Alex appears to be that Alex was recommending that Nick's approach from #13 simply be expanded to cover a larger number of known inheritance cases (taxonomy, author, target groups, etc).
Robert's approach introduced in #17 and followed in a number of subsequent comments, which is to make a separate module that pokes the particular inherited attribute into the $node object can therefore be regarded as simply a workaround for the lack of inheritance in Feeds, and useful as it may be for individual cases, is probably not a step towards closing this issue.
Alex's offer of support in #53 for some developer to step up and go the whole nine yards makes this seem quite daunting (particularly the scalability). With effort, I might be able to hack FeedsNodeProcessor.inc to cover the inheritance cases supported by feedapi_inherit, but I'm a hobbyist-level developer at best and anything I wrote would certainly not come close to clearing the bar for an acceptable patch. Should I even try?
Or would it be better to try to build up a bounty and continue to try to find/cajole another developer?
Comment #56
pdcarto CreditAttribution: pdcarto commentedAttached is my attempt to merge the feedapi_inherit module's functionality in FeedsNodeProcessor.inc. The configuration settings and inheritance functions seem to be working for og, taxonomy and author (I'm not using language, so that's not been tested).
I have no clue as to how to set up a SimpleTest case, but would be willing to try given some encouragement and help.
Comment #57
AntiNSA CreditAttribution: AntiNSA commentedSuper effort! This is in my nodequeue to test. Hopefully I can test in the next 24 hours
Comment #58
poloparis CreditAttribution: poloparis commentedHi, regarding #21 paganwinter's solution, it passes along the correct taxonomy for me but it does not save it to to the feed item node.
I tried a drupal_write_record('feeds_node_item', $node, 'nid'); without success
Could anyone point me in the right direction please ?
Comment #59
AntiNSA CreditAttribution: AntiNSA commentedThanks for the effort! So Far I get this error :
Warning: Call-time pass-by-reference has been deprecated in /home/cyberfan/htdocs/sites/all/modules/feeds/plugins/FeedsNodeProcessor.inc on line 74
and this error:
warning: Cannot modify header information - headers already sent by (output started at /home/cyberfan/htdocs/sites/all/modules/feeds/plugins/FeedsNodeProcessor.inc:74) in /home/cyberfan/htdocs/includes/common.inc on line 148.
Still testing...
Comment #60
AntiNSA CreditAttribution: AntiNSA commentedOk I cant get past this error... gives a white screen :
Warning: Call-time pass-by-reference has been deprecated in /home/cyberfan/htdocs/sites/all/modules/feeds/plugins/FeedsNodeProcessor.inc on line 74
Comment #61
alex_b CreditAttribution: alex_b commented#55: very nice work.
I should have expanded a little more on what I meant with 'scalability' but you got it right: we're looking for an approach that makes inheritance extensible so that we don't need to patch Feeds for every use case that needs it.
The patch attached (untested) adds such extensibility very closely following the pattern set by Feeds' Mapping API:
FNP::getInheritors()
declares a list of available inheritors and callbacks for them. It invokes ahook_feeds_node_processor_inheritors_alter()
that can be implemented to add more inheritors.FNP::inherit()
invokes these callbacks if the inheritor has been selected by a site builder. Please review.Further I have made these modifications:
* Indentation needs to be 2 spaces instead of 4.
* I renamed feeds_inherit_do_inherit() to inherit() (BTW, use lower camelCase for methods).
* Check whether feed_nid is present before inheriting
if ($source->feed_nid) {
* Not 'parent feed' but 'feed node' in descriptions.
* No need to pass objects by reference as of PHP 5
* Disable inheritance options if importer is not attached to feed content type.
* Currently the inheritors just live in FeedsNodeProcessor.inc. They are very short so I didn't want to create a separate location for them like mappers have. I think that's fine for now.
Outstanding:
* Testing (I have only tested the UI, not the actual inheritance).
* Write simpletests
* Documentation in feeds.api.php
* Documentation in Drupal handbooks
* Shouldn't language inheritance depend on locale?
Comment #62
alex_b CreditAttribution: alex_b commentedAntiNSA: please refrain from attaching demands to the issue title. And again: please make sure your issue reports are specific, ie. we need to be able to reproduce them. So: where exactly do errors happen, after what steps, with which configuration?
Comment #63
pdcarto CreditAttribution: pdcarto commentedRe #61: Alex's patch works perfectly in my first testing of a couple rss feeds: UI and og/taxonomy/author inheritance all doing what they're supposed to. Very nice work! Thank you very much!
I'd be happy to try adding the Drupal handbooks: site-builder's documentation but I don't think you'd want me touching the developers documentation. In any case, that should happen after the feature request has been committed, right?
Comment #64
AntiNSA CreditAttribution: AntiNSA commented#62 If you could focus on debugging and developing the patch for feed inheritance as well as you do on criticizing my iupdate of the title, Im sure we can see an increase in efficiency!
I updated the title to reflect teh fact that this thread now has a patch, for those who thought it was lacking, and reflected the current patch development status.
As for the error message, immediatly after installing they appeared. After trying to import a feed I recieved the error / whitescreen. I will try to install the updated version of the patch and see what happens.
Comment #65
AntiNSA CreditAttribution: AntiNSA commentedAll apologies. I love drupal.. and thanks for the effort. to not mistake determination wwith anything else.
Comment #66
Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented@ AntiNSA: I am happy to see that you apparently plan to give up your disruptive behaviour. At drupal.org, we try to provide space for people like Alex to work on improving Drupal. We are not happy if somebody uses this space to disrupt their hard work through unsubstantiated demands. So please restrain yourself, maybe help Alex a bit by documenting Feeds and relax.
However, if you should take up your former behaviour (and since you say you speak German): Wir können auch anders.
Comment #67
pdcarto CreditAttribution: pdcarto commentedAlex asked me to try to write some simpletests for #61.
Oh boy - a new learning opportunity! ;-)
Stay tuned (and poke me if I haven't reported anything back in a few days)...
Comment #68
klonos@Gerhard Killesreiter: We had a private email exchange with AntiNSA (just before his reply in#65) and he is not a mean person at all. He simply is an impatient one perhaps, but he means well. After all we all are friends here.
I am begging everyone (also note to self) to stop this meaningless series of postings that don't have anything to do with the progress of this issue. If you need to say something personal to a person, say it in a private mail and stop polluting the issue queue. Now, I'll shut up because I am doing it too ;)
Comment #69
AntiNSA CreditAttribution: AntiNSA commentedThe newest patch seems to work great! Thanks. I am hesitant though to allow organic groups the right to create their own feeds.. imagine 800 users with 20 feeds each updating on a chron run... I am sure my server would time out.... how do you manage to allow soo many feeds to update?
Comment #70
iaminawe CreditAttribution: iaminawe commentedI can confirm that the lasy patch works nicely to fix the author inherit issue.
It does not seem to be inheriting the authors default input filter so all imported content type still default to the site default which in my case is plain text.
This same issue was also brought up here http://drupal.org/node/652180#comment-2706472
Is there any known way around this?
Thanks
Comment #71
AntiNSA CreditAttribution: AntiNSA commentedyes you can use the module called "Better Formats" do set default content filter formats. Thats what I did
Comment #72
skizzo CreditAttribution: skizzo commentedI would like to test the patch. I am running domain and feeds 6.x-1.0-alpha13
but I don't see a checkbox for domain inheritance (mentioned in #16 and #17).
Could it be added to patch #61?
Comment #73
slicedsoup CreditAttribution: slicedsoup commentedThis is working for me. Great patch.
Comment #74
bflora CreditAttribution: bflora commentedpatch is working for me. Thanks!
Comment #75
idflorin CreditAttribution: idflorin commentedpatch #61 works for me with 6.x-1.0-alpha12
---------
earth climate made with feeds module
Comment #76
michepatch #61
ONLY tested "inherit feed author" on 6.x-1.0-alpha14. WORKED!
Comment #77
Michelle#61 works for me with the latest dev snapshot. I tested taxonomy and author, not OG.
Thanks!
Michelle
Comment #78
srobert72 CreditAttribution: srobert72 commentedSubscribing
Need inheritance for Language and some CCK fields (NodeReference).
I've post another issue #777376: Inherited fields in Feed Items from their Feed parent which I realize now is duplicate of this one.
This one appears to be more generic.
Comment #79
alex_b CreditAttribution: alex_b commentedSetting to NW as patch needs tests (#67) - pdcarto, still on to it? Does somebody else want to step up?
Comment #80
srobert72 CreditAttribution: srobert72 commentedHere is my feedback.
I've just tested patch #61 and it works well for me.
TEST
Module version : Feeds 6.x-1.x-dev (2010-May-06)
Options from patch : Language + Taxonomy inheritance
Test with 2 Feed in different language which import 25 FeedItem each.
I'd like also to inherit CCK values from Feed to FeedItem children.
Do you think it could be implemented soon ?
Comment #81
dixon_Here is a re-roll of the patch from #61. I've added documentation to
feeds.api.php
and made the translation inheritor depend onlocale.module
instead. I've also fixed three code style (indentation) issues.I've been able to test the user, taxonomy and language inheritors without any problems! I haven't been able to test the og inheritor yet.
What's left:
Even though is still not 100% complete, I'm marking this as "needs review" to get some eyes on it.
//dixon_, NodeOne
Comment #82
dixon_And here is the patch ;)
// dixon_, NodeOne
Comment #83
srobert72 CreditAttribution: srobert72 commentedI tested it again but I have this error now while updating :
Fatal error:
Cannot redeclare class FeedsImporter in /home/drupal/sites/all/modules/feeds/includes/FeedsImporter.inc on line 31
EDIT : Sorry my mistake
It's was just right access problem. No problem in your patch.
Comment #84
srobert72 CreditAttribution: srobert72 commentedHere is my feedback.
TEST patch #82
Module version : Feeds 6.x-1.x-dev (2010-May-06)
Options from patch : Language + Taxonomy inheritance
Test with 2 Feed in different language which import 25 FeedItem each.
RESULT
After deleting all old FeedItem, I click on "Import" link of each Feed.
25 FeedItem are imported for each. Great.
But after a while (maybe many cron launches) I have also 25 FeedItem "Language neutral".
It's not due to new patch version. I have noticed also same issue with #61 patch after a while.
Comment #85
ehudash CreditAttribution: ehudash commentedI have tested patch from #82 as well and found no problems.
Now I wonder how can I add the Domain Access properties inheritance from the Feed node onto the imported Feed Item nodes.
I'm a bit of a newbie so can someone guide me on how to do this correctly, or maybe post an updated patch?
Thanks!
Comment #86
ehudash CreditAttribution: ehudash commentedWould love to see Domain Access inheritance as well.
Working on it myself but I'm a bit of a newbie, so it may take a while :)
Comment #87
pdcarto CreditAttribution: pdcarto commentedMy apologies to everyone. I said I'd try to write the simpletests without having any idea what that entailed in terms of know-how. If someone show knows his way around testing would be willing to pick this up, I would be very much obliged.
Changing this back to `needs review` - since the formal testing remains to be done.
Comment #88
alex_b CreditAttribution: alex_b commented#82 looks good. #81 describes next actions. I am not working actively on this, so if someone wants to pick up the test torch, go for it!
Comment #89
srobert72 CreditAttribution: srobert72 commentedpatch #82 is no more compatible with last DEV release (2010-May-17)
@dixon_
Could you update it ? Thanks a lot
Comment #90
ccoppen CreditAttribution: ccoppen commentedIn patching using either #61 or #82, I get a reject error for plugins/FeedsNodeProcessor.inc.
I'm using the 6.x-1.x-dev version. I was using alpha15 but switched to make sure everything would patch correctly.
I'm attaching the rej file.
When I attempted to reverse the file, I got Hunk #1 failed at 69 as well in plugins/FeedsNodeProcessor.inc. So, it is neither succeeding in patching the file, nor reversing the file. Though, only 1 out of 4 hunks FAILED.
Comment #91
ccoppen CreditAttribution: ccoppen commentedAttaching small patch to fix what's broken in #82. Maybe someone wants to reroll everything back into one?
EDIT: I get the inheritance checkbox on the Feed Settings page, but the inheritance is not happening.
Any thoughts?
Comment #92
ccoppen CreditAttribution: ccoppen commentedSorry to be posting again so soon, but attached is a patch with the changes from #82 rolled with the fixes from 91. I'm still testing to see if it really works. I get the checkboxes, but not sure if the inheritance really works.
Comment #93
skizzo CreditAttribution: skizzo commentedApplied patch #92 cleanly to 6.x-1.0-alpha15. I see the 3 checkboxes (language, taxonomy, author) and all 3 attributes are inherited correctly in my setup. As #86, I would really like to see domain inheritance implemented so it can be tested before the patch is committed. @ehudash: any progress?
Comment #94
srobert72 CreditAttribution: srobert72 commentedPatch #92 works for me on last DEV release (May 17, 2010).
It repairs correctly broken patch #82.
I've tested inherited language and taxonomy.
Thank you @ccoppen
Comment #95
ccoppen CreditAttribution: ccoppen commentedWell, my work did finally pay off.
I figured out that you can't use the Content Taxonomy CCK in order for this work. *slaps forehead*
I'm glad it worked for you.
Now to get my emfield working again. :/
Comment #96
Exploratus CreditAttribution: Exploratus commentedAll this needs is Domain Access, and it would be complete.
Comment #97
arlind CreditAttribution: arlind commentedDefinitely a feature I need... subscribing....
Comment #98
deggertsen CreditAttribution: deggertsen commentedTested #92 and works great. No problems so far. This needs to be put in.
Thanks!
Comment #99
ufku CreditAttribution: ufku commentedI think this is ready to go.
Please submit other inheritance related feature requests as separate issues.
Comment #100
alex_b CreditAttribution: alex_b commentedWe are very close here, yep, but we need tests!
Comment #101
michellezeedru CreditAttribution: michellezeedru commentedSubscribing .. would be happy to help test, but running into problems applying the patch for #92. Maybe because I'm trying to apply against 6.x-1.0-alpha15? I placed the patch in sites/all/modules/feeds, but when I try to run it, I get an error:
patching file feeds.api.php
can't find file to patch at input line 31
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff -urp ./plugins/FeedsNodeProcessor.inc ../NEWfeeds/plugins/FeedsNodeProcessor.inc
|--- ./plugins/FeedsNodeProcessor.inc 2010-05-20 10:46:35.000000000 -0400
|+++ ../NEWfeeds/plugins/FeedsNodeProcessor.inc 2010-05-20 11:11:39.000000000 -0400
------
Thanks for this module - looking forward to getting this feature rolled in!
Comment #102
Anonymous (not verified) CreditAttribution: Anonymous commentedJust my 2 cents, but do feed nodes also inhert i18n settings from the feed? E.g. the feed has Drupal language set to "French", and then all the nodes derived from it get that "French" setting as well. I'm asking because I'm doing tests with multilingual feeds, and haven't found out how to do it properly. My feed nodes all get the default languages.
Comment #103
klonosI am not testing atm, but I too am importing single-language feeds and I need to set the language appropriately so that feed items inherit language setting(s).
Comment #104
Jerimee CreditAttribution: Jerimee commentedsubscribing
Comment #105
NCtiNet CreditAttribution: NCtiNet commentedsubscribing
Comment #106
arlind CreditAttribution: arlind commentedHi,
Thanks for the great work on the patch - could it be made into a module, then it can be tested by a bigger audience and move it forward a to a alpha release - like me who doesn't know how to patch ;-)
thanks Bo
Comment #107
alex_b CreditAttribution: alex_b commented#106: I'd rather see tests than a separate module. We are very close, it's not hard at all to write tests. Just takes taking a little bump on the learning curve ;-)
Comment #108
alex_b CreditAttribution: alex_b commentedMay be of use: http://drupal.org/simpletest-tutorial
Comment #109
deggertsen CreditAttribution: deggertsen commentedIt doesn't really make sense to make this a separate module as this functionality should really be included in this module, so a patch is appropriate. Go to http://drupal.org/patch/apply in order to learn how to apply patches. You may need to know some basic terminal/linux commands in order to navigate folders etc in the command line so check out something like this site http://www.reallylinux.com/docs/basic.shtml for that info. Once you know the basics applying a patch is a piece of cake.
This patch has been working great by the way. Hope we can have it actually put into the dev version soon at least.
Comment #110
hello@melmcdougall.com CreditAttribution: hello@melmcdougall.com commentedPatch #92 is working great for me too. Thanks everyone who has worked on this. Had a couple of issues with the patch file itself, but that was likely me not applying it correctly (this is the first patch i've ever tried to apply).
Just incase it wasn't me, here is result from the .rej file. I just applied the "'inherit' => array()," line manually to FeedsNodeProcessor.inc, and everything went smoothly after that.
***************
*** 145,150 ****
'update_existing' => 0,
'expire' => FEEDS_EXPIRE_NEVER,
'mappings' => array(),
);
}
--- 152,158 ----
'update_existing' => 0,
'expire' => FEEDS_EXPIRE_NEVER,
'mappings' => array(),
+ 'inherit' => array(),
);
}
Comment #111
NCtiNet CreditAttribution: NCtiNet commentedWhy not add these categories in the taxonomy?
Thanks.
Comment #112
palpatine1976 CreditAttribution: palpatine1976 commentedI'm getting same results as #101 and #110. Patch #92 fails on Hunk #2 - adding the line below:
+ 'inherit' => array(),
line into plugins/FeedsNodeProcessor.inc.
If you do this single line insert manually it seems to work - but should we have an updated patch or is this going to be rolled into a Dev build soon?
Great addition to the plugin tho!
Comment #113
klonosas Alex said before (#100)... all we now need is tests and we are ready to commit.
Comment #114
Equinger CreditAttribution: Equinger commentedI need to put a vote in for inheriting OG from the parent node as well. This is a feature we use heavily from the Feed API. I want to transition to this module for more customized parsing, but this feature is necessary.
Indeed, I can see how having the OG manage feeds would cause performance issues, but really, while our site is structured around organic groups, it does not imply that the OG will have the privileges to willy nilly add or control feeds. The OG is used simply to direct the audience and where on the site the feeds go from a user access/ ui perspective.
Comment #115
smithmilner CreditAttribution: smithmilner commentedYou guys really helped me! I was finally able to turn a Group Node into a Feed Node as well. I'm not sure how extensible it is and I'm not sure if this is a complete hack, but I've only been diving into modules for about 6weeks now.
What do you guys think?
Comment #116
awakenedvoice CreditAttribution: awakenedvoice commentedSubscribing
Comment #117
jkdaza CreditAttribution: jkdaza commentedPatch #92 works for me, tested only on Taxonomy terms, with Feeds 6.x-1.0-beta1.
Nice work ccoppen!!
Now, I wonder, how hard is it going to be adding the Location inheritance?
I haven't got enough PHP skills to do it, but I can support with testing as much as I can.
Anyone up to it?
Comment #118
jvieille CreditAttribution: jvieille commentedImpossible to patch the current release, even manually, the code is much too different.
Is this possible to port it the the current Beta 2 release or - better - to commit the change.
Thanks
Comment #119
jvieille CreditAttribution: jvieille commentedsmithmilner, what to do with this code? When where to invoke it, what it does exactly?
Thanks
Comment #120
DanielJohnston CreditAttribution: DanielJohnston commentedSubscribing. Specifically interested in allowing organic group admins to import nodes and users that automatically get assigned as appropriate audience / group members.
Comment #121
smithmilner CreditAttribution: smithmilner commentedOk, Apply Patch from comment #92 and go to plugins/FeedsNodeProcessor.inc
and replace the feeds_inherit_og function with:
be sure to change the Group Node value to whatever the machine name of your group node type is.
I was trying to attach a feed importer directly on a group. I was having troubles because the items from that feed weren't getting placed into the same group. After a bit of research I could safely say it's because a group isn't in a group... it IS the group. This code here is where you declare if your feed importer is attached to a group (not an item in a group) and then inserts corresponding group info into each feed item's object array.
Comment #122
Steven Jones CreditAttribution: Steven Jones commentedsubscribe.
Comment #123
jvieille CreditAttribution: jvieille commentedBut what is the proposed change for this function?
Now waiting for a patch that works with Beta 2...
Comment #124
smithmilner CreditAttribution: smithmilner commentedThe first If statement of that function is not part of the original patch. All you need is to write the machine name of your group node type and you're feed items will be placed in the group. An Example would be if I had a content type called League with a machine name of "league" that was a group.
After you write in the content type name, while editing any league you can input a feed and the feed items will be placed into that league.
Comment #125
epersonae2 CreditAttribution: epersonae2 commentedNCtiNet (#111) - quick use case that got me here:
I want to build a private feed reader, cause I don't much like Google Reader, the other PHP app I've found is slow and clunky, and I like Drupal.
I'm going to be grouping feeds as Local, Webdev, Music and so on. Those categories don't exist inside the feeds on the sites themselves. They are purely my creation.
Additionally, I have no interest in using the site's categories of "house, trance, music" etc. (In your example)
Also...subscribing.
Comment #126
XiaN Vizjereij CreditAttribution: XiaN Vizjereij commentedCan someone please reroll the patch for beta2?
Comment #127
DanielJohnston CreditAttribution: DanielJohnston commented@smithmilner - I applied #92 with manual fixes as in #110 then edited in your code from #121, and my Feeds are now happily importing to inherited organic groups. Obviously before the code can be put into Feeds, it needs either a selector for which content type to look at, or to inherit either the top or all organic groups. Manually editing in the content type is a short term solution, but it's done the job for me. Thank you!
Comment #128
jvieille CreditAttribution: jvieille commentedA patch for beta 2 from #92 + #110 + #121 would be much appreciated!
... before beta 3 comes out!
Thanks
Comment #129
smithmilner CreditAttribution: smithmilner commented@danieljohnston - Glad it's working for you! Yeah it really is a short term solution but got the job done for me. I'll look into a better solution when I have some free time.
Comment #130
jkdaza CreditAttribution: jkdaza commentedI have added few lines to patch #92 in order to support the location inheritance.
It works for me, with Feeds 6.x-1.0-beta1, but please test it.
It's my first tweak to a Drupal module, so any suggestion or comment would be helpful. I do not know how to write a patch so I ll just add the code here, it should be pretty obvious where it goes, after looking at patch #92.
The first bit that add Location to the list of inheritors:
And then the function, very straight forward:
Hope it helps
Comment #131
DanielJohnston CreditAttribution: DanielJohnston commentedHaving tested this for importing nodes, it's working fine. Is there anything similar for importing users though? What I'm looking for is a way of allowing organic group owners / admins to import users and have those users automatically join the import creator's organic group. May be willing to pay a bounty on this if someone can put together a working patch in the very near future, as I'm meant to be going live shortly!
Comment #132
illmatix CreditAttribution: illmatix commentedSubscribing!
Comment #133
jvieille CreditAttribution: jvieille commented#92 + #110 + #121+ #130 => patch beta 2
Thanks!
Comment #134
alex_b CreditAttribution: alex_b commented#133: http://drupal.org/patch Let's keep the noise down.
Comment #135
jvieille CreditAttribution: jvieille commented#134: I am not expert enough to create a patch, even less to create a patch for a version based on a patch from another one.
It is still so much a pain to apply a patch (Tortoise merge on Windows, looks like crap) that I generally modify manually my files... But here it is too complex and the beta 2 code is to much different.
Please apologize for the noise.
Comment #136
alex_b CreditAttribution: alex_b commented#131: I opened a separate issue for your request: #855126: Mapper for a user's OG's
Comment #137
Anonymous (not verified) CreditAttribution: Anonymous commentedYou have the option to use a "stand-alone" form on import. For inheriting language, this stand-alone form should have a language option as well.
Comment #138
1st-angel CreditAttribution: 1st-angel commentedThis is a patch agains beta2 which patches in all the changes from #92 + #110 + #121 and #130.
Comment #139
alex_b CreditAttribution: alex_b commentedGreat!
- Patch should be created against CVS with http://drupal.org/patch/create
- Needs tests.
Comment #140
jvieille CreditAttribution: jvieille commentedThank you so much!
I'll test it
Comment #141
meatbag CreditAttribution: meatbag commentedSubscribing
Comment #142
jvieille CreditAttribution: jvieille commentedThat works perfectly.
Node processor can be set to inherit taxonomy, language and author fro Feed node. This is definitely a splendid achievement!
Thanks!
Comment #143
vlooivlerke CreditAttribution: vlooivlerke commentedIt works!
Thanks, for the great work.
Comment #144
Bartezz CreditAttribution: Bartezz commented#138: I manually added the patch to -beta3 because Tortoise wouldn't apply it.
Nodes do get create upon import yet getting this error:
Cheers
Comment #145
aaron_michels CreditAttribution: aaron_michels commentedSubscribing... thanks for working on this, guys.
Comment #146
domesticat CreditAttribution: domesticat commentedI'm using the patch (hooray and thank you!) but wanted to pose a question to see if this use case has been considered: a site admin can set up taxonomy to inherit if the importer is attached to a node type. However, if the standalone form is used, there's no way to select taxonomy, etc.
I could see this being a design choice, but I wanted to make sure it had been considered.
Comment #147
David Stosik CreditAttribution: David Stosik commentedSubscribing.
Comment #148
Marc Ledergerber CreditAttribution: Marc Ledergerber commentedIt would be great if the "inherit" patch (in #138 above) were committed to the next version of Feeds ==> 6.x-1.0-beta4
Currently, the Feed Node processor does not allow taxonomy to pass to Feed nodes. This patch seems to handle this function.
Please confirm that this function also works in standalone form mode too, so taxonomy can be selected, mapped upon import from a local file (e.g. CSV import of taxonomy and Feed source using Feed Node processor).
Thanks!
Comment #149
klonosIssues are only set to 'patch (to be ported)' once they are fixed in one major version and need to be ported to another (for example fixed in 6.x - need to be ported to 7.x, or the other way 'round). This status is not meant to be used among minor versions (there can never be porting form version 6.x to 6.y).
Also, as repeatedly stated by the module maintainer in several posts (#100, #107, #108, #139) this will only be committed once we have tests for it. I guess he made it pretty clear! I am monitoring all this module's issues and I think it is his 'policy' -so to say- that every new feature needs to have tests before committing. I understand the frustration of people always having to apply patches in order to get things working, but we need to respect the way alex maintains this module and help him get tests. So, setting back to 'needs work'.
Comment #150
jkdaza CreditAttribution: jkdaza commentedCan you please confirm the patch #138 works for beta3?
Thanks
Comment #151
klonosI myself use latest devs in general. I could set up a test environment and test patch #138 with beta3 ...but so can you or anyone else that currently uses beta3 ;)
Just remember to back up all patched files and you can revert to them if something happens to go wrong.
Comment #152
Marc Ledergerber CreditAttribution: Marc Ledergerber commentedSee http://drupal.org/node/862874 re Taxonomy support for mapping fields using "Feed Node processor" when importing from local CSV file with standalone form
Comment #153
alex_b CreditAttribution: alex_b commentedMake it easier to find this issue.
Comment #154
jvieille CreditAttribution: jvieille commented#149, could you clarify?
1) Do you mean we have to wait this module to be final (not beta) to get this feature in?
2) What does "tested" means, in addition to the several posts telling it works great?
Comment #155
aidanlis CreditAttribution: aidanlis commentedI've removed the fieldset from the settings page and used checkboxes for a more consistent look, and I've scaffolded the unit tests. I'll aim to get the tests done tomorrow. New patches attached.
Comment #156
klonos1) I wasn't trying to make any point. You were just using the 'patch to be ported' status the wrong way (Please check the Status link available right under the drop-down menu that allows you to set it)
2) Also check the same link, but you need to know the following because you seem to be confusing testing done by users (should be called 'trials' or something instead of tests) with the SimpleTest... When we talk about tests here, we mean automated tests that are written and meant to provide quality assurance. They help automate test most common tasks a module should be able to do without errors (pass these tests with green).
Comment #157
alex_b CreditAttribution: alex_b commented#155: this looks very nice.
In the test I saw you use
module_exists()
.This will bite you as it will check whether the module is installed in the test while you need to install it in setUp().Just do the same that we do in all other tests: assume that these modules are present, install them in setUp() and add a note into the tests' description that it requires OG and Location (the other modules are core anyway).
We will then add the required modules for tests to the Feeds test profile - which btw I recommend using for tests if you are familiar with drush / drush make.
Comment #158
DanielJohnston CreditAttribution: DanielJohnston commented#155: looks good and have patched successfully with beta 4, BUT it looks like organic groups inheritance still requires manual editing of FeedsNodeProcessor.inc to replace 'mygroupnode' with the name of the content type that you want to check for organic groups membership, as explained in #121 and #124. Is there any way of making this a text box or radio selector in the importer settings?
Comment #159
aidanlis CreditAttribution: aidanlis commented#158: I've used og_get_types, will that do? I initially had a select box, then realised you could have more than one group node so used checkboxes, then realised there's probably a function for this, and voila.
#157: I've fixed up the test to setUp those modules and removed the module_exists checks.
To test inheritance I wrote the function createCustomFeedNode, which allowed me to set arbitrary properties on the node. This could be combined with createFeedNode for simplicity, but I didn't want to poke around too much.
I've added a test for the Author inheritance which passes on my setup, and scaffolded the rest of the tests so that it should be easy for the next person. I haven't used Drupal's SimpleTest before so this was all quite a challenge, I hope it's on the right track.
Comment #160
aidanlis CreditAttribution: aidanlis commentedTypo in #159
Comment #161
alex_b CreditAttribution: alex_b commented160, is this NR now or are you planning on more work?
Comment #162
alex_b CreditAttribution: alex_b commentedCleaning up tags.
Comment #163
alex_b CreditAttribution: alex_b commentedJust go ahead and patch createFeedNode() - that'll be cleaner.
Only user inheritance is tested so far, hence setting back to NW.
Comment #164
aidanlis CreditAttribution: aidanlis commentedYeh, still definitely NW, I ran out of time and thought I'd upload the progress in case anyone else wants to finish it off.
Comment #165
aidanlis CreditAttribution: aidanlis commentedOkay I've installed the feeds_test package, checked out Feeds from CVS, and run all of the unit tests, I'm getting:
459 passes, 6 fails, and 12 exceptions for Feeds. Is that normal? I want to make sure everything's working before I start.
Comment #166
pdrake CreditAttribution: pdrake commentedI have just completed some work on the ability for imported users to inherit the group from the feed node. That is, there is a setting on the user import processor to allow for inheritance. And I have extended my OG mapper to include the ability to map the new user's groups to the feed node's groups. Attached are the patches to add inheritance to the User processor. I will post the actual mapper in the OG mapper thread.
Comment #168
aidanlis CreditAttribution: aidanlis commentedMaybe it would be better if #166 was raised as a new issue, so it doesn't hold up the changes to the FeedsNodeProcessor (unless you have unit tests for your patch?)
Comment #169
pdrake CreditAttribution: pdrake commentedBecause the patch in #166 does not provide any inheritable fields, it merely patches FeedsUserProcessor to allow other modules to hook in and provide inheritable fields, I'm not sure if there are any unit tests required. I would think the tests would need to be created along with the mapper or module which actually provides an inheritable field. Is that not the case?
If there are unit tests required for this patch, let me know and give me some basic direction and I'll try to get something together in the next day or two.
Comment #170
jrabeemer CreditAttribution: jrabeemer commented+1 to get this patch in ASAP
I was expecting Feeds to be able to do this feature like FeedAPI did. :-(
Comment #171
theshanergy CreditAttribution: theshanergy commentedsubscribing
Comment #172
unrev.org CreditAttribution: unrev.org commentedChiming in for subscription and taxo inheritance for feeds. Otherwise the module is quite slick!
Comment #173
jenna.tollersonWhat would it take to get this committed? Would a bounty help? Any labor I can chip in? I know it's because I'm not used to following dev threads like this, but I am really confused about the progress and it would be great to just have an assignment. :)
Comment #174
jrabeemer CreditAttribution: jrabeemer commented#160 seems to be a good basic fix. #166 added more to that relating to OG module. I think testing is needed.
Comment #175
aidanlis CreditAttribution: aidanlis commentedThe only thing holding this up now is the unit tests. I don't know how to write them, I've done 80% of the work but I can't get them to pass. I ping alex_b every day, but he's been AFK for the last two weeks.
Comment #176
pdrake CreditAttribution: pdrake commentedThere were some problems reported with patch not properly applying my patches in #166 so I re-created them. These are patches against beta4 as opposed to #166 which was against beta3.
Comment #177
klonos... I believe new features are done against latest dev versions in order to avoid such things. This way, you get more chances to be included in the next stable (or an alpha or beta)
Comment #178
pdrake CreditAttribution: pdrake commentedGenerally, I do create patches using the latest dev version. In this case, it was work-for-hire and the patches being rolled for a particular release was specified in the statement of work.
Comment #179
klonosThanx for taking the time to reply Peter. I think the patch still works with the latest dev, though it might need manual patching. Perhaps if you were kind enough you'd roll it against it to spare people from the manual work.
Comment #180
jun CreditAttribution: jun commentedsubscribe
Comment #181
zacho CreditAttribution: zacho commentedsubscribing...
Comment #182
rbrownellSubscribing...
Quick question: has this been committed to a release at all? Reading through all of this, I am kind of lost.
-R
Comment #183
Michelle@nfd: Nope, still waiting on someone who knows how to make simpletests.
Michelle
Comment #184
boogsbobo CreditAttribution: boogsbobo commentedsubscribing
Comment #185
jvieille CreditAttribution: jvieille commentedWhat is the problem with tests? It looks like a new (or mainly forgotten) policy that only applies to some modules.
Anyway, a patch is always painful to apply, letting very few users doing the field testing.
On the other hand, a dev version is supposed to be unstable, so incomplete formal testing would not hurt.
Why not committing to an updated dev?
Comment #186
MichelleThere is no policy. It's up to individual module maintainers if they want to use tests or not. Test driven development means less buggy code so I can understand their reasoning. On the other hand, it does make it difficult when you have people wanting to use a feature, willing to test the feature, but that don't know how to make tests.
Michelle
Comment #187
aidanlis CreditAttribution: aidanlis commentedThe patch is already field tested - it works perfectly. The module maintainer has made the decision not to commit anything to CVS without tests, which I think is a great idea. The only problem is the maintainer has been MIA for 2 weeks, and I can't finish the tests without him.
I've re-rolled the patches #166 and #160 into a single patch, which is now attached.
If you're not sure how to use patch on command line, download patch_manager which makes patching modules a breeze.
Comment #188
alex_b CreditAttribution: alex_b commentedBack from vacation, I need to get some time to review #187. aidanlis: can you summarize your open issues with the patch?
#185 jvieille: Tests are required to avoid regressions. In fact, I would argue that maintaining an integration-heavy module like Feeds while staying sane is only possible with tests. I require them on a per patch basis, otherwise, they just don't happen.
Comment #189
aidanlis CreditAttribution: aidanlis commentedHi Alex, hope you had a nice vacation.
If you apply the patch in #187, you'll see when running the Feeds test with Inheritance testcase, the following errors:
"Your configuration has been created with default settings." found Other feeds.test.inc 99 FeedsWebTestCase->createFeedConfiguration()
"Your changes have been saved." found Other feeds.test.inc 139 FeedsWebTestCase->setSettings()
Valid content type found: Other feeds.test.inc 162 FeedsWebTestCase->createFeedNode()
Then a bunch of other errors. I don't know what's going on ... when I perform the actions manually, I do see the strings that the TestCase is failing to see. I'm new to simpletesting with Drupal, so I don't know how to see what output I'm actually getting when the test fails.
Comment #190
David Goode CreditAttribution: David Goode commentedHey all; I feel like an easier way to implement this would be entirely on the parser side, exposing new sources, and then just let the existing mappers take care of sticking it wherever it wants to go depending on the storage method. This was it automatically works with any processors that have such mappers. I have attached a patch that has much of the previous functionality; it doesn't work for things where mappers don't exist though, such as OG group settings (to my knowledge). Also doesn't have tests :-( I copied parts of the previous patches for things like the mapper functions.
This has been tested with the syndication parser and the data mappers for taxonomy; other testing would be required. Includes minor parser change to both syndication and SimplePie.
Comment #191
David Goode CreditAttribution: David Goode commentedTweak for better performance.
Comment #192
David Goode CreditAttribution: David Goode commentedSupport for module-name based features exported vocabs.
Comment #193
AntiNSA CreditAttribution: AntiNSA commentedI am getting really close to needing my feeds to inherit taxonomy I define when inporting feeds. is this feature near ready, or has anyone got it to work?
in detetail:
CONTENT TYPE A holds the url for the feed to import
---holds a custom taxonomy vocab
i,e----cats
-----dogs
CONTENT TYPE B - nodes created from importing the items from the url provided in content A
--has the same taxonomy vocabulary assigned
--------If I selected cats in content type a, every imported feed/freshly created node of CONTENT B would be assigned that taxonomy , in this case "cats"
Thats what I mean when I say inherit taxonomy... is it possible soon?
Comment #194
aidanlis CreditAttribution: aidanlis commented#193: Yes, just install patch #187.
#190: Can you explain your approaches benefits over what we already have in #187?
Comment #195
Will White CreditAttribution: Will White commentedAntiNSA,
It looks like this patch aims to do what you need.
Will
Comment #196
brycesenz CreditAttribution: brycesenz commentedSubscribing.
I am interested in the idea of feed items being able to inherit CCK fields from the feed node (an idea brought up earlier in this thread). I would be willing to help test out patches which cover this use case, or possibly trying to extend an existing patch if the author feels that such an extension could be handled by an admittedly novice PHP coder.
I would also be willing to chip in a modest amount of money (especially if others feel the same way) to help make this a priority. Big ups to everyone's who has put forth work on this so far; it's turned out to be an incredibly awesome module.
Comment #197
David Goode CreditAttribution: David Goode commentedTo #194
Hey, this version can be implemented very easily in basically all parsers--just add a single line. Then, it will work with all processors. It provides a new source, so any processor can then use it as long as it has a compatible target. Taxonomy is already "inheritable" to nodes and data entries with this patch, for instance, without actually changing either processor. It also doesn't require a new UI and involves less code.
On a higher level, it maintains the separation between "sources," associated with parsers, and "targets," which are the realm of processors and their mappers. Inherited content looks the same to the processor as content actually retrieved from the feed or CSV file or whatever, and can be treated by reusing the same storage stack.
Thanks,
David
Comment #198
ufku CreditAttribution: ufku commentedDavid's method is extending the already available mapping functionality by providing an extra source(feed node). A big +1 from me.
Comment #199
realnerd CreditAttribution: realnerd commentedTo #197
How would i go about this if i wanted to do exactly this, but for the xpath parser module?
the code from the module found here http://drupal.org/node/871384
Comment #200
alex_b CreditAttribution: alex_b commented#190 You went there :) I've avoided going through the mapping stack so far as strictly speaking a subscription's feed node is different from the feed item that is being mapped from. However, I see the benefit of the parser level API allowing any processor to use inheritance.
I will need to do a closer review.
In regards to OG support, the approach from #190 would require #857424: Mapper for Organic Groups.
Comment #201
AntiNSA CreditAttribution: AntiNSA commentedpatch 187 failed to cause feed nodes inherit taxonomy for me.
Comment #202
AntiNSA CreditAttribution: AntiNSA commentedI have applied the patch in #192 and after setting the taxonomy, the nodes created and the taxonomy set where you enter the feed address is not inherited by the created nodes. I installed the patch against the latest dev version.
Comment #203
AntiNSA CreditAttribution: AntiNSA commentedIhave tried to assighn vocabulary with content taxonomy and also tried to select taxonomy by assigning taxonomy vocabulary to content types. none of them are being inherited when nodes are being created.
Comment #204
alex_b CreditAttribution: alex_b commented#190 was completely broken, but inspiring. This patch builds on David's argument from #190 that we should be expanding the mapping API in Feeds to implement inheritance rather than creating a similar separate API.
Here's a quick screenshot that should explain most, see how the mapping UI is used to map feed node properties to feed item node properties: http://skitch.com/alexbarth/dikw2/edit-importer-feed-feeds-test-site
Expanding the mapping API has the additional advantage of laying the ground work of mapping feed level import content (e. g. a feed's description or its icon or its link) to feed items. It also is a first step of mapping feed level import content to the feed node itself - this is going to take some serious additional work though.
Expanding the mapping API comes of course at the cost of a rather invasive patch (compare the ~50k to previous ~20k patches). A lot of refactoring had to be done to expand the mapping API from a mapping-target-only API to a source and target API. Now additionally to the existing hooks allowing other modules to expand on available mapping targets (
hook_feeds_node_processor_targets_alter()
orhook_feeds_user_processor_targets_alter()
) you will find a newhook_feeds_parser_sources_alter()
that allows for injecting new mapping sources. See feeds.api.php for more details.The most important change to the existing API is that map() and related functions (existimgItemId(), setSourceElement()) now require to pass in $batch instead of $item. This is necessary as we need to be able to access batch level data:
The availability of $batch->currentItem() also means that in a processor, the batch object should not be consumed with shiftItem() but just iterated over with nextItem():
All tests passing.
Todo: as we don't consume items with shiftItem() anymore, we may run into issues with batching as we will always store all items between page loads now... This issue would go away with #744660: Expand batch support to fetchers and parsers which I don't think we will be able to commit in Drupal 6.
Comment #205
alex_b CreditAttribution: alex_b commentedSwitched back to a
shiftItem()
based approach. This is close to RTBC now, reviews more than appreciated at this point.I haven't mentioned that in #204 explicitly, but patch contains tests for taxonomy, og, locale and uid inheritance, all passing.
Comment #206
ufku CreditAttribution: ufku commentedTested #205 successfully with "Common syndication parser" for "Taxonomy mapping".
Used 3 vocabularies and they were successfully mapped from Feed node to Feed item nodes.
Comment #207
alex_b CreditAttribution: alex_b commentedTo be released w/ 6.x-1.0-beta5.
Comment #208
alex_b CreditAttribution: alex_b commented#205 is committed now. Thank you everybody.
http://drupal.org/cvs?commit=417350
Comment #209
MichelleThanks, Alex!
Michelle
Comment #210
DanielJohnston CreditAttribution: DanielJohnston commentedQuick question: the committed patch appears to incorporate #166. Does this still require / work with #857424: Mapper for Organic Groups or does it provide the full functionality itself? Looking forward to beta 5!
Comment #211
alex_b CreditAttribution: alex_b commented#210: Thanks for the heads up, I responded on #857424: Mapper for Organic Groups
Comment #212
MichelleI've gotten rather lost in this huge issue... Did this ever cover Domain Access? I've got a hacked up version of Feeds so it works with that as well as passing the author name of the parent node to the feed entry node. Looks like author is covered but what about DA?
I know I'm being lazy... If you don't know this off the top of your head and have to look it up, ignore me and I'll go STFQ :)
Michelle
Comment #213
alex_b CreditAttribution: alex_b commentedDoes not cover DA. Has all the API improvements to implement DA real fast though - have a look at mappers/locale.inc.
Comment #214
MichelleAlex - Thanks! Will have a look. I'd like to get rid of my custom code.
Michelle
Comment #215
rbrownellI keep getting this error... I am thinking it is taxonomy inherit related.
user warning: Duplicate entry '13-11738' for key 1 query: INSERT INTO term_node (nid, vid, tid) VALUES (11738, 11738, 13) in /srv/www/html/modules/taxonomy/taxonomy.module on line 716.?
Comment #216
rbrownellWhen is Beta 5 coming out? I can't seem to find it!
Comment #217
alex_b CreditAttribution: alex_b commented#215: please file a separate issue, *describing which configuration you use and how to reproduce the bug!*
beta 5 will come out some time this or next week.
Comment #218
brycesenz CreditAttribution: brycesenz commentedWill this update cover inheritance of CCK fields, or should that be opened as a separate issue?
Comment #219
alex_b CreditAttribution: alex_b commentedThat's a separate issue.
Comment #220
jrabeemer CreditAttribution: jrabeemer commentedI think this is a big enough feature to mention as a bullet point on the Feeds project homepage.
Comment #221
aidanlis CreditAttribution: aidanlis commentedGreat work alex_b and everyone else who contributed!
Comment #222
rbrownellFrom what I can tell, this was not released in 6.x-1.0-beta5! Can anyone confirm?
If it has been, where are the checkmark boxes that you select to tell it to inherrit the variables from the feed node or is this susposed to be automatic? If it is susposed to be automatic with no checkmark boxes required it is not working.
Comment #223
alex_b CreditAttribution: alex_b commentedThis has been released. Please read #204 and see the screenshot linked from it.
Comment #224
AntiNSA CreditAttribution: AntiNSA commentedmy error/sorry
Comment #225
AntiNSA CreditAttribution: AntiNSA commentedthat was my error,, sorry/
Comment #226
rbrownellalex_b: Thanks for letting me know. I was using one of the other patches that applies the taxonomy to all as opposed to haveing it map at the mapping level. Here is something else for you.
The method used causes a conflict with various parsers that require the individual to type a value as the source. See: xPath Parser #908776: No Source Available For Mapping. Should this be a new issue or the same one?
Comment #227
alex_b CreditAttribution: alex_b commentednfd - this needs to be fixed in parsers - #908776: No Source Available For Mapping. There is nothing we can fix in Feeds for that.
Comment #228
Bartezz CreditAttribution: Bartezz commentedJust installed the latest stable version! Great!!! Author, language and taxonomy from the feed node are perfectly mapped.
Thanx for this great feature!!!!
Cheers
Comment #229
jvieille CreditAttribution: jvieille commentedThis is really a neat enhancement. Thanks!
Comment #230
Sericlaire CreditAttribution: Sericlaire commentedHi folks,
When attempting to apply the patch via patch manager apparently "Patching did not go smoothly".
Followed by:
at the bottom.. Is this an error on my part or?
Comment #231
Bartezz CreditAttribution: Bartezz commentedDon't use patch but download latest version...
Cheers
Comment #232
rbrownell@alex_b
Sorry about that, I wasn't totally sure. Sorry for the inconveience.
-R
Comment #233
ehudash CreditAttribution: ehudash commentedThanks for the fix!
Does anyone have new info regarding the #826964: Inherit settings from parent feed node (Domain Access) ?
Comment #234
endiku CreditAttribution: endiku commentedAs I understand it, #205 is now part of the latest beta. I have beta7 and can see the patch information from #205 in the files. However I am not seeing the Feed Node choices in the source dropdown list for mapping for node processor. Was there some key element I have missed in this thread?
Comment #235
alex_b CreditAttribution: alex_b commented@234: Please read notes around patch that was committed. Also look at screenshot.
Comment #236
endiku CreditAttribution: endiku commentedSorry to repeat my question, especially if this is a dumb question. However I have already read all the patch notes.
Is there more to do beyond having this patch implemented? I thought that the additional mapped sources were included in the update. I see them in the feeds/mappers directory. Such as og.inc and taxonomy.inc. However I do not see any additional sources in the drop down such as the screenshot shows.
My understanding is that this update automatically adds source node sources to the source dropdown list without the need to add these sources manually. Am I wrong in that assumption? I understand there is a developer's guide to mapping (http://drupal.org/node/622700) but I am still a little unclear to whether that pertains to adding mapping sources for other modules as well as the sources mentioned in this patch, such as og and taxonomy.
Comment #237
alex_b CreditAttribution: alex_b commented#236 - the additional sources only show up if they actually exist on the feed node's content type. Are you sure that there is e. g. a taxonomy configured for the feed node?
Comment #238
endiku CreditAttribution: endiku commentedTurns out it is because of iCal feed parser.
http://drupal.org/node/914184
Sorry I didn't find that first. Didn't realize the problem until I rebuilt the feed to check what you were saying and saw the sources when it was using the default parser but then not for ical.
Comment #240
jnettikWill this feature be added to iCal parsers?
Comment #241
Michsk CreditAttribution: Michsk commentedI am aswell not really getting this. Can i set a taxonomy term for a feed that will import nodes. So just like i can set the author, can i also declare the term? My nodetype does have the taxonomy but i can't see nothing of it.
Comment #242
Anonymous (not verified) CreditAttribution: Anonymous commentedHello,
Has this patch be ported on Drupal 7?
Comment #243
skolesnyk CreditAttribution: skolesnyk commentedYes, it has. Check out Feed settings at admin/structure/feeds/feed/settings/FeedsNodeProcessor
Comment #244
m4oliveiI'm having trouble using hook_feeds_parser_sources_alter with a CSV Parser. Is it even possible to make user of hook_feeds_parser_sources_alter when a CSV parser is used? I ask because I can't get it to work and also noticed that the patch committed here includes an override to the getMappingSources() function:
Comment #245
AlfTheCat CreditAttribution: AlfTheCat commented@m4olivei have you been able to get this working for CSV imports? I'm not able to get it going either... I've opened a seperate issue #1587482: Cannot inherit taxonomy from feed node when using CSV parser.
Would appreciate any help!