Here's a thread to propose that flags should be extended to work with any entity in Drupal, not only nodes + comments + users.

There's another thread (#871064: Making flaggings fieldable) that has touched upon this, but the main discussion there is about making flags fieldable.

(I'd be surprised if this feature request hasn't already been discussed among the Flag maintainers, but I really couldn't find an issue for it. And if there's a good reason *not* to have flags on generic entities I'd be happy to hear it.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rlmumford’s picture

This would also include the #1073568: Absorb Flag Terms into Flag Module issue. Looks like its a case of changing the schema to include entity_type and bundle_type to replace the current content_type column then changing everything we need to to carry that translation over.

Then obviously its a case of updating the widgets in the flag settings forms. Interestingly, this becomes pretty similar to the relations module, except you're always linking users to entities rather than arbitrary entities to entities. http://drupal.org/project/relation

quicksketch’s picture

For things like this I'd prefer to wait until 2.0 was released. Because this functionality is Drupal 7 only, I'm also very open to postponing it until Flag 3.x, which will be for Drupal 7 only and we can leave Drupal 6 behind. But first things first, we need to get out the 2.0 release which is feature-equivalent between Drupal 6 and Drupal 7.

dmsmidt’s picture

subscribe

dmsmidt’s picture

--can be removed--

zkday’s picture

subscribe

zkday’s picture

FileSize
8.83 KB

Thanks, I think this is a necessary feature, and I have a patches for this.

rlmumford’s picture

This has been postponed to version 3

quicksketch’s picture

Status: Active » Postponed

Right you are rlmumford.

LSU_JBob’s picture

I would desperately love to have this on my D7 sites.

rlmumford’s picture

If we get version 2 out quickly this will end up in active development quickly :)

geerlingguy’s picture

Subscribe.

bibo’s picture

subscribe

Shadlington’s picture

Itangalo’s picture

Well spotted!

Maintainer's opinion about providing functionality as a Flag patch has now been asked for: #1249744: Merge with Flag module?

fago’s picture

You could also try http://drupal.org/project/rules_link + using a user-reference field for link state storage or so.

mansspams’s picture

I tested them both. flag_entity is still far from ready and I failed to install rules_link successfully (although in my mind I dremt up a solution where I could add boolean field to image entity and manipulate/fill it with rules_link).

Another issue is presentation. What are your ideas on where to put possible flag link on, f. ex., file entity? Like, after each field display?

mh86’s picture

subscribing

mh86’s picture

Status: Postponed » Needs review
FileSize
15.95 KB

Inspired by the Flag Entity module, I've created a very simple flag entity controller that basically supports all available entity types (except those that are used for configurations, like Rules). Node, user and comment entity types extend this controller and add their specific stuff. It would be possible to re-use more of the base flag entity controller, but I didn't want to destroy anything existing.
Tested it with profile2 and it perfectly worked.

mh86’s picture

FileSize
15.99 KB

now without the trailing whitespaces...

mh86’s picture

FileSize
16.89 KB

Updated patch that fixes following two problems:

  • Prevent warnings if a new (non existing id) entity is viewed (e.g. profile2 page)
  • Fix features export code by preventing that the whole entity info array gets exported.
Anonymous’s picture

It may be over the top, but I would also consider using the relation module to control relationships between a user and any entity. Flag could then just be a UI wrapper around relation entities.

The only scenario that might be tricky is making relation entities for "global" flags.

mansspams’s picture

@kevee, it looks like extremely complicated proposal, you probably need to open new issue and come up with damn good arguments ;)

mh86’s picture

I tried using the Relation module in combination with Rule link, but it was quite complicated to set up and as far as I remember I couldn't get everything working.
Storing the relation (flag) between a user and an entity is pretty simple (concerning the data structure), so I don't think the Relation module is of much help at the moment (although I can image that in future more modules will make use of the Relation module for storing such things).

rafamd’s picture

One thing (maybe) worth considering is that Relation module uses one and only one table to store the related entities for all relations, where every related entity gets one row in that table.

Our usecase for Relation (for other functionalities than flagging) could easily generate a couple million rows per year on that table.

This is something I wanted to talk about with the Relation guys but what I want to say here is that one benefit of having a separate solution (schema) for flag is that we would offload that table (that seems to be quite of a bottleneck).

Does this make sense ?

EDIT: I've just checked Relation module schema again and they seem to have moved what before was a custom table to a field ("endpoints"). The question is still valid, as a field is also a table ...

Anonymous’s picture

Well, relation is a perfect use case for projects where you want to add additional fields to relationships between a user and an entity - while flag is awesome and I have used it a lot in the past, you can't have a person flag something and add additional meta-data around that relationship.

The only barrier seems to be the UI of relations rather than anything else. While I would defer to the module maintainer for flag, we have been rolling our own flag-esque module use relation already, so I'll open an issue when that's ready for a sandbox.

quicksketch’s picture

you can't have a person flag something and add additional meta-data around that relationship.

Note that this is a separate feature request over here: #871064: Making flaggings fieldable

nedjo’s picture

The Mark module, http://drupal.org/project/mark, is another option to flagging non-nodes.

dasjo’s picture

fenda’s picture

The patch in #20 did not patch cleanly for me to 7.x-2.0-beta6.
What's the status on this? Flagging should be for any entity type! :)

univate’s picture

Re-rolled this patch - current working here with custom entitys that I have created myself.

joachim’s picture

Status: Needs review » Needs work
+++ b/flag.inc
@@ -16,26 +16,34 @@
+    // Only add flag support for non-config entities.

I'm mostly curious here -- what's a config entity?

+++ b/flag.inc
@@ -16,26 +16,34 @@
+ * Returns the flag handler class for an entity type, which is flag_{entity_type}.
+ * If no entity type specific implementation exists, the default flag_entity
+ * class is used.

Summary should start with a single line, with more details below. Also missing a @param.

+++ b/flag.inc
@@ -1167,16 +1175,175 @@ class flag_flag {
+  function _load_content($content_id) {

Missing @param.

... and lots more missing docblocks and @params.

I'm not sure how strict the maintainer of this module is being on documentation, but it would be really nice if more contrib modules followed core docs standards :)

univate’s picture

FileSize
16.89 KB

Actually that previous patch wasn't against the current dev, this one should apply cleanly.

as for non-config comment - I assume the original author of this patch was referring to this part of the entity api docs:

  * - configuration: (optional) A boolean value that specifies whether the entity
  *   type should be considered as configuration. Modules working with entities
  *   may use this value to decide whether they should deal with a certain entity
  *   type. Defaults to TRUE to for entity types that are exportable, else to
  *   FALSE.
mh86’s picture

thanks for updating the patch. examples for configuration entities are the profile2 types, or rule configs.

joachim’s picture

+++ b/flag.inc
@@ -16,26 +16,34 @@
+    // Only add flag support for non-config entities.

Thanks for the explanation!

But in that case, isn't this wrong? Why should we prevent flagging of profile2 types?

mh86’s picture

At the moment you can flag profiles, but not the profile type. I couldn't think of a use case yet.

JGonzalez’s picture

After applying the patch, I'm getting Warning: class_exists() expects parameter 1 to be string, array given in flag_create_handler() (line 89 of /var/sites/my.aw/web/sites/all/modules/contrib/flag/flag.inc).

Any ideas?

joachim’s picture

> At the moment you can flag profiles, but not the profile type. I couldn't think of a use case yet.

Duh, profile types and not profiles... I see what you mean now, sorry!

JGonzalez’s picture

Around line 87 of flag.inc,

  if (isset($definition) && class_exists($definition['handler'])) {

Should be replaced with:

  if (isset($definition) && array_key_exists('handler',$definition) && is_string($definition['handler']) ) {

Actually, a better solution should be made for this, as there are times when $definition['handler'] isn't a string, but an array, and I'm guessing we have to iterated through all of these items.

If I get time I'll make a patch for this item.

fenda’s picture

Is this feature still being postponed to 3.0? Flag does everything I need it to but just not with my custom entity types.

jxavier’s picture

Would it be possible to show an example of an entity using this patch (http://drupal.org/files/flag-entity-1035410-32.patch)? I am testing it with an entity I've created myself, but the flag link is not displayed. Is there any specific callback that my entity needs to implement or expose? Thanks.

alexweber’s picture

Status: Needs work » Needs review
FileSize
17.38 KB

Patch re-rolled against head, should apply cleanly now. Also incorporated the fix from #38.

joachim’s picture

Status: Needs review » Needs work
FileSize
18.68 KB

Here's an updated patch that fixes a few minor things:

- the label 'If checked, flag is considered "global" and each node ...' should be general to entities
- the flag table header should say 'entity bundle' rather than 'node type'
- flag_entity class should go below its parents in the code

Further things that need work:

- How is 'Flag access by content authorship' going to work with general entities? Surely it doesn't make sense for things such as terms that have no concept of author, and it's going to require work to implement with other entities depending on where they store their author data.
- I can't see the flag JS toggle on either taxonomy terms or commerce products. Those two in particular don't actually have a 'view' mode really -- just a form. I'm not sure whether there's a way to add the flag checkbox to entity forms in general. It might have to be done per entity type, in which case we need to support taxo terms.

micnap’s picture

I tested the patch in #42 and it's working beautifully! Thank you!!!

I'm guessing this needs to be built yet but thought I'd ask.... I've created a view listing the entities that have been flagged. I'd like to use views bulk operations to unflag many entities as once. I don't see the option when I add vbo to the view to include a action for flagging/unflagging the entities. Should this option be showing?

Thanks,
Mickey

joachim’s picture

A further problem to consider: when a node type is deleted, we act on that in a hook and prune those node types from {flag_types}. There's no universal hook to know when an entity bundle is deleted.

I have the feeling we'll have quite a few follow-ups to this...

joachim’s picture

> I'm not sure whether there's a way to add the flag checkbox to entity forms in general.

Hacky, but there's http://api.drupal.org/api/drupal/modules!field!field.api.php/function/ho...

joachim’s picture

Assigned: Unassigned » joachim

Some more thoughts about this. Will reroll when I have more time.

+++ b/flag.inc
@@ -16,23 +16,30 @@
+    if (!isset($entity['configuration']) || !$entity['configuration']) {

I think empty() suffices here?

+++ b/flag.inc
@@ -16,23 +16,30 @@
+        'description' => t('@entity-type entity', array('@entity-type' => $entity['label'])),

This loses us the descriptions we have for nodes, users, and comment.

+++ b/flag.inc
@@ -16,23 +16,30 @@
+function flag_get_handler($entity_type) {

I'm not happy with this function name. get_THING() means actually get the thing that's defined. In this context, it should mean 'go lift it out of the flag definitions'. But this is the opposite: it provides the class names to flag_flag_definitions().

And it's only used there. So I don't think it needs to be a function.

+++ b/flag.inc
@@ -78,7 +85,7 @@ function flag_get_types() {
-  if (isset($definition) && class_exists($definition['handler'])) {
+  if (isset($definition) && array_key_exists('handler',$definition) && is_string($definition['handler']) ) {

I don't know why this change has been made. Flag definitions are still required to supply a handler class, and we've not changed that as we supply the generic flag_entity class.

+++ b/flag.inc
@@ -1239,17 +1246,176 @@ class flag_flag {
+  var $entity_info;

Needs docblock.

Though I'm not sure we need to store this, as
entity_get_info() has fast static caching, and we have to go to the trouble of unsetting it for export...

+++ b/flag.inc
@@ -1239,17 +1246,176 @@ class flag_flag {
+   * Default contructor that furthermore loads the entity info for the current content_type.
+   *
+   * content_type is used as synonym for entity_type in this handler.

Formatting needs fixing.

joachim’s picture

Another problem: if we sweep in and declare a flag type for every entity with handler flag_entity, then how will other modules (such as Commerce, say) provide more sophisticated handlers for their entity types? They'll come too late, as we'll have already stepped in and said we support them with flag_entity!

I see two ways to do this:

A. We only define entities we have handlers for in flag_flag_definitions(), and in flag_flag_definitions_alter() we fill in with flag_entity for any entities not yet supported.

Pro: contrib modules just define their flag types as normal
Con: extra hook to implement on our party

B. We define types for all entities in flag_flag_definitions(), and contrib modules must use hook_flag_definitions_alter() to change the handler from flag_entity to one they provide.

Pro: less work for us. yay!
Con: we effectively now have a hook that only flag module can use, flag_flag_definitions(), as all other modules are relegated to the alter hook. That's a WTF. Furthermore, I tend to follow the principle that the framework or base module should do the heavy work so the implementers don't have to. Therefore I think this is enough to convince me of option A.

alexweber’s picture

I agree 100% with joachim on this, option A seems like the best choice.

andypost’s picture

Suppose both option should go to 3.x branch because a lot of contrib modules work with current architecture. Also please take care about clean-up flag data on entity delete. Currently node and user needs special processing. See #1596492: Flag database data not cleaned up on comment & node delete

joachim’s picture

I think option A lets contrib modules work ok:

1. flag_foo implements hook_flag_definitions() and defines a flag type for the foo entity
2. our flag_flag_definitions_alter() look at the definitions: foo already has a flag type so we do nothing.

For entity clean-up, we'll have to rely on http://api.drupal.org/api/drupal/modules!system!system.api.php/function/....

joachim’s picture

> - How is 'Flag access by content authorship' going to work with general entities? Surely it doesn't make sense for things such as terms that have no concept of author, and it's going to require work to implement with other entities depending on where they store their author data.

I'm moving this bit down to the flag_node class.

The whole system of author access is done in our hook implementation flag_flag_access(), which caters for the two entity types we know how to handle it for: nodes and comments. Having those options as ticky boxes in any other entity type will result in absolutely nothing happening. Hence implementing modules need to provide that anyway, so they should be up to the job of providing the UI. And I think it could do with sprucing up anyway so it's handled in the handler rather than module code, but that's a totally separate issue. Also, in an ideal world, entity handlers could declare which entity property defines the entity's 'author' and flag core takes care of it. But now we're talking ponies ;)

joachim’s picture

> A further problem to consider: when a node type is deleted, we act on that in a hook and prune those node types from {flag_types}. There's no universal hook to know when an entity bundle is deleted.

There's http://api.drupal.org/api/drupal/modules!field!field.api.php/function/ho....

Which won't work for entities that aren't fieldable...
I'm going to leave this one for a follow-up, as it's just cruft in the types table.

joachim’s picture

Status: Needs work » Needs review
FileSize
26.63 KB

Here's what I hope is a patch that's complete. Please review!

- works on taxonomy terms, but the link can't be seen due to a core bug: #1067120: Missing hook_taxonomy_term_view() or hook_entity_view() when viewing a term
- works on Commerce products, but the link can't be seen because there's no 'display' of a product, really
- works on my own custom entity (sandbox at http://drupal.org/sandbox/joachim/1598562)

Follow-ups I'm not going to tackle here:

- #1689686: act on entity bundle delete to clean up {flag_types} table
- #1689704: investigate performance of storing entity info in flag object vs fetching it

alexweber’s picture

Awesome joachim, thanks for the patch!

joachim’s picture

@alexweber: does that mean you're tried it? :)

If someone can review this and confirm it works ok I'll commit it and make a new release!

joachim’s picture

Update: works on commerce orders too :)

Oleksa-1’s picture

Tested patch

In 'Display options':Display link on entity
Seems didn't work for me.

Display checkbox on entity edit form
worked fine

P.S. I used entities from http://drupal.org/project/examples module

joachim’s picture

> In 'Display options':Display link on entity

Which entity type were you using?
That option does not make sense for all entity types, and that's not something we can determine.

Oleksa-1’s picture

From examples module

joachim’s picture

That's a bug in Examples: #1691346: doesn't invoke hook_entity_view() :)

Thanks for testing!

Oleksa-1’s picture

Shame on example_module developers :)

Oleksa-1’s picture

Status: Needs review » Reviewed & tested by the community

Tried with patch in #1691346: doesn't invoke hook_entity_view() and it works, the problem really was in example_module.

Hope to see next feature #871064: Making flaggings fieldable in 7.x-2.x-dev , as well ) Thx a lor for great work!

joachim’s picture

Status: Reviewed & tested by the community » Fixed

Issue #1035410 by mh86, univate, joachim, zkday, alexweber: Added ability to flag any entity type.

Committed.

Thanks everybody! New beta out soon.

alexweber’s picture

AWESOME! :)

rafamd’s picture

clap clap clap joachim++

joachim’s picture

Thanks! :)

I'm hoping to release another beta and then a stable 2.0 fairly soon -- any help with the issue queue would be much appreciated!

Status: Fixed » Closed (fixed)

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

architectJpres’s picture

Has this made it into 7.x-2.0-beta8

joachim’s picture

You can determine that by either comparing the date this was marked as fixed with the date on the release. Or by just reading the release notes!

architectJpres’s picture

Yes, I meant the development version.

Looking forward to seeing it come into beta.

joachim’s picture

It is already in the beta -- read the release notes. http://drupal.org/node/1698648

alexweber’s picture

@joachim, this is awesome thanks! Can you please update the project page to reflect this change? :)

joachim’s picture

I don't see what needs changing right now, though there is an issue open for moving a lot of the branch technical details to a docs page.

alexweber’s picture

I meant that support for generic entities exists now, the project page states:

Using this module, the site administrator can provide any number of flags for nodes, comments, or users.

joachim’s picture

Done.

warmth’s picture

I'm using the beta 8 but I still can't see the link to flag a term after setting it up properly. I can only see the checkbox option when editing a tag.

Where does it should appear? I looked for it on the list of terms associated with a content (i.e. the list of terms in a full content/ view) and in the tags/ view but nothing appeared.

Sorry for cross posting here: http://drupal.org/node/1067120#comment-6400362

Note: I have applied no patches

joachim’s picture

> I'm using the beta 8 but I still can't see the link to flag a term after setting it up properly. I can only see the checkbox option when editing a tag.

That's because of this core bug: #1067120: Missing hook_taxonomy_term_view() or hook_entity_view() when viewing a term.

I'll add that to the release notes.

warmth’s picture

So the only solution is to apply that latest patch to the core?

Exploratus’s picture

I am currently using the Drealty Module, which makes real estate listings into entities. Would love to see Flag working with this.

joachim’s picture

Could you file new issues for new issues please? Though I can stay straight away that it'll be up to that module to work with Flag, not the other way round.

Exploratus’s picture

Cool. Thanks for the prompt response.

bloomt’s picture

Issue summary: View changes

@Exploratus did you ever find a solution for the drealty module?