Thanks for creating this essential community module.

Any plans to port to Drupal 7?

Comments

Bilmar’s picture

I'm really looking forward to seeing this awesome module in Drupal 7!
fyi http://webchick.net/node/74

I would like to help with any testing required.
Thanks very much in advance.

deltab’s picture

+1, I will help out in testing as well.

YK85’s picture

+1 subscribing, I was not able to find any release at Releases for User Relationships page and found this in the queue.
Are there plans to release a d7 dev version for testing before the drupal 7 stable comes out? Thanks

lpeet01’s picture

Same here, I would like this module ported to D7. I dont have the time to to help with coding, but will help financially if needed.

dorien’s picture

Perhaps we can start a chip in (chipin.com) for the contributors?

Berdir’s picture

I've started to port the module to Drupal 7.

Since this is quite a big task, I've pushed my current code to github : http://github.com/Berdir/user_relationships

Once it's finished, I'll create a patch so that it can be commited easily, but until then, please download it from there if you want to try it out.

Curent state:
- user_relationships_api.module ported, 3 test failures remaining

So there isn't much to test for now.

If someone wants to help, we should discuss who's doing what :)

deltab’s picture

Sure, send the tests over. I have D7 alpha 2.

Berdir’s picture

Ok, UI, Blocks, Elaborations and Implications are now ported, you can download the project by clicking on "Download Source" on the above link.

There are still some issues and I have not tested everything, so please report back all errors. Not that there are many notices, which I plan to fix in the 6.x branch instead.

BenK’s picture

Subscribing...

johngriffin’s picture

Is there anything I can help test at the moment?

Berdir’s picture

Status: Active » Needs work

Nothing special. Just install it and try everything and report any bugs you can find.

johngriffin’s picture

Ok will do. I should get time to test later this week. Since there is no 7.x version released though I won't be able to assign bugs to 7.x on d.o issue queue.

Berdir’s picture

Just report them in this issue.

BenK’s picture

Hey Berdir,

I just completed some initial testing of your D7 User Relationships port. I pulled the latest code from your github account and enabled the API, UI, Blocks, Elaborations and Implications sub-modules. Overall, the basic submission/approval process of user relationships seem to be working fine. I did notice, however, the following bugs and error messages:

1. When trying to add more than one relationship to a given user (for instance, first making someone a "friend," then making them a "buddy"), I received the following error message:

Recoverable fatal error: Object of class stdClass could not be converted to string in DatabaseStatementBase->execute() (line 1987 of /home/sandboxes/sandbox10/public_html/includes/database/database.inc).

When this error was displayed, the second relationship type was not added. Note that this error wasn't displayed if the second relationship was added through an "implied relationship" (see below for more on that).

2. When viewing a relationship that was created via an "implied relationship," I received the following error message on every page where the relationship is subsequently being displayed:

Notice: Undefined property: stdClass::$elaboration in user_relationship_elaborations_preprocess_user_relationships() (line 130 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationship_elaborations/user_relationship_elaborations.module).

3. When trying to "remove" a user relationship, I received the following warning message:

Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 104 of /home/sandboxes/sandbox10/public_html/includes/entity.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->cacheGet() (line 264 of /home/sandboxes/sandbox10/public_html/includes/entity.inc).

Despite this warning message, the relationship seems to be successfully deleted.

4. When defining a relationship that can only be used by certain roles (for instance, one that can only be submitted by administrators), I received an error message any time someone who does not have permitted role (a non-administrator) visits the user page of any other user:

Notice: Undefined index: 3 in user_relationships_api_can_request() (line 448 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationships_api/user_relationships_api.api.inc).

There appears to be a new notice for every relationship that is limited to certain roles.

5. When trying to reciprocate a one-way relationship back to a user, I received the following error message:

Notice: Undefined index: 5 in user_relationships_ui_request_form() (line 30 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).
Notice: Undefined variable: relationship in user_relationships_ui_request() (line 133 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).

Additionally, when I then view the relationship submission confirmation page, the one-way reciprocated relationship I selected doesn't appear at all... only the other relationship possibilities appear (not the link that was clicked).

6. After a relationship is successfully added, it does not appear on the user page of the related user. (For instance, if user2 is a friend of user1 and then user1 visits user2's user page, the "Your relationships to this user" section is blank.) The "Relationship actions" section continues to display other possible relationships to submit, but the relationship that has already been established is missing.

7. It appears that every block may be duplicated on the D7 blocks UI page (admin/structure/block). More specifically, every block is listed with the "My Relationships" prefix and then listed again with the "User Relationships" prefix. If these blocks are meant to serve a different purpose, I'm not clear on what the difference is. Also, it appears that only the ones with "My Relationships" actually work (the ones with "User Relationships" don't display).

8. When try to display a block for a one-way relationship--such as "My Relationships: one way friends (Them to You, normal direction)"--I'm getting the following error message shown along with the block:

Notice: Undefined property: stdClass::$sort in template_preprocess_user_relationships_block() (line 371 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationship_blocks/user_relationship_blocks.module).
Notice: Undefined property: stdClass::$size in template_preprocess_user_relationships_block() (line 377 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationship_blocks/user_relationship_blocks.module).

Thanks again for your hard work on this port... looking forward to hearing your thoughts. And I'm available for more testing whenever you need it.

Best,
Ben

Berdir’s picture

Thanks for testing, appreciated!

Updates pushed to my github repository

1. Fixed
2. Was not able to reproduce with the latest changes from the D6 branch merged in, can you still reproduce it?
3. Fixed
4. Should be fixed too
5. Same
6. Also fixed.
7. Yeah, I'm not sure about these blocks, but I'm pretty sure they *exist* in D6 too, but maybe they are broken. Can you compare D6 and D7?
8. Also fixed.

BenK’s picture

Thanks, Berdir! I'm looking forward to testing the latest changes.

In regard to item #7 (duplicate blocks), I tested this in the D6 version of User Relationships and the bug occurs there as well. So it doesn't appear to be a porting related issue. I've created a new issue about it here: http://drupal.org/node/828712.

I'll check out your new github code shortly and try out the fixes.

--Ben

P.S. Have you had a chance to take a look at porting the UR-Defaults, UR-Mailer, and UR-Node Access sub-modules yet?

Berdir’s picture

Status: Needs work » Needs review

If I understand it correctly, then the User* blocks display the relationships to the user you're currently viewing (node author/user profile/....)

Yes, I have ported node access and mailer yesterday, defaults should have already been ported and I just fixed a few bugs.

BenK’s picture

Hey Berdir,

I tried upgrading to your latest github snapshot, but got the following error message when running update.php:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: http://sandbox10.example.org/update.php?id=17&op=do StatusText: OK ResponseText: Fatal error: Call to undefined function _drupal_initialize_schema() in /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationships_api/user_relationships_api.install

I also received the following log message:

"The update process was aborted prematurely while running update #6103 in user_relationships_api.module. All errors have been logged. You may need to check the watchdog database table manually."

Should I just try to install on a fresh D7? I'm not sure if you intended the code to be upgradable since you're doing heavy development right now.

--Ben

P.S. The sandbox site doesn't work now because I'm getting the following error message on most pages:

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'sandbox10.user_relationship_type_roles_receive' doesn't exist: SELECT rtid, rid FROM {user_relationship_type_roles_receive} ORDER BY rtid, rid; Array ( ) in user_relationships_types_load() (line 88 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/user_relationships/user_relationships_api/user_relationships_api.api.inc).

BenK’s picture

Hi Berdir,

I tried installing your latest github code on a fresh D7 site and everything now installs fine. I created some relationships and users (trying to duplicate my old sandbox), but when it came time to request a relationship from a user (by clicking "Become the user's friend" on the user's page), I received a White Screen of Death as the site tried to connect to this page:

http://sandbox10.example.org/relationship/4/request/1?destination=user/4

I examined my Apache logs and here is the error that is showing:

"PHP Fatal error: Cannot use object of type stdClass as array in /home/sandboxes/sandbox10/public_html/includes/theme.inc on line 787, referer: http://sandbox10.example.org/user/4#"

Any ideas as to what could be causing the problem? (I obviously can't test much else until we get the relationship request process to work again.)

Thanks,
Ben

BenK’s picture

Status: Needs review » Needs work
Berdir’s picture

I'm unable to reproduce that, these errors are very hard to track down. I've started #839986: Enforce array arguments (Say good bye to Fatal error: Cannot use object of type stdClass as array errors), please apply that patch and post the new error.

BenK’s picture

Hey Berdir,

As you suggested in #21, I applied your "Enforce array arguments" patch. Then I re-tried the error reported in #19 (clicking "Become the user's friend" results in WSOD). Here's the error message that now results:

Recoverable fatal error: Argument 2 passed to theme() must be an array, object given, called in /Users/git/drupal/sites/all/modules/contrib/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc on line 135 and defined in theme() (line 729 of /Users/git/drupal/includes/theme.inc).

Hope this helps debug the problem. I'm eager to hear your thoughts.

Also, I finally created an account at github, so it should make it much easier to test your latest code.

Cheers,
Ben

Berdir’s picture

Yeah, that's exactly what I was hoping for.

Fixed and pushed.

Berdir’s picture

Status: Needs work » Needs review
BenK’s picture

Hey Berdir,

I just pulled your latest code and can confirm that the error message in #22 is gone. I was able to successfully request and confirm a new friend!

Now that this is working, I'll do more extensive testing of the module and report back. Looking forward to it!

--Ben

P.S. This is a little off topic, but I'm still brand new to git/github and am wondering how you set up your repositories for efficiency. Do you have a separate repository for each module and theme? Or just a "drupal-contrib" repository for all contrib modules? Are these repositories nested with the Drupal 7 core repository? I thought I'd ask since I'm now trying to manage testing of Private Message, User Relationships, and Userpoints all from git.

BenK’s picture

Hey Sascha,

I've begun testing User Relationships in greater depth, but either I'm confused about the meaning of certain module permissions or there are some bugs that are causing unexpected results. Here's my assumptions about each of the permissions:

* "Can have relationships" -- Allows the user to have relationships at all
* "Administer User Relationships" -- Allows site admin to configure module settings.
* "Maintain own relationships" -- Allows users to approve/disapprove relationships for themselves in the UI
* "View user relationships" -- Allows users to view the relationships of other users.

(So I'm basically assuming that the "View user relationships" permission is supposed to function like Private Message's "Read all private messages" permission.)

But when I test the module, here's what I notice:

A. Even a user without the "Can have relationships" permission (or any other permission) still has "All Relationships" (user/[uid]/relationships), "Pending Relationships" (user/[uid]/relationships/requests), and "Friends Relationships" (user/[uid]/relationships/[relationid] pages. I would have expected that if these users didn't have permission to have relationships, they either wouldn't have these pages created at all or wouldn't be able to access them.

B. A user who doesn't have "Maintain own relationships" permission (but does have the "Can have relationships" permission) still sees links and buttons to remove a friend. Eventually, the person reaches an "access denied" page but not before clicking both a remove link and a remove confirmation button. I would have thought that the user would not see any such links or buttons if they didn't have permission to use them.

C. A user who has the "View user relationships" permission can't view the relationships pages of other users. It seems that to view the relationships of other users, you must have the "Administer user relationships" permission. But this seems wrong given that most social networks let users see the friends of their friends (without letting them administer the system). So at first glance, it doesn't appear that the "View user relationships" permission actually does anything.

D. Even if a user doesn't have the "Maintain own relationships" permission, he still sees the "Become [username]'s friend" link on other users' pages." Upon clicking this link, he gets an Access Denied error, but I would have thought these links would have been hidden.

E. Even if a user has "Administer User Relationships" or "View user relationships" permissions, he doesn't see a tab on other users pages to view their relationships. He has to know for himself that the page is there and manually enter the link. I expected to see some type of "Relationships" sub-tab when visiting other users' pages (similar to Private Message's "Messages" tab that is displayed on other users' pages for those with sufficient permission).

So I guess my basic question is whether these represent bugs in the D7 port or whether these are usability issues of the module in general. I haven't used the D6 version of the module, so if you can let me know how things are supposed to work, I can be a better tester.

Hope this all makes sense! ;-)

Cheers,
Ben

P.S. On the permissions page, the description for each permission is still listed as "TODO". So filling those in might help others test as well.

Berdir’s picture

I have a checkout of Drupal 7 (where I've ignored everything in sites and a few other files. Important!) and then each module is a separate git repository. I could also use git submodules (google it) but I'm not used to them yet. Also, I don't yet use github for privatemsg.

I haven't really used UR either, so I'm not sure either, just guessing...

A: I can confirm that the user/ pages are visible, but not the /relationships pages. I think this is a bug in the module itself. For example, it has code for the access types user,edit,view but hook_menu() also uses 'admin'. Also, 'user' does not actually use a permission at all, just checks if it's the current user. I think this is wrong, since there is no point in being able to view your own relationship pages when you can't have any. I fixed it in the port. Thare is more strange stuff in the user_relationships_ui_check_access function, so you might want to start a generic issue about that function. For example, it uses the bitwise operator | to check two permissions instead of the boolean ||. Also, I changed the first check from 'administer users' to 'administer user relationships'. That makes more sense to me.

For reference, this is how the function looks in D7 now (and it should be the same imho in D6:

function user_relationships_ui_check_access($types, $account = NULL) {
  global $user;

  if (user_access('administer user relationships')) {
    return TRUE;
  }

  if (!is_array($types)) {
    $types = array($types);
  }
 
  foreach ($types as $type) {
    switch ($type) {
      case 'view':
        if (user_access('maintain own relationships') || user_access('view user relationships')) {
          return TRUE;
        }
        break;
      case 'edit':
        if (user_access('maintain own relationships') && user_access('can have relationships')) {
          return TRUE;
        }
        break;
      case 'user':
        if (user_access('can have relationships') && $account->uid == $user->uid) {
          return TRUE;
        }
    }
  }
  return FALSE;

Blocks (those that belong to the current user like pending requests or current points) are visible too. I fixed that by adding

  if (!user_access('can have relationships')) {
    return FALSE;
  }

to user_relationship_blocks_block_view(). Again, I suggest you create a issue for D6 about this.

B: I guess you are talking about that link on the user profile page? That is a bit strange. I agree that without the maintain permission, users should not be allowed to remove them either. Now, UR defines two different links to remove a relationship. One starts with /relationship and works currectly (uses the edit permission check which includes view and maintain). And the other starts with user/%user/relationship... and checks access user, so in the old version, only if this is the current user. I changed it to what I would expect now but imho, there is no point in keeping the user/%user/... url if they both are the same now.

C: I don't think the module supports that right now. view user relationships is just for the own user. I updated the description to make that more clear. I consider that not a bug but a feature, so it is something to look at *after* the initial port is done.

D: Another missing permission check, fixed. Exists in D6 too...

E: Now, this is a bug I introduced. More or less. Since the account link is now part of the user-menu, which doesn't have hierarchy support by default (displayed as secondary menu), the link below it was not visible. I changed it to be a local task, similiar to privatemsg's read all messages.

So, here's my suggestion regarding D6:

- Open (maybe check if there isn't already one) a bug issue to fix A (including the blocks), B, D. Maybe there is a reason for B to exist....
- Open a feature request for a "view all user relationships" permission. Or maybe a relationship setting, so you can have public and private relationships.

I added some descriptions and pushed the fixes. You might want to include a link to http://github.com/Berdir/user_relationships/commit/cd88092dbbcc22f5f6639..., as that is the commit that fixed the permission issues.

BenK’s picture

Title: please port to Drupal 7 » Drupal 7 port
Status: Needs review » Needs work

Hey Berdir,

Thanks for all of the permission fixes. Everything is working a lot better and just as you described. Your explanations also helped me to better understand the intended permissions.

At this point, I just think we should fix D6 bugs that we notice when doing the D7 port and then I'll later post equivalent D6 issues and reference our D7 fixes. Here are a few other minor permission issues I noticed in the latest github version:

A. In the User Menu, the "My Relationships" link is visible even if the user does not have the "Can have relationships" permission. Since no relationships are displayed on the resulting page, the link should probably be removed for users without this permission.

B. A user who doesn't have "Maintain own relationships" permission (but does have the "Can have relationships" permission) still sees Approve/Disapprove links at the Pending Relationships page (user/[uid]/relationships/requests). The Approve/Disapprove links should not be shown for a user without "Maintain own relationships" permission.

C. A user without "View User Relationships" permission can still see all of their relationships at user/[uid]/relationships.

D. Even if a user doesn't have the "Maintain own relationships" permission, for relationships (friends) he already has, he still sees the "friend (Remove)" link on other friends' user pages." The link won't work (Access Denied), but it should probably be hidden.

Okay, now that we've pretty thoroughly discussed permissions, I'll move on with my testing to other aspects of the core module and sub-modules...

Cheers,
Ben

P.S. I like your idea of relationship settings, so you can have more fine grained permissions per relationship type. I think that's something we can discuss later after the port is more stable.

ajayg’s picture

Not to change the topic, but just want to point out R6 is not yet released. So wouldn't it serve people working on R7, to atleast get R6 released so you are working off a stable baseline. Otherwise you are working (and re working) against a moving baseline. I know you can't completely wait till everything in R6 is done. That is why modules have different branches. But this is not the case here. We don't have the official R7 branch so we are working against something may change faster.

IMHO the ideal thing would be Alex annouce R6 branch would have no new features, only bug fixes and a list of bugfixes required for release of 6.x . And he also creates an official R7 branche where this development can continue.

Getting R6 out also help growing the ecosystem of module. Unless the module is fully released many clients won't even consider it.

alex.k’s picture

I think it's understood that UR6 has got all the features it's going to get in the near future. Power users are already using not-yet-committed Rules integration which would round up the package. Bugs need to be fixed in both versions and I'm not sure that they overlap very much. So, I don't think that the cool work Berdir and reviewers are doing on UR7 is at much risk. If anything this is a chance to correct some of the awkward spots in UR's design.

BenK’s picture

@alex.k: Thanks for mentioning the D6 Rules integration that already exists. It looks like something that should be added to the D7 version of UR. I'll chat with Berdir about it. We've already been working on porting rules integration for the D7 versions of Private Message and Userpoints, so doing that port should be very similar.

--Ben

ajayg’s picture

Thanks Alex for clarifying. Would it be a hassle for you to create D7 branch for UR ? So atleast that way people who are working on R7 will have a seperate space to work and those trying to get fix for D6 can do so as well. I am not cVS expert so not sure if a seperate branch is burden for you.

I am just suggesting a development approach that most of popular modules follow here on d.o

ajayg’s picture

@Benk related to comment #14

answering your point #7

"My relationship" blocks are meant for logged in user. So if you logged in as X, it will show everything that is your (X'
s) relationship, despite which page you are viewing.
User relationship block is meant (I think) to be displayed on a users profile. So if you are logged in X and viewing profile of Y, it will show all relationships of Y. This is will make sense only as long as you are on pages related to Y (Where Y is being passed as argument )

YK85’s picture

Thanks Berdir for the great work in porting UR to D7 and Benk for the fantastic testing!

@Berdir - I was wondering if you will be receiving commit access to start the 7.x branch on the drupal.org project page for UR? I see you are very active now in the User Points issue queue since #637584: Drupal 7 port and would be SO HAPPY to see you help further develop UR, which is an awesome module. I really like your Privatemsg module and excited to see User Points further develop as well.

Thanks!

BenK’s picture

Hey everyone,

Berdir and I had a nice chat in IRC. He requested that I summarize the current state of the User Relationships D7 port (what currently works and what doesn't yet work). So that's what I'm doing below.

Because there are numerous sub-module that are part of User Relationships, in this initial comment I'm only going to talk about features that are included in the main UR-API and UR-UI modules. Then in follow-up comments I'll talk about each sub-module individually.

So I apologize for the length of this message. But I promise that it is fairly comprehensive! ;-)

-----------
WHAT CURRENTLY WORKS (PLEASE THANK BERDIR)

* Creation of Relationship Types: I've been able to create the full range of relationship types without any errors. This includes mutual, reciprocal, and one-way relationships as well as relationships that require approval and ones that don't. It's all working quite well.

* Editing of Relationship Types: I've been able to edit the full range of relationship types on the fly. This includes changing the approval status and type of relationship. Everything seems to respond great to these changes.

* Many Settings are Working: I've confirmed that the functionality of the following settings seem to be working as intended: "Allow multiple relationships," "Show direct request links," "Show user pictures in relationship pages," "Path to relationship requests," "Informational Messages" (fieldset), and "Error Messages" fieldset. Nice!

* Links on User Pages: It's easy to requests relationships with users because the links on each user's page (/user/[uid]) are functioning well.

* Managing relationship status: It's been easy to remove relationships, cancel pending relationship requests, and more. All of that is working without a hitch.

-----------
WHAT DOESN'T YET WORK

In comment #30 above, UR module maintainer alex.k says that:

Bugs need to be fixed in both versions and I'm not sure that they overlap very much. So, I don't think that the cool work Berdir and reviewers are doing on UR7 is at much risk. If anything this is a chance to correct some of the awkward spots in UR's design.

So I'm taking the approach in the to-do items below that even if the bug also exists in the D6 version, we're going to go ahead and fix it in this D7 port. It's just too hard to get everything working well together without doing this. (I'm happy to post back bug fixes to D6 as needed.)

A. Any time a relationship is listed at /relationships (or at any sub-pages like relationships/1), I'm getting the following notice at the top of the page:

Notice: Undefined property: stdClass::$extra_for_display in include() (line 15 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/templates/user_relationships.tpl.php).

B. If the "Show direct request links" setting is unchecked and you visit the relationship request page (relationship/5/request?destination=user/5), then the following error is shown at the top of the page (if there is an available relationship that requires approval):

Notice: Undefined variable: default_relationship in user_relationships_ui_request_form() (line 30 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).

C. If the "Show direct request links" setting is enabled, there are pending relationship requests with a user, and you visit that user's page (user/[uid]), then the following error message is being displayed at the top of the page:

Notice: Undefined index: 3 in user_relationships_api_can_receive() (line 497 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.api.inc).

Note that the error goes away if you uncheck the "Allow multiple relationships" setting.

D. If you try to request a relationship with a user by filling out the request relationship form (and you already have requested every type of relationship with that user), then the following message is displayed: "This user does not accept relationship requests" (the 'Not Accepting Requests' message). This is not correct because the user does, in fact, accept relationships... it's just that every possible relationship has either been requested or accepted. So we should be displaying the existing 'Too many relationships' message: "You may not create any more relationships to this user." Additionally, I currently get the following error messages at the top of the page:

Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 167 of /Users/benkaplan/git/drupal/includes/entity.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->cacheGet() (line 343 of /Users/benkaplan/git/drupal/includes/entity.inc).
Notice: Undefined index: rtid in user_relationships_ui_request_validate() (line 16 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.actions.inc).
Notice: Trying to get property of non-object in user_access() (line 754 of /Users/benkaplan/git/drupal/modules/user/user.module).

E. The description for the 'Show direct request links' setting current reads: "Show a 'create relationship with...' link for each available relationship type." This isn't actually the link that is displayed if this setting is enabled. (The link will actually read: "Become user1's friend".) As a result, it would be more clear to change the description to read: "On a user's page, show a link for each available relationship type instead of the generic 'Create a relationship with...' link."

F. If the "Allow users to auto approve" setting is enabled, then the following error message is appear on the top of the user edit page (user/[uid]/edit):

Notice: Undefined property: stdClass::$user_relationships_ui_auto_approve in user_relationships_ui_form_alter() (line 702 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.module).

G. If the "Allow users to auto approve" setting is enabled and you try to select certain relationships types for auto approval on the user edit page (user/[uid]/edit), the settings are not being saved. Instead, after trying to save all of the settings appear unchecked. As a result, I can't test the auto approve functionality.

H. The "Relationships per page" setting doesn't appear to be doing anything (at least if I understand the intent of the setting). I set this to "2" and then visited /relationships. If still see way more than two relationships shown on this page. If the intent is something different, it isn't clear. But I don't see any other place where relationships are limited per page or some type of paging system is used.

I. If you append some random letter after the /relationships/requests URL (such as visit /relationships/requests/totallyrandom), then you are taken to a page with a "Sent Requests" section, a "Received Requests" section, and missing usernames of the other users sending/receiving requests. Additionally, the following error messages appear at the top of the page:

Notice: Trying to get property of non-object in template_preprocess_user_relationships_pending_requests() (line 880 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.module).
Notice: Trying to get property of non-object in template_preprocess_user_relationships_pending_requests() (line 895 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.module).
Notice: Trying to get property of non-object in template_preprocess_user_relationships_pending_requests() (line 895 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.module).
Notice: Trying to get property of non-object in include() (line 26 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/templates/user_relationships_pending_requests.tpl.php).
Notice: Trying to get property of non-object in include() (line 34 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_ui/templates/user_relationships_pending_requests.tpl.php).

Instead, going to a made up URL should probably return a Page Not Found error.

J. The "AJAX POPUP POSITIONING" fieldset settings don't appear to be working. With "Show AJAX confirmation popups" enabled, I'm not getting any confirmation screens appearing in a popup. Should we be showing confirmation screens in the D7 overlay if this is enabled? That might be a nice feature (when deleting a user relationship or something like that). No confirmation screens are currently being shown in a popup.

K. When the "Show user pictures in relationship pages" setting is enabled, user pictures are not being shown on the "Pending" page (/relationships/requests). They are being shown on the other tabs, however.

L. When the "Show user pictures in relationship pages" setting is enabled, user pictures are not being shown using the "thumbnail" style. However, the thumbnail style may be quite large by default (100x100) making it difficult to get a lot of relationships on one page. Yet, the site admin may not want to resize the thumbnail settings because it effects other areas of the site. It would be better if the site admin could specify (on the settings page) the image style to be used on relationship pages... alternately the user relationships module could provide its own image style. This may be more of a feature request.

M. When users are displayed in tables at /relationships (or any of the tabs for specific relationship types), there appears to be an extra column in the table where users are displayed. Basically, the fourth column (out of 5) is appearing blank in the default Bartik theme.

N. Only user #1 is currently able to see the relationships of other users. Instead, users with the "Administer User Relationships" permission should also be able to see the relationships of other users. Currently, users with this permission are getting an "Access Denied" message. The permission description even says those with this permission should be able to do this.

O. A user who doesn't have "Maintain own relationships" permission (but does have the "Can have relationships" permission) still sees Approve/Disapprove links at the Pending Relationships page (user/[uid]/relationships/requests). The Approve/Disapprove links should not be shown for a user without "Maintain own relationships" permission.

P. A user without "View User Relationships" permission can still see all of their relationships at user/[uid]/relationships.

Q. I think the "View user relationships" permission should be renamed "View own user relationships" since that is what it does and that is what the description says it does.

R. If user2 doesn't have the "View user relationships" permission, he should not be able to see his relationships with user3 when visiting user3's page (/user/3). However, user2's relationships with user3 are still being displayed to user2.

S. Even if a user doesn't have the "Maintain own relationships" permission, for relationships (friends) he already has, he still sees the "friend (Remove)" link on other friends' user pages." The "friend" part should stay (assuming he has the "View user relationships" permission), but the "Remove" part should be hidden. The remove link currently won't work (Access Denied), but it should probably be hidden.

T. This is a feature request for later, but I'm making a note of it here. The only way currently to see another user's relationships is having the "Administer User Relationships" permission (which only a site admin can have for obvious reasons). However, a primary function of sites like Facebook is to see other users' friends (so that you can friend request them too). There really should be, at least, a "View all user relationships" permission that lets you view the relationships of other users. I just think that's a basic feature for any social network type site. Later, there could be more fine grained permissions like "View all relationships of 'friend' type". This isn't necessarily an action item for the port (since we want to get this port done), but it's one of the first things we should revisit afterward.

That's mostly it for the UR-API and UR-UI modules. One exception: If you also have the Private Message module installed (Berdir is the maintainer of Private Message!), there is some added functionality on the User Relationships settings page. I'll review the state of that PM-UR integration in a subsequent comment.

I'll review the various UR sub-modules in subsequent comments as well....

Cheers,
Ben

Berdir’s picture

A. Don't have that error but added a check so that it shouldn't happen anymore.
B. Fixed
C. I don't see that, seems to be related to roles limitations, should be fixed.

Pushed the changes, will continue to work on this when I find the time...

Berdir’s picture

BTW: To help with backporting bugfixes if necessary at a later point, I've started to do separate commits for every point, see http://github.com/Berdir/user_relationships/commits/DRUPAL-7--1

BenK’s picture

PRIVATE MESSAGE INTEGRATION

The following items deal with User Relationship's Private Message integration. The referenced options only become available when the Private Message module is also installed. The main integration options appear at admin/config/people/relationships/settings in the "PRIVATE MESSAGES" fieldset.

_______
WHAT CURRENTLY WORKS

* If the setting "Allow sending messages to all users" is set, then everything works as expected... Private Message continues to function normally (including sending to roles) with no intervention from User Relationships.

* Role exclusions seem to work great. If a role is excluded, everything behaves for that role just as if "Allow sending messages to all users" was selected.

_______
WHAT IS NOT YET WORKING

U. If you select an option that prevents sending to those without confirmed relationships, but a user tries to send to such a person anyway (ignoring the fact that the autocomplete doesn't show their name), then the message is sent anyway and the following message appears at the top of the page.

Notice: Undefined index: recipient in _privatemsg_validate_message() (line 1620 of /Users/benkaplan/git/drupal/sites/all/modules/privatemsg/privatemsg.module).
Anonymous (not verified) is not a friend of yours.
A message has been sent to person2.

Note that there are a few different problems with the above message. First, there is the error message. Second, there is the problem that the actual user name in the second line is being referred to as "Anonymous (not verified)" even though it was an actual authenticated user. Third, the message says the person is not a "friend" (a specific relationship type) even though we have multiple relationships defined. (It should probably say that: "[username] does not have an established relationship with you.") Finally, the confirmation message of the successfully sent message is bring displayed.

V. I'm getting the following error message on the top of the site Permissions page and on the top of the Relationships list page (admin/config/people/relationships):

Notice: Undefined property: stdClass::$user_relationships_allow_private_message in user_relationships_api_privatemsg_block_message() (line 42 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.privatemsg.inc).

I'm also getting the above error message when logging in.

W. If you select "Allow sending messages to all users, users have option to accept messages only from their confirmed relationships," then an additional fieldset is placed on the user edit page (user/[uid]/edit). But currently, changes to options in that fieldset cannot be saved. I'm not sure if the problem results from this fieldset directly or from the issue described in #35G above. As a result, I can't fully test this functionality.

X. Settings on the user edit page are being displayed in its own fieldset, separate from Private Message module settings. From a usability perspective it would be much better to display anything related to Private Messages in the same fieldset. Actually, I like the fieldset name supplied by this integration ("Private messages") better than the name of the fieldset supplied by the Private Message module itself ("Privatemsg settings"). So maybe this is an issue I can also file with the Private Message module. Regardless, the various PM-related options should appear in the same fieldset.

Y. On the user edit page, change "Allow private messages" to "Allow private messages from..." This will be more clear. Then, for the "My relationships only" option, change this to "Only those who have established relationships with me." Finally, change the description that currently reads: "Allow/disallow users in or outside relationships to send you private messages." Change this to: "Choose who can send you private messages." Since these are user-facing options, it's important that they are clear.

Z. Change the second option on the relationships settings page. It currently reads: "Allow sending messages to all users, users have option to accept messages only from their confirmed relationships" Change this to: "Allow sending messages to all users, but provide users with an option to only receive messages from their confirmed relationships."

AA. The third option on the relationships settings page is unclear. The option currently reads: "Allow sending messages only to confirmed relationships (You probably should enable "Suggest only relationships" below)." What isn't clear is that this option also prevents users from receiving messages from those without relationships (by removing the receiving options on the user edit page). So probably this option should be changed to: "Allow sending messages only to confirmed relationships. Do not provide users with an option for receiving messages (which must be from confirmed relationships)."

BB. The "Suggest only relationships in To: field" functionality seems to be enabled regardless of whether the checkbox is checked or not. No matter what option is specified here, only those with relationships are ever shown in the "To" field (unless the "Allow sending messages to all users" is chosen). All users should be shown in autocomplete fields if this option is unchecked.

CC. The wording of the "Suggest only relationships in To: field" option and its description is a bit unclear. I'd change this option to read: 'When sending a private message, only display confirmed relationships in the "To" autocomplete field.' If this is the option language, then a description might not even be necessary.

DD. When sending a message to a role, I'm getting the following warning message at the top of the page in addition to the "message sent" confirmation:

Notice: Undefined property: stdClass::$uid in user_relationships_api_privatemsg_block_message() (line 43 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.privatemsg.inc).

EE. Additionally, when sending a message to a role, the message is only sent to role members who the person has a relationship with (even though the confirmation message says it was sent to the entire role). It would probably be better if sending by role was totally exempt from relationship limitations (since the purpose is generally to send the message to all users in a role). Sending to all users in the role should be probably be the default behavior. Alternately, we could provide an additional option to limit role sending only to those with relationships, but I'm not sure this would be a common use case.

Well, that's it for Private Message integration. Onward to the sub-modules....

--Ben

BenK’s picture

@Berdir (in #36 and #37): Thanks for starting work on the fixes and posting your finished items as you go. As you fix things, I'll pull down your changes and confirm that each problem has been solved. Since this is such a large module, I think this will be a good workflow.

--Ben

Berdir’s picture

D. This was maybe a follow-up error of B, can you please recheck. When I visit the page, I get this message: "You may not create any more relationships to this user." before I can fill out any forms.

E. Changed.

F. Fixed.

G. Fixed (also for the Privatemsg setting). The problem is that the module relies on the $user->data feature that automatically stores information attached to the $user object when saving. In Drupal 7, this works a bit less automatically because you need to explicitly tell what information to save and it's not automatically added to $user but remains in $user->data. Also, the use of this is discouraged, instead, a separate table should be used to store that information so that it is only loaded when needed and doesn't need to be unserialized for every request.

H. Upsie, Fixed.

Pushed the changes...

Berdir’s picture

I. Actually thought quite a bit of this and commited three different version. But I think we should simply drop any kind of "other-user-handling" from that URL. To view the pending requests of another users, user/%user/relationships/requests should be used instead.

J. I don't think the overlay should be used for that. The overlay is for administration purposes only in my opinion. I fixed the broken implementation but this could probably be simplified by using the new ajax framework that's in core but I don't know much about that...

K. That is simply not implemented, let's do that in a separate issue.

L. Well, it simply uses theme('user_picture') and that uses whatever has been configured as the default image_style and there is no way to override it. Would certainly be a nice feature, but something for a follow, probably together with K.

M. That is a generic field that other modules can use to display information. The only module that seems to use it is elaborations, and it displays the elaboration text there. Doesn't make that much sense to display it there and there will be issues with long elaborations but there is no other place where it could be displayed.

N. Typo on the permission name :) Fixed.

R. Should be fixed. As I said earlier, for Drupal 6 the module more or less assumes that users have both view and maintain permissions and doesn't work well for configurations. But I think have a pretty clean situation now in D6.

O. Untested, but should work now.

P. Same here.

Q. Fine with me, changed.

S. Should be better now.

P. I agree, but there are probably a few issues. For example, should elaborations be visible for other users as well or are they supposed to be something private between the related users? I suggest you start a new issue after this is done where we can discuss what permissions should exist and what exactly each of them is allowed to do/see.

=====

Regarding privatemsg integration, the whole thing is probably a bit broken currently because UR uses the Privatemsg 6.x-1.x API and there were some changes because of the roles thing. Since Drupal 7 (only) has the 6.x-2.x API, we can easily fix it, but will need a more intelligent way in Drupal 6 if we want to keep compatibility with both versions.

U. Should be fixed now.

V. That should be gone, was fixed together with G.

W. Same

X. I agree. For example, the email setting that can be enabled if the mail notification module is a separate fieldset as well. But let's postpone this on #902402: Combine user-facing settings into one fieldset (and change fieldset name). I will fix this when creating a patch for that issue to test that it works.

Y. Agreed and changed. I was wondering if the first character should be upper case or not in the two options?

Z. Changed.

AA. Not sure if that is really cleaner. What about something like this "Allow sending messages between confirmed relationships only"? (basically, add something like "between" to make it more obvious that it works both ways (which is obvious to me because receiving == sending for another user). I am not sure about these options anyway, for example, what happens with one way relationships? Especially when they don't need to approved? If you're a fan of someone else for example... Are you allowed to write him? Or is he allowed to write you? I think these options should be moved to the relationship create/edit form, allow to specific the direction etc. But that does have issues as well...

BB. Should be fixed now..

CC. Changed

DD. Fixed in the same commit as U. only user recipients should be checked now.

EE. This is imho a feature, not a bug :) I think it was actually you who suggested to not send messages to users who have blocked you through roles recipients, this is exactly the same mechanism. The only option that is actually technically possible is the role exclusion feature that UR comes with. When adding hidden recipients based on a role, there is simply no way to provide it with enough context to do that with a per-message settting. Well, maybe there would be, but that requires quit ugly hacks IMHO.

BenK’s picture

Hey Berdir,

I've confirmed that A, B, C, D, E, F, and G are all fixed now. Awesome!

As for H, the problem has been fixed on every page except the "Pending" tab. In the current implementation, the number of relationships displayed is calculated based on the "Sent Requests" section. The number of relationships in the "Received Requests" section is completely ignored.

Additionally, although the number of relationships displayed under "Sent Requests" will be limited by the "Relationships per page" setting, paging is not actually being displayed on this tab. Basically, any number of Sent Requests beyond the max limit are just being hidden, with no apparent paging links to get to them. I've confirmed that the additional pages do exist, just the links aren't appearing. All of this is complicated, of course, because this is the only page with two different sections. One possibility would be to break up Sent and Received into sub-tabs.

One additional problem I noticed: Paging is accomplished by appending "?page=5" to the end of the URL. If you type into the URL made up page numbers greater than the last page, then the last page is shown anyway. This isn't a huge deal, but I wondered if we should be returning a page not found for made up pages.

Finally, one usability question came up on all pages that use paging: What should be the default sort order? Currently, it seems sorted by relationship type (all of the "friend" types, for instance, are placed together). But once I had a bunch of relationships and paging enabled, it became hard to find a particular relationship I was looking for. (I sort of expected it to be sorted alphabetical across all types.) One option would be to add clickable sort headers per column. But if that is too difficult for now, we may want to change the default sort order. What do you think?

--Ben

BenK’s picture

Hey Berdir,

Now that the auto approve settings are being successfully saved I decided to fully test out the auto approve functionality. I can report back that everything is working great. I've tested auto approval with various relationship types and it's working without a hitch.

The only changes I have are related to the wording of the user-facing settings (which I think are a little unclear).

FF. So here are my changes:

i. Change this: "Automatically approve the following relationship requests" To this: "Automatically approve the following relationship types when I receive a request..."

ii. Change this: "Check off the types of relationships you'd like to automatically approve." To this: "The above relationship types usually require your approval each time you receive a relationship request. If you'd like certain relationship types to be approved automatically (without asking for your approval), please check the box next to that type. Any relationship requests that are currently pending will still require your manual approval."

--Ben

Berdir’s picture

H. Ok, should be fixed now...

Regarding ?page=5, and replacing 5 with other values, that is simply the default behavior of Drupal and how the pager works. I'm not going to change that :)

I don't think they are sorted explicitly, they're simply returned in the default order, that's usually in the order they were added. I agree that we should add a proper header to that table and make it sortable...

BenK’s picture

Hey Berdir,

I've just completed some additional testing of your latest fixes:

I. I agree that we should simply drop any kind of "other-user-handling" from that URL. As for returning the "Page Not Found" error, this is working fine except that the Relationship sub-tabs (All, Pending, etc.) are actually still being included on the "page not found" page. If there's no easy way to get rid of these tabs, then that's fine. However, if it's easy, then I'd probably remove those tabs.

J. I updated to your latest fixes and the AJAX popup is now working. Just one issue I noticed: On the "Pending" page (/relationships/requests), when clicking the "Cancel" link, the D7 overlay is firing in addition to the popup. Only the popup should be firing. One other thing: Using the default Bartik theme, the body text in the popup is white text on a very light gray background... don't know if it's possible to make the text black (or if the theme is controlling this), but currently it's very, very hard to read. I agree that the current implementation could probably be improved a lot at some point using D7 core features.

K and L. I agree that we should postpone these for now.

M. Okay, I understand now why it's there. Let's postpone. We'll discuss elaborations when I test that sub-module.

That's it for now...

--Ben

BenK’s picture

Berdir,

I've tested more of your fixes below....

N, O, P, Q, R and S: All of these are fixed. Permissions system is now working great! Thanks for the terrific permissions cleanup. :-)

T. Okay, after the porting is done, I'll create a new issue to discuss further permissions. But for now, we don't need to worry about this.

U and V. Confirmed these are fixed!

W. Confirmed this is fixed. Also, I tested the main functionality and it is working great!

X. Sounds good.

Y. Now that you mention it, I think the first character in each option should be lower case. And perhaps we should get rid of the ending period (".") on each option.

Z. Looks great.

AA. I like your suggestions and would love to eventually move these settings to relationship create/edit form (so they can be specified on a per-relationship basis). But for the meantime, how about this slight change to the your suggested wording: "Only allow sending messages between confirmed relationships". And I think we should get rid of the "You should probably enable..." part. It's not needed.

While you're at it, I'd also probably change the sentence that reads: "You may choose which roles are allowed to send message to all users, even if it is otherwise restricted." I'd change this to: "You may exclude specific roles from the above restrictions. Any roles that are checked below may send messages to all users."

As for your thoughts about one-way relationships, I agree it's confusing. I tested this a bunch to figure out how it's currently working. Basically, if you have a one-way relationship with someone that didn't need to be approved (a fan), then you are allowed to send to them (they are a relationship of yours). But if they try to write you back, they can't unless they first become a fan of you, too. This is because a one-way relationship is only considered a confirmed relationship for the party initiating the relationship (even though the relationship will nevertheless appear on the relationships page of the receiving party). The way to solve this is probably to allow such options to be specified on a per-relationship basis on the relationship create/edit form. Also, we may want to rethink whether a one-way relationship should appear on the Relationship page of the receiving party. Probably, this should be a setting on the relationship edit/create page as well. Anyway, we can wait until after the port to tackle this.

BB. This is mostly fixed based on what I previously reported. I tested it and it's working great. The one thing I noticed, however, is that if you have "Allow sending messages to all users" selected and also "Suggest only relationships in To: field" enabled, then things don't work as you would expect. Basically, in this case you'd expect that you can send to anyone, but only your relationships would appear in the autocomplete. But in such a configuration, all users are currently shown in the autocomplete. I'm not sure how difficult this would be to fix given the current code. It's actually kind of a cool feature to have a way to limit the autocomplete but still have all users available for sending.

CC. The description now reads great. I'd change the actual checkbox label to be a bit more clear. It currently reads: "Suggest only relationships in To: field" I'd change this to: 'Only suggest confirmed relationships as message recipients'

DD. I confirmed this is fixed.

EE. Okay, I can see how that would be a feature. And having the role exclusions provides enough options. So I agree there's no need to worry about this.

--Ben

BenK’s picture

user_relationship_blocks

I've begun testing the user_relationship_blocks sub-module. Here is what I noticed in terms of bugs:

GG. The most glaring bug is that none of the blocks prefixed with "User Relationships" on the block admin page appear to be working (or even showing up). This includes the 'User Relationships: All Relationships' block, the 'User Relationships: Actions' block, and all of the UR-prefixed relationship type specific blocks (such as 'User Relationships: friends'). This is in contrast to the blocks prefixed with 'My Relationships,' which are displaying properly.

The 'User Relationships' prefixed blocks are intended to show the relationships of other users (when visiting a user's page or when visiting a node page authored by another user). So I'm unclear why this isn't working. Also, I'm unclear what permissions is meant to govern this block visibility. Is it possible we are missing an intended permission from the Permissions page? The only permission we've seen so far that grants a user the ability to see other users' relationships is the Administer User Relationships permission (which only applies to site admins). So is there anything else?

HH. For the 'My Relationships: All Relationships' block, I changed the number of relationships to display and clicked save. I got the following error at the top of the page:

Notice: Undefined variable: key in user_relationship_blocks_block_save() (line 329 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationship_blocks/user_relationship_blocks.module).

II. For the 'My Pending Relationships' block, I changed the block title and clicked save. I received the following error messages at the top of the page:

Notice: Undefined index: get_account in user_relationship_blocks_block_save() (line 324 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationship_blocks/user_relationship_blocks.module).
Notice: Undefined property: stdClass::$bid in drupal_write_record() (line 6215 of /Users/benkaplan/git/drupal/includes/common.inc).
Notice: Undefined variable: key in user_relationship_blocks_block_save() (line 329 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationship_blocks/user_relationship_blocks.module).
Notice: Undefined index: pending in user_relationship_blocks_block_save() (line 338 of /Users/benkaplan/git/drupal/sites/all/modules/user_relationships/user_relationship_blocks/user_relationship_blocks.module).

The changes to the block title, however, seemed to be saved properly.

JJ. For the 'My Pending Relationships' block, if you have a pending request under "Sent Requests" and also have the AJAX popup setting enabled, then when clicking "Cancel" on a pending request, the D7 core overlay is firing in addition to the AJAX popup. If the popup is being used, then the overlay shouldn't be used.

KK. None of the blocks appear to respect the 'View own user relationships' or 'Maintain own relationships' permissions. For instance, even if a user doesn't have 'View own user relationships' permission, all of their relationships are showing up to them in the 'My Relationships: All Relationships' block. Likewise, if a user doesn't have 'Maintain own relationships' permission, they are still seeing 'Approve' and 'Disapprove' links in the 'My Pending Relationships' block (even though if they try to click them, they get an "Access Denied" error). If a user doesn't have the necessary permissions, certain blocks (and links in blocks) should be hidden.

Thanks,
Ben

Berdir’s picture

J. Can't reproduce the overlay thing. And yes, it looks hard to read and ugly, but that's something for later imho...

AA. Changed the strings..

BB. Should work now.

CC. Changed.

Berdir’s picture

GG. The blocks currently have a hidden dependency on php.module, you need to enable that. The reason is that it executes custom PHP to load the $account. This is really bad and we probably want to get away from it but I'm not sure how.

HH. Fixed

II. Fixed.

JJ. Ah, I had overlay.module disabled that's why this didn't happen for me. I don't think this should open in the overlay in the first place, need to figure out how to do that. Unfortunatly, neither #87994: Quit clobbering people's work when they click the filter tips link nor #647228: Links are needlessly unable to fully participate in D7 AJAX framework features has landed in core just yet so I'm not sure what would best way to implement the popups. Thinking about just dropping it for now and let other modules take care of it, for example http://drupal.org/project/modalframe.

KK. Uhm. Well, there are simply no access checks except of the "can have relationships" and that is checked for the current user (not the one that might be displayed). Since all the User Relationships: * blocks make no sense if they're visible to admins only, this really needs a new permission. Actually, I think the whole thing needs to go further than that, when I think about privacy. For example, only being allowed to view friends of user X if you're his friend as well. But that is nothing that's specific to this module.

Anyway, I pushed some changes.

alex.k’s picture

NathanM’s picture

Subscribing.

sammyman’s picture

Subscribing.

BenK’s picture

Hey all,

As discussed with Berdir in IRC, we'd like to get this port stable enough to open up an official branch around the same time that Drupal 7 is officially released. So I've reviewed our prior notes on this thread and made a list of items trelated to the core User Relationships module that still need to be completed. I'll tackle the sub-modules in a subsequent post.

So here's the UR core to-do list:

1. On user account edit page (user/[ui]/edit), change field group name from "Relationship" to "Relationships". A corresponding change may need to be made to the "Automatically approve" setting and the "Receive relationship email notifications" setting so that they continue to appear in this "Relationships" field group.

2. If you are looking at another user's relationships at user/[uid]/relationships, the page title says "My account". Instead, it should probably display the username of the user you are looking at (just like Private Message module does).

3. Clean up the following UI strings on the user edit page:

a. Change the label "Automatically approve the following relationship requests" so that it reads: "Automatically approve relationship requests from other users:"

b. Change the description to read: "When another user requests a relationship with you, we usually require your approval. If you'd like certain relationship types to be approved automatically, check the box next to that type."

c. Change the label "Receive relationship email notifications" to "Receive e-mail notification of relationship activity". Change the description to read: "If checked, we will e-mail you when there are changes to your relationship status with other users."

4. Move the core UR module to the main directory. See http://drupal.org/node/990422 and http://drupal.org/node/986616 for more info.

5. On the UR global settings page, use vertical tabs just like we did on the Userpoints global settings page.

6. Clean up the following UI strings on the global settings page:

a. Multiple relationships description. Change to: "If checked, a user may create multiple relationships (each relationship of a different type) with another user."

b. Change the label "Show direct request links" to this: "Show a separate link per relationship type". Change the description to read: "On a user's page, show a separate link for each available relationship type (instead of the generic "Create a relationship" link).

c. User pictures description. Change to: "Show a picture next to each user's name on "My relationships" pages.

d. Auto approve description. Change to: "Provide users with an option to automatically approve all requested relationships."

e. Change the "Email Options" fieldset to "E-mail Notification"

f. Change: "Allow users to turn off relationship messages". To this: "Allow users to disable e-mail notifications" Change the description to read: "If checked, users can control e-mail notifications by changing a setting on their account edit page. If left unchecked, e-mail notifications will be sent to all users based on the global settings shown below."

g. In the "E-mail Notifications" fieldset, change the following notification labels:

* Change "Send Request messages" to "Notify user when a relationship request is made"
* Change "Send Cancel messages: to "Notify user when a relationship request is cancelled"
* Change "Send Approve messages: to "Notify requesting user when a relationship request is approved"
* Change "Send Disapprove messages" to "Notify requesting user when a relationship request is declined". Also, change the fieldset label to be "Decline" (instead of "Disapprove").
* Change "Send Remove messages" to "Notify user when an existing relationship is deleted by another user". Also, change the fieldset label to be "Delete" (instead of "Remove").
* Change "Send Pre-approved messages" to "Notify user when a relationship request is pre-approved"

h. When the tokens are listed in each E-mail Notification sub-fieldset, change "Replacement strings are:" to this: "Available tokens include:". Add a line of white space above the tokens list so that it doesn't run into the text area quite so much.

i. Change the "Messages" fieldset to be labeled "Custom Screen Messages". Change the description to read: "Customize the confirmation messages displayed to users when specific relationship events occur."

j. Change the "Path to relationship requests" description to read: "Only change this setting if a user's pending relationship requests have a different location than the default path (relationships/requests).

k. In both the "Information messages" and "Error messages" fieldsets, add the following description: "The following tokens are available for use in your custom messages: !requester, !requestee, %relationship_name, %relationship_plural_name, !pending_relationship_requests

l. Change the label on the "Private Messages" fieldset. Change it to: "Private Message Integration" Change the description to read: "Configure integration with the Private Message module. These settings apply to all users except the default admin user."

m. Change the label "Restrict Private Messaging to only related users". To this: "Permitted message recipients:"

n. Change the label from "Role Exclusions" to "Role Exceptions". Change the description to this: "Any roles checked below are exempt from the above restrictions and may send private messages to all users." Add an extra line of white space below the description and above the checkboxes.

7. Improve consistency of Realname module integration. Currently, if you request a "friend" (two-way) relationship with another user, the confirmation dialog uses the user's Realname (which is good). But if you request a one-way relationship, then the user's username is used. Additionally, in the "Become person1's friend" direct request link, the username is used instead of the realname (regardless of whether it is a one-way or two-way relationship). Conversely, the confirmation message printed with drupal_set_message() always uses the Realname.

Because of the nature of this module, I think it's important that we fully support Realnames. A lot of people (myself included) will want to use Realnames instead of usernames when creating user relationships. We just need to make all of this consistent and predictable.

8. The following notice is being printed at the top of my UR settings page:

Notice: Undefined property: stdClass::$realname in user_relationship_mailer_replacements() (line 74 of /Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationship_mailer/user_relationship_mailer_defaults.inc).
Notice: Undefined property: stdClass::$realname in user_relationship_mailer_replacements() (line 75 of /Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationship_mailer/user_relationship_mailer_defaults.inc).

9. If you try to request a relationship with a user (by visiting relationship/[uid]/request) and you already have requested every type of relationship with that user, the page title reads "Home" instead of "Request relationship". Also, change the page text that currently reads: "You may not create any more relationships to this user." To this: "You are not permitted to create any more relationships with this user."

10. If you try to request a relationship with yourself (by visiting relationship/[your_uid]/request), the page title reads "Home" instead of "Request relationship". Also, change the page text that currently reads: "You may not create a relationship to yourself." To this: "You cannot create a relationship with yourself."

11. In the user-facing UI, change the word "Disapprove" to "Decline". Here are the instances I found:

a) On the relationship confirmation page, the title needs to change from "Disapprove relationship" to "Decline relationship". And the text needs to change from "Are you sure you want to disapprove the friend relationship request from [user]?" to this: "Are you sure you want to decline the friend relationship request from [user]?

b) On the Pending relationships page, the action link needs to change from "Disapprove" to "Decline". This change in language will mirror what we did on the Userpoints module.

12. If a user doesn't have any relationships yet, all of the relationship list pages currently read: "No relationships found". For the "All" relationships page, change this to: "You do not have any relationships with other users." For the pending requests page, change this to: "You do not have any relationship requests currently pending." For the relationship type specific pages (such as for "friends"), use: "You do not have any friends."

13. On the private message integration settings that appear on the user edit page (if the "Allow sending messages to all users, but provide users with an option to only receive messages from their confirmed relationships." option is checked), put an initial capital letter on both options ("Everyone" and "Only those..."). This will make it consistent with our other settings. Additionally, change the "Only those..." option to read: "Only those who have an established relationship with me"

--Ben

P.S. My sub-modules to-do list will follow shortly. :-)

Berdir’s picture

Starting this again, merged in the changes from the 6.x branch.

1. Renamed.

2. I need to figure out how we did this in Privatemsg. From looking at the code, I can't see why it displays My Account here...

4. This would work best if is delayed after d.o has switched to git, because git allows moving directories around, CVS does not (everything needs to be removed and re-added as new files, all history is lost). Also, see the question I posted in the linked issue, what exactly *is* the main/core UR module? :) Usually that is the module which matches the project name, but no such thing exists for UR. So move around might not be enough, we actually have to rename it.

Everything else is TBD :)

alex.k’s picture

One bad issue with how user_relationships_api and *_ui are laid out in D6 is that the UI module contains all of the administrative functions. If I were doing this over again, I would keep all of that stuff in _api. Or even merge the two modules altogether. In the D6 version the _api module is pretty useless on its own because all of the request/approve logic is actually in _ui. This is one aspect of UR I would not carry over to D7.

Berdir’s picture

2. Found the privatemsg issue #933214: When viewing other users' messages, the current user's username is displayed as page title on message list page. But this was a different issue.

3. Changed.

4. Postponed.

5. Initial conversion done, no summaries yet. Collapsible fieldsets inside vertical_tabs don't work, I'm not sure if that's a bug or not. I think so, because they are completely gone. Converted these to inner vertical_tabs, there's obviously still room for improvements...

6. Changed. Did not add any space, especially since all the token stuff will probably be updated to use real tokens at one point.

7. This should work now, format_username() is now consistently used.

8. This should be fixed, there is no special realname integration anymore, it works through format_usename() now.

More will follow later...

BenK’s picture

Berdir,

Your updates are looking good. I'm in the process of testing the various UR sub-modules, but wanted to post this note first.

One sub-module that we need to add to the Drupal 7 port is user_relationships_rules. This rules integration sub-module was committed to the 6.x branch after we split off the 7.x branch on github. So it's not included in the D7 port yet.

So to continue my prior list, here is a new item #14:

14. Add user_relationships_rules sub-module and port it to D7.

--Ben

BenK’s picture

user_relationship_blocks

I just finished testing the user_relationship_blocks sub-module. It's working well. I just have two items:

15. I got the following notices and errors when visiting this link: http://localhost/user/2/relationships/requested/15/cancel?destination=us... . The weird thing, however, is that I'm not sure how I got to that link. I was canceling a pending friend request (I think), but it's proven difficult to recreate what I clicked to get to that link. But if I just cut and paste that link in my browser, I get the errors every time. So here are the error messages:

Notice: Undefined index: 15 in user_relationships_load() (line 407 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.api.inc).
Notice: Trying to get property of non-object inuser_relationships_ui_pending_requested() (line 170 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).
Notice: Trying to get property of non-object inuser_relationships_ui_pending_requested() (line 202 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).
Notice: Trying to get property of non-object inuser_relationships_ui_pending_requested() (line 202 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).
Notice: Trying to get property of non-object inuser_relationships_ui_pending_requested() (line 203 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).
Notice: Trying to get property of non-object inuser_relationships_ui_pending_requested() (line 203 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.forms.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! inDrupalDefaultEntityController->load() (line 178 of/Users/benkaplan/git/drupal-rc1/includes/entity.inc).
Warning: array_flip(): Can only flip STRING and INTEGER values! inDrupalDefaultEntityController->cacheGet() (line 354 of/Users/benkaplan/git/drupal-rc1/includes/entity.inc).
Recoverable fatal error: Argument 2 passed to realname_username_alter() must be an instance of stdClass, boolean given, called in /Users/benkaplan/git/drupal-rc1/includes/module.inc on line 1002 and defined in realname_username_alter() (line 47 of/Users/benkaplan/git/drupal-rc1/sites/all/modules/realname-DRUPAL-7--1/realname.module).

Any ideas? Or should we just hide such notices?

16. As previously documented (by Berdir), there are no access checks for blocks except for the "can have relationships" that is checked for the current user (not the one that might be displayed). I'm wondering what we can do to add some permissions (without getting too fancy either). How about adding two permissions:

* 'View all relationship blocks': Allows you to view all of the blocks prefixed with "User Relationships" for any user
* 'View blocks for established relationships': Allows you to only view the blocks of those who have an established relationship with you (a friend, etc.)

If a user has neither of these two permissions, he would only be able to view the blocks prefixed with "My Relationships" (assuming that he has the 'can have relationships' permission). What do you think?

The only other issue we have is the fact that all of the blocks require enabling php.module (which isn't too good). But in the interest of getting this port done, perhaps that should be postponed for a later issue.

--Ben

BenK’s picture

user_relationship_defaults

I just finished testing the user_relationship_defaults sub-module and it worked great. There is nothing to fix with this sub-module! :-)

--Ben

verynic’s picture

Thanks Benk,I'm waiting Ur to use Drupal 7.

Berdir’s picture

Status: Needs work » Needs review

9-11: Changed. I think you could do this renames yourself, just fork my UR project on github, do search & replace to find and replace these strings in all files (ide's like eclipse and netbeans make this easy over multiple files) and commit it. Make sure to make separate commits for every change and give them meaninful commit messages. Then I can simply merge them in. You can still leave the more complex stuff to me.

12. "You do not have any friends.". That is a very mean thing to say :-) Changed.

13. Changed.

14. The code has been merged with the D7 branch, but I'm thinking about postponing this after a first -dev release. Rules integration can take a long time, we learned that much when porting Privatemgs and UR.

15. Fixed. What you're accessing is the patch to cancel a relationship but you already canceled it. Trying to load such a canceled relationship caused these errors. Should point to 404 page now.

16. Postpone, *please* :) We really need to re-think the whole concept, these kind of permissions don't belong into the block module, they need to be global permission which all modules can use.

BenK’s picture

Thanks, Berdir. In the future, I'll try to commit some of the simple string changes myself to a separate fork. And yes, we can postpone the block permission stuff. And it sounds like we should also postpone moving the directory of the core UR module as well (since it sounds trickier than I thought).

As for the rules stuff, I'm wondering if we can try to get some of the basic porting stuff done now. Doesn't need to totally work before we create the -dev release, but enough so that we can enable the rules sub-module and basically view the events and actions in the Rules UI. That would be enough to make it easier to file individual issues against the -dev release. What do you think?

--Ben

BenK’s picture

Status: Needs review » Needs work

Two more issues I just realized regarding UR core:

17. Just like we did for Private Message and Userpoints, we should add clickable and sortable column headers for the relationship tables listed at /relationships (including the tables at sub-tabs like "/relationships/requests" and "/relationships/[rid]"). Once a user has more than just a few relationships and relationship types, it gets difficult to find the relationship you're looking for without some type of sorting. (Imagine if you had hundreds of relationships like the typical Facebook account).

18. Instead of having the individual sub-tabs for each relationship type (which takes up a lot of space if you have many types), I'm wondering if we should just add a "Filter by relationship type" drop-down filter at the top of the /relationships page. This would look exactly like the "Filter by points category" filter that we did for Userpoints module. I think it would look better and be much more efficient. Then, we would only need to have two tabs: "All" and "Pending". In fact, if there were only two tabs, we could probably change "All" to either "Current" or "Confirmed". What do you think?

--Ben

BenK’s picture

user_relationship_implications

I just finished testing the user_relationship_implications sub-module. Here are my to-do items:

19. If you click a relationship type to be implied (but then leave "strict" and "reverse" unchecked), then things function as expected. But once you return to the relationship type configuration page, "strict" and "reverse" will be checked (even though the system is functioning as if they were unchecked). Then, if you save the configuration page again (and aren't paying attention closely), then "strict" and "reverse" will be checked for real. It appears that the form is making "strict" and "reverse" default to checked even if you previously had them unchecked.

20. If I have a "parent" relationships that implies a "child" relationship and it is set to "strict", then deleting the "child" relationship should cause the "parent" relationship to also be deleted. This is successfully happening, but the following notice is being printed at the top of the page:

Notice: Undefined variable: category in user_relationship_implications_user_relationships() (line 194 of /Users/benkaplan/git/drupal-rc1/sites/all/modules/user_relationships/user_relationship_implications/user_relationship_implications.module).

21. If I have two one-way reciprocal relationships ("donor" and "supporter") and then set an implied relationship so that a "donor" implies a "supporter" (with "reverse" unchecked), then you would expect the following to be created:

* Donor (You to Them)
* Supporter (You to Them)

But there is one configuration where this fails: If there already exists a reverse "Supporter (Them to You)" relationship, then the implied "Supporter (You to Them)" relationship will not be created.

22. Change "This Relationship Implies" fieldset label to "Implied relationships".

23. Perhaps make the "Implied relationships" section a vertical tab?

24. Update the bullet point explanations to read as follows:

* Check a relationship type to create an implied relationship.
* This implied relationship will be created automatically when the primary relationship is created.
* By default, removing an implied relationship will not cause the primary relationship to be removed.
* But if "strict" is checked, then removing the implied relationship will also cause the primary relationship to be removed.
* By default, an implied one-way relationship is created in the forward direction ("You to Them").
* But if "reverse" is checked, then an implied one-way relationship is created in the reverse direction ("Them to You").

--Ben

BenK’s picture

user_relationship_migrate

25. The user_relationship_migrate module is meant to migrate users from Buddylist2 to UR. Since Buddylist2 was mainly a Drupal 5 module that is not widely used in Drupal 6 (because people already migrated), I recommend that we deprecate this module and remove it from the D7 version.

--Ben

BenK’s picture

user_relationship_elaborations

At the moment, the user_relationship_elaborations module has quite a few limitations that make it difficult to use. For instance, you can't allow elaborations only on certain relationship types, you can't edit elaborations after a relationship is confirmed, and the receiving party can modify the elaboration with no recourse from the person who submitted it. But in the interest of getting a -dev branch going, perhaps these types of feature requests should be postponed.

So at this point I just think we should do the following quicker fixes:

26. Put the elaboration module settings in a vertical tab on the global settings page.

27. When making a relationship request and the elaboration UI displays, change the field title from "Elaboration" to "Comments". Change the field description that currently reads: "Please elaborate on this relationship (how/where you met, etc)" Change to this: "Add more details about your relationship. Both parties will be able to view these comments."

28. Elaborations should probably be visible in the table at /relationships/requests (pending relationships page). It is important to show a receiving user the comment before he/she declines the relationship. Title this column "Comments".

29. On the disapprove (renamed "decline") confirmation screen, we also should display the elaboration/comment. Currently, the elaboration/comment is only displayed on the approval confirmation screen.

30. For the user requesting the relationship, the Elaboration/Comments field appears above the "Are you sure you want to..." message. But for the user receiving the relationship request, the Elaboration/Comments field appears below the "Are you sure you want to..." message. Instead, this should be consistent. In both cases, I like it better with the "Are you sure you want to..." message above the elaboration/comments field.

--Ben

BenK’s picture

user_relationship_node_access

31. The user_relationship_node_access sub-module doesn't appear to be working for me. I read the README.txt and I'm supposed to be seeing a variety of new permissions on the Permissions page, but I don't see these permissions. I only see the global settings, but no access control settings are available when creating or editing a node (of a content type that has been enabled on the global settings).

Once we get the permissions sorted out, I can do more testing.

--Ben

BenK’s picture

user_relationship_views

I've tested views integration and it is somewhat working. Here is my list of to-do items:

32. I'm getting a broken handler notice when adding the "User relationship types: Type" field to a view:

Broken/missing handler 
Broken handler user_relationship_types.is_oneway

33. When creating a user relationship view or a standard user view, I'm getting broken handler notices when adding the following fields:

USER_RELATIONSHIPS.APPROVED
USER_RELATIONSHIPS.RTYPE
USER_RELATIONSHIPS.REQUESTEE_ID
USER_RELATIONSHIPS.REQUESTER_ID
USER_RELATIONSHIPS.STATUS_LINK

--Ben

BenK’s picture

As for user_relationship_invites, user_relationship_service, and user_relationships_panels_visibility, my suggestion is that we postpone porting these integration sub-modules until after we open the official -dev branch. That would also allow some time for the D7 ports of the main modules themselves to stabilize.

Well, that should be about it... I've now tested or otherwise addressed all UR sub-modules. We're not too far away from having a working port and opening up an official branch! :-)

--Ben

giorgio79’s picture

Will the D7 port integrate with the Relation module?
Previously Awesome Relationships http://drupal.org/project/awesomerelationship

http://drupal.org/project/relation

As I understand, users will become entities, just like any other node.

Berdir’s picture

Status: Needs work » Needs review

17. Good idea, but I think we should postpone this. Reason is that the the data is loaded through api functions and not direct sql queries and these apis need to be extended to support table sorting.

18. Same here. This is something that needs more discussion imho, not sure if I like the idea and it's not something that is a bug that needs to be fixed, just something that could be done differently.

19. Interesting. I can't reproduce this, strict/reversed always stay unchecked for me (edit: by default, I mean)

20. Fixed. Please verify that the correct confirmation message shows up now.

21. Should be fixed but I haven't tested it. Please verify.

22. Changed.

23. Interesting idea, but what exactly should be a vertical tab? Just a single one doesn't make sense. What we could think about is converting all those different settings into a vertical tabs thingy. Or did you mean making a vertical tab for each relationship? Suggestions are welcome.

24. I actually find that bullet description thing very strange. What about fixing it properly while we're on it? Any reason to not simply make a single description paragraph without bullets? Or maybe two paragraphs...?

25. Agreed, removed.

26. Is it already?

27. Changed, I found two almost identical placed which used that title/description, changed both: Edit, ah, the second one was the approve form.

28. Agreed, but the problem is that this is more complicated than it sounds and requires quite some changes to the user_relationships_ui module. The table is built in a template file and other modules have absolutely no chance to alter this. Just like I did for userpoints.module, we should probably replace/extend these templates with renderable structures. Then we can add a simple alter hook over the while thing and other modules can do whatever they want with these tables. I suggest a single "Revamp relationship listings" follow-up issue where we can do this.

29. Ok, changed.

30. Agreed, below it is ;)

31. Permissions settings should now show up, totally untested though. There are a few quite strange things that could be fixed here I guess, including the weirdly positioned global settings form and the interface strings (Posting to social network... WTF? ;))

32/33. The views handler classes weren't defined in the .info file. Try again after clearing the caches (Maybe you need to re-install the module)

#69. Agreed.

#70. We had the idea too, but not any time soon. The thing is that we are not talking about integrating with that but about completely depending on it and probably remove a large part of user_relationships_api module. And that will be a huge effort, will take a long time and probably include quite some extensions to the relationship project too. It will also need to be discussed with the maintainers of that project. So, maybe in 7.x-2.x...

Setting back to review, since the remaining stuff needs more testing/discussing/verifying.

ajayg’s picture

72. Need to change readme.txt file to refer drupal 7 and up the version number/date. As this is taking shape it would be difficult to otherwise keep track of which package is which. Any other dependencies need to be updated too.

jlporter’s picture

long thread is long

please publish 7.x-dev branch plox

kthx

BenK’s picture

Status: Needs review » Needs work

I'm about half way through testing Berdir's latest fixes. A couple new errors/notices surfaced in my testing that I thought I'd post first:

34. On the "Add Relationship Type" page (admin/config/people/relationships/add), I'm getting the following notice:

Notice: Undefined property: stdClass::$rtid in user_relationships_ui_type_edit() (line 271 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationships_ui/user_relationships_ui.admin.inc).

35. If either "Allow sending messages to all users, but provide users with an option to only receive messages from their confirmed relationships." OR "Only allow sending messages between confirmed relationships." is selected in the Private Message Integration settings, then the following notice is appearing on the top of the account view page (user/[uid]):

Notice: Undefined property: stdClass::$type in user_relationships_api_privatemsg_block_message() (line 38 of/Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.privatemsg.inc).
Notice: Undefined property: stdClass::$type in user_relationships_api_privatemsg_block_message() (line 38 of/Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.privatemsg.inc).

--Ben

ccheu’s picture

subscribing

BenK’s picture

I'm now about 75% done with testing Berdir's fixes. A few more to-do items showed up along the way:

36. Any way to control the weight of the Relationships fieldset on the account view page? I created two fields on the user entity (first name and last name) so that I could test Realname integration. But the Relationships fieldset appears below first name and above last name (which looks weird) on the account view page. Berdir suggested in IRC that to fix this maybe we can expose it as an extra field similiar to what we did for privatemsg's extra fields.

37. On the account edit page, it kind of looks like "Receive e-mail notification of relationship activity" is a sub-option beneath "Automatically approve relationship requests from other users" (because the latter is in bold). Let's swap the order of the options in the fieldset (put "Receive e-mail notification of relationship activity" as the first option) so that this is more clear.

38. The "Show AJAX confirmation popups" option is checked by default. Since the AJAX popups don't work that great (and most people shouldn't use them), we should have this unchecked by default.

39. There are some strings that are using inconsistent language or missing links:

a. Currently: "Are you sure you wish to send a new friend request to Ed Editor?" Change "wish" to "want" (since we use "want" most other places)

b. Currently: "Are you sure you wish to delete the recommender relationship with Ed Editor?" Change "wish" to "want" (since we use "want" most other places)

c. Currently: "Are you sure you want to become Ed Editor's recommender?" The username should be a link, but it's not currently a link.

d. Currently: "Are you sure you want to approve the friend relationship request from Bob Smith? The username should be a link, but it's not currently a link.

e. Currently: "The friend relationship of Bob Smith to Ed Editor has been deleted." Change to: "The friend relationship between Bob Smith and Ed Editor has been deleted." (This is better grammar.)

40. If you do not have direct links turned on and go to the "Request relationship" page, then the order of the text is strange. Currently, it reads:

Request relationship

My relationship is:
* friend
* fan
How do you relate to Al Authenticated?

We should delete "My relationship is" and put "How do you relate to [username]?" above the radial options. To make make it more clear, we should also change "How do you relate..." to: "What type of relationship are you requesting with [username]?" Thus, it would read:

Request relationship

What type of relationship are you requesting with Al Authenticated?
* friend
* fan

--Ben

Berdir’s picture

14. As discussed, initial rules port done but totally untested.

36. Added as extra field, order can now be controlled in the manage fields/display page. This is actually a nice idea and something that all modules (meaning: UP/PM) should be doing. Can you open an issue for each of those?

37. Changed

38. Changed

39. Changed except d) which I can't reproduce. It's a link for me.

40. Changed.

design.er’s picture

It's amazing that you guys are working so hard on a D7 port. Thank you so much for that!
Can you tell us anything about the current status (~30%, ~90% ...) and when you plan to release a dev version so that we can test it and give you feedback?

Berdir’s picture

I think we are close to an official -dev version, the module is working well, we are just doing minor fixes and adjustments now.

If you want to help test, the most recent code is on https://github.com/Berdir/user_relationships. You can either download it directly (there is a big Downloads button on that page) or clone it with git then you can very easily get the updates.

BenK’s picture

I've now gone through and tested all of Berdir's fixes. Here's my report on what's left to be done:

2. The page title still says "My account" instead of the username of the user you are looking at. Any other ideas on how to solve?

6. It looks like you may have inadvertently changed the field label (instead of the field descriptions) on the following strings. I've noted how they should actually be below:

c. Label should read: "Show user pictures on relationship pages". Description should read: "Show a picture next to each user's name on 'My relationships' pages.

d. Label should read: "Allow users to auto approve". Description should read: "Provide users with an option to automatically approve all requested relationships."

n. Label should read: "Role Exceptions". Description should read: "Any roles checked below are exempt from the above restrictions and may send messages to all users."

7. One problem persists: In the "Become person1's friend" direct request link, the username is still being used instead of the realname (regardless of whether it is a one-way or two-way relationship). The realname should be used.

11. On the relationship decline confirmation screen, the text currently reads: "Are you sure you want to disapprove the friend relationship request from Bob Smith?" The word "disapprove" should be "decline".

15. If I try to visit "http://localhost/user/2/relationships/requested" (or other made up URL in that folder hierarchy), I get the following notice at the top of the page:

Notice: Trying to get property of non-object in include() (line 29 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationships_ui/templates/user_relationships.tpl.php).

Is it possible to instead return "Page Not Found"?

23. I just meant some vertical tabs to organize all of the settings on a particular relationship type. Here are my suggested vertical tabs:

General
Implied Relationships
Allowed Roles

The "General" tab would be for everything up until the Implied Relationships section on the current page. The "Allowed Roles" tab would be for the 'Request access' and 'Receive access' settings.

24. Update the bullet point explanations to read in paragraph form as follows:

Check a relationship type to create an implied relationship. This implied relationship will be created automatically when the primary relationship is created.

By default, removing an implied relationship will not cause the primary relationship to be removed. But if "strict" is checked, then removing the implied relationship will also cause the primary relationship to be removed. Note that an implied one-way relationship, by default, is created in the forward direction ("You to Them"). But if "reverse" is checked, then it is created in the reverse direction ("Them to You").

33. When creating a regular "user" view (not a "user relationships" view), I tried adding all available UR fields. I received the following notices at the top of the view (displayed as a page in table style):

Notice: Undefined index: in views_handler_field_user_relationships_status->render() (line 16 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationship_views/views_handler_field_user_relationships_status.inc).
Notice: Undefined index: in views_handler_field_user_relationships_name->render() (line 33 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationship_views/views_handler_field_user_relationships_name.inc).

Note that everything is fine (no notices) on a "user relationship" type view. The above notices just appear on a "user" type view.

37. In the "Automatically approve relationship requests from other users" area, is it possible to put the description text ("When another user requests...") above the checkboxes instead of below it? I think it's a bit more user friendly this way.

39. The following strings still aren't corrected for me:

c. Currently: "Are you sure you want to become Ed Editor's recommender?" The username should be a link, but it's not currently a link. Note that the "recommender" type shown above is a one-way, reciprocal relationship type. Maybe you added the link for a different relationship type?

e. Currently: "The friend relationship of Bob Smith to Ed Editor has been deleted." Change to: "The friend relationship between Bob Smith and Ed Editor has been deleted." Note that "of" has become "between" and "to" is now "and". (This is better grammar.) Note that in my testing the "friend" relationship is a type that requires approval... maybe you changed this for just one-way relationship types?

So that's it for now. We're very close to being done. The only things I have left to do is test the Rules integration and the user_relationship_node_access sub-module.

--Ben

jarush’s picture

Thanks BenK for this module.
I will help testing it too.
:)

BenK’s picture

user_relationship_node_access

31. I tried testing the latest changes/fixes to the user_relationship_node_access sub-module, but I'm still not getting any permissions to show up.

41. Instead of having its own major horizontal tab, the global settings for this module should be placed in a vertical tab (at admin/config/people/relationships/settings). Instead of "Posting to Social Network," label the vertical tab "Share Content". Add a description in this vertical tab that reads: "Add a check mark next to each content type that users should be allowed to share with their established relationships. Only these content types will have "Share content" options on their content creation and edit forms."

42. On the content creation and edit forms, change "Posting to social network" to "Share content". (Because the module permissions aren't visible yet, I'm suggesting this change without having seen how the module actually adds the fieldset to the content creation/edit forms. But that "Posting..." language is just weird.)

That's all I have on this for now... once we get permissions working, I will do some basic testing to make sure the permissions work as expected.

--Ben

BenK’s picture

43. A new notice/error has surfaced. When I request a relationship that requires approval, I'm now getting the following notice:

Notice: Undefined index: 5 in user_relationships_request_relationship() (line 215 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/user_relationships/user_relationships_api/user_relationships_api.api.inc).

Note that this notice only appears when requesting a relationships that requires approval. (It appears on both mutual and one-way relationships.) If the relationship doesn't need approval, then everything is fine. This bug didn't exist in prior testing, so possibly a recent fix caused it.

--Ben

BenK’s picture

user_relationships_rules

I did some preliminary testing of Rules integration and it's actually working pretty well. Nice job! I was able to successfully execute rules actions on UR events.

To test, on the "A user relationship has been approved" event, I awarded some points to the requester via the Userpoints module. It worked well. I then added a rule condition that would execute the rule only if the relationship type being approved was a "friend". This worked well, too.

The problems I experienced mostly dealt with UR Rules actions. I've described them below:

44. When using the "Delete, cancel or disapprove relationships between users" action, I'm not getting any other options besides the relationship type, requester, or requestee. Is there supposed to be some more options in which you choose whether to delete, cancel, or disapprove? In any event, none of those three things is happening to the specified relationship when the rule is executed. To test, on the "User was awarded points" event, I tried to delete the relationship between the current user (points giver) and points receiver.

45. When using the "Request, create, or approve relationships between users" action, it's creating a relationship between a user and himself. For instance, on the "User awarded points event," I tried to create a friend relationship between the current user who is awarding the points (Bob Smith) and the user who is receiving the points (Al Authenticated). I set the tokens properly using data selectors, but ended up with a friend relationship between Bob Smith and himself. I also tried this on the "After saving a new comment" event. I tried to create a relationship between the comment author (Bob Smith) and the node author (Al Authenticated). But upon execution of this rule, a relationship was created between Bob Smith and Bob Smith (the comment author).

As discussed, I'm not getting into the nitty-gritty of testing every last Rules event and action during this port. I'm trying to focus on the big stuff and we can get into the minutia after we have an official 7.x branch.

--Ben

Berdir’s picture

Status: Needs work » Needs review

2. I have no idea. I can't see a difference between PM and UR here and it works fine in PM. Can we postpone this into a follow-up issue?

6. Changed. n) used some very strange structure, there was a fieldset and inside that a select and the title/description was on the fieldset. Cleaned that up to only use a simple select.

7. Fixed. That used $relate_to as the user object, that's why I haven't found it before.

11. Fixed

15. Fixed, anything that is not a valid rtid should result in a Page not found message now.

23. Ok, changed. Also clean up the weirdly structured (just like privatemsg roles settings, I assume they was copied from this) roles fieldset/checkboxes thing.

24. Yep, looks much better now. Especially inside the vertical_tab where the list looked even stranger ;). Changed.

33. I fixed the notices - but I have no clue what that view is supposed to show and around 50% of my rows are empty ;)

37. Hm, not without quite some effort, including defining our own theme function.

39. Changed.

31. Found the culprit. The module checked some permissions internally (it even translated them before checking o.0) without actually defining them. I've defined them now. Try again :)

41. Changed. Not sure if we should add the description to the fieldset or the checkboxes, because there is currently only a single setting. They are currently on the fieldset but I think they might look better below the checkboxes. I also added a title to those checkboxes.

42. Changed. It actually used either the posting.. or "User relationship node access" as title depending on the form complexity (= number of relationship types)

43. Should be fixed. That was probably added when I merged in the changes from 6.x-1.x.

44. I haven't found any options when looking at the code. I guess it's named like that because the actual action depends on the state of the relationship. If it is approved, then it is deleted and if it's pending, then it gets declined. Or something like that, not sure :) We could also rename it to simply say 'Remove' or so.

45. Kinda funny :) Both 44 (aka: it tried to delete a relationship with the current user) and 45 was because I forgot the change requester to requestee when copying the code around. Both should now work.

Changes pushed.

I think once you've verified the latest fixes we should commit this to d.o. I'll contact alex.k. As part of that, I will also remove all translation directories and files within, because that is now handled on localize.drupal.org.

BenK’s picture

Status: Needs review » Reviewed & tested by the community

Hey Berdir,

All of your fixes are working great! Really nice job. This is RTBC and ready to be committed. Now we can officially open the 7.x branch! :-)

--Ben

P.S. I did find a few very, very minor things if you want to fix them while you're opening the new 7.x branch. Or we can wait and fix them on separate issue threads. So here they are:

* In regard to 41., I agree with your suggestion that the description would look better below the checkboxes (instead of being on fieldset).

* When you decline a relationship request, there is no drupal_set_message() to confirm the decline. It should say something like: "Bob Smith's friend request has been declined." "Bob Smith" would be a link to the user's account page. This would be the same structure as what is displayed when the request is approved.

* In the views integration, the "Requestee user" and "Requester user" fields are strange. They display the word "view" as a link to the user's account page. We should change this to just display the username/realname (with the same link to the account page).

Berdir’s picture

Status: Reviewed & tested by the community » Fixed

Ok, commited!

I branched of from HEAD, as it is usually suggested for CVS. That might not have been a very good idea, since HEAD seems to be a very very old version. Meaning, it was a lot of work to get the commit right so that all files are there that should be and no others :)

See http://drupal.org/cvs?commit=481868 ;)

I also removed all translation related files as that should be handled on localize.drupal.org now.

From the additional points, I changed the description but not the other two things.

I've created a -dev version which should show up within the next 12 hours.

Yay! :)

Status: Fixed » Closed (fixed)

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

Negor’s picture

Assigned: Unassigned » Negor
Category: task » support

Moved here

Berdir’s picture

Create a new issue, this is not the correct place.