I've read the code comments for why flagging is disabled for entities that aren't fieldable and I understand the logic.

However in my tests I've found that this makes taxonomy vocabularies impossible to be flag!

There's no "view" page for a taxonomy vocabulary; it has no view modes and, in fact, Display Suite won't even let you add any. Therefore, the only way to flag a vocabulary would be through the edit form, but the option is disabled because it's not fieldable.

I realize that adding the flag checkbox through Form Alter & API is not as clean as doing it through the Field API so my question is: how badly do we want to support flagging taxonomy vocabularies? Because as it stands, it's just taking up space in the list and isn't actually usable... :)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

> but the option is disabled because it's not fieldable

It's like that because the only way we can add our flag checkboxes to entity forms in a generic way is via the field_attach_form() system.

For entities which are not fieldable, it's up to them to:

a) provide form alteration to put the flag widget on their form
b) provide a handler class to expose a user admin option for that

> Because as it stands, it's just taking up space in the list and isn't actually usable... :)

Yup. I think the actual fix here might be that we exclude vocabs from being flaggable.

alexweber’s picture

> Yup. I think the actual fix here might be that we exclude vocabs from being flaggable.

That's what I was going for! It's really just a technicality, like being able to Flag Profile2 types. In general, if an entity is being used as the entity type and another as the bundles (a pretty common pattern), in general we can assume that the "type" entity shouldn't be flagged because they are just there to use the Entity API Admin UI features and aren't meant to be viewed directly, but that's another issue.

Can we change this issue's title and exclude taxonomy vocabularies? Maybe we can have an API function to return entity types that aren't supposed to be flaggable and drupal_alter() it so that contribs can use it too?

joachim’s picture

> Can we change this issue's title and exclude taxonomy vocabularies?

Yup.

This is again a core/EntityAPI thing. EntityAPI adds two properties to entity info that are of interest:

- $entity_info['configuration'] is TRUE if the entity is configuration rather than content, like profile2 types
- $entity_info['bundle of'] is TRUE if the entity is the type of another entity, again like profile2 types.

We use the first to skip config entities, but again, core of course doesn't support this.

> Maybe we can have an API function to return entity types that aren't supposed to be flaggable and drupal_alter() it so that contribs can use it too?

That seems a bit fiddly to me.

As you've said ether further up or on another issue, most custom entity modules will be using Entity API, and if they're config entities they'll set the property that we check.

alexweber’s picture

Title: Taxonomy Vocabularies and flagging disabled for entities that aren't fieldable » Don't allow flagging taxonomy vocabularies
Category: support » task
alexweber’s picture

@joachim, what do you reckon is the best way to implement this? there's a bunch of different approaches:

function flag_flag_type_info_alter(&$definitions) {
  foreach (entity_get_info() as $entity_type => $entity_info) {
    // Only add flag support for entities that don't yet have them, and which
    // are non-config entities.
    if (!isset($definitions[$entity_type]) && empty($entity_info['configuration'])) {
      $definitions[$entity_type] = array(
        'title' => $entity_info['label'],
        'description' => t('@entity-type entity', array('@entity-type' => $entity_info['label'])),
        'handler' => 'flag_entity',
      );
    }
  }
}

I want to check if ($entity_type === 'taxonomy_vocabulary') and if so, continue; but I think you're gonna yell at me if I do this :)

joachim’s picture

> I want to check if ($entity_type === 'taxonomy_vocabulary') and if so, continue; but I think you're gonna yell at me if I do this :)

:D

I don't think there's a better way, so it's going to have to be that. Stick a comment on to explain we have to deal with the taxo vocab entity as a special case.

alexweber’s picture

Status: Active » Needs review
FileSize
891 bytes

There we go! Feel free to reword the comment in any way you see fit! :)

socketwench’s picture

Status: Needs review » Needs work

Maybe I'm not understanding the issue, but it still appears I can create Flags for vocabularies. Shouldn't that function be removed as well?

Other than that, it tests clean.

alexweber’s picture

@socketwench are you running a clean dev clone or do you have any other patches applied by any chance?

Also, this is cached, so you gotta flush them to update the list in flag/add

alexweber’s picture

Status: Needs work » Needs review

I've just tested this again against a clean 7.x-3.x dev branch and can confirm that it works 100% but requires a cache flush to take effect.

@socketwench, please confirm.

Thanks!

joachim’s picture

Status: Needs review » Fixed

Committed, with a tweak to comment wrapping. I've also removed the link to this issue -- I think the comment explains it all. And there's always git blame ;)

git commit -m "Issue #1905766 by alexweber: Removed declaration of flag type for taxonomy vocabularies." --author="alexweber "

alexweber’s picture

Awesome!

Status: Fixed » Closed (fixed)

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