Add implementation of hook_entity_info_alter() to domain

Domains are not entities. I suspect this code would explode badly.

See #1142558: domains as entities comments 3 and 5 are most relevant here.

I'm skeptical about non-core hook support. But if it makes life much easier for people...

The 'domains' property works great, and is necessary to use Domain Access with Search API.

Not sure about the domain_source part, since its FALSE for everything for me.

Needs description comments.

To fully support Search API, you need this hook as well.

The 'domains' property works great, and is necessary to use Domain Access with Search API.

Note that I'm not sure the second half of this sentence is actually true. I think that the "correct" solution in our usecase (making solr-based views respect node access) is to configure the search index to include node access data, as described on http://drupal.org/node/1254452#search_api_node_access:

This is done by adding a new field, „Node access information“ that stores the relevant access data. When the field is present and indexed, appropriate filters will be automatically added to all searches so that they only return results that the current user is allowed to view.

No module should need special domain integration to filter by domain; it should only need to support core node access.

However, the hook provides a simple workaround, and there are certainly other usecases for it.

It seems the patch in #5 is only needed because I was uid 1; the instructions xjm posted work great for other users.

Or more specifically, because of if (user_access('bypass node access', $account)) { in node_access().

Yes, the Search API built-in support for node access works to limit results based on a user's access perms, however what if a user has access to more than one domain (and all content on those domains), but you want search to only return results for the current domain?

That seems to me to be the real use of this hook with Search API (and related and/or similar modules). Not to limit by a user's access (though you should do that as well with the built-in node access), but to limit to content assigned to the current domain. So despite comment #6, I do think this is a very valid patch.

The second part of the patch, the hook_search_api_query_alter, could in theory be implemented by a separate module depending on your site's needs. But IMO the hook_entity_property_info_alter should really be provided by domain.module since it's purpose it to provide domain info to other modules. Yes, it's technically a non-core hook, but similar to D6 CCK, it's effectively a core hook since it's filling a hole in a core API.

Under normal circumstances, a user's Domain Access permissions are only the current domain and content viewable on that domain. I don't think this patch would change that. You can disable Domain Access on search pages, too.

This patch might allow fine-grained filtering / faceting IFF access rules are disabled when searching, but I don't know.

But I don't want to disable domain access on the search page, rather I want nodes to be indexed by their assigned domains. User access (via na) is different from assigned domain. IMO, search should first filter by assigned domain (optionally I suppose), and then filter then by node access (i.e which nodes does the domain have access to and then which does the user have access to).

But again, the first patch really has nothing to do with search, and instead provides domain information to all entity-aware modules. This is **a very good thing**.

As I said, the second part of the patch could (and likely should) be in a separate module (like search_api_domain), since there's a lot more one would want to do when integrating domains with the search API.

p.s. And "normal circumstances"? Is there really such a thing? ;) On the site I'm building users typically have access to all nodes, but can only view them on the assigned domain. This may not be the way Domain Access is typically used, but works really well (while allowing for future node access limitations, but even then users will likely have access to multiple domains).

I think #5 is a step too far. #4 seems noncontroversial and VERY helpful.

What I am saying is that "which domains should this node be visible on" and "which domains can the user see" are actually the exact same question, unless you enable Domain Strict, which forces "membership" on users.

@tim.plunkett I agree completely. I think the Search API integration should be another module (but requires the patch in #4).

@agentrickard Maybe I'm not being very clear, but I don't think what you're describing is what I'm suggesting at all. What I'm saying is that nodes are assigned to certain domains, and searching on a domain should return the nodes for that domain. It shouldn't return all the nodes the user preforming the search has access to, which might include nodes which aren't assigned to that domain (maybe as you say this isn't how most people use DA, but is totally possible and not a constraint anywhere else AFAIK.)

This is similar to how in Views one could/would add a filter to limit nodes displayed by the current domain. The "filter" is what is needed with the Search API integration, which requires indexing the assigned domains for each node. Which is what patch #4 enables. (Patch #5 is the "filter", it's what limits the search by the current domain. Again, this probably should be configurable, and is best owned by a separate module. Which I will gladly build if Entity API support is added to domain.)

I understand the reticence to add support for other contrib modules to DA, but Entity API is as close to a critical contrib module as D7 has. And hopefully much of it will be part of D8 natively...

That filtering should be done by the node access system already. If Search API module bypasses that, it's a security bug in the module.

Core search and SOLR search (with SOLR's node access module) will only return the results you describe: items assigned to the current domain or to "all affiliates".

I'm not sure how to explain it any further... I believe you're not understanding what I'm trying to say here and I can't figure out how to explain it so you do. It has nothing to do with node access; Search API respects NA (and does so properly). It is an issue with a node's assigned domain. Obviously this isn't how you use DA, so possibly that's the problem.

I will gladly try to explain again if you want me to. But rest assured, the first patch will "fix" this issue by making a node's assigned domain (and source) available through the Entity API. And hopefully the patch gets in, for many more reasons than Search API support...

OK. So let's try another question: How does this interact with #1684920: Add a 'domain' field type?

I suspect the answer is "they are totally separate".

I really have no idea how to review this.

Yes, as Dave Reid said, I believe they are different. If and when the time comes that DA switched to his field to assign domains to an entity, then this patch would be superfluous.

And as for reviewing, kpr(entity_get_property_info('node')) before and after will show you it's there. In order to test it's usage, you'd have to use it somewhere. The second patch demonstrates that...