Skip to content

Commit 7a04343

Browse files
authored
fail if request does not exist (#230)
* fail if request does not exist * runtimeexception -> exception * implement toString * cover weird edge case * throw exception
1 parent 5582ea4 commit 7a04343

File tree

4 files changed

+32
-4
lines changed

4 files changed

+32
-4
lines changed

resources/lib/UnityGroup.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ public function equals($other_group)
4747
return $this->getPIUID() == $other_group->getPIUID();
4848
}
4949

50+
public function __toString()
51+
{
52+
return $this->getPIUID();
53+
}
54+
5055
/**
5156
* Returns this group's PI UID
5257
*
@@ -133,6 +138,12 @@ public function requestGroup($send_mail_to_admins, $send_mail = true)
133138
*/
134139
public function approveGroup($operator = null, $send_mail = true)
135140
{
141+
if (!$this->SQL->requestExists($this->getOwner()->getUID())) {
142+
throw new Exception(
143+
"attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'"
144+
);
145+
}
146+
136147
// check for edge cases...
137148
if ($this->exists()) {
138149
return;
@@ -277,6 +288,12 @@ public function cancelGroupJoinRequest($user, $send_mail = true)
277288
*/
278289
public function approveUser($new_user, $send_mail = true)
279290
{
291+
if (!$this->requestExists($new_user)) {
292+
throw new Exception(
293+
"attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'"
294+
);
295+
}
296+
280297
// check if user exists
281298
if (!$new_user->exists()) {
282299
$new_user->init();
@@ -382,15 +399,17 @@ public function removeUser($new_user, $send_mail = true)
382399
public function newUserRequest($new_user, $send_mail = true)
383400
{
384401
if ($this->userExists($new_user)) {
402+
UnitySite::errorLog("warning", "user '$new_user' already in group");
385403
return;
386404
}
387405

388406
if ($this->requestExists($new_user)) {
407+
UnitySite::errorLog("warning", "user '$new_user' already requested group membership");
389408
return;
390409
}
391410

392-
// check if account deletion request already exists
393411
if ($this->SQL->accDeletionRequestExists($new_user->getUID())) {
412+
throw new Exception("user '$new_user' requested account deletion");
394413
return;
395414
}
396415

resources/lib/UnityUser.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public function equals($other_user)
3838
return $this->getUID() == $other_user->getUID();
3939
}
4040

41+
public function __toString()
42+
{
43+
return $this->uid;
44+
}
45+
4146
/**
4247
* This is the method that is run when a new account is created
4348
*

test/functional/PiMemberApproveTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,12 @@ public function testApproveNonexistentRequest()
6464
$this->assertEmpty($piGroup->getRequests());
6565

6666
try {
67+
$this->expectException(Exception::class);
6768
$piGroup->approveUser($notRequestedUser);
68-
$this->assertEquals([$pi->getUID()], $piGroup->getGroupMemberUIDs());
69-
$this->assertFalse($piGroup->userExists($notRequestedUser));
7069
} finally {
71-
$piGroup->removeUser($notRequestedUser);
70+
if ($piGroup->userExists($notRequestedUser)) {
71+
$piGroup->removeUser($notRequestedUser);
72+
}
7273
}
7374
}
7475
}

test/functional/PiRemoveUserTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ public function testRemoveUser()
3030
foreach ($memberUIDs as $uid) {
3131
if ($uid != $piUid) {
3232
$memberToDelete = new UnityUser($uid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK);
33+
if ($memberToDelete->hasRequestedAccountDeletion()) {
34+
continue;
35+
}
3336
break;
3437
}
3538
}

0 commit comments

Comments
 (0)