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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex_b’s picture

Title: Taxonomy Propogation » Taxonomy inheritance

This 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.

nickbits’s picture

Hi 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

nickbits’s picture

Hi 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

nickbits’s picture

Hi,

Has anyone else tested this yet? Is it a feature people want?

Nick

willmoy’s picture

No (sorry), yes (please). :)

tobedeleted’s picture

Very 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.

alex_b’s picture

Status: Active » Needs review

#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.

nickbits’s picture

#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

alex_b’s picture

Status: Needs review » Needs work

Great.

- 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).

nickbits’s picture

Alex,

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

alex_b’s picture

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.

Yeah, you gotta speak the local dialect here ;-)

nickbits’s picture

Yeah, you gotta speak the local dialect here ;-)

It's just a case of remembering where "here" is! Catches me out every time!

nickbits’s picture

Hi,

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:

$this->drupalPost('admin/build/feeds/edit/feed/settings/FeedsNodeProcessor', $edit, 'Save');
$this->assertText('Do not allow feed nodes to inherit taxonomy terms.');

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

nickbits’s picture

Priority: Minor » Normal
Status: Needs work » Needs review
paganwinter’s picture

Subscribing...
And what about any other CCK field, like how the FeedAPI Field Inherit module did?

robertDouglass’s picture

Title: Taxonomy inheritance » Inherit settings from parent feed node (like taxonomy)
Status: Needs review » Needs work

I 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.

robertDouglass’s picture

As 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:


function roller_nodeapi(&$node, $op) {
  switch ($op) {
    case 'insert':
      if ($node->type == 'video_external') {
        if ($feed_nid = db_result(db_query("SELECT feed_nid FROM {feeds_node_item} WHERE nid=%d", $node->nid))) {
          $feed_node = node_load($feed_nid);
          $node->domain_site = $feed_node->domain_site;
          $node->domains     = $feed_node->domains;
          $node->subdomains  = $feed_node->subdomains;
        }
      }

      break;
  }
}

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.

Anonymous’s picture

So 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

paganwinter’s picture

Subscribing...

Anonymous’s picture

As 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:

function roller_nodeapi(&$node, $op) {
  switch ($op) {
    case 'insert':
      if ($node->type == 'video_external') {
        if ($feed_nid = db_result(db_query("SELECT feed_nid FROM {feeds_node_item} WHERE nid=%d", $node->nid))) {
          $feed_node = node_load($feed_nid);
          $node->domain_site = $feed_node->domain_site;
          $node->domains     = $feed_node->domains;
          $node->subdomains  = $feed_node->subdomains;
        }
      }

      break;
  }
}

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.

What 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

paganwinter’s picture

Create 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:

  print_r($node).

This will cause all data associated with your node for that node type, to be displayed when you view the respective nodes.

  print_r($node->taxonomy).

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:

function roller_nodeapi(&$node, $op) {
  switch ($op) {
    case 'insert':
      if ($node->type == 'feeds_item') {
        if ($feed_nid = db_result(db_query("SELECT feed_nid FROM {feeds_node_item} WHERE nid=%d", $node->nid))) {
          $feed_node = node_load($feed_nid);
          $node->taxonomy = $feed_node->taxonomy;
        }
      }
      break;
  }
}

I think this should do it.

(Boy, am I bad at explaining stuff...)

skizzo’s picture

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?

@cfaadev: As I understand it, that's what nickbits's patch Taxonomy Inheritance Patch is supposed to do...

skizzo’s picture

@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...

drewish’s picture

subscribing.

lennart’s picture

A 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.

AntiNSA’s picture

This 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.

AntiNSA’s picture

Can someone give a status update on this issue? At least the option of inheriting taxonomy?

AntiNSA’s picture

When 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);

AntiNSA’s picture

Priority: Normal » Critical
alex_b’s picture

Priority: Critical » Normal

#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).

AntiNSA’s picture

ok 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?

AntiNSA’s picture

Title: Inherit settings from parent feed node (like taxonomy) » Inherit settings from parent feed node (like taxonomy) Organic group Audience inheritance; where do I put this?

Can 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?

jboeger’s picture

Definitely a feature I need... subscribing....

Dmxr100’s picture

Inheritance 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.

drupallogic’s picture

without mapping terms to feed items, I can't use Feeds and need to stay with FeedAPI...

hope it's added.

AntiNSA’s picture

Title: Inherit settings from parent feed node (like taxonomy) Organic group Audience inheritance; where do I put this? » Inherit settings from parent feed node (like taxonomy); This should be a core feeds function Sign petition!

Seems 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.

alex_b’s picture

Title: Inherit settings from parent feed node (like taxonomy); This should be a core feeds function Sign petition! » Inherit settings from parent feed node (like taxonomy)

PLease accept this ongoing petition from feeds users and feedapi converts.

There 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.

alex_b’s picture

#17 is a good stop gap solution.

#13 is very close.

  • The portion of the code in FeedsNodeProcessor::process() needs to be broken out into a method FeedsNodeProcessor::inherit() (for easier overridability)
  • there needs to be a check whether $feed_nid is non-zero before attempting to load the node (this is essentially what I said in the IRC excerpt in #16).
alex_b’s picture

#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.

AntiNSA’s picture

:) 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

gravelpot’s picture

subscribing

squinternata’s picture

excuse 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

AntiNSA’s picture

No, it seems that it is not a priority at all for feeds. I hope it will be though!!!1

squinternata’s picture

ok 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

AntiNSA’s picture

44 replies and no solution? Come on Drupal!!!!! Go Go Go! Where is the solution? We need this!!!!

skizzo’s picture

uhm... 10 out of 45 come from the same user :-)

bflora’s picture

Subscribing...

pdcarto’s picture

On 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.

pdcarto’s picture

I'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?

AntiNSA’s picture

I 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....

jtr’s picture

For 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:

<?php
function feeds_custom_inherit_nodeapi(&$node, $op) {
  switch ($op) {
    case 'prepare':
    case 'insert':
      if ($node->type == 'feed_item') {
          if($op == 'prepare') {
              $feed_node = node_load((int)$node->feeds_node_item->feed_nid);
          } else {
            if ($feed_nid = db_result(db_query("SELECT feed_nid FROM {feeds_node_item} WHERE nid=%d", $node->nid))) {
               $feed_node = node_load((int)$feed_nid);
            } else {break;}
          }
          $node->uid = $feed_node->uid;
          $node->og_groups = $feed_node->og_groups;
          $node->og_groups_both = $feed_node->og_groups_both;
          $node->og_public = $feed_node->og_public;
          $node->taxonomy = $feed_node->taxonomy;
        }
      break;
  }
}
?>

I also had to comment out the line in FeedsNodeProcessor.inc that does this:

$node->uid = 0;

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.

pdcarto’s picture

W/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

alex_b’s picture

Pat - 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.

AntiNSA’s picture

I am a poor teacher in China I would help if I could but I cant afford it and have no credit card :(

pdcarto’s picture

Re #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?

pdcarto’s picture

Attached 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.

AntiNSA’s picture

Super effort! This is in my nodequeue to test. Hopefully I can test in the next 24 hours

poloparis’s picture

Hi, 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 ?

AntiNSA’s picture

Title: Inherit settings from parent feed node (like taxonomy) » Inherit settings from parent feed node (like taxonomy) WE HAVE PATCH! Help Debug PLease!!!

Thanks 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...

AntiNSA’s picture

Ok 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

alex_b’s picture

FileSize
5.98 KB

#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 a hook_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?

alex_b’s picture

Title: Inherit settings from parent feed node (like taxonomy) WE HAVE PATCH! Help Debug PLease!!! » Inherit settings from parent feed node (like taxonomy)

AntiNSA: 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?

pdcarto’s picture

Status: Needs work » Needs review

Re #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?

AntiNSA’s picture

#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.

AntiNSA’s picture

All apologies. I love drupal.. and thanks for the effort. to not mistake determination wwith anything else.

Gerhard Killesreiter’s picture

@ 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.

pdcarto’s picture

Alex 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)...

klonos’s picture

@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 ;)

AntiNSA’s picture

The 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?

iaminawe’s picture

I 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

AntiNSA’s picture

yes you can use the module called "Better Formats" do set default content filter formats. Thats what I did

skizzo’s picture

I 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?

slicedsoup’s picture

This is working for me. Great patch.

bflora’s picture

patch is working for me. Thanks!

idflorin’s picture

patch #61 works for me with 6.x-1.0-alpha12
---------
earth climate made with feeds module

miche’s picture

patch #61
ONLY tested "inherit feed author" on 6.x-1.0-alpha14. WORKED!

Michelle’s picture

#61 works for me with the latest dev snapshot. I tested taxonomy and author, not OG.

Thanks!

Michelle

srobert72’s picture

Subscribing
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.

alex_b’s picture

Status: Needs review » Needs work

Setting to NW as patch needs tests (#67) - pdcarto, still on to it? Does somebody else want to step up?

srobert72’s picture

Here 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 ?

dixon_’s picture

Status: Needs work » Needs review

Here is a re-roll of the patch from #61. I've added documentation to feeds.api.php and made the translation inheritor depend on locale.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:

  • Write simple tests for all inheritors. (This is a commit blocker if I understand things right.)
  • Document the inheritor functionality and the new hook in the handbook here on d.o. (I'll gladly do that when this gets committed.)

Even though is still not 100% complete, I'm marking this as "needs review" to get some eyes on it.

//dixon_, NodeOne

dixon_’s picture

FileSize
7.03 KB

And here is the patch ;)

// dixon_, NodeOne

srobert72’s picture

I 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.

srobert72’s picture

Here 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.

ehudash’s picture

I 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!

ehudash’s picture

Status: Needs review » Reviewed & tested by the community

Would 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 :)

pdcarto’s picture

Status: Reviewed & tested by the community » Needs review

My 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.

alex_b’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

#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!

srobert72’s picture

patch #82 is no more compatible with last DEV release (2010-May-17)

@dixon_
Could you update it ? Thanks a lot

ccoppen’s picture

In 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.

ccoppen’s picture

FileSize
631 bytes

Attaching 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?

ccoppen’s picture

FileSize
6.99 KB

Sorry 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.

skizzo’s picture

Applied 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?

srobert72’s picture

Patch #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

ccoppen’s picture

Well, 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. :/

Exploratus’s picture

All this needs is Domain Access, and it would be complete.

arlind’s picture

Definitely a feature I need... subscribing....

deggertsen’s picture

Tested #92 and works great. No problems so far. This needs to be put in.

Thanks!

ufku’s picture

Status: Needs work » Reviewed & tested by the community

I think this is ready to go.
Please submit other inheritance related feature requests as separate issues.

alex_b’s picture

Status: Reviewed & tested by the community » Needs work

We are very close here, yep, but we need tests!

michellezeedru’s picture

Subscribing .. 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!

Anonymous’s picture

Just 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.

klonos’s picture

I 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).

Jerimee’s picture

subscribing

NCtiNet’s picture

subscribing

arlind’s picture

Hi,

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

alex_b’s picture

#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 ;-)

alex_b’s picture

deggertsen’s picture

It 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.

hello@melmcdougall.com’s picture

Patch #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(),
);
}

NCtiNet’s picture

Why not add these categories in the taxonomy?

<?xml version="1.0" encoding="utf-8"?>
<rss version="2.0" xmlns:media="http://search.yahoo.com/mrss/">
<channel>
	<pubDate>Mon, 28 Jun 10 13:26:54 -0400</pubDate>
	<language>en</language>

	<item>
		<title>Avril Lavigne - Hot</title>
		<description><![CDATA[Music video by Avril Lavigne performing Hot. (C) 2007 RCA/JIVE Label Group, a unit of Sony Music Entertainment]]></description>
		<link>http://youtube.com/?v=fzb75m8NuMQ</link>
		<media:player url="http://youtube.com/?v=fzb75m8NuMQ" />
		<media:thumbnail url="http://img.youtube.com/vi/fzb75m8NuMQ/default.jpg" width="120" height="90" />
		<media:title>Avril Lavigne - Hot</media:title>

		<media:category label="Tags">Avril, Lavigne, RCA, Records, Label, Pop</media:category>
		<media:credit>AvrilLavigneVEVO</media:credit>
		<enclosure url="http://youtube.com/v/fzb75m8NuMQ.swf" length="202" type="application/x-shockwave-flash"/>
	</item>
	<item>
		<title>Inna - Hot (True Love Video Edit) (On Ultra Dance 11 out now)</title>
		<description><![CDATA[Inna - Hot (True Love Video Edit) from Ultra Records. Get Ultra T-shirts here: www.ultrarecords.storenvy.com Ultra Dance 11 out now. Buy the US album on iTunes bit.ly UK Release Date March 14]]></description>

		<link>http://youtube.com/?v=1rh3_r0nbKs</link>
		<media:player url="http://youtube.com/?v=1rh3_r0nbKs" />
		<media:thumbnail url="http://img.youtube.com/vi/1rh3_r0nbKs/default.jpg" width="120" height="90" />
		<media:title>Inna - Hot (True Love Video Edit) (On Ultra Dance 11 out now)</media:title>
		<media:category label="Tags">Inna, Hot, True, Love, Video, Edit, ultra, records, house, trance, music, progressive, electro</media:category>
		<media:credit>UltraRecords</media:credit>
		<enclosure url="http://youtube.com/v/1rh3_r0nbKs.swf" length="225" type="application/x-shockwave-flash"/>

	</item>
</channel>
</rss>

Thanks.

palpatine1976’s picture

I'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!

klonos’s picture

as Alex said before (#100)... all we now need is tests and we are ready to commit.

Equinger’s picture

I 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.

smithmilner’s picture

You 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.

	if ($feed_node->type == 'mygroupnode') {
		//set the group array just incase
		$item_node->og_groups = array();
		//take mygroupnode's nid as the group id
		$gid = $feed_node->nid;
		//inserts the mygroupnode NID as the og_group value
		$item_node->og_groups = array( $gid => $gid );
		//inserts the mygroupnode title as group name
		$item_node->og_groups_both = array( $gid => $feed_node->title);
	}else {

	  if (isset($feed_node->og_public) ) {
	    if (!isset($item_node->og_public)) {
	      $item_node->og_public = 1;
	    }
	    $item_node->og_public = $item_node->og_public & $feed_node->og_public;
	  }
	  if (isset($feed_node->og_groups)) {
	    if (!(isset($item_node->og_groups) && is_array($item_node->og_groups))) {
	      $item_node->og_groups = array();
	    }
	    $item_node->og_groups = array_merge($feed_node->og_groups, $item_node->og_groups);
	    foreach ($item_node->og_groups as $gid) {
	      if ($gid != 0) {
	        $transformed_groups[$gid] = $gid;
	      }
	    }
	    $item_node->og_groups = $transformed_groups;
	  }
	  if (isset($feed_node->og_groups_names)) {
	    if (!is_array($item_node->og_groups_names)) {
	      $item_node->og_groups_names = array();
	    }
	    $item_node->og_groups_names = array_merge($feed_node->og_groups_names, $item_node->og_groups_names);
	  }
				
	}

What do you guys think?

awakenedvoice’s picture

Subscribing

jkdaza’s picture

Patch #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?

jvieille’s picture

Impossible 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

jvieille’s picture

smithmilner, what to do with this code? When where to invoke it, what it does exactly?

Thanks

DanielJohnston’s picture

Subscribing. Specifically interested in allowing organic group admins to import nodes and users that automatically get assigned as appropriate audience / group members.

smithmilner’s picture

Ok, Apply Patch from comment #92 and go to plugins/FeedsNodeProcessor.inc

and replace the feeds_inherit_og function with:

<?php
/**
 * Inherit OG properties.
 */
function feeds_inherit_og($item_node, $feed_node) {
	if ($feed_node->type == 'mygroupnode') {
        //set the group array just incase
        $item_node->og_groups = array();
        //take mygroupnode's nid as the group id
        $gid = $feed_node->nid;
        //inserts the mygroupnode NID as the og_group value
        $item_node->og_groups = array( $gid => $gid );
        //inserts the mygroupnode title as group name
        $item_node->og_groups_both = array( $gid => $feed_node->title);
	}else {

	  if (isset($feed_node->og_public) ) {
	    if (!isset($item_node->og_public)) {
	      $item_node->og_public = 1;
	    }
	    $item_node->og_public = $item_node->og_public & $feed_node->og_public;
	  }
	  if (isset($feed_node->og_groups)) {
	    if (!(isset($item_node->og_groups) && is_array($item_node->og_groups))) {
	      $item_node->og_groups = array();
	    }
	    $item_node->og_groups = array_merge($feed_node->og_groups, $item_node->og_groups);
	    foreach ($item_node->og_groups as $gid) {
	      if ($gid != 0) {
	        $transformed_groups[$gid] = $gid;
	      }
	    }
	    $item_node->og_groups = $transformed_groups;
	  }
	  if (isset($feed_node->og_groups_names)) {
	    if (!is_array($item_node->og_groups_names)) {
	      $item_node->og_groups_names = array();
	    }
	    $item_node->og_groups_names = array_merge($feed_node->og_groups_names, $item_node->og_groups_names);
	  }
				
	}
}

?>

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.

Steven Jones’s picture

subscribe.

jvieille’s picture

But what is the proposed change for this function?
Now waiting for a patch that works with Beta 2...

smithmilner’s picture

The 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.

<?php
    if ($feed_node->type == 'league') {
        //set the group array just incase
        $item_node->og_groups = array();
        //take mygroupnode's nid as the group id
        $gid = $feed_node->nid;
        //inserts the mygroupnode NID as the og_group value
        $item_node->og_groups = array( $gid => $gid );
        //inserts the mygroupnode title as group name
        $item_node->og_groups_both = array( $gid => $feed_node->title);
    }else {
?>

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.

epersonae2’s picture

NCtiNet (#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.

XiaN Vizjereij’s picture

Can someone please reroll the patch for beta2?

DanielJohnston’s picture

@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!

jvieille’s picture

A patch for beta 2 from #92 + #110 + #121 would be much appreciated!
... before beta 3 comes out!
Thanks

smithmilner’s picture

@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.

jkdaza’s picture

I 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:

 if (module_exists('location')) {
      $inheritors['location'] = array(
        'name' => t('Inherit location'),
        'description' => t('Feed nodes inherit location settings from feed node.'),
        'callback' => 'feeds_inherit_location',
      );
   }

And then the function, very straight forward:


/**
 * Inherit location properties.
 */
function feeds_inherit_location($item_node, $feed_node) {
  $item_node->locations[0] = $feed_node->locations[0];
  $item_node->locations[0] = $feed_node->locations[0];
}


Hope it helps

DanielJohnston’s picture

Having 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!

illmatix’s picture

Subscribing!

jvieille’s picture

#92 + #110 + #121+ #130 => patch beta 2
Thanks!

alex_b’s picture

#133: http://drupal.org/patch Let's keep the noise down.

jvieille’s picture

#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.

alex_b’s picture

#131: I opened a separate issue for your request: #855126: Mapper for a user's OG's

Anonymous’s picture

You 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.

1st-angel’s picture

Status: Needs work » Needs review
FileSize
7.87 KB

This is a patch agains beta2 which patches in all the changes from #92 + #110 + #121 and #130.

alex_b’s picture

Status: Needs review » Needs work

Great!

- Patch should be created against CVS with http://drupal.org/patch/create
- Needs tests.

jvieille’s picture

Thank you so much!
I'll test it

meatbag’s picture

Subscribing

jvieille’s picture

That works perfectly.
Node processor can be set to inherit taxonomy, language and author fro Feed node. This is definitely a splendid achievement!

Thanks!

vlooivlerke’s picture

It works!

Thanks, for the great work.

Bartezz’s picture

#138: I manually added the patch to -beta3 because Tortoise wouldn't apply it.

Nodes do get create upon import yet getting this error:

warning: Invalid argument supplied for foreach() in C:\xampplite\htdocs\drupal\sites\all\modules\feeds\plugins\FeedsNodeProcessor.inc on line 412.
#line 412:
 foreach ($this->config['inherit'] as $key => $value) {

Cheers

aaron_michels’s picture

Subscribing... thanks for working on this, guys.

domesticat’s picture

I'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.

David Stosik’s picture

Subscribing.

Marc Ledergerber’s picture

Status: Needs work » Patch (to be ported)
Issue tags: -Needs tests +6.x-1.0-beta4

It 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!

klonos’s picture

Status: Patch (to be ported) » Needs work
Issue tags: -6.x-1.0-beta4 +Needs tests

Issues 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'.

jkdaza’s picture

Can you please confirm the patch #138 works for beta3?
Thanks

klonos’s picture

I 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.

Marc Ledergerber’s picture

See http://drupal.org/node/862874 re Taxonomy support for mapping fields using "Feed Node processor" when importing from local CSV file with standalone form

alex_b’s picture

Title: Inherit settings from parent feed node (like taxonomy) » Inherit properties from parent feed node (taxonomy, author, OG, language)

Make it easier to find this issue.

jvieille’s picture

#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?

aidanlis’s picture

FileSize
2.82 KB
6.97 KB

I'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.

klonos’s picture

1) 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).

alex_b’s picture

#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.

DanielJohnston’s picture

#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?

aidanlis’s picture

FileSize
6.79 KB
5.01 KB

#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.

aidanlis’s picture

FileSize
6.78 KB

Typo in #159

alex_b’s picture

Status: Needs work » Needs review

160, is this NR now or are you planning on more work?

alex_b’s picture

Cleaning up tags.

alex_b’s picture

Status: Needs review » Needs work

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.

Just go ahead and patch createFeedNode() - that'll be cleaner.

Only user inheritance is tested so far, hence setting back to NW.

aidanlis’s picture

Yeh, still definitely NW, I ran out of time and thought I'd upload the progress in case anyone else wants to finish it off.

aidanlis’s picture

Okay 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.

pdrake’s picture

I 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.

aidanlis’s picture

Maybe 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?)

pdrake’s picture

Because 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.

jrabeemer’s picture

+1 to get this patch in ASAP
I was expecting Feeds to be able to do this feature like FeedAPI did. :-(

theshanergy’s picture

subscribing

unrev.org’s picture

Chiming in for subscription and taxo inheritance for feeds. Otherwise the module is quite slick!

jenna.tollerson’s picture

What 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. :)

jrabeemer’s picture

#160 seems to be a good basic fix. #166 added more to that relating to OG module. I think testing is needed.

aidanlis’s picture

The 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.

pdrake’s picture

FileSize
2.82 KB
684 bytes

There 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.

klonos’s picture

... 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)

pdrake’s picture

Generally, 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.

klonos’s picture

Thanx 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.

jun’s picture

subscribe

zacho’s picture

subscribing...

rbrownell’s picture

Subscribing...

Quick question: has this been committed to a release at all? Reading through all of this, I am kind of lost.

-R

Michelle’s picture

@nfd: Nope, still waiting on someone who knows how to make simpletests.

Michelle

boogsbobo’s picture

subscribing

jvieille’s picture

What 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?

Michelle’s picture

There 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

aidanlis’s picture

The 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.

alex_b’s picture

Status: Needs work » Needs review

Back 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.

aidanlis’s picture

Hi 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.

David Goode’s picture

FileSize
5.3 KB

Hey 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.

David Goode’s picture

FileSize
5.33 KB

Tweak for better performance.

David Goode’s picture

FileSize
5.72 KB

Support for module-name based features exported vocabs.

AntiNSA’s picture

I 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?

aidanlis’s picture

#193: Yes, just install patch #187.
#190: Can you explain your approaches benefits over what we already have in #187?

Will White’s picture

AntiNSA,
It looks like this patch aims to do what you need.

Will

brycesenz’s picture

Subscribing.

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.

David Goode’s picture

To #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

ufku’s picture

David's method is extending the already available mapping functionality by providing an extra source(feed node). A big +1 from me.

realnerd’s picture

Issue tags: +Needs tests

To #197

this version can be implemented very easily in basically all parsers--just add a single line.

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

alex_b’s picture

#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.

AntiNSA’s picture

patch 187 failed to cause feed nodes inherit taxonomy for me.

AntiNSA’s picture

I 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.

AntiNSA’s picture

Ihave 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.

alex_b’s picture

Issue tags: -Needs tests
FileSize
49.48 KB

#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() or hook_feeds_user_processor_targets_alter()) you will find a new hook_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:

// Previously:
$this->map($item, $target) {
  // Map from $item;
}

// Now:
$this->map($batch, $target) {
  // Map from $batch->currentItem();
  // or map from $batch->feedNode();
}

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():

// Previously:
while ($item = $batch->shiftItem()) {
}
// Now:
while ($item = $batch->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.

alex_b’s picture

Issue tags: -Needs tests
FileSize
50.06 KB

Switched 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.

ufku’s picture

Tested #205 successfully with "Common syndication parser" for "Taxonomy mapping".
Used 3 vocabularies and they were successfully mapped from Feed node to Feed item nodes.

alex_b’s picture

Issue tags: +6.x-1.0-beta5

To be released w/ 6.x-1.0-beta5.

alex_b’s picture

Status: Needs review » Fixed

#205 is committed now. Thank you everybody.

http://drupal.org/cvs?commit=417350

Michelle’s picture

Thanks, Alex!

Michelle

DanielJohnston’s picture

Quick 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!

alex_b’s picture

#210: Thanks for the heads up, I responded on #857424: Mapper for Organic Groups

Michelle’s picture

I'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

alex_b’s picture

Does not cover DA. Has all the API improvements to implement DA real fast though - have a look at mappers/locale.inc.

Michelle’s picture

Alex - Thanks! Will have a look. I'd like to get rid of my custom code.

Michelle

rbrownell’s picture

I 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.?

rbrownell’s picture

When is Beta 5 coming out? I can't seem to find it!

alex_b’s picture

#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.

brycesenz’s picture

Will this update cover inheritance of CCK fields, or should that be opened as a separate issue?

alex_b’s picture

That's a separate issue.

jrabeemer’s picture

I think this is a big enough feature to mention as a bullet point on the Feeds project homepage.

aidanlis’s picture

Great work alex_b and everyone else who contributed!

rbrownell’s picture

Status: Fixed » Active

From 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.

alex_b’s picture

Status: Active » Fixed

This has been released. Please read #204 and see the screenshot linked from it.

AntiNSA’s picture

my error/sorry

AntiNSA’s picture

that was my error,, sorry/

rbrownell’s picture

Status: Fixed » Active

alex_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?

alex_b’s picture

Status: Active » Fixed

nfd - this needs to be fixed in parsers - #908776: No Source Available For Mapping. There is nothing we can fix in Feeds for that.

Bartezz’s picture

Just installed the latest stable version! Great!!! Author, language and taxonomy from the feed node are perfectly mapped.
Thanx for this great feature!!!!

Cheers

jvieille’s picture

This is really a neat enhancement. Thanks!

Sericlaire’s picture

Hi folks,

When attempting to apply the patch via patch manager apparently "Patching did not go smoothly".

Followed by:

Patching file tests/feeds_mapper_taxonomy.test using Plan A...
Hunk #1 FAILED at 38.
Hunk #2 succeeded at 55 with fuzz 2 (offset -21 lines).
Hunk #3 FAILED at 149.
Hunk #4 succeeded at 390 with fuzz 2 (offset 170 lines).
2 out of 4 hunks FAILED -- saving rejects to file tests/feeds_mapper_taxonomy.test.rej
done

at the bottom.. Is this an error on my part or?

Bartezz’s picture

Don't use patch but download latest version...

Cheers

rbrownell’s picture

@alex_b

Sorry about that, I wasn't totally sure. Sorry for the inconveience.

-R

ehudash’s picture

Thanks for the fix!
Does anyone have new info regarding the #826964: Inherit settings from parent feed node (Domain Access) ?

endiku’s picture

As 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?

alex_b’s picture

@234: Please read notes around patch that was committed. Also look at screenshot.

endiku’s picture

Sorry 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.

alex_b’s picture

#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?

endiku’s picture

Turns 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.

Status: Fixed » Closed (fixed)

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

jnettik’s picture

Will this feature be added to iCal parsers?

Michsk’s picture

I 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.

Anonymous’s picture

Hello,

Has this patch be ported on Drupal 7?

skolesnyk’s picture

Yes, it has. Check out Feed settings at admin/structure/feeds/feed/settings/FeedsNodeProcessor

m4olivei’s picture

I'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:

class FeedsCSVParser extends FeedsParser {

...

  /**
   * Override parent::getMappingSources().
   */
  public function getMappingSources() {
    return FALSE;
  }

...

}
AlfTheCat’s picture

@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!