Attach multiple importers to one content type (in D7)

Subscribe

Some initial work on this, completely untested.

No upgrade path at the moment either.

The following project (only D6 right now) seems to do the very thing. Am I right?
http://drupal.org/project/feeds_node_multisource

No, I think that other module is taking multiple feeds and combining them to create the same nodes, for example I might want to take RSS feeds from BBC news and Guardian news and create a single node for each story with both the BBC and Guardian versions of the story.

This issue is about just having multiple feed importers associated with a single node type, so that I can add multiple feed sources when creating a feed-node. These feeds will be handled completely separately however.

Changing title to differ between #634462 and #1127696.

Previous patch was broken, this has now been tested, and you can import/clear individual sources.

Actually that one was broken also, this is very much work-in-progress stuff.

More patches to get the weight of the importers correct.

Hi,
last patch works for me, good job. Thank you.

Just a note, in node form importers should be optional because if using a single importer the following had to be configured too. An example:
- mailhandler importer
- rss importer

both have required fields, how could setup only one?
Maybe as in the import/delete-items tabs, could be listed as checkboxes and created via ahah callbacks?

Patch does not apply. Please re-roll so I can test.

I've manually applied the patch, re-rolled it and tested it. I may have misunderstood, but I assumed that you could have 1 content type that could optionally attach to any of the importers. For example I have 4 importers: vimeo importer, youtube importer, flickr importer and soundcloud importer. I don't want to set up 4 different content types for these, I just want "feed importer". Feed importer nodes should be able to choose 1 (or more) importers and it should certainly not be mandatory to attach a feed of each type.

This seems similar to the major change in Views Bulk Operations recently where instead of creating a VBO display type they changed VBO to a field. Maybe we need to create a Feed Importer field with importer type and url. Then one importer node could handle many feeds of many types (or many of one type).

I've done some work to make importers optional, and also I put the importers in vertical_tabs to clean it up a bit. I've only tested this with FeedsHTTPFetcher. I was able to import 2 different feeds. I still think a better approach would be an importer field rather than an importer content type.

i couldn't get this patch to apply using cygwin.

This is the command i ran patch -p0 < 1127696-Attach-multiple-importers-to-one-content-type2.patch

this is what was returned:

can't find file to patch at input line 17
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|From c156aa9ad157a548c57c044eb98184707db2cbfe Mon Sep 17 00:00:00 2001
|From: Michael S <>
|Date: Mon, 21 Nov 2011 13:35:06 +1000
|Subject: [PATCH] #1127696 - Attach multiple importers to one content type (in D7)
|
|---
| feeds.module | 199 ++++++++++++++++++++++++++++++----------
| feeds.pages.inc | 135 ++++++++++++++++++++-------
| includes/FeedsImporter.inc | 7 ++
| plugins/FeedsNodeProcessor.inc | 46 +++++----
| 4 files changed, 286 insertions(+), 101 deletions(-)
|
|diff --git a/feeds.module b/feeds.module
|index 543b24a..1cde14c 100644
|--- a/feeds.module
|+++ b/feeds.module

Patches can be created with many mechanisms, as long as they will apply with patch -p1 or git apply.

http://drupal.org/node/1054616

Use patch -p1 instead of patch -p0.

I applied the patch in #12 and tried it with the file upload fetcher. I am no longer able to upload any files, even with importers that are not attached to nodes.

BTW: I like the concept of an importer field.

Hi, what is the current workaround for this issue?

For example. I too have a content type (called Student) that would ideally have multiple Feeds (twitter, blog, ...).

I have created multiple importers and I can actually assign these tho the same content type.

BUT, I only see one Feed field under my Student content type.

Logically the "Student" content-type should somehow have connections/references to different feeds.

So what is the solution/workaround? What are my options?

@colle901 - I didn't test with file uploader at all, so that's a shame it doesn't work. I likely won't have time to look in to it.

@mareks - Applying the patch in #12 should provide a workaround for you for now, but beware there could be other issues, such as mentioned in #15.

#12 is not the way to go. The form alter shouldn't remove the required attribute and change the title for the source config form. It might work for some, but will fail on others.

There must be another way...

@Xen what about the idea of an importer field rather than an importer node.

@mstrelean
Frankly, I personally don't like it. It makes feeds part of the content type, which may not always be what you want. I'm using feeds to pull in commit logs for Project nodes, and the custom fetcher knows how to extract the information it needs to generate urls from the Project node. There's nothing on the project node that's specific for the feeds it might, heh, feed. The reason why I want to attach multiple feeds is that the projects might be hosted different places, requiring different importers (different formats, etc), and the plan is to have more importers that pulls other kind of activity.

It wont be impossible with a feed field, but I'll have to update the content type when adding feeds, and it just feels cleaner that feeds attaches itself to nodes rather than the other way round.

Well, that's my personal feeling, anyway.

But I do appreciate a certain elegance in the field way. Comparing with VBO doesn't make any sense, though, as we're talking views displays and views fields, contra custom attaching and Field API fields (unless I've missed something).

As for the issue at hand, I think it would make more sense to be able to mark the importers required or not in the importer setup, and modify the existing fetchers so they only make their form fields required when the importer is. Then there needs to be a way for feeds to ask the fetcher if the submitted data is valid for running the fetcher, or whether it should be considered 'empty' and thus skipped. Well, that's off the top of my head.

Slightly related to #1406260: Fetchers without source configuration fails..

I can confirm that #11 works, for attaching and running multiple importers.

I suggest that we get this committed, and work on the issue of required arguments to importers in another issue, as the current patch solves some peoples problems.

Hi

Thanks for the update do you think this will work for my case? http://drupal.org/node/1352048

thanks

Lee

@alwoodman
I doubt it. This patch only ensures that multiple importers on the same node can exist. Sounds like your problem is that one feed doesn't recognize the GUIDs of the others. But you can try it out.

@alwoodman, the problem is that a feed/importer always imports a complete node. Partially completing a node is possible with this patch #1047894: Behavior when fields/columns are not in import file: do not clear field, but leave field untouched..
Also, you cannot update nodes which are not created by Feeds itself, because the GUID is stored in a feeds-maintained table. But, there's a patch for that: #661606: Support unique targets in mappers

subscribing

subscribing

subscribing

patch no longer applies cleanly:

atch -p1 < 1127696-Attach-multiple-importers-to-one-content-type2.patch
patching file feeds.module
Hunk #1 succeeded at 371 (offset 16 lines).
Hunk #2 succeeded at 519 (offset 25 lines).
Hunk #3 succeeded at 544 (offset 25 lines).
Hunk #4 FAILED at 572.
Hunk #5 FAILED at 589.
Hunk #6 succeeded at 629 (offset 25 lines).
Hunk #7 succeeded at 640 (offset 25 lines).
Hunk #8 succeeded at 650 (offset 25 lines).
Hunk #9 succeeded at 756 (offset 25 lines).
2 out of 9 hunks FAILED -- saving rejects to file feeds.module.rej
patching file feeds.pages.inc
patching file includes/FeedsImporter.inc
Hunk #1 succeeded at 242 (offset 6 lines).
Hunk #2 succeeded at 270 (offset 6 lines).
patching file plugins/FeedsNodeProcessor.inc
Hunk #1 succeeded at 267 (offset 77 lines).
Hunk #2 succeeded at 347 with fuzz 2 (offset 102 lines).
Hunk #3 succeeded at 383 with fuzz 2 (offset 105 lines).

Needs re-roll then.

can someone, please, create new patch?

here it is, re-rolled against current dev.

still works fine so rtbc.

here is also the interdiff of the additional effort in #12 - as of #15 this seemed to have issues.

as i understand it this is NOT for commit, only for analysis.

+++ b/feeds.module
@@ -371,22 +371,30 @@ function feeds_theme() {
+  // Loop through all importers. if the user has access to one, they have access
+  // to the item.
+  foreach ($importer_ids as $importer_id){
+    // Check for permissions if feed id is present, otherwise return FALSE.
+    if ($importer_id) {
+      if (user_access('administer feeds') || user_access($action . ' ' . $importer_id .' feeds')) {
+        return TRUE;
+      }
     }
   }

This piece of access check doesn't look right to me. "If user has access to one, they have access to the item."

So does this means a user can get undesired access to additional importers? I don't see the access being checked anywhere else, so I think we should either change this to a restrictive check, i.e "if one fails, all fail" or check access for each importer on node form and import form, which is a little more difficult

We also need tests for this, but it's looking good

I'm using the latest dev version (7.x-2.0-alpha5+21-dev) and patch from #31. It doesn't seem to work:

(Stripping trailing CRs from patch.)
patching file feeds.module
Hunk #1 FAILED at 371.
Hunk #2 succeeded at 543 (offset 24 lines).
Hunk #3 succeeded at 569 (offset 24 lines).
Hunk #4 FAILED at 637.
Hunk #5 succeeded at 654 (offset 24 lines).
Hunk #6 succeeded at 665 (offset 24 lines).
Hunk #7 succeeded at 675 (offset 24 lines).
Hunk #8 succeeded at 770 (offset 24 lines).
2 out of 8 hunks FAILED -- saving rejects to file feeds.module.rej
(Stripping trailing CRs from patch.)
patching file feeds.pages.inc
Hunk #1 FAILED at 66.
Hunk #2 FAILED at 96.
Hunk #3 succeeded at 119 (offset 6 lines).
Hunk #4 succeeded at 178 (offset 6 lines).
Hunk #5 succeeded at 190 (offset 6 lines).
Hunk #6 succeeded at 255 (offset 6 lines).
2 out of 6 hunks FAILED -- saving rejects to file feeds.pages.inc.rej
(Stripping trailing CRs from patch.)
patching file includes/FeedsImporter.inc
(Stripping trailing CRs from patch.)
patching file plugins/FeedsNodeProcessor.inc
Hunk #1 succeeded at 204 (offset 10 lines).
Hunk #2 succeeded at 284 (offset 10 lines).
Hunk #3 succeeded at 320 (offset 10 lines).

Am I doing something wrong?

report: i'm actively using this patch, but due to it's global nature it has several flaws and i'm having numerous additional patch fixes here. i think the only sensible way to handle this is to create a feature branch. if someone (@franz?) gives me the rights i will create one.

Wow, less than a month and already breaking the patch that much. I'm ok with creating the branch, but I don't have access to adding more people. Try leaving a message to twistor.

You can always publish on github or something, we can merge later.

Was anyone able to re-roll this?

although there seem to be no european feeds lovers who will sprint with me, i can look into this in the next week.

Wow, this was a major re-roll because the codebase ran away.
Also i did thorough testing in production and fixed quite some issues.
as this really is a monster patch i created a branch in my sandbox for it:
git clone --recursive --branch multiple-importers http://git.drupal.org/sandbox/axel.rutz/1739562.git feeds_sandbox
and am willing to integrate further fixes.

i really hope we soon get this tested and in because rebasing is always pain with such a big patch.
So please help testing is, it's one of the last issues blocking a new beta imho.

It seems like this is a lot of work to support a stupid legacy behavior of Feeds. What I would really like to see is each Feed become its own entity, but that will take a 3.x branch I believe.

For 2.x, why not make the field source a proper field? We should be able to upgrade existing importers to use it and attach it to nodes on update. This would take care of, #1257314: Ability to attach a feeds source to any entity, not only nodes at the same time. I know what I'm proposing is an even bigger task, it just seems that this task is large, and I do not see the use case.

Another note, how do you handle access control? What if a user has permission to import one feed, but not another?
This begs, what happens when you run import, does it import both? Are you adding separate imports. Sorry, don't have time to do a proper review atm.

Subscribing

I'm currently using 7.x-2.x-alpha3 in production with loads of patches against it (including #8 above), and am trying to upgrade to a newer version, starting with -alpha4. As such, I've rerolled #8 against alpha4 and attached it below just in case anyone else is in the same situation as me. I may come back and look at a longer term solution once I'm on a more recent version of feeds.

THE ATTACHED PATCH SHOULD ONLY BE USED IF YOU'RE STUCK ON ALPHA4. SEE #31 AND #40 IF YOU'RE ON NEWER VERSIONS.

Patch against 7.x-2.x-alpha7 based on #43.

I re-rolled the above for the latest dev version. I also made it so that the feed source could be empty on the node it is attached to.

I did this in support of the idea put forward in comment #41 (using actual fields attached to nodes as sources). I also created http://drupal.org/sandbox/acouch/1952744 which creates two plugins, "File Field Fetcher" and 'Link Field Fetcher", which allow you to select a field attached to an entity instead of the upload or link form elements supplied by Feeds. Feeds can't expect that those fields are always required like the Feeds form elements hence the ability to have empty sources without breaking things.

This attached patch works for the scenarios I could think of testing by hand. For a module this big automated tests should be run, updated if this is to be included IMO. I'd be up for adding those if folks like the general direction this is going in.

I'll work on the tests.

shouldn't the url fields be optional? http://drupal.org/node/856316 http://drupal.org/node/1463906

currently all import feed url fields are required.

I had to use form alter hook to make them optional. http://drupal.org/node/1369588#comment-6947586

function module_name_form_alter(&$form, $form_state, $form_id)
{
  if ($form_id == 'content_type_node_form') {
    $form['feeds']['feed_importer_name1']['FeedsHTTPFetcher']['source']['#required'] = FALSE;
	$form['feeds']['feed_importer_name2']['FeedsHTTPFetcher']['source']['#required'] = FALSE;
  }
}

also I had to add this to below validate function if(!empty($values['source'])){
feeds\plugins\FeedsHTTPFetcher.inc http://drupal.org/node/1369588#comment-6952390

public function sourceFormValidate(&$values) {
    $values['source'] = trim($values['source']);
	if(!empty($values['source'])){
		if (!feeds_valid_url($values['source'], TRUE)) {
		  $form_key = 'feeds][' . get_class($this) . '][source';
		  form_set_error($form_key, t('The URL %source is invalid.', array('%source' => $values['source'])));
		}
		elseif ($this->config['auto_detect_feeds']) {
		  feeds_include_library('http_request.inc', 'http_request');
		  if ($url = http_request_get_common_syndication($values['source'])) {
			$values['source'] = $url;
		  }
		}
	}
  }

and two different importers are not able to use same content type in "Node processor" settings
http://sitename.com/admin/structure/feeds/importer1/settings/FeedsNodePr...
http://sitename.com/admin/structure/feeds/importer2/settings/FeedsNodePr...

Settings for Node processor

Content type *

when I select the same content type in one importer, the other one gets unselected.

*EDIT* solved this issue using http://drupal.org/project/feeds_node_helper

Feeds Node Helper provides helpers for the following:

UUIDs
Book parent based UUIDs
Book Weights
Node Types (by default Feeds just maps to 1 type, this allows multiple)

Is this still work in progress I am extremely interested in a patch!

#45: feeds-1127696-multiple-importers-per-content-type-45.patch queued for re-testing.

I would really like to see this functionality to. - Im creating one KPI Report (content type) and wanna fetch multiple datasets from various social APIs using feeds. Hope someone bright and helpful can finish the patch?

Would love this functionality as well!

I apologize, I was using this with https://drupal.org/project/feeds_field_fetcher which doesn't use the standard form. The attached patch adds the additional form for multiple importers on a single node. Please provide any feedback you have. Will try and track this more closely and write the necessary tests.

The patch in #55 did it for me! - Thanks acouch! - This should go into feeds as default functionality.

Attached is a version that passes all of the feeds tests. Need to write one that specifically tries multiple importers per bundle.

Simpletest is making me a liar. This should pass the current tests.

Marking back to needs work since that only passes the current tests.

For reference, I created a patch that does something similar, but allows for nodes in a content type to have different importers, not two applied to the same node.

Well, @acouch, your patch in #59 is looking very promising, but I have seem to run into some sort of hitch (after working many hours on this project...).

My setup is a bit complex, but I will try to describe it as thoroughly as possible. I am connecting to the Dota 2 video game web API, which provides a match history listing and match details. You can see samples of the data at history sample and details sample.

I have a single content type "Dota2 Match" (dota2_match), with the default body field and a field collection field, "Players" (field_players) set to unlimited on the content type. The field_players has an integer field "Player Slot" (field_player_slot) and a text field "Hero" (field_hero_id).

The first Feeds importer "Dota2 Matches Source" looks at the match history API call to generate feed nodes of type "Dota2 Matches". To generate the nodes it is NOT attached to a content type (use standalone form), weight is set to 0, periodic import is set to "As often as possible" (to pull new nodes in when the match history gets updated with new match ids, import on submission is checked, and process in background is not checked.

"Dota2 Matches Source" uses the JSONPath parser and the node processor. For the node processor settings, the bundle selected is "Dota2 Match", do not update existing nodes, author is set the superuser, authorize is checked, and expire nodes = never.

The mappings for "Dota2 Matches Source" are as follows:
Context: $.result.matches.*

• Title: match_id (Unique*)
• Feed source Dota2 Match Details: match_id
• Feed source Dota2 Match Players: match_id

The mappings for Feed source Dota2 Match Details and Feed source Dota2 Match Players are tampered on this importer to form them into the URL for the API call to retrieve the match details. Tamper:Rewrite https://api.steampowered.com/IDOTA2Match_570/GetMatchDetails/V001/?match...

The second Feeds importer, "Dota2 Match Details", is attached to the content type Dota2 Match, assigned a weight of 1, periodic import is as often as possible, import on submission is checked and process in background is not checked.

"Dota2 Match Details" uses the JSONPath parser and the Self Node processor. The Self Node processor settings have "Dota2 Match" selected as the bundle, update existing nodes, author is set to the superuser, authorize is checked, and expire nodes is set to never.

There is only one mapping for "Dota2 Match Details" for testing at this point:
Context: $.result.

• Body: start_time

The UNIX start_time timestamp is tampered into a PHP format.

The third Feeds imported, "Dota2 Match Players", is necessary to import from the match details API call to populate the field collection "Players". "Dota2 Match Players" is attached to the content type "Dota2 Match", assigned a weight of 2, periodic import is set to as often as possible, import on submission is checked, and process in background is not checked.

"Dota2 Match Players" uses the JSONPath parser and the Field Collection processor. The Field Collection processor settings have Field collection field_players as the bundle, update existing field collection item, field name is field_players, host entity type is node, "Is field" is not checked, Field/property name of Host entity GUID is nid, Identifier field name is field_player_slot (unique).

The mappings for "Dota2 Match Players" are as follows:
Context: $.result.players.*

• Host Entity GUID: Feed node Node ID
• Player Slot: player_slot
• Hero: hero_id

No tampering on this last one yet.

Now, before I stumbled across the patch in #59, I had gotten everything set up well, with the correct JSONPaths and what not, so that when I did my import of "Dota2 Matches Source" from the match history URL, it would create the 100 nodes from the results, with the match_id as the title. But, since I had attached both "Dota2 Match Details" and "Dota2 Match Players" to "Dota2 Match", the parser settings for "Dota2 Match Details" were not showing up when editing an imported node as "Dota2 Match Players" was the last attached to the content type. If I hit import on a node, it would bring in the 10 field collection items for players nicely, but not the "Dota2 Match Details". If I detached the "Dota2 Match Players" from the node type, then I could get "Dota2 Match Details" to import as it was the only importer on the node then, but then the players field collection/third importer is of no effect! You can imagine how frustrating this might be... So, I was so happy when I saw this patch, because it seemed to be exactly what I need.

But, now when I go to do the initial Source import, with debug on, the context shows up properly, and the three parsers are properly pulling the match_id field, like:

• jsonpath_parser:2:
• 417226405
• jsonpath_parser:3:
• 417226405
• jsonpath_parser:4:
• 417226405
• jsonpath_parser:2:
• ...
• jsonpath_parser:3:
• ...
• jsonpath_parser:4:
• ...
• jsonpath_parser:2:
• 417226253
• jsonpath_parser:3:
• 417226253
• jsonpath_parser:4:
• 417226253
• Created 100 nodes.

Then there is a big red error box with 100 entries of:

• Download of failed with code -1002.

As the importer reports, the 100 nodes are created, and have the match_id as their titles. However, when editing an imported node, the URLs for the Dota2 Match Details and Dota2 Match Players feed importers are blank. The parser settings for both are fine, but can't do much without the URLs.

By the way, I did clear my caches a couple times after applying the patch. I have tried various different settings combos, to no avail. Is there something I'm missing, or is the patch not yet fully functional? Is it possible that the Tamper on the Feeds source mappings is not being triggered, and therefore the values get thrown out? Any help would be most appreciated!

I'm using patch #59.

File element in FeedsFileFetcher always has files[feeds] as name and thus same file is used for all importers with file upload.

Attached patch changes file element names.

I've been chasing this issue for over a year. Just found your thread yesterday. Applied patch and tested. Still seeing duplicate nodes created by two different importers and error:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'AVI-99356527700' for key 'SKU'

Importing with just one feed importer works fine. Any content using the same unique field value (SKU in my case) and a second importer results in the duplicate node and error. I am trying to add content from difference sources and need to use multiple feeds to build the database.

Thanks for your help! -Ed

8: feeds-1127696-multiple-feeds-per-node.patch queued for re-testing.

I applied #64 but I have an error msg "Download of failed with code -1002" when I import the feed. I am not sure how to parameter the importer. I have two importers :
1) "My feed" attached to the "Feed" content type and having as bundle the "Feed item" content type in its processor settings
2) "My feed item 1" and "My feed item 2" depending on the "Feed item" content type and having as bundle the "Feed item" content type in its processor settings

I can see on "My feed" importer mappings there are two "feed source My feed item 1" and "feed source My feed item 1" in the target selection, but what should I set in the source selection ? I put "Item URL (link)" as source, but it doesn't seem to have any effect. When I import the feed, the feed item's nodes are created and when I edit them there is a feed form with two sections composed by an "url field" and an "XPath Parser Settings". The "url field" is empty while the "XPath Parser Settings" correspond to my two item importers ("My feed item 1" and "My feed item 2"). I can't go further as "url field" is required but empty ! This is my first problem with the patch.

The second one is the way the patch goes to answer the question of attaching various importers to the same content type. If I understand, one has to create as many importers as the feeds to import. Then ha has to go to the feed importer and map each importer. Then all these importers are listed inside the created notes (feed items). Shouldn't it be simpler/easier to just have a "select list" of the importers near the "url field" when adding new (feed) content ? So, we'll just have to add feed urls, one by one, and define the associated importer. Each feed url will have its own node with the associated importer stored, much easier to edit and maintain. We can be inspired by the feeds_imagegrabber approach.

@bblake - your option at #61 is the one I'd be looking for but now that it is implemented not sure where it is controlled or applied. It would save having to create hundreds of product type displays on my site.

Has anyone else tried this option and what did you do to apply it to your imports?

Are these patches to be included with the next release for feeds? I'm hesitant to try them without some added feedback. I will test if helpful.

Allowing multiple feeds to import to the same records in nodes would be a huge benefit for applications requiring consolidation of data from multiple sources and formats.

-Ed

#64 works like a charm except for the fact that source mapping only works with the first source URL.
No change if I use #59

I attached 3 importers to a content type which all create this same content type.
Each importer has a mapping set for each feed source url.
Only the first URL will have a value after import, no matter how I set the values.

Patch in #64 no longer applies cleanly, re-rolled against latest dev.

I'd like to try this again in the next couple months. Any sense of how long it might be before the next Feeds update (would prefer to wait if a few weeks away...). Thanks again for your help!

The patch still has some issues, unfortunately. When I was testing. it would always create duplicate nodes rather than updating the same set of nodes, even when used alongside the patch at https://drupal.org/node/1539224#comment-8375315, which allows GUIDs to be unique sitewide.

When using feeds-1127696-multiple-importers-per-content-type-59.patch with DKAN, when user adds resource but has missing title field, feeds tries to fetch the title from the resource which causes various issues - Commenting out that check.

patch in 76 does not apply cleanly to dev (Aug 28,2014) version:

patch -p1 < feeds-multiple-importers-per-content-type-1127696-76-7.31.patch
patching file feeds.module
Hunk #1 succeeded at 421 (offset 21 lines).
Hunk #2 succeeded at 579 (offset 21 lines).
Hunk #3 succeeded at 605 (offset 21 lines).
Hunk #4 FAILED at 635.
Hunk #5 succeeded at 672 (offset 20 lines).
Hunk #6 succeeded at 692 (offset 20 lines).
Hunk #7 succeeded at 703 (offset 20 lines).
Hunk #8 succeeded at 805 (offset 20 lines).
1 out of 8 hunks FAILED -- saving rejects to file feeds.module.rej
patching file feeds.pages.inc
Hunk #1 succeeded at 132 (offset 14 lines).
Hunk #2 succeeded at 195 (offset 14 lines).
Hunk #3 succeeded at 207 (offset 14 lines).
Hunk #4 succeeded at 294 (offset 14 lines).
patching file feeds_news/feeds_news.feeds_importer_default.inc
patching file feeds_ui/feeds_ui.test
Hunk #1 succeeded at 108 (offset 4 lines).
patching file includes/FeedsImporter.inc
Hunk #1 succeeded at 193 (offset -49 lines).
Hunk #2 succeeded at 221 (offset -49 lines).
patching file includes/FeedsSource.inc
Hunk #1 succeeded at 554 (offset 64 lines).
patching file plugins/FeedsHTTPFetcher.inc
Hunk #1 FAILED at 175.
1 out of 1 hunk FAILED -- saving rejects to file plugins/FeedsHTTPFetcher.inc.rej
patching file plugins/FeedsNodeProcessor.inc
Hunk #1 succeeded at 224 (offset -9 lines).
Hunk #2 succeeded at 322 (offset -9 lines).
Hunk #3 succeeded at 345 (offset -9 lines).
Hunk #4 succeeded at 360 (offset -9 lines).
patching file tests/feeds.test
Hunk #1 succeeded at 249 (offset 1 line).
Hunk #2 succeeded at 261 (offset 1 line).
Hunk #3 succeeded at 297 (offset 1 line).
patching file tests/feeds_processor_node.test
Hunk #1 FAILED at 366.
Hunk #2 FAILED at 375.
Hunk #4 FAILED at 395.
3 out of 5 hunks FAILED -- saving rejects to file tests/feeds_processor_node.test.rej
patching file tests/feeds_scheduler.test
Hunk #1 FAILED at 157.
1 out of 1 hunk FAILED -- saving rejects to file tests/feeds_scheduler.test.rej

Hello,

I have rebased manually (rewritten each lines) the patch #76. Maybe I have forgotten some changes.

Patch in #78 causes WSOD:

Create a feed
add it as standalone or a feed content type
Use feeds_facebook parser
Add the feed
WSOD - PHP Fatal error: Call to undefined method FeedsImporter::schedule() in feeds/feeds.module on line 702,

Isn't the goal of this thread the same as this one Add support for unique fields to be unique site wide?

Edit: Sorry, it's actually not :)

I removed line 675 which caused the error described in #79. Not sure of the implication tbh. but needed a quick fix.

Added

elseif ($param instanceof FeedsImporter) {
    $importer_ids[$param->id] = $param->id;
}

in feeds_access callback for standalone importers.

Okay this issue is now really messy. Trying to figure out what's going on.

This is from comment #78

+++ b/plugins/FeedsNodeProcessor.inc
@@ -351,7 +352,7 @@ class FeedsNodeProcessor extends FeedsProcessor {
-  protected function existingEntityId(FeedsSource $source, FeedsParserResult $result) {
+  function existingEntityId(FeedsSource $source, FeedsParserResult $result) {

+++ b/tests/feeds_processor_node.test
@@ -394,8 +394,8 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
-  public function testFeedURLValidation() {
...
+  function testFeedURLValidation($id = 'syndication') {

@@ -403,7 +403,7 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
-  public function testOddFeedSchemes() {
+  function testOddFeedSchemes($id = 'syndication') {

@@ -423,7 +423,7 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
-  public function testNonFeedNodeUI() {
+  function testNonFeedNodeUI($id = 'syndication') {

Something has gone wrong with the manually applied changes as changes from other commits are now being removed. The patch before that, #76, is actually not latest version at all but instead it's a modified version of #59. So now some changes from patches before that are lost.

It looks like patch from #73 is the correct one that needs a proper reroll and then changes from #83 and maybe #81 need to be added on top of that.

This is a rather big patch, please include interdiffs with changes so it's easier to follow.

I've now rerolled the patch from #73 and then I added the changes made by @fubhy in #81 and @mexthecat in #83. I also made all the necessary fixes to make tests pass.

@HHMU8 (#76), your changes are now missing from this patch. You used some older patch to add these changes to, was there a reason for that? Is that still an issue with this new rerolled patch? I have now idea what DKAN is.

minor fix to Example feature that I missed from last patch

Hello,
Thank you for your patch, it applied well without problem. Now I see 2 URL fields on the Feed content type form. Maybe I misunderstood the purpose of this functionality, but "has is", I'm lost...

I explain:

1. The 2 fields are labeled "URL" > How can I know which field uses which importer?
2. Both fields are mandatory > What if I just want to create some content with one of my 2 importer but not both of them?

I though this patch was meant to add the possibility to assign different importer to a unique content type so I could import content from one kind of feed, or from another kind and not have to create a feed content type (+corresponding feed items) for each importer.

Am I wrong?

I've tested the patch and can confirm it's working - great effort!! Such an important feature for our complex import processes.

The issues mentioned by anou don't affect me too much because I have a custom fetcher, so I just override the sourceForm() function and add specific titles for each importer and remove the required flag.

As a side note, does anyone else experience the problem I logged in this issue #2450365: Importer is not rescheduled for new sources when using attach to node? (comment over there if you have, cheers).

Deleted comment - wrong issue

I've tested this patch but in my case it wasn't working.

I shortly explain my case.
I have 3 feeds importers. One using a standalone form (let's call it "master") and 2 others attached to my content type ("slave 1" and "slave 2").
I am using the "master" to initially create the nodes and initialize the feeds source URL for "slave 1" and "slave 2".
Once the nodes are created, "slave 1" and "slave 2" are executed to get extra data for these nodes.

When I applied the patch 97 I got multiple feeds source URL's available in the "mapping" part of feeds config (Mapping for Node processor). But when my "master" feed got executed, the nodes are created but the feeds source URL's are empty.

Together with nico.knaepen we have found the solution. We have added next code into FeedsNodeProcessor.inc, function setTargetElement:

    if (substr($target_element, 0, 13) == 'feeds_source_') {
      foreach ($target_node->feeds as $key => $feed) {
        if ($target_element == 'feeds_source_' . $key) {
          $class = get_class(feeds_importer($id)->fetcher);
          $target_node->feeds[$key][$class]['source'] = $value;
        }
      }
    }

According to us, next code can be removed but we decided to keep it, so everybody can test his case:

    if ($target_element == 'feeds_source_' . $id) {
      // Get the class of the feed node importer's fetcher and set the source
      // property. See feeds_node_update() how $node->feeds gets stored.
      $class = get_class(feeds_importer($id)->fetcher);
      $target_node->feeds[$id][$class]['source'] = $value;
      // This effectively suppresses 'import on submission' feature.
      // See feeds_node_insert().
      $target_node->feeds['suppress_import'] = TRUE;
    }

Near this change I also have implemented a solution to solve the issue 1 explained by @anou (#98).

The 2 fields are labeled "URL" > How can I know which field uses which importer?

Please review it.

Subscribing

Few coding standards issues and some performance tweaks required.

1. +++ b/feeds.module
   @@ -427,26 +427,33 @@ function feeds_theme() {
   +    // If checking the access for a content type, check all importers available
   

   Try to shorten sentence so it fits in one line for ex.
   On access check for a content type, check all available importers.

2. +++ b/feeds.module
   @@ -427,26 +427,33 @@ function feeds_theme() {
   +   // Loop through all importers. if the user has access to one, they have access
   

   Try to shorten the sentence here also.

3. +++ b/feeds.module
   @@ -427,26 +427,33 @@ function feeds_theme() {
   +       if (user_access('administer feeds') || user_access($action . ' ' . $importer_id .' feeds')) {
   

   Move user_access('administrator feeds') outside the foreach loop. Otherwise the user_access function is being called for each item in the $importer_ids. Make a variable of it and use this one in the if statement.

4. +++ b/feeds.module
   @@ -580,11 +587,25 @@ function feeds_entity_delete($entity, $type) {
   +/*
   

   Apply correct comment block standards
   /**
   * Implements hook_node_prepare().
   */

5. +++ b/feeds.module
   @@ -593,28 +614,30 @@ function feeds_node_validate($node, $form, &$form_state) {
   +    if (trim($node->title) == '') {
   

   Create variable with value trim($node->title) outside foreach statement. Use this variable in if statement.

6. +++ b/feeds.module
   @@ -593,28 +614,30 @@ function feeds_node_validate($node, $form, &$form_state) {
   +          throw new Exception();
   

   Add message on throwing a new Exception:
   throw new Exception(t('Could not retrieve title from feed'));

7. +++ b/feeds.module
   @@ -593,28 +614,30 @@ function feeds_node_validate($node, $form, &$form_state) {
   +        form_set_error('title', t('Could not retrieve title from feed.'), array('error' => array('title')));
   

   Move form_set_error below code line 635. It does not belong here. Think about when you want to raise another exception in the function.

8. +++ b/feeds.module
   @@ -658,10 +683,15 @@ function feeds_node_insert($node) {
   +  if (isset($node->feeds) && $importer_ids = feeds_get_importer_ids($node->type)) {
   

   Change line to:
   if (isset($node->feeds) && ($importer_ids = ...)) {
   for more clear reading purposes.

9. +++ b/feeds.module
   @@ -682,25 +714,32 @@ function feeds_node_delete($node) {
   +      $form['#attributes']['enctype'] = 'multipart/form-data';
   

   Set enctype outside foreach. Maybe just apply if (count($importer_ids)) around it.

10. +++ b/feeds.module
    @@ -682,25 +714,32 @@ function feeds_node_delete($node) {
    +      $feed_title = $form['feeds'][$importer_id][$class]['source']['#title']
    

    Do not make variable of it but add it directly to the rule below.
    $form['feeds']... = $form['feeds'][$importer_id][...];

11. +++ b/feeds.module
    @@ -894,6 +933,48 @@ function feeds_get_importer_id($content_type) {
    + * @param $content_type
    

    Add param type. Ex: @param string $content_type.

12. +++ b/feeds.module
    @@ -894,6 +933,48 @@ function feeds_get_importer_id($content_type) {
    + * @param $feed_nid
    

    Add param type.

13. +++ b/feeds.module
    @@ -894,6 +933,48 @@ function feeds_get_importer_id($content_type) {
    + *  Nid for feed.
    

    Coding standards issue:
    Add an extra whitespace in front of sentence.

14. +++ b/feeds.module
    @@ -894,6 +933,48 @@ function feeds_get_importer_id($content_type) {
    + * @return
    

    Add return type.

15. +++ b/feeds.module
    @@ -894,6 +933,48 @@ function feeds_get_importer_id($content_type) {
    +function _feeds_get_importer_weights($importers, $sorted = TRUE){
    

    Add comment block.

16. +++ b/feeds.module
    @@ -894,6 +933,48 @@ function feeds_get_importer_id($content_type) {
    +      $importer_weights[$importer->id] = isset($importer->config['weight']) ? $importer->config['weight'] : '0';
    

    Line exceeds 80 chars.

17. +++ b/feeds.pages.inc
    @@ -131,25 +131,60 @@ function feeds_import_form_submit($form, &$form_state) {
    +    $form = confirm_form($form, t('Import all content from source?'), 'node/' . $node->nid, '', t('Import'), t('Cancel'), 'confirm feeds update');
    

    Line exceeds 80 chars.

18. +++ b/feeds.pages.inc
    @@ -131,25 +131,60 @@ function feeds_import_form_submit($form, &$form_state) {
    +        '#title' => t('@source_name: Status', array('@source_name' => $source->importer->config['name'])),
    

    Line exceeds 80 chars.

19. +++ b/feeds.pages.inc
    @@ -131,25 +131,60 @@ function feeds_import_form_submit($form, &$form_state) {
    +          t('Importing (@progress %)', array('@progress' => number_format(100 * $progress, 0)));
    

    Line exceeds 80 chars.

20. +++ b/feeds.pages.inc
    @@ -169,30 +206,80 @@ function feeds_import_tab_form_submit($form, &$form_state) {
    +          '#title' => t('@source_name: Status', array('@source_name' => $source->importer->config['name'])),
    

    Line exceeds 80 chars.

21. +++ b/feeds.pages.inc
    @@ -169,30 +206,80 @@ function feeds_import_tab_form_submit($form, &$form_state) {
    +          '#title' => t('@source_name: Status', array('@source_name' => $source->importer->config['name'])),
    

    Line exceeds 80 chars.

22. +++ b/feeds.pages.inc
    @@ -169,30 +206,80 @@ function feeds_import_tab_form_submit($form, &$form_state) {
    +    // Set importer ids. If this is a stand-alone form then importer will be
    

    Try to avoid having multiple lines of a sentence in comments.

23. +++ b/feeds.pages.inc
    @@ -169,30 +206,80 @@ function feeds_import_tab_form_submit($form, &$form_state) {
    +    $form = confirm_form($form, t('Delete all items from source?'), $form['#redirect'], '', t('Delete'), t('Cancel'), 'confirm feeds update');
    

    Line exceeds 80 chars.

24. +++ b/feeds.pages.inc
    @@ -169,30 +206,80 @@ function feeds_import_tab_form_submit($form, &$form_state) {
    +          t('Deleting (@progress %)', array('@progress' => number_format(100 * $progress, 0)));
    

    Line exceeds 80 chars.

25. +++ b/feeds_ui/feeds_ui.test
    @@ -108,7 +108,7 @@ class FeedsUIUserInterfaceTestCase extends FeedsWebTestCase {
    +      'feeds[test_feed][FeedsHTTPFetcher][source]' => $GLOBALS['base_url'] . '/' . drupal_get_path('module', 'feeds') . '/tests/feeds/developmentseed.rss2',
    

    Line exceeds 80 chars.

26. +++ b/includes/FeedsSource.inc
    @@ -563,24 +563,25 @@ class FeedsSource extends FeedsConfigurable {
    +      if (db_query_range("SELECT 1 FROM {feeds_source} WHERE id = :id AND feed_nid = :nid", 0, 1, array(':id' => $this->id, ':nid' => $this->feed_nid))->fetchField()) {
    

    Line exceeds 80 chars.

27. +++ b/plugins/FeedsFileFetcher.inc
    @@ -147,7 +148,7 @@ class FeedsFileFetcher extends FeedsFetcher {
    +      elseif ($file = file_save_upload($this->id, array('file_validate_extensions' => array(0 => $this->config['allowed_extensions'])), $feed_dir)) {
    

    Line exceeds 80 chars.

28. +++ b/plugins/FeedsNodeProcessor.inc
    @@ -362,11 +372,13 @@ class FeedsNodeProcessor extends FeedsProcessor {
    +      if (isset($this->config['content_type']) && $ids = feeds_get_importer_ids($this->config['content_type'])) {
    

    Line exceeds 80 chars.

29. +++ b/tests/feeds_processor_node.test
    @@ -356,7 +356,7 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
    +      'files[syndication_standalone]' => $this->absolutePath() . '/tests/feeds/drupalplanet.rss2',
    

    Line exceeds 80 chars.

#105 patch applied cleanly.

When importing multiple I get this error

Exception: Download of failed with code -1002. in FeedsHTTPFetcherResult->getRaw() (line 52 of /var/www/drupal/sites/all/modules/feeds/plugins/FeedsHTTPFetcher.inc).

error image

Applied coding standards from comment #107

Patch wasn't applying cleanly and was still a problem with the fields being required. Should apply well against current dev.

I applied the latest patch to the latest dev and this was the result:

Hunk #4 FAILED at 615.
Hunk #5 succeeded at 673 (offset 4 lines).
Hunk #6 succeeded at 691 (offset 4 lines).
Hunk #7 succeeded at 711 (offset 4 lines).
Hunk #8 FAILED at 718.
Hunk #9 succeeded at 958 (offset 70 lines).
Hunk #10 succeeded at 1002 (offset 70 lines).
Hunk #11 succeeded at 1012 (offset 70 lines).
Hunk #12 succeeded at 1025 (offset 70 lines).
Hunk #13 succeeded at 1087 (offset 70 lines).
Hunk #14 succeeded at 1342 (offset 75 lines).
Hunk #15 succeeded at 1366 (offset 75 lines).
Hunk #16 succeeded at 1431 (offset 75 lines).
Hunk #17 succeeded at 1466 (offset 75 lines).
Hunk #18 succeeded at 1483 (offset 75 lines).
Hunk #19 succeeded at 1517 (offset 75 lines).
Hunk #20 succeeded at 1545 (offset 75 lines).
Hunk #21 succeeded at 1580 (offset 75 lines).
2 out of 21 hunks FAILED -- saving rejects to file feeds.module.rej
patching file feeds.pages.inc
patching file feeds_import/feeds_import.test
patching file feeds_news/feeds_news.feeds_importer_default.inc
patching file feeds_ui/feeds_ui.test
patching file includes/FeedsImporter.inc
patching file includes/FeedsSource.inc
patching file plugins/FeedsFileFetcher.inc
patching file plugins/FeedsHTTPFetcher.inc
patching file plugins/FeedsNodeProcessor.inc
patching file tests/feeds.test
patching file tests/feeds_processor_node.test

bump, this is a feature I used to use all of the time, but the patch won't apply anymore.

bump. Really need this feature on multiple websites.

Tagging as "needs reroll".

This is really quite a big patch. I see that the latest patch also improves the code style of Feeds here and there, which strictly spoken would not be really necessary for this feature. These extra changes are great, but it also increases the chance that the patch won't apply anymore (which apparently has happened now).

IMPORTANT NOTE
For the 7.x-2.0-beta4 version of Feeds a change is planned which would make this feature harder to implement. This is to disable the possibility of selecting a content type that already has an other importer attached. This is to minimize user errors/confusion. See #2640108: Display an error message when attaching an importer to a content type that is already in use by an other importer.

reroll

The patch in #120 works for me! It did ask me to specify where some of the submodule files were, but it worked. :)

Okay, great! Then this issue only needs tests. Who wants to write them?

Note: if you don't know how to code the tests, it would also already be helpful to write down a list of scenario's to test (preferable with steps). For example:

• Scheduling testing
  1. Create two importers, attach them to same content type.
  2. Set one importer to use periodic import and the other one not.
  3. Create a feed node, setup a source for both.
  4. Trigger periodic import, ensure only one source gets imported.

sorry for my English
Can you help me
I use path
And when I want to add feeds node
display error

Notice: Undefined variable: importer_id in feeds_node_validate() (line 627 of /var/www/admin/www/mysite.ru/sites/all/modules/feeds/feeds.module).
InvalidArgumentException: Empty configuration identifier. in FeedsConfigurable::instance() (line 60 of /var/www/admin/www/mysite.ru/sites/all/modules/feeds/includes/FeedsConfigurable.inc)

How do I fix it????

sorry for my English
Can you help me
I use path
And when I want to add feeds node
display error
Notice: Undefined variable: importer_id in feeds_node_validate() (line 627 of /var/www/admin/www/mysite.ru/sites/all/modules/feeds/feeds.module).
InvalidArgumentException: Empty configuration identifier. in FeedsConfigurable::instance() (line 60 of /var/www/admin/www/mysite.ru/sites/all/modules/feeds/includes/FeedsConfigurable.inc)
How do I fix it????

I received the same error with this patch

Patch applied on 7.x-2.0-beta3 but produced an error. I rerolled patch for dev version and eliminated the error from 7.x-2.0-beta3.

There are still a lot of unrelated changes in the patch, which increases the chance that the patch won't apply anymore in the future. These changes should be removed and instead be provided as a separate patch in an other issue.

Quick glance over, I believe the following can be removed from the patch:

+++ b/feeds.module
@@ -515,7 +523,7 @@ function feeds_exit() {
-     }
+    }

@@ -956,11 +999,11 @@ function feeds_menu_local_tasks_alter(&$data, $router_item, $root_path) {
- * @param $load_disabled
+ * @param bool $load_disabled
...
- * @return
+ * @return array

@@ -1010,10 +1053,10 @@ function feeds_enabled_importers() {
- * @param $content_type
+ * @param string $content_type
...
- * @return
+ * @return array

@@ -1034,7 +1128,8 @@ function _feeds_importer_digest() {
-        $importers[$importer->id] = isset($importer->config['content_type']) ? $importer->config['content_type'] : '';
+        $importers[$importer->id] = isset($importer->config['content_type']) ?
+          $importer->config['content_type'] : '';

@@ -1288,6 +1383,9 @@ function feeds_include_library($file, $library) {
+ *
+ * @return bool
+ *   If library exists or not.

@@ -1309,7 +1407,7 @@ function feeds_library_exists($file, $library) {
- /**
+/**

@@ -1374,7 +1472,7 @@ function feeds_alter($type, &$data) {
- * @see valid_url().
+ * @see valid_url()

@@ -1409,7 +1507,7 @@ function feeds_valid_url($url, $absolute = FALSE) {
- * @return
+ * @return array

@@ -1426,7 +1524,7 @@ function feeds_set_subscription_job(array $job = NULL) {
- * @return
+ * @return array

@@ -1460,7 +1558,7 @@ function feeds_entity_property_info_alter(&$info) {
-  list($entity_id, , ) = entity_extract_ids($entity_type, $entity);
+  list($entity_id, ,) = entity_extract_ids($entity_type, $entity);

@@ -1488,7 +1586,7 @@ function feeds_file_download($uri) {
-   // Get the file record based on the URI. If not in the database just return.
+  // Get the file record based on the URI. If not in the database just return.

+++ b/feeds.pages.inc
@@ -257,7 +357,7 @@ function feeds_unlock_tab_form_submit($form, &$form_state) {
-  //Is there a more API-friendly way to set the state?
+  // Is there a more API-friendly way to set the state?

@@ -281,7 +381,7 @@ function feeds_fetcher_callback($importer, $feed_nid = 0) {
- * Template generation
+ * Template generation.

@@ -318,18 +418,26 @@ function theme_feeds_source_status($v) {
-    $items[] = t('Importing - @progress % complete.', array('@progress' => $progress));
+    $items[] = t('Importing - @progress % complete.', array(
+      '@progress' => $progress,
+    ));
...
-    $items[] = t('Deleting items - @progress % complete.', array('@progress' => $progress));
+    $items[] = t('Deleting items - @progress % complete.', array(
+      '@progress' => $progress,
+    ));
...
-        $items[] = t('Last import: @ago ago.', array('@ago' => format_interval(REQUEST_TIME - $v['imported'], 1)));
+        $items[] = t('Last import: @ago ago.', array(
+          '@ago' => format_interval(REQUEST_TIME - $v['imported'], 1),
+        ));
...
-      $items[] = t('@count imported items total.', array('@count' => $v['count']));
+      $items[] = t('@count imported items total.', array(
+        '@count' => $v['count'],
+      ));

@@ -357,7 +465,9 @@ function theme_feeds_upload($variables) {
-      $summary .= t('URI scheme %scheme not available.', array('%scheme' =>  file_uri_scheme($uri)));
+      $summary .= t('URI scheme %scheme not available.', array(
+        '%scheme' => file_uri_scheme($uri),
+      ));

@@ -369,7 +479,8 @@ function theme_feeds_upload($variables) {
-  $element['#children'] = '<div class="feeds-file">' . $summary . '<div class="feeds-file-upload">' . $element['#children'];
+  $element['#children'] = '<div class="feeds-file">' . $summary .
+    '<div class="feeds-file-upload">' . $element['#children'];

+++ b/feeds_import/feeds_import.test
@@ -100,9 +100,10 @@ class FeedsExamplesNodeTestCase extends FeedsWebTestCase {
-      'files[feeds]' => $this->absolutePath() . '/tests/feeds/nodes.tsv',
+      'files[node]' => $this->absolutePath() . '/tests/feeds/nodes.tsv',

+++ b/includes/FeedsSource.inc
@@ -33,7 +33,7 @@ interface FeedsSourceInterface {
-   * @return
+   * @return bool

@@ -124,9 +124,9 @@ class FeedsState {
-   * @param $total
+   * @param int $total
...
-   * @param $progress
+   * @param int $progress

@@ -217,7 +217,8 @@ class FeedsSource extends FeedsConfigurable {
-   * Returns the FeedsImporter object that this source is expected to be used with.
+   * Returns the FeedsImporter object that this source is expected
+   * to be used with.

@@ -226,7 +227,7 @@ class FeedsSource extends FeedsConfigurable {
-   * @return
+   * @return object

@@ -368,7 +369,7 @@ class FeedsSource extends FeedsConfigurable {
-   * @return
+   * @return float

@@ -496,7 +497,7 @@ class FeedsSource extends FeedsConfigurable {
-   * @return
+   * @return float

@@ -594,10 +595,10 @@ class FeedsSource extends FeedsConfigurable {
-   * @param $stage
+   * @param string $stage
...
-   * @return
+   * @return object

@@ -701,7 +714,7 @@ class FeedsSource extends FeedsConfigurable {
-   * @see FeedsConfigurable::existing().
+   * @see FeedsConfigurable::existing()

@@ -738,7 +751,7 @@ class FeedsSource extends FeedsConfigurable {
-   * @return
+   * @return array

@@ -805,10 +818,10 @@ class FeedsSource extends FeedsConfigurable {
-   * @see FeedsSource::startImport().
-   * @see FeedsSource::startClear().
+   * @see FeedsSource::startImport()
+   * @see FeedsSource::startClear()
...
-   * @param $method
+   * @param string $method

@@ -828,13 +841,13 @@ class FeedsSource extends FeedsConfigurable {
-   * @see FeedsSource::startImport().
-   * @see FeedsSource::startClear().
+   * @see FeedsSource::startImport()
+   * @see FeedsSource::startClear()
...
-   * @param $title
+   * @param string $title
...
-   * @param $method
+   * @param string $method

+++ b/plugins/FeedsFileFetcher.inc
@@ -30,7 +30,11 @@ class FeedsFileFetcherResult extends FeedsFetcherResult {
-      throw new Exception(t('File @filepath is not accessible.', array('@filepath' => $this->file_path)));
+      throw new Exception(t('File @filepath is not accessible.',
+        array(
+          '@filepath' => $this->file_path,
+        )
+      ));

@@ -65,14 +69,15 @@ class FeedsFileFetcher extends FeedsFetcher {
-    throw new Exception(t('Resource is not a file or it is an empty directory: %source', array('%source' => $source_config['source'])));
+    throw new Exception(t('Resource is not a file or it is an empty directory: %source', ¶
+      array('%source' => $source_config['source'])));
...
-   *   A stream wreapper URI that is a directory.
+   *   A stream wrapper URI that is a directory.

@@ -118,8 +127,10 @@ class FeedsFileFetcher extends FeedsFetcher {
-        '#description' => t('Specify a path to a file or a directory. Prefix the path with a scheme. Available schemes: @schemes.', array('@schemes' => implode(', ', $this->config['allowed_schemes']))),
-        '#default_value' => empty($source_config['source']) ? '' : $source_config['source'],
+        '#description' => t('Specify a path to a file or a directory. Prefix the path with a scheme. Available schemes: @schemes.',
+          array('@schemes' => implode(', ', $this->config['allowed_schemes']))),
+        '#default_value' => empty($source_config['source']) ?
+        '' : $source_config['source'],

@@ -139,15 +150,27 @@ class FeedsFileFetcher extends FeedsFetcher {
-          form_set_error('feeds][FeedsFileFetcher][source', t('Upload failed. Please check the upload <a href="@link">settings.</a>', array('@link' => $link)));
+          form_set_error('feeds][FeedsFileFetcher][source',
+            t('Upload failed. Please check the upload <a href="@link">settings.</a>',
+              array('@link' => $link))
+          );
...
-          form_set_error('feeds][FeedsFileFetcher][source', t('Upload failed. Please contact your site administrator.'));
+          form_set_error('feeds][FeedsFileFetcher][source',
+            t('Upload failed. Please contact your site administrator.')
+          );
...
-        watchdog('feeds', 'The upload directory %directory required by a feed could not be created or is not accessible. A newly uploaded file could not be saved in this directory as a consequence, and the upload was canceled.', array('%directory' => $feed_dir));
+        watchdog('feeds', 'The upload directory %directory required by a feed could not be created or is not accessible. A newly uploaded file could not be saved in this directory as a consequence, and the upload was canceled.',
+          array('%directory' => $feed_dir)
+        );
...
-      elseif ($file = file_save_upload('feeds', array('file_validate_extensions' => array(0 => $this->config['allowed_extensions'])), $feed_dir)) {
+      elseif ($file = file_save_upload($this->id,
+        array(
+          'file_validate_extensions' => array(
+            0 => $this->config['allowed_extensions'],
+          ),
+        ), $feed_dir)) {

@@ -162,11 +185,17 @@ class FeedsFileFetcher extends FeedsFetcher {
-        form_set_error('feeds][FeedsFileFetcher][source', t("The file needs to reside within the site's files directory, its path needs to start with scheme://. Available schemes: @schemes.", array('@schemes' => implode(', ', $this->config['allowed_schemes']))));
+        form_set_error('feeds][FeedsFileFetcher][source',
+          t("The file needs to reside within the site's files directory, its path needs to start with scheme://. Available schemes: @schemes.",
+            array(
+              '@schemes' => implode(', ', $this->config['allowed_schemes']),
+            ))
+        );
...
-        form_set_error('feeds][FeedsFileFetcher][source', t('The specified file or directory does not exist.'));
+        form_set_error('feeds][FeedsFileFetcher][source',
+          t('The specified file or directory does not exist.'));

@@ -255,11 +284,17 @@ class FeedsFileFetcher extends FeedsFetcher {
-      '#description' => t('Directory where uploaded files get stored. Prefix the path with a scheme. Available schemes: @schemes.', array('@schemes' => implode(', ', $this->getSchemes()))),
+      '#description' => t('Directory where uploaded files get stored. Prefix the path with a scheme. Available schemes: @schemes.',
+        array('@schemes' => implode(', ', $this->getSchemes()))
+      ),
...
-        'visible' => array(':input[name="direct"]' => array('checked' => FALSE)),
-        'required' => array(':input[name="direct"]' => array('checked' => FALSE)),
+        'visible' => array(
+          ':input[name="direct"]' => array('checked' => FALSE),
+        ),
+        'required' => array(
+          ':input[name="direct"]' => array('checked' => FALSE),
+        ),

@@ -270,7 +305,9 @@ class FeedsFileFetcher extends FeedsFetcher {
-          'visible' => array(':input[name="direct"]' => array('checked' => TRUE)),
+          'visible' => array(
+            ':input[name="direct"]' => array('checked' => TRUE),
+          ),

@@ -300,7 +337,8 @@ class FeedsFileFetcher extends FeedsFetcher {
-        form_set_error('directory', t('Please enter a valid scheme into the directory location.'));
+        form_set_error('directory',
+          t('Please enter a valid scheme into the directory location.'));

@@ -309,7 +347,8 @@ class FeedsFileFetcher extends FeedsFetcher {
-        form_set_error('directory', t('The chosen directory does not exist and attempts to create it failed.'));
+        form_set_error('directory',
+          t('The chosen directory does not exist and attempts to create it failed.'));

+++ b/plugins/FeedsNodeProcessor.inc
@@ -96,7 +96,9 @@ class FeedsNodeProcessor extends FeedsProcessor {
-        throw new FeedsAccessException(t($message, array('%uid' => $entity->uid)));
+        throw new FeedsAccessException(t($message,
+          array('%uid' => $entity->uid))
+        );

@@ -129,7 +131,7 @@ class FeedsNodeProcessor extends FeedsProcessor {
-       $entity->uid = $this->config['author'];
+      $entity->uid = $this->config['author'];

@@ -186,7 +188,7 @@ class FeedsNodeProcessor extends FeedsProcessor {
-      '#default_value' => empty($author->name) ?  'anonymous' : check_plain($author->name),
+      '#default_value' => empty($author->name) ? 'anonymous' : check_plain($author->name),

@@ -194,7 +196,10 @@ class FeedsNodeProcessor extends FeedsProcessor {
-    $period = drupal_map_assoc(array(FEEDS_EXPIRE_NEVER, 3600, 10800, 21600, 43200, 86400, 259200, 604800, 2592000, 2592000 * 3, 2592000 * 6, 31536000), 'feeds_format_expire');
+    $period = drupal_map_assoc(array(
+      FEEDS_EXPIRE_NEVER, 3600, 10800, 21600, 43200, 86400, 259200, 604800,
+      2592000, 2592000 * 3, 2592000 * 6, 31536000,
+    ), 'feeds_format_expire');

@@ -368,16 +382,24 @@ class FeedsNodeProcessor extends FeedsProcessor {
-          $nid = db_query("SELECT nid FROM {node} WHERE nid = :nid", array(':nid' => $value))->fetchField();
+          $nid = db_query("SELECT nid FROM {node} WHERE nid = :nid",
+            array(':nid' => $value))->fetchField();
...
+
...
-          $nid = db_query("SELECT nid FROM {node} WHERE title = :title AND type = :type", array(':title' => $value, ':type' => $this->bundle()))->fetchField();
+          $nid = db_query("SELECT nid FROM {node} WHERE title = :title AND type = :type",
+            array(':title' => $value, ':type' => $this->bundle()))
+            ->fetchField();

+++ b/tests/feeds_processor_node.test
@@ -24,7 +24,8 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
-    // Set the front page to show 20 nodes so we can easily see what is aggregated.
+    // Set the front page to show 20 nodes so we can easily see what
+    // is aggregated.

@@ -356,7 +357,8 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
-      'files[feeds]' => $this->absolutePath() . '/tests/feeds/drupalplanet.rss2',
+      'files[syndication_standalone]' => $this->absolutePath() .
+      '/tests/feeds/drupalplanet.rss2',

@@ -418,15 +420,18 @@ class FeedsRSStoNodesTest extends FeedsWebTestCase {
-      $feed_url = strtr($url, array('http://' => $scheme . '://', 'https://' => $scheme . '://'));
+      $feed_url = strtr($url, array(
+        'http://' => $scheme . '://',
+        'https://' => $scheme . '://',
+      ));

I'll see what i can do soon, thanks. I didn't really investigate every chunk in the patch, just made it work in dev.

I was able to apply patch #126 to the dev version of feeds. It seems to have worked, but I did receive these errors while applying the patch.

patching file feeds.module
Hunk #1 succeeded at 458 (offset 2 lines).
Hunk #2 succeeded at 525 (offset 2 lines).
Hunk #3 succeeded at 620 (offset 2 lines).
Hunk #4 succeeded at 646 (offset 2 lines).
Hunk #5 succeeded at 703 (offset 2 lines).
Hunk #6 succeeded at 721 (offset 2 lines).
Hunk #7 succeeded at 741 (offset 2 lines).
Hunk #8 succeeded at 752 (offset 2 lines).
Hunk #9 succeeded at 1001 (offset 2 lines).
Hunk #10 succeeded at 1055 (offset 2 lines).
Hunk #11 succeeded at 1068 (offset 2 lines).
Hunk #12 succeeded at 1130 (offset 2 lines).
Hunk #13 succeeded at 1385 (offset 2 lines).
Hunk #14 succeeded at 1409 (offset 2 lines).
Hunk #15 succeeded at 1474 (offset 2 lines).
Hunk #16 succeeded at 1509 (offset 2 lines).
Hunk #17 succeeded at 1526 (offset 2 lines).
Hunk #18 FAILED at 1558.
Hunk #19 succeeded at 1622 (offset 36 lines).
1 out of 19 hunks FAILED -- saving rejects to file feeds.module.rej
patching file feeds.pages.inc
Hunk #6 FAILED at 357.
Hunk #7 succeeded at 377 (offset -4 lines).
Hunk #8 succeeded at 414 (offset -4 lines).
Hunk #9 succeeded at 461 (offset -4 lines).
Hunk #10 succeeded at 475 (offset -4 lines).
1 out of 10 hunks FAILED -- saving rejects to file feeds.pages.inc.rej
can't find file to patch at input line 695
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/feeds_import/feeds_import.test b/feeds_import/feeds_import.test
|index ee62de0..e5a1f7a 100644
|--- a/feeds_import/feeds_import.test
|+++ b/feeds_import/feeds_import.test
--------------------------
File to patch: all/modules/feeds/feeds_import/feeds_import.test
patching file all/modules/feeds/feeds_import/feeds_import.test
can't find file to patch at input line 711
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/feeds_news/feeds_news.feeds_importer_default.inc b/feeds_news/feeds_news.feeds_importer_default.inc
|index 3fa5ee7..2820269 100644
|--- a/feeds_news/feeds_news.feeds_importer_default.inc
|+++ b/feeds_news/feeds_news.feeds_importer_default.inc
--------------------------
File to patch: all/modules/feeds/feeds_news/feeds_news.feeds_importer_default.inc
patching file all/modules/feeds/feeds_news/feeds_news.feeds_importer_default.inc
can't find file to patch at input line 724
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/feeds_ui/feeds_ui.test b/feeds_ui/feeds_ui.test
|index 7378b91..58ac0cd 100644
|--- a/feeds_ui/feeds_ui.test
|+++ b/feeds_ui/feeds_ui.test
--------------------------
File to patch: all/modules/feeds/feeds_ui/feeds_ui.test
patching file all/modules/feeds/feeds_ui/feeds_ui.test
Hunk #1 succeeded at 112 (offset 4 lines).
can't find file to patch at input line 741
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/includes/FeedsImporter.inc b/includes/FeedsImporter.inc
|index cad7695..a111224 100644
|--- a/includes/FeedsImporter.inc
|+++ b/includes/FeedsImporter.inc
--------------------------
File to patch: all/modules/feeds/includes/FeedsImporter.inc
patching file all/modules/feeds/includes/FeedsImporter.inc
Hunk #1 succeeded at 225 (offset 32 lines).
Hunk #2 succeeded at 253 (offset 32 lines).
can't find file to patch at input line 766
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/includes/FeedsSource.inc b/includes/FeedsSource.inc
|index b5577a1..6dded29 100644
|--- a/includes/FeedsSource.inc
|+++ b/includes/FeedsSource.inc
--------------------------
File to patch: all/modules/feeds/includes/FeedsSource.inc
patching file all/modules/feeds/includes/FeedsSource.inc
Hunk #6 succeeded at 503 (offset 6 lines).
Hunk #7 succeeded at 601 (offset 6 lines).
Hunk #8 succeeded at 653 (offset 23 lines).
Hunk #9 succeeded at 687 (offset 23 lines).
Hunk #10 succeeded at 768 with fuzz 1 (offset 54 lines).
Hunk #11 succeeded at 805 (offset 54 lines).
Hunk #12 succeeded at 872 (offset 54 lines).
Hunk #13 succeeded at 895 (offset 54 lines).
can't find file to patch at input line 954
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/plugins/FeedsFileFetcher.inc b/plugins/FeedsFileFetcher.inc
|index 88292ae..6ed8ac3 100644
|--- a/plugins/FeedsFileFetcher.inc
|+++ b/plugins/FeedsFileFetcher.inc
--------------------------
File to patch: all/modules/feeds/plugins/FeedsFileFetcher.inc
patching file all/modules/feeds/plugins/FeedsFileFetcher.inc
Hunk #1 FAILED at 30.
Hunk #2 succeeded at 63 (offset -2 lines).
Hunk #3 succeeded at 106 (offset -2 lines).
Hunk #4 succeeded at 121 (offset -2 lines).
Hunk #5 succeeded at 144 (offset -2 lines).
Hunk #6 succeeded at 179 (offset -2 lines).
Hunk #7 succeeded at 278 (offset -2 lines).
Hunk #8 succeeded at 299 (offset -2 lines).
Hunk #9 succeeded at 331 (offset -2 lines).
Hunk #10 succeeded at 341 (offset -2 lines).
1 out of 10 hunks FAILED -- saving rejects to file all/modules/feeds/plugins/FeedsFileFetcher.inc.rej
can't find file to patch at input line 1123
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/plugins/FeedsHTTPFetcher.inc b/plugins/FeedsHTTPFetcher.inc
|index a6f2f08..2c55531 100644
|--- a/plugins/FeedsHTTPFetcher.inc
|+++ b/plugins/FeedsHTTPFetcher.inc
--------------------------
File to patch: all/modules/feeds/plugins/FeedsHTTPFetcher.inc
patching file all/modules/feeds/plugins/FeedsHTTPFetcher.inc
Hunk #1 succeeded at 246 (offset 5 lines).
can't find file to patch at input line 1136
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/plugins/FeedsNodeProcessor.inc b/plugins/FeedsNodeProcessor.inc
|index 0087d2d..d3912fe 100644
|--- a/plugins/FeedsNodeProcessor.inc
|+++ b/plugins/FeedsNodeProcessor.inc
--------------------------
File to patch: all/modules/feeds/plugins/FeedsNodeProcessor.inc
patching file all/modules/feeds/plugins/FeedsNodeProcessor.inc
Hunk #1 succeeded at 115 (offset 19 lines).
Hunk #2 succeeded at 150 (offset 19 lines).
Hunk #3 succeeded at 207 (offset 19 lines).
Hunk #4 succeeded at 215 (offset 19 lines).
Hunk #5 succeeded at 259 (offset 19 lines).
Hunk #6 succeeded at 287 (offset 19 lines).
Hunk #7 succeeded at 372 (offset 19 lines).
Hunk #8 succeeded at 401 (offset 19 lines).
can't find file to patch at input line 1278
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/feeds.test b/tests/feeds.test
|index c486773..7915ed5 100644
|--- a/tests/feeds.test
|+++ b/tests/feeds.test
--------------------------
File to patch: all/modules/feeds/tests/feeds.test   
patching file all/modules/feeds/tests/feeds.test
can't find file to patch at input line 1313
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/feeds_processor_node.test b/tests/feeds_processor_node.test
|index 25ec79a..171a960 100644
|--- a/tests/feeds_processor_node.test
|+++ b/tests/feeds_processor_node.test
--------------------------
File to patch: all/modules/feeds/tests/feeds_processor_node.test
patching file all/modules/feeds/tests/feeds_processor_node.test

@g33kg1rl
Some of the errors indicate that the patch was only applied partly, so you may be missing something important. The patch will need to be rerolled again.

As I mentioned before, it would also be better if all unrelated changes (coding standards improvements) are removed from the patch and instead be provided as a separate patch in its own issue (see comment #127).

And an automated test is badly needed to ensure that the requested feature keeps working in the future.

I started to do a reroll for this but it is not that simple so I reverted most the unrelated changes first. Here is the result of that. I didn't have enough time to complete the actual reroll now.

And here is reroll of the patch, hopefully it still works. There was quite a bit of manual work required to resolve all the conflicts.

feeds_form_node_form_alter assumed that all fetchers have source element and that this source element contains a #title attribute. However this is not a case with file fetcher at least.

Attached patch changes this a bit. Now each source config form has own fieldset and the title of fieldset is used instead. I also added a todo to #required attribute change that is also not working with file fetcher.

Thanks mikran, I've added the relevant coding standards improvements that you removed from the patch to #2342143-5: Fix coding standards.
Do you want to write the tests as well? A "plan" for writing the tests is also fine. See #122.

Yes tests are absolutely required for this. I don't have the hours to put to it right now, maybe later. Anyway here is a small bug fix patch again.

I change the status to 'needs review' just to see what current tests think of the patch.

one more bugfix

Reroll for latest dev.
Because of http://cgit.drupalcode.org/feeds/commit/?id=bcc20555ba82cdc2eabb23c81b1b...
One of the hunks failed.

Attached a diff.txt that shows the diff command output for the previous patch and this one. Only a code part is removed due the above commit, and the line numbers have changed.

It seems to work for me, but have not done a lot of testing so far.

Patch rerolled.

Attach an empty patch to see which fails are due to the patch and which are present in current dev branch.

OK. We have a lot of work with tests.

Reroll #142.