Removing a user from a group does not remove the roles assigned.

Can you provide a patch + test?

jzornig and I have developed this patch to remove roles when a user is removed from a group or deleted.

try again

try again

+++ B/og.module	2012-06-05 09:54:14.000000000 +1000
@@ -1120,8 +1120,18 @@
+    //remove possible records in table {og_users_roles}

Start with capital letter, and with dot.

+++ B/og.module	2012-06-05 09:54:14.000000000 +1000
@@ -1120,8 +1120,18 @@
+    }    ¶

Remove space.

Any chance for a test ? :)

patch + test
the original code causes 2 fails in testing, while the modified code pasts

Thanks.

+++ og_modified/og.module	2012-06-13 14:41:46.000000000 +1000
@@ -1128,8 +1128,18 @@
+    //Remove possible records in the table {og_users_roles}.

Space missing after //

+++ og_modified/og.module	2012-06-13 14:41:46.000000000 +1000
@@ -1128,8 +1128,18 @@
+    if((!empty($membership->group_type)) && (!empty($membership->gid)) && (!empty($membership->etid)))  {

Why do we need this IF?

+++ og_modified/og.test	2012-06-13 14:47:40.000000000 +1000
@@ -1065,3 +1065,74 @@
+    //Grant 'role1' to $user1 at $entity1 and 'role2' to $user1 at $entity2

In the comments lets reference them as user1 and user2 (i.e. remove the $ prefix)

+++ og_modified/og.test	2012-06-13 14:47:40.000000000 +1000
@@ -1065,3 +1065,74 @@
+    //watchdog('roles', serialize($roles));

Please remove.

+++ og_modified/og.test	2012-06-13 14:47:40.000000000 +1000
@@ -1065,3 +1065,74 @@
+    foreach($roles as $rid => $name)  {
+      if($name == 'role1')  {
+        og_role_grant('entity_test', $entity1->pid, $user1->uid, $rid);
+      }
+      if($name == 'role2')  {
+        og_role_grant('entity_test', $entity2->pid, $user1->uid, $rid);
+      }

Since we have 2 roles, instead of foreach with IF, just write the 2 lines.

+++ og_modified/og.test	2012-06-13 14:47:40.000000000 +1000
@@ -1065,3 +1065,74 @@
+    $this->assertFalse(og_get_user_roles('entity_test', $entity1->pid, $user1->uid, FALSE), t('User is unsubscribed from group, so return value is empty'));

Instead of "so return value is..." maybe "role was removed"

Space missing after //

space removed.

Why do we need this IF?

If some codes unset or empty these variables before og_og_membership_delete, we won't access database.

In the comments lets reference them as user1 and user2 (i.e. remove the $ prefix)

Changed.

Please remove.

Removed.

Since we have 2 roles, instead of foreach with IF, just write the 2 lines.

Because 'og_role_save' does not reture $rid, I have to call 'og_roles' to get all roles which include not only 'role1', 'role2', but also 'non-member', 'member', and 'administrator'. Do you know any better way to get $rid of role1 and role2?

Instead of "so return value is..." maybe "role was removed"

Changed.

Anymore suggestions, just let me know. Thanks.

> If some codes unset or empty these variables before og_og_membership_delete, we won't access database.

I can't think of such a behavior. I think we can remove that IF.

> Because 'og_role_save' does not reture $rid,

$role is an object passed by reference, so you actually have the RID inside $role1 and $role2 (you can debug($role1) to see it.

> If some codes unset or empty these variables before og_og_membership_delete, we won't access database.

I can't think of such a behavior. I think we can remove that IF.
------------------------------------
IF is already removed. But because og_og_membership_delete is a hook, I still think the case maybe happens because we can not foresee other modules' codes.

> Because 'og_role_save' does not reture $rid,

$role is an object passed by reference, so you actually have the RID inside $role1 and $role2 (you can debug($role1) to see it.
-------------------------------------
Yes, thank you. og role grant is changed to be two lines.

Thanks, I've cleaned code a bit and committed.

I have the reverse problem:

• Adding a user to the group (group/node/24/admin/people/add-user) doesn't give them any roles.