diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 0a91a9ee..f74aad7c 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -47,6 +47,11 @@ public function equals($other_group) return $this->getPIUID() == $other_group->getPIUID(); } + public function __toString() + { + return $this->getPIUID(); + } + /** * Returns this group's PI UID * @@ -133,6 +138,12 @@ public function requestGroup($send_mail_to_admins, $send_mail = true) */ public function approveGroup($operator = null, $send_mail = true) { + if (!$this->SQL->requestExists($this->getOwner()->getUID())) { + throw new Exception( + "attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'" + ); + } + // check for edge cases... if ($this->exists()) { return; @@ -277,6 +288,12 @@ public function cancelGroupJoinRequest($user, $send_mail = true) */ public function approveUser($new_user, $send_mail = true) { + if (!$this->requestExists($new_user)) { + throw new Exception( + "attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'" + ); + } + // check if user exists if (!$new_user->exists()) { $new_user->init(); @@ -382,15 +399,17 @@ public function removeUser($new_user, $send_mail = true) public function newUserRequest($new_user, $send_mail = true) { if ($this->userExists($new_user)) { + UnitySite::errorLog("warning", "user '$new_user' already in group"); return; } if ($this->requestExists($new_user)) { + UnitySite::errorLog("warning", "user '$new_user' already requested group membership"); return; } - // check if account deletion request already exists if ($this->SQL->accDeletionRequestExists($new_user->getUID())) { + throw new Exception("user '$new_user' requested account deletion"); return; } diff --git a/resources/lib/UnityUser.php b/resources/lib/UnityUser.php index 594e7af6..04c412b9 100644 --- a/resources/lib/UnityUser.php +++ b/resources/lib/UnityUser.php @@ -38,6 +38,11 @@ public function equals($other_user) return $this->getUID() == $other_user->getUID(); } + public function __toString() + { + return $this->uid; + } + /** * This is the method that is run when a new account is created * diff --git a/test/functional/PiMemberApproveTest.php b/test/functional/PiMemberApproveTest.php index dd51b571..0b8b2024 100644 --- a/test/functional/PiMemberApproveTest.php +++ b/test/functional/PiMemberApproveTest.php @@ -64,11 +64,12 @@ public function testApproveNonexistentRequest() $this->assertEmpty($piGroup->getRequests()); try { + $this->expectException(Exception::class); $piGroup->approveUser($notRequestedUser); - $this->assertEquals([$pi->getUID()], $piGroup->getGroupMemberUIDs()); - $this->assertFalse($piGroup->userExists($notRequestedUser)); } finally { - $piGroup->removeUser($notRequestedUser); + if ($piGroup->userExists($notRequestedUser)) { + $piGroup->removeUser($notRequestedUser); + } } } } diff --git a/test/functional/PiRemoveUserTest.php b/test/functional/PiRemoveUserTest.php index 79ecd1ee..c02a8908 100644 --- a/test/functional/PiRemoveUserTest.php +++ b/test/functional/PiRemoveUserTest.php @@ -30,6 +30,9 @@ public function testRemoveUser() foreach ($memberUIDs as $uid) { if ($uid != $piUid) { $memberToDelete = new UnityUser($uid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); + if ($memberToDelete->hasRequestedAccountDeletion()) { + continue; + } break; } }