Problem/Motivation
Exact same as the above feature request which looks like its solved for d6.
For convenience here is copy pasted issue
I can assign more than one feed to a content type by setting the 'Attach to content type:' to the same content type for two different feeds. However, when I go to create my node, I only see one textfield to enter in the feed URL. I should see two.
My use case: I'm using the project.module to detail projects that I maintain on Drupal.org and I want to attach a few feeds to the node. The CVS commit history, release history, issue queue etc. So, when I go to create a new project node I should see textfields for each associated feed, allowing me to associate all the feeds with the project (I'm using the data storage backend).
Proposed resolution
Node form alters are modified so that config forms of multiple importers are displayed right there on node form if multiple importers have been added to same node type.
Remaining tasks
- Write more tests
- Review the code
- Manual testing
User interface changes
Multiple importer config forms are now shown on node edit forms.
API changes
-
Comment | File | Size | Author |
---|---|---|---|
#146 | feeds-multiple-importers-1127696-142-reroll.patch | 36.52 KB | mr.york |
#144 | test-current-fails.patch | 329 bytes | oriol_e9g |
| |||
#142 | feeds-multiple-importers-1127696-142.patch | 35.58 KB | oriol_e9g |
Comments
Comment #1
XiaN Vizjereij CreditAttribution: XiaN Vizjereij commentedSubscribe
Comment #2
Steven Jones CreditAttribution: Steven Jones commentedSome initial work on this, completely untested.
No upgrade path at the moment either.
Comment #3
johnvThe following project (only D6 right now) seems to do the very thing. Am I right?
http://drupal.org/project/feeds_node_multisource
Comment #4
Steven Jones CreditAttribution: Steven Jones commentedNo, 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.
Comment #5
johnvChanging title to differ between #634462 and #1127696.
Comment #6
Steven Jones CreditAttribution: Steven Jones commentedPrevious patch was broken, this has now been tested, and you can import/clear individual sources.
Comment #7
Steven Jones CreditAttribution: Steven Jones commentedActually that one was broken also, this is very much work-in-progress stuff.
Comment #8
Steven Jones CreditAttribution: Steven Jones commentedMore patches to get the weight of the importers correct.
Comment #9
muka CreditAttribution: muka commentedHi,
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?
Comment #10
mstrelan CreditAttribution: mstrelan commentedPatch does not apply. Please re-roll so I can test.
Comment #11
mstrelan CreditAttribution: mstrelan commentedI'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).
Comment #12
mstrelan CreditAttribution: mstrelan commentedI'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.
Comment #13
leewoodman CreditAttribution: leewoodman commentedi 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:
Comment #14
mstrelan CreditAttribution: mstrelan commentedhttp://drupal.org/node/1054616
Use
patch -p1
instead ofpatch -p0
.Comment #15
colle901 CreditAttribution: colle901 commentedI 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.
Comment #16
mareks CreditAttribution: mareks commentedHi, 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?
Comment #17
mstrelan CreditAttribution: mstrelan commented@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.
Comment #18
Xen CreditAttribution: Xen commented#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...
Comment #19
mstrelan CreditAttribution: mstrelan commented@Xen what about the idea of an importer field rather than an importer node.
Comment #20
Xen CreditAttribution: Xen commented@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..
Comment #21
Xen CreditAttribution: Xen commentedI 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.
Comment #22
leewoodman CreditAttribution: leewoodman commentedHi
Thanks for the update do you think this will work for my case? http://drupal.org/node/1352048
thanks
Lee
Comment #23
Xen CreditAttribution: Xen commented@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.
Comment #24
johnv@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
Comment #25
timb CreditAttribution: timb commentedsubscribing
Comment #26
Ivan Simonov CreditAttribution: Ivan Simonov commentedsubscribing
Comment #27
keereelsubscribing
Comment #28
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedpatch 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).
Comment #29
franzNeeds re-roll then.
Comment #30
R@100 CreditAttribution: R@100 commentedcan someone, please, create new patch?
Comment #31
geek-merlinhere it is, re-rolled against current dev.
still works fine so rtbc.
Comment #32
geek-merlinhere 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.
Comment #33
franzThis 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
Comment #34
aTrotter CreditAttribution: aTrotter commentedI'm using the latest dev version (7.x-2.0-alpha5+21-dev) and patch from #31. It doesn't seem to work:
Am I doing something wrong?
Comment #35
geek-merlinreport: 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.
Comment #36
franzWow, 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.
Comment #37
franzYou can always publish on github or something, we can merge later.
Comment #38
franzWas anyone able to re-roll this?
Comment #39
geek-merlinalthough there seem to be no european feeds lovers who will sprint with me, i can look into this in the next week.
Comment #40
geek-merlinWow, 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.
Comment #41
twistor CreditAttribution: twistor commentedIt 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.
Comment #42
NWOM CreditAttribution: NWOM commentedSubscribing
Comment #43
ianthomas_ukI'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.
Comment #44
dmitrit CreditAttribution: dmitrit commentedPatch against 7.x-2.x-alpha7 based on #43.
Comment #45
acouch CreditAttribution: acouch commentedI 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.
Comment #47
acouch CreditAttribution: acouch commentedI'll work on the tests.
Comment #48
TechNikh CreditAttribution: TechNikh commentedshouldn'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
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
Comment #49
TechNikh CreditAttribution: TechNikh commentedand 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)
Comment #50
flier CreditAttribution: flier commentedIs this still work in progress I am extremely interested in a patch!
Comment #51
gints.erglis CreditAttribution: gints.erglis commented#45: feeds-1127696-multiple-importers-per-content-type-45.patch queued for re-testing.
Comment #53
sbilde CreditAttribution: sbilde commentedI 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?
Comment #54
dobe CreditAttribution: dobe commentedWould love this functionality as well!
Comment #55
acouch CreditAttribution: acouch commentedI 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.
Comment #56
sbilde CreditAttribution: sbilde commentedThe patch in #55 did it for me! - Thanks acouch! - This should go into feeds as default functionality.
Comment #57
acouch CreditAttribution: acouch commentedAttached is a version that passes all of the feeds tests. Need to write one that specifically tries multiple importers per bundle.
Comment #59
acouch CreditAttribution: acouch commentedSimpletest is making me a liar. This should pass the current tests.
Comment #60
acouch CreditAttribution: acouch commentedMarking back to needs work since that only passes the current tests.
Comment #61
bblake CreditAttribution: bblake commentedFor 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.
Comment #62
mobonobomo CreditAttribution: mobonobomo commentedWell, @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.*
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.
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.*
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:
Then there is a big red error box with 100 entries of:
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!
Comment #63
mikran CreditAttribution: mikran commentedI'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.
Comment #64
mikran CreditAttribution: mikran commentedAttached patch changes file element names.
Comment #65
Ed Vogt CreditAttribution: Ed Vogt commentedI'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
Comment #66
erwangel CreditAttribution: erwangel commented8: feeds-1127696-multiple-feeds-per-node.patch queued for re-testing.
Comment #69
erwangel CreditAttribution: erwangel commentedI 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.
Comment #70
caseyb CreditAttribution: caseyb commented@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?
Comment #71
Ed Vogt CreditAttribution: Ed Vogt commentedAre 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
Comment #72
adrien.felipe CreditAttribution: adrien.felipe commented#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.
Comment #73
littledynamo CreditAttribution: littledynamo commentedPatch in #64 no longer applies cleanly, re-rolled against latest dev.
Comment #74
Ed Vogt CreditAttribution: Ed Vogt commentedI'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!
Comment #75
littledynamo CreditAttribution: littledynamo commentedThe 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.
Comment #76
HHMU8 CreditAttribution: HHMU8 commentedWhen 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.
Comment #77
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedpatch 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
Comment #78
GrimreaperHello,
I have rebased manually (rewritten each lines) the patch #76. Maybe I have forgotten some changes.
Comment #79
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedPatch 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,
Comment #80
adrien.felipe CreditAttribution: adrien.felipe commentedIsn'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 :)
Comment #81
fubhy CreditAttribution: fubhy commentedI removed line 675 which caused the error described in #79. Not sure of the implication tbh. but needed a quick fix.
Comment #83
mexthecat CreditAttribution: mexthecat commentedAdded
in feeds_access callback for standalone importers.
Comment #84
mexthecat CreditAttribution: mexthecat commentedComment #85
mexthecat CreditAttribution: mexthecat commentedComment #92
mikran CreditAttribution: mikran commentedComment #93
mikran CreditAttribution: mikran commentedOkay this issue is now really messy. Trying to figure out what's going on.
This is from comment #78
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.
Comment #94
mikran CreditAttribution: mikran commentedI'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.
Comment #95
mikran CreditAttribution: mikran commentedComment #97
mikran CreditAttribution: mikran commentedminor fix to Example feature that I missed from last patch
Comment #98
anouHello,
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:
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?
Comment #99
littledynamo CreditAttribution: littledynamo commentedI'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).
Comment #100
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedDeleted comment - wrong issue
Comment #101
michel.settembrino CreditAttribution: michel.settembrino as a volunteer commentedI'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:
According to us, next code can be removed but we decided to keep it, so everybody can test his case:
Near this change I also have implemented a solution to solve the issue 1 explained by @anou (#98).
Please review it.
Comment #102
michel.settembrino CreditAttribution: michel.settembrino as a volunteer commentedComment #104
nico.knaepen CreditAttribution: nico.knaepen as a volunteer commentedSubscribing
Comment #105
michel.settembrino CreditAttribution: michel.settembrino as a volunteer commentedComment #106
michel.settembrino CreditAttribution: michel.settembrino as a volunteer commentedComment #107
nico.knaepen CreditAttribution: nico.knaepen as a volunteer commentedFew coding standards issues and some performance tweaks required.
Try to shorten sentence so it fits in one line for ex.
On access check for a content type, check all available importers.
Try to shorten the sentence here also.
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.
Apply correct comment block standards
/**
* Implements hook_node_prepare().
*/
Create variable with value trim($node->title) outside foreach statement. Use this variable in if statement.
Add message on throwing a new Exception:
throw new Exception(t('Could not retrieve title from feed'));
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.
Change line to:
if (isset($node->feeds) && ($importer_ids = ...)) {
for more clear reading purposes.
Set enctype outside foreach. Maybe just apply if (count($importer_ids)) around it.
Do not make variable of it but add it directly to the rule below.
$form['feeds']... = $form['feeds'][$importer_id][...];
Add param type. Ex: @param string $content_type.
Add param type.
Coding standards issue:
Add an extra whitespace in front of sentence.
Add return type.
Add comment block.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Try to avoid having multiple lines of a sentence in comments.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Line exceeds 80 chars.
Comment #108
No Sssweat CreditAttribution: No Sssweat commented#105 patch applied cleanly.
When importing multiple I get this error
error image
Comment #109
_Geertje CreditAttribution: _Geertje at Pàu for Colruyt Group Services commentedApplied coding standards from comment #107
Comment #111
_Geertje CreditAttribution: _Geertje at Pàu for Colruyt Group Services commentedComment #113
nlambert CreditAttribution: nlambert as a volunteer commentedPatch wasn't applying cleanly and was still a problem with the fields being required. Should apply well against current dev.
Comment #114
Jeroen_005 CreditAttribution: Jeroen_005 at Pàu for Colruyt Group Services commentedComment #116
g33kg1rl CreditAttribution: g33kg1rl commentedI 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
Comment #117
g33kg1rl CreditAttribution: g33kg1rl commentedbump, this is a feature I used to use all of the time, but the patch won't apply anymore.
Comment #118
g33kg1rl CreditAttribution: g33kg1rl commentedbump. Really need this feature on multiple websites.
Comment #119
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedTagging 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.
Comment #120
jday CreditAttribution: jday commentedreroll
Comment #121
g33kg1rl CreditAttribution: g33kg1rl commentedThe patch in #120 works for me! It did ask me to specify where some of the submodule files were, but it worked. :)
Comment #122
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedOkay, 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:
Comment #123
garegin1987 CreditAttribution: garegin1987 commentedsorry for my English
Can you help me
I use path
And when I want to add feeds node
display error
How do I fix it????
Comment #124
garegin1987 CreditAttribution: garegin1987 as a volunteer and commentedsorry 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????
Comment #125
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedI received the same error with this patch
Comment #126
pookmish CreditAttribution: pookmish commentedPatch 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.
Comment #127
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedThere 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:
Comment #128
pookmish CreditAttribution: pookmish commentedI'll see what i can do soon, thanks. I didn't really investigate every chunk in the patch, just made it work in dev.
Comment #129
g33kg1rl CreditAttribution: g33kg1rl commentedI 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.
Comment #130
MegaChriz CreditAttribution: MegaChriz as a volunteer commented@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.
Comment #131
mikran CreditAttribution: mikran at Mediamaisteri Oy commentedI 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.
Comment #132
mikran CreditAttribution: mikran at Mediamaisteri Oy commentedAnd here is reroll of the patch, hopefully it still works. There was quite a bit of manual work required to resolve all the conflicts.
Comment #133
mikran CreditAttribution: mikran at Mediamaisteri Oy commentedfeeds_form_node_form_alter
assumed that all fetchers havesource
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.Comment #134
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedThanks 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.
Comment #135
mikran CreditAttribution: mikran at Mediamaisteri Oy commentedYes 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.
Comment #137
mikran CreditAttribution: mikran at Mediamaisteri Oy commentedone more bugfix
Comment #138
nagy.balint CreditAttribution: nagy.balint commentedReroll 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.
Comment #139
nagy.balint CreditAttribution: nagy.balint commentedIt seems to work for me, but have not done a lot of testing so far.
Comment #140
oriol_e9gPatch rerolled.
Comment #142
oriol_e9gComment #144
oriol_e9gAttach an empty patch to see which fails are due to the patch and which are present in current dev branch.
Comment #145
oriol_e9gOK. We have a lot of work with tests.
Comment #146
mr.york CreditAttribution: mr.york at Agence Inovae commentedReroll #142.