Instead of using the standard flag confirm as the link type, flag friend should create its own link type. And have a its own hook_flag_link(). Within that it can add its magik, and create its own form instead of form_altering and requiring that the flag_name be 'friend'.

Furthermore, I believe you could then use this instead of flag_friend_create_link(). This could be achieved by form_altering the flag_add form, adding in the textfields for the various text that you would want to display. This text can then be accessed using $flag->get_label('flag_friend_remove_message', $content_id).

This has the incredible advantage of then allowing the theme to call $flag->theme('flag', $content_id) and get the proper link. No other magik would then be required.

And you could then easily add ctools modal integration... :-D

Comments

sirkitree’s picture

Assigned: Unassigned » sirkitree

Wow! Great idea! Thank you!

sirkitree’s picture

I think this will be part of the 2.x branch which I'm prepping for.

crea’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev

Moving.

crea’s picture

Am I right in understanding that this will allow us to have different types of relationships ?

crea’s picture

Assigned: sirkitree » crea

I'm working on it. My goal in the end to have different relationship types (same as User Relationships module). Flag friend will be using arbitrary user flags, instead of default flag. Let me know if that's not good idea.

crea’s picture

Title: Implement hook_flag_link_types() and hook_flag_link() » Implement own link type as a way of turning any user flag into friend flag
Scott Reynolds’s picture

What different types of relationships? Isn't there only two, Friend and Follow.

What would be the 'follow' relationship? What would that provide that Flag: User doesn't provide?

crea’s picture

With custom link it will be possible to store flag name in our table as separate column, so it will be possible to setup different relationships via different user flags. Example: some (weird ?) social networks allow confirmation on additional relationship types such as "X guy is my friend", "Y girl is my wife". Having only one "friend" relationship is quite limiting IMO. That's why there's User Relationship module but it's messy. I looked at UR code and found it badly designed (monstrous code, many hardcoded stuff etc). Flag/Flag Friend combo is much much better.

What would be the 'follow' relationship?

Follow is configurable with just Flag module alone.

What would that provide that Flag: User doesn't provide?

Arbitrary number of mutual relationships that require confirmation.

crea’s picture

Wow! Really working with Flag module is a pleasure. Those link options automatically pulled in are fantastic.

crea’s picture

@sirkitree:
Maybe split mail notifications into separate module and move third party modules integrations into own include files ? Do you mind if I include these changes in my patch ? I know it will be harder to review, but it will be much easier for me to make it as one big patch. Considering changes are already big, 2.x branch will be rather kind of "small rewrite" than "patch" anyway.

sirkitree’s picture

I'd much rather them be separate patches for review, but if have one big patch that is for 2.x 'rewrite' then I'm game. might take a bit longer to get committed though.

Also, yes, I'm open to a /contrib folder as well.

crea’s picture

Title: Implement own link type as a way of turning any user flag into friend flag » Refactoring of the module in 2.x branch

We can also remove "unfriend" action in 2 steps:
1) As soon as 2 users are friends, create the relationship but dont unflag them. This will allow to still use "unflag" action as a way of breaking the relationship.
2) As a way of denying the incoming request, create special link which lets user to unflag himself.

UPD: I mostly done this steps. I believe this is more clean approach cause we are not inventing special actions here. Just "Deny" link is special.

crea’s picture

Unfortunately there will be no smooth upgrade path. We will have in hook_update_N() following pieces:
1) Additional fid column with index. We can fill it with fid of "friend" flag without problems. I already included that in hook_update_N.
2) Default "friend" flag can be upgraded: we can "friend"-flag legacy friends using Batch API to reflect the change in flag states. As I said above, we no more have FLAG_FRIEND_FLAGGED as "friends" state so any "friends" in 2.x branch should always have each other flagged.
3) Views. Views must be changed, because we have fid as "relationship type id" and also because we have changed fields. People who use customized views won't have big problems: they are used to play with settings. Those who use default views will have problems, because I have no idea how to upgrade default Views. If we provide new default Views it may or may not work depending on if they are overriden or not. I guess we could tell people to revert their views, so they return to our upgraded versions.

crea’s picture

I implemented views filter handler that filters on the type of relationship (fid). Then we need to have flag_friend table properly joined. I wonder what is the proper way to join flag_friend table to the users table ? Problem is we have to join both on "uid" and "friend_uid" columns since for every Flag Friend relationship we have 1 record and user could be in either column. I tried different things:
1) Writing custom join handler that joins like "ON (users.uid = flag_friend.uid OR users.uid = flag_friend.friend_uid). That doesn't work for some reason (ON clause gets broken).
2) Writing custom relationship that joins flag_friend table 2 times (with different aliases). Should work with LEFT join, but if we INNER JOIN same table like that wouldn't that totally break ?
3) Describing pseudo table in hook_views_data() that is alias of flag_friend table. This is almost same as 2 except we have to ensure that this pseudo table always joined together with real table. OTOH using custom relationship gives us advantage of always having joins we need without any worries.

So what would be best way to do that ?

crea’s picture

Having only single record in the flag_friend table for every relationship is a major headache. I believe we could change this to 2 symmetric records for every relationship. Then all our queries would be SO much simplier. For example we wouldn't need to use subselects in "Flag Friend: friends of" views argument handler.
This would also give us possibility to have uni-directional relationships in the future, where each uni-directional relationship would be *single* record. Now if you tell me that we already have it with Flag module, Flag alone doesn't allow to have confirmation together with uni-directional relationship. Flag is a binary system and confirmations essentially make it non-binary, since they add additional transitional states.

UPD: This is mostly done. Having 2 records per relationship greatly simplified Views integrations: we don't need subselects anymore and JOINS are much easier to grok and to implement. There's also initial support for uni-directional relationships.

crea’s picture

StatusFileSize
new126.18 KB

Attaching first "preview" patch. Basic stuff should work (requesting, removing, etc). Integrations with other modules are not ready yet - also I've splitted mail notification into separate flag_friend_mail.module which is not included in this patch. "Flag Friend Access" probably doesn't work too.

Before installing this patch backup your database! This is not ready for production sites yet!

To install

  • Install Flag 2.x if you haven't done it yet.

  • Apply the patch

  • Should apply to the current code in 2.x branch.

  • Run update.php

  • Please ensure that you have "friend" flag enabled before running it.

  • Edit "friend" flag

  • Replace default texts with custom ones if needed.

  • Revert "friend" view to default state and edit it.

  • Make sure relationships in the view use your "Friend" flag. Also make sure that "Flag Friend: relationship" filter uses "Friend" flag too. You may need to re-save some settings in the process.

Patch summary

  • Our relationships are now Flag-specific

  • New "fid" column in the flag_friend table. Thus every relationship is flag-specific. New primary key and indexes.

  • States slightly changed

  • FLAG_FRIEND_FLAGGED was renamed to FLAG_FRIEND_RELATIONSHIP that means "relationship is established". Also we don't unflag unless relationship is breaking. FLAG_FRIEND_BOTH is now transitional state.

  • We no more remove old request messages

  • This could allow to have history of request messages e.g. to investigate abuse reports.

  • Flag Friend now has own "msg_confirm" link type

  • We now have both menu callback and own form processing which allows us to flexibly route all flag operations, and to have own "link options" of Flag.

  • All texts are now customizable per-flag

  • All texts are configured and stored per-flag using Flag module link options feature. This together with fid-aware relationships allows us to use Flag Friend for any kind of relationship that requires confirmation from both parties.

  • We now store 2 entries in the flag_friend table for every relationship

  • This greatly simplified Views integration and other queries. We always store (originating) user in the "uid" column and his friends ("terminating user") in the "friend_uid" column. For 2-way symmetric relationships (e.g. current "friend" relationship) "originating" and "terminating" have no meaning. They will have meaning when we implement support for 1-way relationship: we will be able to store 1 entry in the flag_friend for every 1-way relationship. Thus relationship direction will start to make a difference.

  • Views integration changed

  • No more argument with subselects. Implemented filter handler to filter using specific relationship (flag id). Implemented 2 relationships as a way of choosing JOIN method: either on "uid" column or on "friend_uid" column. Implemented "User: current user" and "User: current user or admin" argument validators to replace custom php code.

crea’s picture

Status: Active » Needs review
StatusFileSize
new134.73 KB

Updated patch:
- removed 6.x compatibility of "flag friend access" module until we make it compatible with 2.x branch. It needs some UI changes probably, to let user choose which flag to use in access rules.
- fixed "author pane" integration. Now the target is Author Pane 2.x
- added replacements for flag_friend_get_friends() and flag_friend_get_friend_count()
- removed function flag_friend_get_flags() cause I couldn't find a place where its being used
- added FLAG_FRIEND_ERROR state describing inconsistency between Flag tables and flag_friend table
- added flag_friend_relationship() function which is sort of our public API to manage flag friend relationships. It handles both flag states and flag_friend table entries.
- added support for transactions (our operations are non-atomic and DB could end up in incosistent state).

crea’s picture

StatusFileSize
new134.73 KB

Previous patch contained stupid bug in the flag_friend_relationship_status() function. Attaching fixed version.

crea’s picture

StatusFileSize
new135 KB

Hmm, bug was still there. We need additional transitional state when relationship is breaking. So I just added FLAG_FRIEND_BREAKING.

crea’s picture

StatusFileSize
new135.33 KB

updated "author pane" integration and fixed a bug in it

crea’s picture

Status: Needs review » Fixed

Jerad kindly gave me committer rights so I committed this.

BenK’s picture

Just keeping track of this thread...

Status: Fixed » Closed (fixed)

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