From 541dfa04fe98d0995679d44fdc5d0c5dc8ceca77 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Mon, 21 Mar 2011 14:35:29 -0700 Subject: [PATCH] Switch things from calling Group_member::join & leave & calling events manually to running through Profile::joinGroup() && Profile::leaveGroup(), with the events encapsulated. --- EVENTS.txt | 8 +++---- actions/apigroupjoin.php | 5 +--- actions/apigroupleave.php | 5 +--- actions/atompubmembershipfeed.php | 5 +--- actions/atompubshowmembership.php | 5 +--- actions/joingroup.php | 5 +--- actions/leavegroup.php | 5 +--- classes/Group_member.php | 1 + classes/Profile.php | 30 ++++++++++++++++++++++++ classes/User.php | 27 +++++++++++++++++++++ lib/activityimporter.php | 5 +--- lib/activitymover.php | 2 +- lib/command.php | 10 ++------ plugins/ForceGroup/ForceGroupPlugin.php | 5 +--- plugins/OStatus/OStatusPlugin.php | 2 +- plugins/OStatus/actions/groupsalmon.php | 15 ++---------- plugins/OStatus/actions/ostatusgroup.php | 16 ++++--------- 17 files changed, 81 insertions(+), 70 deletions(-) diff --git a/EVENTS.txt b/EVENTS.txt index 54d06655ee..1494a9c890 100644 --- a/EVENTS.txt +++ b/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 diff --git a/actions/apigroupjoin.php b/actions/apigroupjoin.php index 2e35cb87de..7124a4b0f0 100644 --- a/actions/apigroupjoin.php +++ b/actions/apigroupjoin.php @@ -126,10 +126,7 @@ class ApiGroupJoinAction extends ApiAuthAction } try { - if (Event::handle('StartJoinGroup', array($this->group, $this->user))) { - Group_member::join($this->group->id, $this->user->id); - Event::handle('EndJoinGroup', array($this->group, $this->user)); - } + $this->user->joinGroup($this->group); } 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. diff --git a/actions/apigroupleave.php b/actions/apigroupleave.php index 083ebd890f..35a4e04d78 100644 --- a/actions/apigroupleave.php +++ b/actions/apigroupleave.php @@ -117,10 +117,7 @@ class ApiGroupLeaveAction extends ApiAuthAction } try { - if (Event::handle('StartLeaveGroup', array($this->group,$this->user))) { - Group_member::leave($this->group->id, $this->user->id); - Event::handle('EndLeaveGroup', array($this->group, $this->user)); - } + $this->user->leaveGroup($this->group); } 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. diff --git a/actions/atompubmembershipfeed.php b/actions/atompubmembershipfeed.php index b52583314d..f7882d97d3 100644 --- a/actions/atompubmembershipfeed.php +++ b/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))) { - $membership = Group_member::join($group->id, $this->auth_user->id); - Event::handle('EndJoinGroup', array($group, $this->auth_user)); - } + $this->auth_user->joinGroup($group); Event::handle('EndAtomPubNewActivity', array($activity, $membership)); } diff --git a/actions/atompubshowmembership.php b/actions/atompubshowmembership.php index 098cca8b3e..8bf62912f5 100644 --- a/actions/atompubshowmembership.php +++ b/actions/atompubshowmembership.php @@ -151,10 +151,7 @@ class AtompubshowmembershipAction extends ApiAuthAction " membership."), 403); } - if (Event::handle('StartLeaveGroup', array($this->_group, $this->auth_user))) { - Group_member::leave($this->_group->id, $this->auth_user->id); - Event::handle('EndLeaveGroup', array($this->_group, $this->auth_user)); - } + $this->auth_user->leaveGroup($this->_group); return; } diff --git a/actions/joingroup.php b/actions/joingroup.php index 4c45ca8b9d..8675dbaed3 100644 --- a/actions/joingroup.php +++ b/actions/joingroup.php @@ -129,10 +129,7 @@ class JoingroupAction extends Action $cur = common_current_user(); try { - if (Event::handle('StartJoinGroup', array($this->group, $cur))) { - Group_member::join($this->group->id, $cur->id); - Event::handle('EndJoinGroup', array($this->group, $cur)); - } + $cur->joinGroup($this->group); } 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. diff --git a/actions/leavegroup.php b/actions/leavegroup.php index f5d1ccd08c..9e560b9717 100644 --- a/actions/leavegroup.php +++ b/actions/leavegroup.php @@ -123,10 +123,7 @@ class LeavegroupAction extends Action $cur = common_current_user(); try { - if (Event::handle('StartLeaveGroup', array($this->group, $cur))) { - Group_member::leave($this->group->id, $cur->id); - Event::handle('EndLeaveGroup', array($this->group, $cur)); - } + $cur->leaveGroup($this->group); } 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. diff --git a/classes/Group_member.php b/classes/Group_member.php index 2cf31cf123..30b79bb931 100644 --- a/classes/Group_member.php +++ b/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 diff --git a/classes/Profile.php b/classes/Profile.php index 88edf5cbb3..a80f121fc4 100644 --- a/classes/Profile.php +++ b/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); diff --git a/classes/User.php b/classes/User.php index 970e167a3b..31b132d0f3 100644 --- a/classes/User.php +++ b/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(); diff --git a/lib/activityimporter.php b/lib/activityimporter.php index aa9b95e084..270b285a26 100644 --- a/lib/activityimporter.php +++ b/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))) { - Group_member::join($group->id, $user->id); - Event::handle('EndJoinGroup', array($group, $user)); - } + $user->joinGroup($group); } // XXX: largely cadged from Ostatus_profile::processNote() diff --git a/lib/activitymover.php b/lib/activitymover.php index 495d7b4caa..b308ad5624 100644 --- a/lib/activitymover.php +++ b/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: diff --git a/lib/command.php b/lib/command.php index 03baa8212d..5b9964c5b1 100644 --- a/lib/command.php +++ b/lib/command.php @@ -352,10 +352,7 @@ class JoinCommand extends Command } try { - if (Event::handle('StartJoinGroup', array($group, $cur))) { - Group_member::join($group->id, $cur->id); - Event::handle('EndJoinGroup', array($group, $cur)); - } + $cur->joinGroup($group); } 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))) { - Group_member::leave($group->id, $cur->id); - Event::handle('EndLeaveGroup', array($group, $cur)); - } + $cur->leaveGroup($group); } 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. diff --git a/plugins/ForceGroup/ForceGroupPlugin.php b/plugins/ForceGroup/ForceGroupPlugin.php index fb98644846..5925dcaef0 100644 --- a/plugins/ForceGroup/ForceGroupPlugin.php +++ b/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))) { - Group_member::join($group->id, $user->id); - Event::handle('EndJoinGroup', array($group, $user)); - } + $profile->joinGroup($group); } catch (Exception $e) { // TRANS: Server exception. // TRANS: %1$s is a user nickname, %2$s is a group nickname. diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 738481149c..e75130b9e9 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/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 */ diff --git a/plugins/OStatus/actions/groupsalmon.php b/plugins/OStatus/actions/groupsalmon.php index 024f0cc217..a9838f6e1b 100644 --- a/plugins/OStatus/actions/groupsalmon.php +++ b/plugins/OStatus/actions/groupsalmon.php @@ -149,14 +149,7 @@ class GroupsalmonAction extends SalmonAction } try { - // @fixme that event currently passes a user from main UI - // 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)); - //} + $profile->joinGroup($this->group); } 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 - //if (Event::handle('StartLeaveGroup', array($this->group, $profile))) { - Group_member::leave($this->group->id, $profile->id); - //Event::handle('EndLeaveGroup', array($this->group, $profile)); - //} + $profile->leaveGroup($this->group); } 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.'), diff --git a/plugins/OStatus/actions/ostatusgroup.php b/plugins/OStatus/actions/ostatusgroup.php index 24fbaac9ca..245c56a68e 100644 --- a/plugins/OStatus/actions/ostatusgroup.php +++ b/plugins/OStatus/actions/ostatusgroup.php @@ -141,18 +141,12 @@ class OStatusGroupAction extends OStatusSubAction return; } - if (Event::handle('StartJoinGroup', array($group, $user))) { - $ok = Group_member::join($this->oprofile->group_id, $user->id); - if ($ok) { - Event::handle('EndJoinGroup', array($group, $user)); - $this->success(); - } else { - // TRANS: OStatus remote group subscription dialog error. - $this->showForm(_m('Remote group join failed!')); - } - } else { + try { + $user->joinGroup($group); + } catch (Exception $e) { // TRANS: OStatus remote group subscription dialog error. - $this->showForm(_m('Remote group join aborted!')); + $this->showForm(_m('Remote group join failed!')); + return; } }