If you select a specific bundle that's referencable, EntityReference will do a entityCondition('bundle') in the EntityFieldQuerys at entityreference_get_referencable_count() and entityreference_get_referencable_entities().

However, taxonomy_terms and comments don't support that condition, because they have no exact bundle keys in the database.
So you get a crash.

For comments I suggest removing the ability to select a specific bundle in the field settings.

Taxonomy terms will be fixed by #1054162: Taxonomy bundles not supported by EntityFieldQuery (followup) in core, so we have several options:
1) Wait for core to be fixed and do nothing
2) Implement the same alter as the core patch does
3) Disallow specific bundles to be selected in field settings for terms as well.

Thoughts?

Comments

Forgot to mention, in regards to taxonomy terms, I'm for #2. It's a simple copy-paste from the core patch, and it won't break after the core patch lands.

Status:Active» Needs review
StatusFileSize
new2.71 KB

Bug reports are always more fun when they have patches attached. Here's one as an example.

Status:Needs review» Needs work

Given the sad state of Drupal core, it seems to be a pipe dream to be able to support all the entity types consistently. I suggest we implement those special cases properly. I envision a generic entity handler (as a class) + specific versions for core entities (as child classes). I think we need the following methods:

- Give me the bundles you control: special case for comments and taxonomy terms
- Get referencable entities (same as the current entityreference_get_referencable_entities): special case for access control for nodes (+status = 1), users (+ status = 1), comments (+has access to the parent node), files (access control is very messy for this thing)
- Get label: user needs a special case because it is brain-dead

We should see how much we can get into Entity API. These are common problems (I have special case code for user label handling in VBO as well).

Status:Needs work» Needs review
StatusFileSize
new16.01 KB

This looks like it could do the trick. Part of the refactoring is to extract the business logic into a class, so that we can more easily swap it (for example for the Views-based selection of entities).

Title:Can't reference comments and taxonomy_terms of specific bundles (fatal error)Implement special handling for some entity type (and factor out the main business logic)
StatusFileSize
new34.77 KB

New patch up for review.

StatusFileSize
new36.1 KB

With the upgrade path.

StatusFileSize
new48.88 KB

And with the missing files.

+  /**
+   * Helper method: pass a query to the alteration system again.
+   */
+  protected function reAlterQuery(SelectQueryInterface $query, $tag, $base_table) {

Can we have a comment explaining why we need to realter the query? It's not immediately clear from reading the code
(saying something like "EntityReference alters the query first, then the realter fires so that everyone else can do their altering")

+  public function getLabel($entity) {
+    // entity_label() doesn't work at all for users.
+    // @see http://drupal.org/1261918
+    return format_username($entity);
+  }

The link is wrong, and this is only true for D8, not for D7. We can just kill the whole method, I guess.

In general the new patch is much nicer, cleaner, I love the interface and the usage of ctools plugins. I will take it for a spin once I finish work for today.

StatusFileSize
new48.88 KB

Thanks for review. New patch integrating #9.

StatusFileSize
new48.84 KB

Come on. What's with me today?

Status:Needs review» Fixed

Merged in 7.x-1.x (420df4b).

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.