Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Should Domain make it's information available through hook_entity_info_alter() ? This makes domain information accessible to Rules, Search API and other modules that use entity API data.
Are changes like this ruled out for 7.x-2.x?
I'd need to reed the Entity API docs more closely before submitting a patch. I think it wouldn't need to be much more than this:
/**
* Implements hook_entity_property_info_alter().
*
* Adding domain properties.
*/
function domain_entity_property_info_alter(&$info) {
$properties = &$info['node']['properties'];
$properties['domain_source'] = array(
'label' => t('Domain Source'),
'description' => t(''),
'type' => 'integer',
);
$properties['domains'] = array(
'label' => t('Domains'),
'description' => t(''),
'type' => 'list<integer>',
);
}
Comment | File | Size | Author |
---|---|---|---|
#5 | domain-1427552-5.patch | 911 bytes | tim.plunkett |
#4 | domain-1427552-4.patch | 687 bytes | tim.plunkett |
Comments
Comment #1
agentrickardDomains are not entities. I suspect this code would explode badly.
See #1142558: domains as entities comments 3 and 5 are most relevant here.
Comment #2
stevectorThis is not domains as entities. This is "domain module creates $node->domains and $node->domain_source, let other modules know those properties are available on node objects."
Comment #3
agentrickardI'm skeptical about non-core hook support. But if it makes life much easier for people...
Comment #4
tim.plunkettThe '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.
Comment #5
tim.plunkettTo fully support Search API, you need this hook as well.
Comment #6
xjmNote 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:
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.
Comment #7
tim.plunkettIt 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().Comment #8
joshuajabbour CreditAttribution: joshuajabbour commentedYes, 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.
Comment #9
agentrickardUnder 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.
Comment #10
joshuajabbour CreditAttribution: joshuajabbour commentedBut 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).
Comment #11
tim.plunkettI think #5 is a step too far. #4 seems noncontroversial and VERY helpful.
Comment #12
agentrickardWhat 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.
Comment #13
joshuajabbour CreditAttribution: joshuajabbour commented@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...
Comment #14
agentrickardThat 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".
Comment #15
joshuajabbour CreditAttribution: joshuajabbour commentedI'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...
Comment #16
agentrickardOK. 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".
Comment #17
agentrickardI really have no idea how to review this.
Comment #18
joshuajabbour CreditAttribution: joshuajabbour commentedYes, 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...