Add support for Entity API properties

This patch needs to be updated work with the latest versions of flag.

+++ b/flag.module
@@ -1836,3 +1836,76 @@ function flag_set_sid($uid = NULL, $create = TRUE) {
+ * Implements hook_entity_property_info().
+ */
+function flag_entity_property_info() {

This looks like it should be done in hook_entity_property_info_alter(), as we're changing the properties of existing entities, not adding out own.

Also, hook_entity_property_info()/alter() go in a .inc file -- see entity API.

Tagging.

+++ b/flag.module
@@ -5,6 +5,10 @@
+// Manually include the properties info file, because it contains the properties
+// getter callbacks.
+require_once 'flag.info.inc';

Isn't there a Drupal API function for that?

I really don't like automatic requires/includes. They negate the whole point of having separate files.

Could we just have the getters in the .module file then?

Also could their docblocks look like:

* Entity metadata callback: returns the current user's shopping cart order.

A few more things I've noticed:

+++ b/flag.info.inc
@@ -0,0 +1,84 @@
+    $info['user']['properties']['flag_' . $flag->name . '_flagged'] = array(

I'd really like some commenting above each of the things we put into the $info array.

+++ b/flag.info.inc
@@ -0,0 +1,84 @@
+ *
+ * @param $type
+ *   The type can either be the bundle or the entity type.

That's really ugly. If we're going to need this parameter juggling then I don't see the point of this helper function -- let's just fold it into the caller.

+++ b/flag.info.inc
@@ -0,0 +1,84 @@
+    'description' => t('Returns a list of users who flagged an enttiy with flag @flag.', array('@flag' => $flag->name)),

This will sort of work for global flags, but ideally I'd like a boolean for those.

@joachim

I'll work on an updated version of the patch, with your suggestions included, and flag count snippet from #1634620: Flag module integration?.

I haven't actually used global flags, but my understanding that it adds a boolean flag to a node or user. So, there is no flag count for global flags-- it's just TRUE or FALSE, right?

I can modify the entities appropriately with a boolean property. Should we also add a property to specify WHO flagged the content?

Given that I'm currently using Flag 7.x-2.x, I'm going to start with a patch for that branch. Afterwards, I'll update the patch for 7.x-3.x, and submit it separately.

A number of changes:

• Nested 'flag_' . $flag->name . '_flagged' under ['user']['bundles']['user']['properties'] for consistency.
• Replaced 'flag_name' => $flag->name, with 'data' => array('flag' => $flag), so that we can access all object properties.
• I decided NOT to make a separate boolean property for global flags—it seems that the 'count' field will sufficiently handle global flags by returning a count of either 0 or 1. Let me know I'm overlooking an incompatible use-case here.
• Added in descriptive comments.

I'm just starting to test this, so I may post an update with bug fixes. For now, let me know if you disagree with any of the changes.

Status update

This new patch doesn't seem like it's based on the earlier one, as we've lost a few things!

> I decided NOT to make a separate boolean property for global flags—it seems that the 'count' field will sufficiently handle global flags

Technically that's true, but I don't think it's good DX to have a property called foo_count that returns a boolean.

> Should we also add a property to specify WHO flagged the content?

That's not available for global flags.

+++ b/flag.module
@@ -2096,3 +2096,111 @@ function flag_set_sid($uid = NULL, $create = TRUE) {
+/**
+ * Implements hook_entity_property_info_alter().
+ */
+function flag_entity_property_info_alter(&$info) {

This needs to be in an inc file like in the earlier patch.

+++ b/flag.module
@@ -2096,3 +2096,111 @@ function flag_set_sid($uid = NULL, $create = TRUE) {
+    elseif (count($flag->types)) {
+      $bundles = $flag->types;
+    }
+    // If there are no bundles for this content type, move to next flag.
+    else {
+      continue;
+    }

$flag->types can be empty to mean the flag applies to all.

I was partway through a rework of the original patch myself, so I'll try to get round to finishing that this week.

I can easily move new code into include file, and also create the boolean field. I'll test the boolean flags before the next submit. Are there any other necessary changes?

Here's my work in progress -- I was still at the stage of widening the scope of hook_entity_property_info_alter().

As I noted in my earlier comments, I'm not that keen on the helper function here, at least in its current form.

Tagging as release blocker.

Assigning to myself; I have work in progress for this.

Updated patch:

- moved the callbacks to the module file, as I don't like having to load a .inc file unconditionally
- change the getter callbacks to use our API rather than their own queries, as that way they benefit from caching
- removed the info helper, as I think it was just confusing to have that broken up into different functions. Creating the array then using it in several places works well though.
- finished boolean getter callback

Committed.

Thanks everyone who worked on this :)

untag

Playing with the latest patch and seeing some errors:

Notice: Undefined property: flag_node::$entity_type in flag_entity_property_info_alter() (line 52 of /var/www/dev-www/sites/all/modules/flag/flag.info.inc).
Notice: Undefined property: flag_node::$entity_type in flag_entity_property_info_alter() (line 58 of /var/www/dev-www/sites/all/modules/flag/flag.info.inc).
Notice: Undefined property: flag_node::$entity_type in flag_entity_property_info_alter() (line 62 of /var/www/dev-www/sites/all/modules/flag/flag.info.inc).
Notice: Undefined property: flag_node::$entity_type in flag_entity_property_info_alter() (line 82 of /var/www/dev-www/sites/all/modules/flag/flag.info.inc).
Notice: Undefined index: entity keys in entity_views_field_definition() (line 196 of /var/www/dev-www/sites/all/modules/entity/views/entity.views.inc).
Notice: Undefined index: label in search_api_search_api_item_type_info() (line 726 of /var/www/dev-www/sites/all/modules/search_api/search_api.module).

My thought was that if I installed this, then a search API view would have access to the flag that is attached to my content type. But I don't see a way to add the flag field or a relationship to the flag. Am I wrong or am I doing something wrong?

> Playing with the latest patch

What do you mean by the 'latest patch'? The patch above was committed ages ago; it's part of the code now.

> then a search API view would have access to the flag that is attached to my content type

I suspect that is out of the scope of this issue.

The patch above was committed ages ago; it's part of the code now.

If you have any problems with the module, please raise the separate issue.

.

You may want #1362298: Independent views query backend flag links.