The attached patch allows feed items to inherit domain access info from the parent feed node.

CommentFileSizeAuthor
feedapi-domain-inherit.patch2.03 KBjpp

Comments

jpp’s picture

Status: Active » Needs review
skizzo’s picture

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

agentrickard’s picture

Status: Needs review » Needs work

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

jpp’s picture

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

jpp’s picture

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

jpp’s picture

Status: Needs work » Needs review

setting it back to code needs review - I think it's a legitimate patch for D6 FeedAPI

skizzo’s picture

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

skizzo’s picture

Will this patch find its way into an official release? Thank you.

agentrickard’s picture

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

skizzo’s picture

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

agentrickard’s picture

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

alex_b’s picture

Status: Needs review » Needs work

I 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

agentrickard’s picture

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

function _feedapi_inherit_do_inherit(&$item_node, $feed_node) {
 // ... move existing code into a hook implementation.
 drupal_alter('feedapi_inherit', $item_node, $feed_node);

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

function domain_feedapi_inherit_alter(&$item_node, $feed_node) {
  $item_node['domain_site'] = $feed_node['domain_site'];
  $item_node['domains'] = $feed_node['domains'];
}

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

alex_b’s picture

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

/**
 * Implementation of hook_nodeapi().
 */
function da_inherit_nodeapi($node, $op) {
  if (!isset($node->feedapi_node)) {
    return;
  }
  
  switch ($op) {
    case 'prepare':
      if ($node->feedapi_node->feed_nids) {
        foreach ($node->feedapi_node->feed_nids as $feed_nid) {
          $feed_node = node_load($feed_nid);
          if (feedapi_enabled_type($feed_node->type, 'da_inherit')) {
            _da_inherit_do_inherit($node, $feed_node);
          }
        }
      }
      break;
  }
}

/**
 * Implementation of hook_feedapi_item().
 * Do nothing here, this function only makes us a feedapi processor,
 * which means that we can be enabled/disabled on a per node type 
 * basis.
 */
function da_inherit_feedapi_item($op) {
}

function _da_inherit_do_inherit($node, $feed_node) {
 // @todo: inherit domain settings.
}
agentrickard’s picture

That 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_alter might 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.

skizzo’s picture

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

alex_b’s picture

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

agentrickard’s picture

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

szy’s picture

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

agentrickard’s picture

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

alex_b’s picture

#18 / #20

which I would implement.

Ok, you persuaded me. Drop the patch, I'll commit : )

agentrickard’s picture

@alex
You going to PublicMedia Camp? We could hack it out there in like 20 minutes....

szy’s picture

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

agentrickard’s picture

Back on topic, does this patch now need to target Feeds?

szy’s picture

Title: Patch to support inheriting domain access settings » Patch to support inheriting Domain Access settings
webwriter’s picture

Subscribe. Please target feeds!

Exploratus’s picture

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

ayesh’s picture

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

ayesh’s picture

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

if (isset($feed_node->domain_source)) {
		$item_node->domain_source = $feed_node->domain_source;
	}

4. Ran cron.php from base domain.
5. It worked!

skizzo’s picture

...but won't domain_source radically alter the behaviour of your system?

ayesh’s picture

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