Switch things from calling Group_member::join & leave & calling events manually to running through Profile::joinGroup() && Profile::leaveGroup(), with the events encapsulated.

EVENTS.txt

@@ -742,19 +742,19 @@ EndUnsubscribe: when an unsubscribe is done
 
 StartJoinGroup: when a user is joining a group
 - $group: the group being joined
-- $user: the user joining
+- $profile: the local or remote user joining
 
 EndJoinGroup: when a user finishes joining a group
 - $group: the group being joined
-- $user: the user joining
+- $profile: the local or remote user joining
 
 StartLeaveGroup: when a user is leaving a group
 - $group: the group being left
-- $user: the user leaving
+- $profile: the local or remote user leaving
 
 EndLeaveGroup: when a user has left a group
 - $group: the group being left
-- $user: the user leaving
+- $profile: the local or remote user leaving
 
 StartShowContentLicense: Showing the default license for content
 - $action: the current action

actions/apigroupjoin.php

@@ -126,10 +126,7 @@ class ApiGroupJoinAction extends ApiAuthAction
         }
 
         try {
-            if (Event::handle('StartJoinGroup', array($this->group, $this->user))) {
+            $this->user->joinGroup($this->group);
-                Group_member::join($this->group->id, $this->user->id);
-                Event::handle('EndJoinGroup', array($this->group, $this->user));
-            }
         } catch (Exception $e) {
             // TRANS: Server error displayed when joining a group failed in the database.
             // TRANS: %1$s is the joining user's nickname, $2$s is the group nickname for which the join failed.

actions/apigroupleave.php

@@ -117,10 +117,7 @@ class ApiGroupLeaveAction extends ApiAuthAction
         }
 
         try {
-            if (Event::handle('StartLeaveGroup', array($this->group,$this->user))) {
+            $this->user->leaveGroup($this->group);
-                Group_member::leave($this->group->id, $this->user->id);
-                Event::handle('EndLeaveGroup', array($this->group, $this->user));
-            }
         } catch (Exception $e) {
             // TRANS: Server error displayed when leaving a group failed in the database.
             // TRANS: %1$s is the leaving user's nickname, $2$s is the group nickname for which the leave failed.

actions/atompubmembershipfeed.php

@@ -275,10 +275,7 @@ class AtompubmembershipfeedAction extends ApiAuthAction
                 throw new ClientException(_('Blocked by admin.'));
             }
 
-            if (Event::handle('StartJoinGroup', array($group, $this->auth_user))) {
+            $this->auth_user->joinGroup($group);
-                $membership = Group_member::join($group->id, $this->auth_user->id);
-                Event::handle('EndJoinGroup', array($group, $this->auth_user));
-            }
 
             Event::handle('EndAtomPubNewActivity', array($activity, $membership));
         }

actions/atompubshowmembership.php

@@ -151,10 +151,7 @@ class AtompubshowmembershipAction extends ApiAuthAction
                                         " membership."), 403);
         }
 
-        if (Event::handle('StartLeaveGroup', array($this->_group, $this->auth_user))) {
+        $this->auth_user->leaveGroup($this->_group);
-            Group_member::leave($this->_group->id, $this->auth_user->id);
-            Event::handle('EndLeaveGroup', array($this->_group, $this->auth_user));
-        }
 
         return;
     }

actions/joingroup.php

@@ -129,10 +129,7 @@ class JoingroupAction extends Action
         $cur = common_current_user();
 
         try {
-            if (Event::handle('StartJoinGroup', array($this->group, $cur))) {
+            $cur->joinGroup($this->group);
-                Group_member::join($this->group->id, $cur->id);
-                Event::handle('EndJoinGroup', array($this->group, $cur));
-            }
         } catch (Exception $e) {
             // TRANS: Server error displayed when joining a group failed in the database.
             // TRANS: %1$s is the joining user's nickname, $2$s is the group nickname for which the join failed.

actions/leavegroup.php

@@ -123,10 +123,7 @@ class LeavegroupAction extends Action
         $cur = common_current_user();
 
         try {
-            if (Event::handle('StartLeaveGroup', array($this->group, $cur))) {
+            $cur->leaveGroup($this->group);
-                Group_member::leave($this->group->id, $cur->id);
-                Event::handle('EndLeaveGroup', array($this->group, $cur));
-            }
         } catch (Exception $e) {
             // TRANS: Server error displayed when leaving a group failed in the database.
             // TRANS: %1$s is the leaving user's nickname, $2$s is the group nickname for which the leave failed.

classes/Group_member.php

@@ -28,6 +28,7 @@ class Group_member extends Memcached_DataObject
 
     /**
      * Method to add a user to a group.
+     * In most cases, you should call Profile->joinGroup() instead.
      *
      * @param integer $group_id   Group to add to
      * @param integer $profile_id Profile being added

classes/Profile.php

@@ -339,6 +339,36 @@ class Profile extends Memcached_DataObject
         return $groups;
     }
 
+    /**
+     * Request to join the given group.
+     * May throw exceptions on failure.
+     *
+     * @param User_group $group
+     * @return Group_member
+     */
+    function joinGroup(User_group $group)
+    {
+        $ok = null;
+        if (Event::handle('StartJoinGroup', array($group, $this))) {
+            $ok = Group_member::join($group->id, $this->id);
+            Event::handle('EndJoinGroup', array($group, $this));
+        }
+        return $ok;
+    }
+
+    /**
+     * Leave a group that this profile is a member of.
+     *
+     * @param User_group $group 
+     */
+    function leaveGroup(User_group $group)
+    {
+        if (Event::handle('StartLeaveGroup', array($this->group, $this))) {
+            Group_member::leave($this->group->id, $this->id);
+            Event::handle('EndLeaveGroup', array($this->group, $this));
+        }
+    }
+
     function avatarUrl($size=AVATAR_PROFILE_SIZE)
     {
         $avatar = $this->getAvatar($size);

classes/User.php

@@ -68,6 +68,9 @@ class User extends Memcached_DataObject
     /* the code above is auto generated do not remove the tag below */
     ###END_AUTOCODE
 
+    /**
+     * @return Profile
+     */
     function getProfile()
     {
         $profile = Profile::staticGet('id', $this->id);
@@ -596,6 +599,30 @@ class User extends Memcached_DataObject
         return $profile->getGroups($offset, $limit);
     }
 
+    /**
+     * Request to join the given group.
+     * May throw exceptions on failure.
+     *
+     * @param User_group $group
+     * @return Group_member
+     */
+    function joinGroup(User_group $group)
+    {
+        $profile = $this->getProfile();
+        return $profile->joinGroup($group);
+    }
+
+    /**
+     * Leave a group that this user is a member of.
+     *
+     * @param User_group $group
+     */
+    function leaveGroup(User_group $group)
+    {
+        $profile = $this->getProfile();
+        return $profile->leaveGroup($group);
+    }
+
     function getSubscriptions($offset=0, $limit=null)
     {
         $profile = $this->getProfile();

lib/activityimporter.php

@@ -163,10 +163,7 @@ class ActivityImporter extends QueueHandler
             throw new ClientException(_("User is already a member of this group."));
         }
 
-        if (Event::handle('StartJoinGroup', array($group, $user))) {
+        $user->joinGroup($group);
-            Group_member::join($group->id, $user->id);
-            Event::handle('EndJoinGroup', array($group, $user));
-        }
     }
 
     // XXX: largely cadged from Ostatus_profile::processNote()

lib/activitymover.php

@@ -116,7 +116,7 @@ class ActivityMover extends QueueHandler
             $sink->postActivity($act);
             $group = User_group::staticGet('uri', $act->objects[0]->id);
             if (!empty($group)) {
-                Group_member::leave($group->id, $user->id);
+                $user->leaveGroup($group);
             }
             break;
         case ActivityVerb::FOLLOW:

lib/command.php

@@ -352,10 +352,7 @@ class JoinCommand extends Command
         }
 
         try {
-            if (Event::handle('StartJoinGroup', array($group, $cur))) {
+            $cur->joinGroup($group);
-                Group_member::join($group->id, $cur->id);
-                Event::handle('EndJoinGroup', array($group, $cur));
-            }
         } catch (Exception $e) {
             // TRANS: Message given having failed to add a user to a group.
             // TRANS: %1$s is the nickname of the user, %2$s is the nickname of the group.
@@ -400,10 +397,7 @@ class DropCommand extends Command
         }
 
         try {
-            if (Event::handle('StartLeaveGroup', array($group, $cur))) {
+            $cur->leaveGroup($group);
-                Group_member::leave($group->id, $cur->id);
-                Event::handle('EndLeaveGroup', array($group, $cur));
-            }
         } catch (Exception $e) {
             // TRANS: Message given having failed to remove a user from a group.
             // TRANS: %1$s is the nickname of the user, %2$s is the nickname of the group.

plugins/ForceGroup/ForceGroupPlugin.php

@@ -68,10 +68,7 @@ class ForceGroupPlugin extends Plugin
             $group = User_group::getForNickname($nickname);
             if ($group && !$profile->isMember($group)) {
                 try {
-                    if (Event::handle('StartJoinGroup', array($group, $user))) {
+                    $profile->joinGroup($group);
-                        Group_member::join($group->id, $user->id);
-                        Event::handle('EndJoinGroup', array($group, $user));
-                    }
                 } catch (Exception $e) {
                     // TRANS: Server exception.
                     // TRANS: %1$s is a user nickname, %2$s is a group nickname.

plugins/OStatus/OStatusPlugin.php

@@ -675,7 +675,7 @@ class OStatusPlugin extends Plugin
      * it'll be left with a stray membership record.
      *
      * @param User_group $group
-     * @param User $user
+     * @param Profile $user
      *
      * @return mixed hook return value
      */

plugins/OStatus/actions/groupsalmon.php

@@ -149,14 +149,7 @@ class GroupsalmonAction extends SalmonAction
         }
 
         try {
-            // @fixme that event currently passes a user from main UI
+            $profile->joinGroup($this->group);
-            // Event should probably move into Group_member::join
-            // and take a Profile object.
-            //
-            //if (Event::handle('StartJoinGroup', array($this->group, $profile))) {
-                Group_member::join($this->group->id, $profile->id);
-                //Event::handle('EndJoinGroup', array($this->group, $profile));
-            //}
         } catch (Exception $e) {
             // TRANS: Server error. %1$s is a profile URI, %2$s is a group nickname.
             $this->serverError(sprintf(_m('Could not join remote user %1$s to group %2$s.'),
@@ -181,11 +174,7 @@ class GroupsalmonAction extends SalmonAction
         $profile = $oprofile->localProfile();
 
         try {
-            // @fixme event needs to be refactored as above
+            $profile->leaveGroup($this->group);
-            //if (Event::handle('StartLeaveGroup', array($this->group, $profile))) {
-                Group_member::leave($this->group->id, $profile->id);
-                //Event::handle('EndLeaveGroup', array($this->group, $profile));
-            //}
         } catch (Exception $e) {
             // TRANS: Server error. %1$s is a profile URI, %2$s is a group nickname.
             $this->serverError(sprintf(_m('Could not remove remote user %1$s from group %2$s.'),

plugins/OStatus/actions/ostatusgroup.php

@@ -141,18 +141,12 @@ class OStatusGroupAction extends OStatusSubAction
             return;
         }
 
-        if (Event::handle('StartJoinGroup', array($group, $user))) {
+        try {
-            $ok = Group_member::join($this->oprofile->group_id, $user->id);
+            $user->joinGroup($group);
-            if ($ok) {
+        } catch (Exception $e) {
-                Event::handle('EndJoinGroup', array($group, $user));
-                $this->success();
-            } else {
-                // TRANS: OStatus remote group subscription dialog error.
-                $this->showForm(_m('Remote group join failed!'));
-            }
-        } else {
             // TRANS: OStatus remote group subscription dialog error.
-            $this->showForm(_m('Remote group join aborted!'));
+            $this->showForm(_m('Remote group join failed!'));
+            return;
         }
     }