The attached patch allows feed items to inherit domain access info from the parent feed node.
| Comment | File | Size | Author |
|---|---|---|---|
| feedapi-domain-inherit.patch | 2.03 KB | jpp |
The attached patch allows feed items to inherit domain access info from the parent feed node.
| Comment | File | Size | Author |
|---|---|---|---|
| feedapi-domain-inherit.patch | 2.03 KB | jpp |
Comments
Comment #1
jpp commentedComment #2
skizzo commentedIf it isn't too much of a hassle, could you please provide a D5 patch?
I am asking, but I don't really know how much different D5 and D6 are,
hopefully not too much. Thank you.
Comment #3
agentrickardWe should not have to set a module-specific patch here. The question is: what part of the sequence is causing the node access rules not to be set?
Comment #4
jpp commentedI'm not sure what is causing it - the module specific patch activates a feature in the FeedAPI module that is meant for this kind if thing - causing new nodes to inherit from the node that defines the feed - so while I agree with you that a module specific patch is not the "right" solution it's not a kludge either.
I'm not quite sure where to look to find out why this bug happen in the first place I'm new to Drupal development - if you can point me at a section of code I'll add debug and see if I can figure out what's broken.
Comment #5
jpp commentedSorry I don't run of develop for D5 - it's only about 10 lines of code so it shouldn't be too hard if you have the environment set up to test it.
Comment #6
jpp commentedsetting it back to code needs review - I think it's a legitimate patch for D6 FeedAPI
Comment #7
skizzo commentedI am not in the position to review the code. After upgrading to D6 I could only test this patch functionality, and I see the domain assignment propagate from feed to feed items, as expected. Incidentally, I also applied patch http://drupal.org/node/252877, on the domain side.
Comment #8
skizzo commentedWill this patch find its way into an official release? Thank you.
Comment #9
agentrickardI think we have accounted for this in DA HEAD -- but it does not inherit the parent node settings. just the domain from which the update is run.
Comment #10
skizzo commentedI think that inheriting the domain from which the update is run may be undesirable when running the updates automatically via cron (which I believe happens quite often). There is a direct "relationship" between parent feed and children feed-item nodes, so it seems appropriate that the setting be inherited from the node's parent. I am successfully using this patch (along with DA 6.x-2.0-rc6) to automatically assign feedapi nodes to different domains. If both behaviours are desirable, could it be made an option in DA? Whether it's in FeedAPI or DA, I guess it would be a "module-specific" change in anyway... but maybe that can't be avoided, as we are talking about feeds AND domains.
Comment #11
agentrickardIt would need to be module specific, I think. DA's default only kick in if nothing is set, so it you have a module that is intervening and setting a domain based on the parent, then you should be fine.
Nothing much to do here but write bridge code as you have done, AFAIK.
Comment #12
alex_b commentedI tend to agree with agentrickard in #3: contrib module specific code can and should live in the respective contrib modules... exceptions are _really_ widespread modules like views or og (and core modules which are not contrib modules, of course).
Should patch go into Domain Access? Or is this a case of custom code that does not need to be contributed? From my point of view this issue either needs to go to Domain Access' queue or it needs to be set to won't fix. Holding off for now as I am not 100 % sure which way to go.
In any case, this patch needs work. It does not adhere to http://drupal.org/coding-standards
Comment #13
agentrickard@alex_b
Right now, I think this is a case for a specific bridge module. If FeedAPI were core (or should I say _when_ FeedAPI is core), this would be supported directly in Domain Access.
But as it is, I can't maintain code for all use cases. The code in DA works fine now -- assuming that the Feed and its child nodes are processed from the proper domain.
Telling DA to read the domain settings of the parent node (the Feed node) when processing new child nodes (feed items) is the work of an additional module that uses available APIs.
Glancing briefly at the FeedAPI Inherit code (which is new to me), I think the best solution is to abstract that code into a full API, rather than hardcoding exceptions for OG and Taxonomy.
Then, if I could implement a domain_feedapi_inherit_alter() hook, I would gladly do so.
Could you simply do the following:
(Or, at least, add that line to the bottom of the existing function.) That would use drupal_alter() to pass &$item_node by reference, along with $feed_node. Then I can do this:
As it stands now, I would have to hack in a solution at the hook_node_access_records() level, and this seems much more Drupal-y.
So my stance is "This is a bug in the design of FeedAPI Inherit that we can and should easily fix."
Comment #14
alex_b commented#13: I see....
You should be able to implement this functionality nearly as easy with the existing APIs with the advantage that you'll be able to configure your module on a per content type basis:
Comment #15
agentrickardThat makes sense. Looks like 3rd-party bridge code to me. I would hope to avoid running that node_load() for every item, which suggests that using
feedapi_inherit_altermight be better, since that function explicitly passes the $feed_node object already.I think the inherit hook is still a good idea. There is too much clutter in hook_nodeapi() already.
Comment #16
skizzo commentedI have been using the OP patch for few months now, but it does not apply cleanly anymore to feedapi 6.x-1.9-beta1 or other recent releases. Re solutions #13 #14 (which not being a developer I understand only vaguely): should I issue a request for a "bridge module" in http://groups.drupal.org/contributed-module-ideas, or is this issues queue the proper place for such request? Thank you.
Comment #17
alex_b commented#17: I would rather try to team up with a developer who is interested in writing this bridge module (very simple based on #14) and maintain it on d. o.
Comment #18
agentrickardalex_b: I would still encourage you to introduce a hook inside the inherit function. What you have now is an exception handler for OG and Taxonomy. By making it a hook, it's easier to integrate into other modules without the need for bridge code.
Comment #19
szy commentedDo i remember your discussion correctly - there is no working
combination of FeedAPI and Domain?
What version of FeedAPI should I apply mentioned patch to?
Thanks,
Szy.
Comment #20
agentrickardYou need a custom bridge module to implement hook_nodeapi() and make modifications to the domain value of the node being saved based on the feed parent node.
I'm not putting that in DA, since I think its a module-specific hack. My argument is for FeedAPI to use hook_feed_inherit() or similar, which I would implement. Right now, FeedAPI hardcodes exceptions for taxonomy and OG, which I think is the wrong approach.
Comment #21
alex_b commented#18 / #20
Ok, you persuaded me. Drop the patch, I'll commit : )
Comment #22
agentrickard@alex
You going to PublicMedia Camp? We could hack it out there in like 20 minutes....
Comment #23
szy commentedAnd again, it's not about hurrying you up guys, but... about
projects waiting for a patch.
Maybe there is a possibility to donate to this patch?
* * *
It looks like people don't use Domain Access with FeedAPI
and XML Sitemap (I wrote yesterday about it), because
there are no working combinations of them.
... strange, as both of them are powerful projects!
Szy.
Comment #24
agentrickardBack on topic, does this patch now need to target Feeds?
Comment #25
szy commentedComment #26
webwriter commentedSubscribe. Please target feeds!
Comment #27
Exploratus commentedWas this commited? Or is this still in development. When using cron, feeds does not inherit domain access settings. Only when I run it manually from the domain.
Any info would be appreciated...
Comment #28
ayesh commentedThis patch didn't work for me with a univesal cron!
I tried with 1.x - dev and latest version of feedapi to the date.
i patched manually to the current version of feedapi, because line numbers are different.
Can anyone provide me a patch or a link to the exact file which has modified to support domain inheritance ?
I see the check box to enable domain inheritance, but it doesn't do its job.
However, I can get this work if I added new crontabs for each site.
Comment #29
ayesh commentedI could avoid the cron problem.
1. Enabled domain_source module.
2. Unchecked cron.php as a special request.
3. Added following lines to the patch to be patched before subdomain inheritance.
4. Ran cron.php from base domain.
5. It worked!
Comment #30
skizzo commented...but won't domain_source radically alter the behaviour of your system?
Comment #31
ayesh commentedYes it doesn't.
But somehow it worked very well for me... Thanks again for this.
Can we solve the problem with Job Sheduler ? (another Alex_b's module)