fail if request does not exist (#230)

* fail if request does not exist

* runtimeexception -> exception

* implement toString

* cover weird edge case

* throw exception

Author: simonLeary42

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;
         }
 

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
      *

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);
+            }
         }
     }
 }

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;
             }
         }