clarify reciprocal & one-way relationships

The way default theming works is you get both users marked as having the relationship (with clarifications You to Them/Them to You). The best solution would be to override default theming to hide or alter the wording to suit your use case.

I can see this would be quite common request from websites that are using One-way relationship. If anybody has already accomplished this (either by theming or otherwise) That would be a great help to several of us so we don't have to reinvent wheel.

I agree with the original report here. UR should be intuitive enough to see if the relationship is one-way and if so, only show the one-way. The relationship descriptions are confusing enough already for two-ways...

This is even more relevant to the UR node access module. If I want to allow "them-to-me" access to my content (because it is a one-way relationship), it is very confusing for my site's users to see:

Post to Approved Audience (them to you) view update delete
Post to Approved Audience (you to them) view update delete

What this code does is shows "fans" and "followers" separately in a oneway follower/fan relationship. The blocks for fan and followers existed in UR block but it was not easy to change the title of the blocks. works in rc4

Enable user_relationship_blocks module. Add the blocks on your profile in some region. Add the following function to you theme template.php file

/* Overrides the title of relationship blocks (mainly used on profile) */
  /**
 * Theme function to generate the title of a block
 */
function themename_user_relationship_block_subject($bid, $account, $rtid, $extra) {
  if ($bid == 'pending') {
    return t('Relationship requests');
  }
  else if ($bid == 'actions') {
    return t('Relationship actions');
  }
  //one-way relationships need special wording depending on the direction
  //this implies that we're showing a single relationship, no need to check for UR_BLOCK_ALL_TYPES
  else if ($extra == 'you_to_them') {
    global $user;
    $rtype = user_relationships_type_load($rtid);
    
    if ($account->uid == $user->uid) {
      // override if fan/follower relationship
      if ($rtid == 1){
        $rel_type_lbl ="My UR-follower";
      } else {
        $rel_type_lbl ="I am a @rel_name of";
      }
      return t($rel_type_lbl, array('@rel_name' => ur_tt("user_relationships:rtid:$rtid:name", $rtype->name), '@rel_plural_name' => $rtype->plural_name));
    }
    else {
      // override if fan/follower relationship
      if ($rtid == 1){
        $rel_type_lbl ="@username's UR-follower";
      } else {
        $rel_type_lbl ="@username is a @rel_name of ";
      }
      return t($rel_type_lbl, array('@username' => $account->name, '@rel_name' => ur_tt("user_relationships:rtid:$rtid:name", $rtype->name), '@rel_plural_name' => $rtype->plural_name));
    }
  }
  else {
    global $user;
    $user_name = ($account->uid == $user->uid) ? 'My' : t("@username's", array('@username' => $account->name));
    if ($rtid == UR_BLOCK_ALL_TYPES) {
      $output = ($account->uid == $user->uid) ? t('My relationships') : t("@username's relationships", array('@username' => $account->name));
    }
    else {
      $rtype = user_relationships_type_load($rtid);
      $output = ($account->uid == $user->uid) 
                ? t('My @relationships', array('@relationships' => ur_tt("user_relationships:rtid:$rtid:plural_name", $rtype->plural_name))) 
                : t("@username's @relationships", array('@username' => $account->name, '@relationships' => ur_tt("user_relationships:rtid:$rtid:plural_name", $rtype->plural_name)));
    }
    return $output;
  }
}

Optionally Now all labels can now be changed using locale module and changing translation string. Or you can change in the function above directly if you don't use locale module.

Does anyone know if one-way relationship functionality as described in this issue ever existed in User relationships?

Classifying this as a bug report based on the second sentence of the User relationships project description:

Relationship types can be setup to be one-way or mutual. If a relationship type is one-way (subscriber) only the requester is shown as relating to the requestee

Setting status back to "active" since there's no patch to review yet.

Are you seeing this as a bug because of the fact user B knows the relationship was created, or just the fact that relationship can't be distinguished from other relationships? Also for a bit further clarification are the standard blocks showing the wrong information, or is this limited to views?

My understanding of this feature right now is that it should replicate twitter style follow until the relationship is reciprocated, after that its essentially the same as a 'standard' relationship as far as UR is concerned with no concept of directionality.

There are some really big limitations to how 1-way is currently implemented that we frequently bump up against especially when it comes to improving views integration, right now the plan is to tackle this in the 7.x-2.0 push to incorporate relation module, I'm not sure anyone is up for really fixing the current implementation but if there are some easy bugs to improve I'm all for it.

Are you seeing this as a bug because of the fact user B knows the relationship was created, or just the fact that relationship can't be distinguished from other relationships? Also for a bit further clarification are the standard blocks showing the wrong information, or is this limited to views?

I see it as a bug because in with a one-way relationship, each of the following should be its own unique relationship:
User A => User B
User B => User A

Whereas the system is treating these as a single relationship.

I'm seeing this in the standard blocks added on hook_user().

Well, bug report or feature request I am not planning to spend time on architectural changes to 6.x at this stage.

For better or worse we're stuck with the wacky way that 1-way is currently handled, there is probably some room to change how this is presented to the user, but storage and api changes are just too daunting especially when a major refactor for all this is already on the roadmap.

If anyone has a strong desire to really solve this I've wanted to see this fixed since I first looked at UR and would love to see it resolved.

Haha guess the secret of where i work is out, that last comment was supposed to be from me...

See #1104274: Improve handling of one way relationships for a meta issue where some more discussion about this problem area has already taken place.

Here's a patch that provides the behavior that I and others would expect for a one-way relationship: A=>B and B=>A being two, separate and unrelated relationships.

I tested this patch with a relationship that was one way and then toggled it back to 2-way, and the 2-way setting was observed: A=>B was appropriate reflected on B's user page.

This patch also updates some uses of $form_values, which became $form_state['values'] in the API changes between Drupal 5 and Drupal 6.

My motivation for working on this bug is that it is one of the most reported issues within the Commons distribution: #1244998: User relationships not working as expected (version 2).

It would be great to get some maintainer code review as well as a functional review from folks in the queue.

I haven't looked detailed yet, just wondering if it's possible to split up the obvious bugfixes (wrong variable names, E_NOTICE fixes) and stuff like that into a separate issue so that we can get that it soon.

Right now, I'm not sure which parts of your parts actually change the behavior and which are just bugfixes.

Also, I have fixed numerous bugs while porting this module to 7.x and I am pretty sure that you can have two unrelated relationships (A -> B, B -> A) if the they are marked as being reciprocal. Any chance you can try out the current 7.x-1.x-dev version to see if that works like you'd expect it to?

After testing on a site with 2 relationship types (Friend and follower) I've revised the patch. I'll post screenshots shortly that help explain the before and after behavior here.

Here are some screenshots that help illustrate the problem with a simple 1-way relationship for clarity. In each screenshot, we have 2 browser windows, and in each window a user is viewing another user's profile.

In my testing, the patch in comment #15 preserves expected functionality when 2 relationship types are present, and one is symmetrical while the other is one-way.

Reviewing your patch, quite a few strange things in there which made we wonder if you aren't simply missing the reciprocal flag.

So I installed UR 6.x-1.0 on a D6 test site, created a relationship type with the name "fan", checked both the "This is a one-way relationship" and "This one-way relationship can be reciprocated" (only shows up when the one-way one is checked, wording could probably be improved) and... everything seems to be working just fine:

The only thing that is discussable is how and if the relationhips from the other person to you should show up, but we're working on that in 7.x.

For completeness, below is the review I did before checking this, but my check ensured my feeling that a large part of your patch is actually backwards...

+++ b/user_relationships_api/user_relationships_api.api.inc
@@ -198,7 +198,7 @@ function user_relationships_request_relationship($requester, $requestee = NULL,
   // check whether this relationship aleady exists
-  if (!$type->is_oneway || !$type->is_reciprocal) {
+  if (!$type->is_oneway || $type->is_reciprocal) {
     $existing = user_relationships_load(array('between' => array($requester->uid, $requestee->uid), 'rtid' => $type->rtid), array('count' => TRUE));
   }

Not sure if this condition is correct.

We need to look for duplicates if the type is two-way or not reciprocal. Which is what the old condition did?

e.g. if a type is reciprocal and one-way we must not look for duplicates because a reverse duplicate is allowed.

+++ b/user_relationships_ui/user_relationships_ui.actions.inc
@@ -12,11 +12,10 @@
-  $rtid = $form_state['values']['rtid'];
-  
+  $relationship_type = $form_state['values']['rtid'];
   if (user_access('can have relationships', $requestee)) {
     //check that a type has been chosen
-    if (!$rtid || !($rtype = user_relationships_type_load($rtid))) {
+    if (!$relationship_type || !($rtype = user_relationships_type_load($relationship_type->rtid))) {

not sure what you're doing here. did you change rtid to the whole type object? If so, then the type_load() call is pointless.

+++ b/user_relationships_ui/user_relationships_ui.actions.inc
@@ -68,28 +67,37 @@ function user_relationships_ui_request_validate($form, &$form_state) {
+        // Don't prevent the present user from creating a new one-way
+        // relationship because another user has created one pointing to
+        // the present user.
+        if ($form_state['values']['requester'] != $val_rel->requester) {
+          continue;
+        }
+        ¶

Reverse one-way relationships are only allowed if the type is reciprocal. And this is what's already covered below by the third check which only adds them if $requester->uid and $val_rel->requester_id are equal and ignores them otherwise?

+++ b/user_relationships_ui/user_relationships_ui.actions.inc
@@ -68,28 +67,37 @@ function user_relationships_ui_request_validate($form, &$form_state) {
         if ($requester->uid == $val_rel->requester_id) {
-          $test_relationships[$key]=$value;
+          $test_relationships[$val_rel->$rtid] = $val_rel->$rtid;
         }

$var_rel->$rtid looks like a typo to me, that doesn't make sense.

+++ b/user_relationships_ui/user_relationships_ui.actions.inc
@@ -68,28 +67,37 @@ function user_relationships_ui_request_validate($form, &$form_state) {
+    $result = db_result(db_query($query, array_keys($test_relationships)));
+    if (1 > $result) {

$result is usually used for result sets, I would use something like $existing or $available or something like that. Also, kind of a weird condition, the the only way this is true is if the type is 0. So why not simply check !$result or $result == 0? The first is what I do in http://api.worldempire.ch/api/user_relationships/user_relationships_ui--...

+++ b/user_relationships_ui/user_relationships_ui.actions.inc
@@ -114,23 +122,12 @@ function user_relationships_ui_request_validate($form, &$form_state) {
-  //#578372 leave all form values in the object for other functions to use
-  $relationship = (object) $form_state['values'];
-  $relationship->requester = user_load($form_state['values']['requester']);
-  $relationship->requestee = user_load($form_state['values']['requestee']);
-  $relationship->type = user_relationships_type_load($form_state['values']['rtid']);
-
-  $relationship = user_relationships_request_relationship($relationship);
-
-  if ($relationship === FALSE) {
+ ¶
+  $relationship = user_relationships_request_relationship($form_state['values']['requester'], $form_state['values']['requestee'], $form_state['values']['rtid']->rtid);
+  if (!$relationship) {

Why are you changing this?

As the comment mentions, this was done specifically so that the form can be extended by other modules, e.g. elaborations and the stuff is passed through to hooks.

+++ b/user_relationships_ui/user_relationships_ui.forms.inc
@@ -10,13 +10,13 @@ module_load_include('inc', 'user_relationships_ui', 'user_relationships_ui.actio
- * @param $form_values array $form['values'] when called from form_alter() 
+ * @param $form_state array when called from form_alter() ¶

Trailing space (also in some other places, try to view the patch in dreditor to soo them)

+++ b/user_relationships_ui/user_relationships_ui.forms.inc
@@ -10,13 +10,13 @@ module_load_include('inc', 'user_relationships_ui', 'user_relationships_ui.actio
-  //try to find out desired relationship type
-  if (isset($form_values['rtid'])) {//given through form
-    $default_relationship = $form_values['rtid'];
+  // Check if a default relationship type is set.
+  if (isset($form_state['values']['rtid'])) {
+    $default_relationship = $form_state['values']['rtid'];
   }
   elseif (is_numeric(arg(3)) && user_relationships_type_load(arg(3))) {//given through URL arguments, e.g. relationship/{uid}/request/{rtid}
     $default_relationship = arg(3);
@@ -26,14 +26,14 @@ function user_relationships_ui_request_form($requester, $requestee, $form_values

@@ -26,14 +26,14 @@ function user_relationships_ui_request_form($requester, $requestee, $form_values
     $default_relationship = $rtids[0];
   }
   //verify default relationship may be requested
-  $default_relationship = $relationships[$default_relationship] ? $default_relationship : NULL;
+  $default_relationship = isset($relationships[$default_relationship->rtid]) ? $default_relationship : NULL;
   
-  if (count($relationships)) {
+  if (count($relationships) > 1) {
     //hide control if the relationship is chosen
     if ($default_relationship && variable_get('user_relationships_show_direct_links', 1)) {
       $form['rtid'] = array(
-        '#type' => 'hidden',
-        '#default_value' => $default_relationship,
+        '#type' => 'value',
+        '#value' => $default_relationship,
       );
     }
     else {
@@ -41,10 +41,16 @@ function user_relationships_ui_request_form($requester, $requestee, $form_values

@@ -41,10 +41,16 @@ function user_relationships_ui_request_form($requester, $requestee, $form_values
         '#title'          => t('My relationship is'),
         '#type'           => 'radios',
         '#options'        => $relationships,
-        '#default_value'  => $default_relationship,
+        '#default_value'  => $default_relationship->rid,
       );
     }
   }
+  else {
+    $form['rtid'] = array(
+      '#type' => 'value',
+      '#value' => $default_relationship,
+    );
+  }

This looks like a weird mix of using $default_relationship as object and then again not. Also, rid is certainly wrong, should be rtid.

Doesn't matter if type radios is used or value, the value needs to be consistent. and because radios obviously only support using the rtid, this always needs to use the rtid.

Looks like some of these fixes will also need to applied against 7.x, although the $form_state stuff might not even be necessary. If there is already something in $form_state, it has preference over the #default_value anyway so maybe it can just be dropped completely.

+++ b/user_relationships_ui/user_relationships_ui.forms.inc
@@ -72,10 +78,10 @@ function user_relationships_ui_request_ajax($requestee) {
   $requester = $user;
-
+  $form_state['values']['rtid'] = $relationship_type;
   $script = '<script type="text/javascript">setTimeout(\'Drupal.user_relationships_ui.hidePopup()\', 10000);</script>';

This looks like a hack to me. If we want to pass in the desired type to the form function, then we should just do so directly, without injecting something in $form_state['values']. Doing so would also allow us to move the arg(3) stuff from the request_form() function to this one, making the form more re-usable.

+++ b/user_relationships_ui/user_relationships_ui.forms.inc
@@ -116,28 +122,33 @@ function user_relationships_ui_request(&$form_state, $requestee) {
-  if (!$form['rtid']) {
+  if (!$form_state['values']['rtid']) {

This is not the same thing. The old check checked if the function returned a rtid form element, the new if there already is a predefined value, which only works because you override it above.

+++ b/user_relationships_ui/user_relationships_ui.module
@@ -261,7 +259,7 @@ function _user_relationships_ui_actions_between(&$viewer, &$viewed, $action_type
-      if ($relationship->approved && !isset($list[$relationship->rid])) {
+      if ($relationship->approved && !isset($list[$relationship->rid]) && !($relationship->is_oneway && $relationship->requester_id != $viewer->uid)) {

Also missing the is_reciprocal check I think.

+++ b/user_relationships_ui/user_relationships_ui.module
@@ -544,13 +542,13 @@ function user_relationships_ui_menu() {
-  $items['relationship/%user/request'] = array(
+  $items['relationship/%user/request/%user_relationships_type'] = array(
     'title'             => 'Create a relationship',
     'type'              => MENU_CALLBACK,
     'access callback'   =>  'user_relationships_ui_check_access',
     'access arguments'  => array('edit'),
     'page callback'     => 'user_relationships_ui_request_ajax',
-    'page arguments'    => array(1),
+    'page arguments'    => array(1, 3),
     'file'              => 'user_relationships_ui.forms.inc',
   );

This change makes it impossible to call the request form without a predefined type.

Thanks for that explanation and the detailed patch review, @Berdir!

Indeed, making the one-way relationship reciprocal made this function very close to how I would have expected. The only exception is that a user who (to use the example above) is followed by another can force the following user to unfollow herself. As you point out that's being addressed for the Drupal 7 version of User relationships.

Since there are others who made the same mistake, I'd like to add to the UR documentation to help clarify how reciprocal relationships are supposed to be used.

Currently, that documentation says,

One-way relationships may be something like "manager," "follower," "stalker." You may be my manager, but I'm not yours. You have the option to allow a one way relationship to be reciprocated, but this is not enabled by default.

I'm actually not clear based on my screenshots above how a non-reciprocal one-way relationship is expected to be used, but with some clarification I'd be happy to update the documentation.

Changing issue title + status to reflect the nature of the work that remains here.

The only use case I have seen for one way with no-reciprocation relationships is for cases where the person being connected to is of a role with less permissions and can't manage their own relationships. So a teacher could "student" all of their students, but a student wouldn't be able to remove that relationship.

So If I have understood this correctly, there is no need for a code patch, just documentation change to clarify. Correct?

Re use cases, it's not a permission thing IMHO. A non-reciprocal relationship is one that doesn't make sense to be possible in both directions at the same time. As you said, for example teacher -> students (doesn't matter if they can manage it themself or not), boss/employer -> employee, sales person -> customers...

What I'd like to see is improvements to the user interface, especially the title and description of the reciprocal checkbox.

I'm not a native speaker, so please make suggestions on how to improve these strings. Basically, we need to tell users that if they want something like Twitter, they need to make the relationship reciprocal. Maybe change it to be enabled by default, because that's the more common case..

I updated the main documentation page to hopefully make this a little bit more clear.

I fully agree that a few UI changes could make this a lot easier. If this is pretty much still the same in the D7 branch would it make sense to fix it there first and backport the change?

Not sure how easy it would be to backport the changes, would probably require some adjustments, as we're using #states to hide that option based on the one-way checkbox for example. Also, the admin UI has moved to the the user_relationships module (which was user_relationships_api).

Anyway, it probably still makes sense to do this in 7.x first since the real work here is figuring out the correct wording/descriptions IMHO, the patch itself should be simple enough.

Here is a first go at the help text. It is definitely an improvement over what is there currently, but my head is so deep in this module at this point I'm not sure it would make total sense to someone who just saw this screen for the first time.

Hm..

I'm wondering if we can get rid of stuff like reciprocal (which sounds extremely technical to me..) and one-way in the UI completely and also change the checkbox labels...

Today is the day of small steps :)

Commited, back to active for further improvements. Maybe you want to commit this to 6.x-1.x as well..

#25

I'm wondering if we can get rid of stuff like reciprocal (which sounds extremely technical to me..) and one-way in the UI completely and also change the checkbox labels...

I completely agree on this. I still get confused when I configure this and after reading this forum post, things hasn't cleared up for me :-S