From c4b503c83ffc687d07168a84438587a4fff49606 Mon Sep 17 00:00:00 2001 From: John Saigle Date: Tue, 22 Oct 2019 14:57:16 -0400 Subject: [PATCH 1/5] [User Accounts] Hide UI option allowing users to edit their own approval status --- modules/user_accounts/php/edit_user.class.inc | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index f4030fb69af..25d99460b8b 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', @@ -844,14 +849,20 @@ class Edit_User extends \NDB_Form // 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. 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', + ) + ); + } //------------------------------------------------------------ From 7dcb261acf63023b49565cdf7e7cb4d94d7cec7f Mon Sep 17 00:00:00 2001 From: ridz1208 Date: Mon, 18 Nov 2019 12:01:19 -0500 Subject: [PATCH 2/5] fix tests --- modules/user_accounts/php/edit_user.class.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index 25d99460b8b..95800afa846 100644 --- a/modules/user_accounts/php/edit_user.class.inc +++ b/modules/user_accounts/php/edit_user.class.inc @@ -1214,8 +1214,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. From b9bb64f682cd2616001ce13ad218ba6e66b5077e Mon Sep 17 00:00:00 2001 From: ridz1208 Date: Mon, 18 Nov 2019 13:47:34 -0500 Subject: [PATCH 3/5] fix tests --- modules/user_accounts/php/edit_user.class.inc | 12 ++-- .../user_accounts/test/user_accountsTest.php | 57 ++++++++++++++++++- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index 95800afa846..0830130e8f3 100644 --- a/modules/user_accounts/php/edit_user.class.inc +++ b/modules/user_accounts/php/edit_user.class.inc @@ -823,9 +823,9 @@ class Edit_User extends \NDB_Form "examiner_pending", "Pending Approval: ", array( - null => "", - 'Y' => 'Yes', - 'N' => 'No', + null => "", + 'Y' => 'Yes', + 'N' => 'No', ) ); } @@ -858,8 +858,8 @@ class Edit_User extends \NDB_Form 'Pending_approval', 'Pending approval', array( - 'Y' => 'Yes', - 'N' => 'No', + 'Y' => 'Yes', + 'N' => 'No', ) ); } @@ -1215,7 +1215,7 @@ class Edit_User extends \NDB_Form } if (!$matched && ($values['examiner_radiologist'] !== '' - || $values['examiner_pending'] ?? '' !== '') + || $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..b4a7aad1f35 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' => 999991, + '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' => 999991, + 'CenterID' => 1, + ) + ); + + $this->DB->insert( + "user_project_rel", + array( + 'UserID' => 999991, + '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" => 999991)); + $this->DB->delete("user_project_rel", array("UserID" => 999991)); + $this->DB->delete("users", array("UserID" => 'UnitTesterTwo')); parent::tearDown(); } } From e33ef0b33b08629032c78a5dd2b0c72a575835bb Mon Sep 17 00:00:00 2001 From: John Saigle Date: Mon, 18 Nov 2019 15:32:42 -0500 Subject: [PATCH 4/5] Hide activity status as well --- modules/user_accounts/php/edit_user.class.inc | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index 0830130e8f3..0d757859cb0 100644 --- a/modules/user_accounts/php/edit_user.class.inc +++ b/modules/user_accounts/php/edit_user.class.inc @@ -847,12 +847,10 @@ class Edit_User extends \NDB_Form unset($groupB); } - // active - $this->addSelect('Active', 'Active', array('Y' => 'Yes', 'N' => 'No')); // It doesn't make sense for a user to be editing their own - // approval status. In fact this could result in a Darwinian - // self-lockout. Let us help those who cannot help themselves. + // 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', @@ -862,33 +860,38 @@ class Edit_User extends \NDB_Form '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) From 0dfe2494e0d7fce23ecb8ce31f862a35c81e9b3c Mon Sep 17 00:00:00 2001 From: ridz1208 Date: Tue, 19 Nov 2019 13:31:45 -0500 Subject: [PATCH 5/5] tests --- modules/user_accounts/php/edit_user.class.inc | 9 ++++----- modules/user_accounts/test/user_accountsTest.php | 10 +++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/modules/user_accounts/php/edit_user.class.inc b/modules/user_accounts/php/edit_user.class.inc index 0d757859cb0..ec91ff77b8e 100644 --- a/modules/user_accounts/php/edit_user.class.inc +++ b/modules/user_accounts/php/edit_user.class.inc @@ -847,9 +847,8 @@ class Edit_User extends \NDB_Form unset($groupB); } - // 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 + // 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( @@ -862,10 +861,10 @@ class Edit_User extends \NDB_Form ); $this->addSelect( - 'Active', - 'Active', + 'Active', + 'Active', array( - 'Y' => 'Yes', + 'Y' => 'Yes', 'N' => 'No' ) ); diff --git a/modules/user_accounts/test/user_accountsTest.php b/modules/user_accounts/test/user_accountsTest.php index b4a7aad1f35..e081aa7f671 100644 --- a/modules/user_accounts/test/user_accountsTest.php +++ b/modules/user_accounts/test/user_accountsTest.php @@ -64,7 +64,7 @@ public function setUp() $this->DB->insert( "users", array( - 'ID' => 999991, + 'ID' => 999995, 'UserID' => 'UnitTesterTwo', 'Real_name' => 'Unit Tester 2', 'First_name' => 'Unit 2', @@ -82,7 +82,7 @@ public function setUp() $this->DB->insert( "user_psc_rel", array( - 'UserID' => 999991, + 'UserID' => 999995, 'CenterID' => 1, ) ); @@ -90,7 +90,7 @@ public function setUp() $this->DB->insert( "user_project_rel", array( - 'UserID' => 999991, + 'UserID' => 999995, 'ProjectID' => 1, ) ); @@ -406,8 +406,8 @@ function _accessUser($page, $userId) function tearDown() { $this->DB->delete("users", array("UserID" => 'userid')); - $this->DB->delete("user_psc_rel", array("UserID" => 999991)); - $this->DB->delete("user_project_rel", array("UserID" => 999991)); + $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(); }