diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index f4030fb69af..ec91ff77b8e 100644 --- a/modules/user_accounts/php/edit_user.class.inc +++ b/modules/user_accounts/php/edit_user.class.inc @@ -812,18 +812,23 @@ class Edit_User extends \NDB_Form 'N' => 'No', ) ); - $groupB[] = $this->createLabel( - "Pending Approval:" - ); - $groupB[] = $this->createSelect( - "examiner_pending", - "Pending Approval: ", - array( - null => "", - 'Y' => 'Yes', - 'N' => 'No', - ) - ); + + // A user should not be able to edit their own examiner approval + // status. + if (!$this->_isEditingOwnAccount()) { + $groupB[] = $this->createLabel( + "Pending Approval:" + ); + $groupB[] = $this->createSelect( + "examiner_pending", + "Pending Approval: ", + array( + null => "", + 'Y' => 'Yes', + 'N' => 'No', + ) + ); + } $this->addGroup( $groupA, 'examiner_sites', @@ -842,42 +847,50 @@ class Edit_User extends \NDB_Form unset($groupB); } - // active - $this->addSelect('Active', 'Active', array('Y' => 'Yes', 'N' => 'No')); - $this->addSelect( - 'Pending_approval', - 'Pending approval', - array( - 'Y' => 'Yes', - 'N' => 'No', - ) - ); - - //------------------------------------------------------------ + // It doesn't make sense for a user to be editing their own + // approval status or active status. In fact this could result in a + // Darwinian self-lockout. Let us help those who cannot help themselves. + if (!$this->_isEditingOwnAccount()) { + $this->addSelect( + 'Pending_approval', + 'Pending approval', + array( + 'Y' => 'Yes', + 'N' => 'No', + ) + ); - // active time windows + $this->addSelect( + 'Active', + 'Active', + array( + 'Y' => 'Yes', + 'N' => 'No' + ) + ); - $dateOptions = array( - 'language' => 'en', - 'format' => 'YMd', - 'addEmptyOption' => 'true', - ); + $dateOptions = array( + 'language' => 'en', + 'format' => 'YMd', + 'addEmptyOption' => 'true', + ); - $dateAttributes = ['class' => 'form-control input-sm input-date']; + $dateAttributes = ['class' => 'form-control input-sm input-date']; - $this->addBasicDate( - 'active_from', - 'Active from', - $dateOptions, - $dateAttributes - ); + $this->addBasicDate( + 'active_from', + 'Active from', + $dateOptions, + $dateAttributes + ); - $this->addBasicDate( - 'active_to', - 'Active to', - $dateOptions, - $dateAttributes - ); + $this->addBasicDate( + 'active_to', + 'Active to', + $dateOptions, + $dateAttributes + ); + } // checking if the account can be rejected (no password hash // pending approval) @@ -1203,8 +1216,8 @@ class Edit_User extends \NDB_Form } } if (!$matched - && (($values['examiner_radiologist'] !== '' - || $values['examiner_pending'] !== '')) + && ($values['examiner_radiologist'] !== '' + || $values['examiner_pending'] ?? '' !== '') ) { $errors['examiner_sites'] = "Please select at least one examiner site or clear the 'Examiner status' fields below (i.e. diff --git a/modules/user_accounts/test/user_accountsTest.php b/modules/user_accounts/test/user_accountsTest.php index c005d4c10f5..e081aa7f671 100644 --- a/modules/user_accounts/test/user_accountsTest.php +++ b/modules/user_accounts/test/user_accountsTest.php @@ -49,6 +49,53 @@ class UserAccountsIntegrationTest extends LorisIntegrationTest private $_table = "#dynamictable > tbody > tr:nth-child(1)"; private $_addUserBtn = "#default-panel > div > div > div.table-header >". " div > div > div:nth-child(2) > button:nth-child(1)"; + + /** + * Does basic setting up of Loris variables for this test, such as + * instantiting the config and database objects, creating a user + * to user for the tests, and logging in. + * + * @return void + */ + public function setUp() + { + parent::setUp(); + $password = new \Password($this->validPassword); + $this->DB->insert( + "users", + array( + 'ID' => 999995, + 'UserID' => 'UnitTesterTwo', + 'Real_name' => 'Unit Tester 2', + 'First_name' => 'Unit 2', + 'Last_name' => 'Tester 2', + 'Email' => 'tester2@example.com', + 'Privilege' => 0, + 'PSCPI' => 'N', + 'Active' => 'Y', + 'Password_hash' => $password, + 'Password_expiry' => '2099-12-31', + 'Pending_approval' => 'N', + ) + ); + + $this->DB->insert( + "user_psc_rel", + array( + 'UserID' => 999995, + 'CenterID' => 1, + ) + ); + + $this->DB->insert( + "user_project_rel", + array( + 'UserID' => 999995, + 'ProjectID' => 1, + ) + ); + } + /** * Tests that, when loading the User accounts module > edit_user submodule, some * text appears in the body. @@ -103,7 +150,6 @@ function testUserAccountsMyPreferencesDoespageLoad() function testUserAccountsFilterClearBtn() { $this->safeGet($this->url . "/user_accounts/"); - //testing data from RBdata.sql $this-> _testFilter($this->_name, $this->_table, null, "UnitTester"); $this-> _testFilter($this->_site, $this->_table, "1 rows", "3"); } @@ -183,7 +229,7 @@ function testUserAccountEdits() ); $this->_verifyUserModification( 'user_accounts', - 'UnitTester', + 'UnitTesterTwo', 'Active', 'No' ); @@ -195,10 +241,12 @@ function testUserAccountEdits() ); $this->_verifyUserModification( 'user_accounts', - 'UnitTester', + 'UnitTesterTwo', 'Pending_approval', 'No' ); + //TODO:add test case to ensure pending_approval + //DOES NOT show up on UnitTester since logged in user is UnitTester } /** * Tests various My Preference page edit operations. @@ -358,6 +406,9 @@ function _accessUser($page, $userId) function tearDown() { $this->DB->delete("users", array("UserID" => 'userid')); + $this->DB->delete("user_psc_rel", array("UserID" => 999995)); + $this->DB->delete("user_project_rel", array("UserID" => 999995)); + $this->DB->delete("users", array("UserID" => 'UnitTesterTwo')); parent::tearDown(); } }