Support for flagging flaggings

aaron - October 9, 2008 - 20:32
Project:Flag
Version:6.x-1.x-dev
Component:Flag core
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs work
Issue tags:flag 1.2, interface language
Description

How difficult would it be to add a layer of reciprocity to Flag? For instance, an administrator could mark a particular flag as requiring reciprocity. Then, a view filter could be added for something like 'Reciprocal flags', which would only return objects with both flags set.

An example: a 'Friend' flag is allowed for users. For two users to be considered 'Friends', they must both have the same flag relationship set. In theory, this could be handled for nodes or other objects allowed as well, although I'm not sure of a use-case off hand. (Although I guess it could be used to tally 'votes' of a node by users?)

There's been discussion about this on the Social Networking Sites group, and I see this as the main lacking feature of Flag that would kill all the other friend-type modules.

#1

quicksketch - October 10, 2008 - 05:31

Thanks for open an issue for this, it's something that I've been thinking a lot about also.

What you're describing would require and additional database column for "temporary" or similar in the flag_content table, as the flag isn't "accepted" until the reciprocating user flags back.

My general feeling however is that this is specific to users (a comment/node isn't going to flag you back). So I best feel that this would be handled in a separate module, but that flag would enable that module to do exactly this. We've been discussing this in #285237: Ability to disallow a flag/unflag operation, where Flag would provide a (tentatively named) hook_flag_access. A "flag friends" module then could implement this hook, and deny the flagging unless it was reciprocating. It would maintain it's own table of temporary flaggings until they were approved by reciprocating user.

#2

mooffie - October 19, 2008 - 14:14

What you're describing would require and additional database column for "temporary" or similar in the flag_content table

A different approach (a quick one, which I haven't thought about much):

1. Our module can flag nodes/comment/users right now. We should add the ability to flag flags ("to flag flaggings", to be exact).

2. We should revisit the proposal to create views of "flaggings".

(For these two features, a "primary key" for the flag_content table is needed.)

#3

aaron - October 31, 2008 - 03:10

great idea! maybe even allow a hook for other data types to be flagged as well (taxonomy terms, files, custom data created by other modules, etc)

#4

mooffie - October 31, 2008 - 08:39

maybe even allow a hook for other data types to be flagged as well (taxonomy terms, files, custom data created by other modules, etc)

It's already possible. In fact, this hook_flag_definitions() is how the current three flags (nodes/comments/users) are plugged into the system.

#5

sun - November 4, 2008 - 20:43

a comment/node isn't going to flag you back

I think that's possible. Think double-opt-in/opt-out systems, action/rules based applications, automated tresholds, aso.

#6

doublejosh - April 17, 2009 - 00:05

The flag friend modules seems more about the reciprocity aspect.

#7

Amitaibu - May 3, 2009 - 13:50
Status:active» needs work

Ok, just for the (geeky) fun of it, I've started implementing what mooffie suggested on #2 - flag flaggings.

* Current patch allows adding a a new flag of 'flag' type.
* There's no user interface to flag flaggings. One should do something like this:

<?php
  $flag
= flag_get_flag('flag');
 
$flag->flag('flag', 100); // <-- 100 is the number found in the fcid column of {flag_content}.
?>

AttachmentSize
319224-flag-flaggings-7.patch 4.55 KB
snap1.png 18.79 KB

#8

mooffie - May 3, 2009 - 22:12

* There's no user interface to flag flaggings.

There is: there's Views' "Ops" field. See #371432: Exposing the flaggings table, which completes your patch.

+ 'flag' => array(
+ 'title' => t('Flags'),
+ 'description' => t('Flags are an object created by the flag module.'),

It'd be very sad if we pick the word "flag" for this. We should use "flagging" (pl. "flaggings") to describe these objects.

/**
+ * Load a flag content. Flag content is defined as fcid in flag_content table.
[...]
+function flag_load_flag_content($fcid = 0) {
[...]
+ [...] db_fetch_object(db_query("SELECT fid FROM {flag_content} WHERE fcid = %d ", $fcid));

1. A "flagging" object should be the complete record, not just the 'fcid' column.
2. No need for a separate function; move this code into flag_flagging::_load_content().

(See how confusing everything looks when the English word "flag" is used instead of "flagging". Not only it "looks" bad, it also behaves bad: it's the mental leap from "flag" to "flagging" that allows one to see that this object should contain the complete record.)

#9

mooffie - May 3, 2009 - 22:45

+1

I believe the ability to flag flaggings is extremely useful. It will solve many feature and support requests here, in an elegant manner.

This feature goes hand-in-hand with #371432. These two issues are the two sides of the same coin (though #371432 can exist independently).

Even if you, the reader, don't see the necessity of this feature, the necessity of #371432 can be easily seen. And since these two features are complementary, they both should go in.

Moreover, it's trivial to implement this feature (these two features). Amitai provided the handler code, and #371432 provides the Views half.

#10

mooffie - May 3, 2009 - 22:53

(Amitai, thanks for looking into this. It's going to be a valuable contribution.)

#11

Amitaibu - May 4, 2009 - 19:44

@Mooffie,
thank you for the review - I've addressed your issues in this patch. Rules integration still pending.

AttachmentSize
319224-flag-flaggings-11.patch 3.8 KB

#12

Amitaibu - May 4, 2009 - 07:02
Title:Reciprocity for Flags?» Implement flag flaggings

better title.

#13

mooffie - May 5, 2009 - 23:20
Title:Implement flag flaggings» Support for flagging flaggings

Title: Reciprocity for Flags? » Implement flag flaggings

better title.

It isn't. We want to flag flaggings, not to flag flags ;-)

"Flaggings" are like dog ears in a book: they are the little entities one creates when one flags something. A "flag", on the other hand, is the *hand* used to make these dog-ears.

I've noticed that users sometimes use the term "flag" when they really mean "a flagging", but we programmers should use the right terms in the source code.

+ 'description' => t('Flagging are flags that were flagged.'),

Maybe "For advanced users only. Flaggings are the entities created whenever you flag something. They are like the dog-ears in a book." Mmmm... we could certainly use some help from English wizards here...

#14

mooffie - May 5, 2009 - 23:17

+function flag_load_flag_content($fcid = 0) {

[...]
2. No need for a separate function; move this code into flag_flagging::_load_content().

BTW, you have a healthy mind! You did recognize that it's a good thing to be able to load a content object (the flagging object) without necessarily having a flag handler object. That's why you provided a function and not a handler method. But note that _load_content() is defined to be a 'static' method (see '@static' in the doxygen; all because PHP4 doesn't support this keyword), so it *can* be called directly, as a function.

(However, that method is marked '@private'. Oh, that's a long story.)

#15

mooffie - May 5, 2009 - 23:23

Maybe "For advanced users only. Flaggings are the entities [...]

Correction: "For advanced usage only. [...]"

#16

Amitaibu - May 6, 2009 - 07:04

@mooffie,
I like your dog-ears analogy! :)

1) I've tried rephrasing the description.
2) flag_load_flag_content() already moved to _load_content().
3) What do you this should be done with 'Display options' - keep it there, or remove until the Views on flaggings is implemented?
4) I was thinking that maybe we can commit the patch (when it's ready), and then do the Rules integration, as a follow up patch.

AttachmentSize
319224-flagging-flaggings-16.patch 3.89 KB

#17

mooffie - May 7, 2009 - 03:35

I was thinking that maybe we [...]

Can you do some Views development? There isn't much point in having this feature without having the #371432: Exposing the flaggings table patch in. Now, the "patch" at #371432 isn't really a patch (I cobled it up hastily when no longer using Drupal), and it needs some work. I'll happily review your code if you decide to pick up that task (though I won't be able to actually run any Drupal code).

#18

Amitaibu - May 8, 2009 - 18:55

>>Can you do some Views development?

I haven't done so far, but I guess it's a good time to start... I'll have a look on that issue.

#19

opensanta - May 17, 2009 - 05:03

Tagging.

#20

Amitaibu - May 18, 2009 - 14:47
Component:Code» Flag core

Just putting my work so far - not much, but a little progress with the Views stuff.

AttachmentSize
Snap1.png 12.72 KB
319224-flagging-flaggings-20.patch 10.65 KB

#21

opensanta - May 18, 2009 - 20:57

Not sure if it's just me, but both the attachments in #20 are broken.

Here's the patch: http://drupal.org/files/issues/319224-flagging-flaggings-20.patch

#22

Amitaibu - May 20, 2009 - 13:17

weird... thanks mithcell. here's the snap shot again

AttachmentSize
Snap1.png 12.72 KB

#23

Amitaibu - May 25, 2009 - 13:39

@mooffie,
A little busy, so It takes me some time - but I'm getting there. Question - what is the definition that should be in get_views_info() ?

 
 

Drupal is a registered trademark of Dries Buytaert.