From 43e4c23bfaf543b83776fcf506b2ddf8ea9a47f3 Mon Sep 17 00:00:00 2001 From: CamilleBeau Date: Tue, 27 Feb 2024 16:39:55 -0500 Subject: [PATCH 1/6] [user_accounts] Limit sites in table filters --- .../user_accounts/php/user_accounts.class.inc | 17 ++++++++++++++++- .../php/useraccountrowprovisioner.class.inc | 8 +++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/modules/user_accounts/php/user_accounts.class.inc b/modules/user_accounts/php/user_accounts.class.inc index d4db5401c85..f6b1e4daa39 100644 --- a/modules/user_accounts/php/user_accounts.class.inc +++ b/modules/user_accounts/php/user_accounts.class.inc @@ -77,9 +77,24 @@ class User_Accounts extends \DataFrameworkMenu 'Y' => 'Yes', 'N' => 'No', ]; + // Get user + $factory = \NDB_Factory::singleton(); + $user = $factory->user(); + + // Without user_acounts_multisite permission, only show user sites + if ($user->hasPermission('user_accounts_multisite')) { + $sites = \Utility::getSiteList(false); + } else { + $sites = []; + foreach (\Utility::getSiteList(false) AS $centerID => $centerName) { + if ($user->hasCenter(new \CenterID($centerID))) { + $sites[$centerID] = $centerName; + } + } + } return [ - 'sites' => \Utility::getSiteList(false), + 'sites' => $sites, 'projects' => \Utility::getProjectList(), 'actives' => $yesNoOptions, 'pendingApprovals' => $yesNoOptions, diff --git a/modules/user_accounts/php/useraccountrowprovisioner.class.inc b/modules/user_accounts/php/useraccountrowprovisioner.class.inc index 382a70b8673..f0cf20459fb 100644 --- a/modules/user_accounts/php/useraccountrowprovisioner.class.inc +++ b/modules/user_accounts/php/useraccountrowprovisioner.class.inc @@ -17,6 +17,10 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione */ function __construct() { + // Get user + $factory = \NDB_Factory::singleton(); + $user = $factory->user(); + parent::__construct( "SELECT GROUP_CONCAT(DISTINCT upsr.CenterID) as centerIds, GROUP_CONCAT(DISTINCT uprr.ProjectID) as projectIds, @@ -29,9 +33,11 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione FROM users u LEFT JOIN user_psc_rel upsr ON (upsr.UserID=u.ID) LEFT JOIN user_project_rel uprr ON (uprr.UserID=u.ID) + LEFT JOIN user_psc_rel upsr2 ON (upsr2.CenterID=upsr.CenterID) + WHERE upsr2.UserID=:uid GROUP BY u.ID ORDER BY u.UserID", - [] + ['uid' => $user->getId()] ); } From 8a12485f2d631e70773e1e307d05ba09d5b1aec1 Mon Sep 17 00:00:00 2001 From: CamilleBeau Date: Tue, 27 Feb 2024 16:46:12 -0500 Subject: [PATCH 2/6] Test plan update --- modules/user_accounts/test/TestPlan.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/user_accounts/test/TestPlan.md b/modules/user_accounts/test/TestPlan.md index 8083e718e51..c1ba6680f9b 100644 --- a/modules/user_accounts/test/TestPlan.md +++ b/modules/user_accounts/test/TestPlan.md @@ -41,8 +41,8 @@ When creating or editing a user: (subtest: edit_user) 13. Check that if generating a new password for a user an email is sent to that user containing the new password (requires email server). 14. Check that when editing a user account it is not possible to set the password to its actual value (i.e. it needs to change). [Automated] -15. Verify that if the editor does not have permission 'Across all sites add and edit users' then the site drop-down list is populated with - the editor's associated sites, otherwise all sites are displayed. +15. Verify that if the editor does not have permission 'User Accounts: View/Create/Edit User Accounts - All Sites' then the multi-select + site field in both edit user page and the filters for the data table are populated with the editor's associated sites, otherwise all sites are displayed. 16. Check that if the 'Additional user information' entry is set to false in the Configuration module, fields Degree, Academic Position, Institution, Department, Street Address, City, State/Province, Country and FAX are not shown. 17. Check that the 'Examiner At:' and 'Examiner Status' sections are available only if you have the 'Across all sites add and certify examiners'. From c0a2083c77f8584c76decb373d4e44da89c80cb7 Mon Sep 17 00:00:00 2001 From: CamilleBeau Date: Tue, 27 Feb 2024 16:52:29 -0500 Subject: [PATCH 3/6] fix --- .../php/useraccountrowprovisioner.class.inc | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/modules/user_accounts/php/useraccountrowprovisioner.class.inc b/modules/user_accounts/php/useraccountrowprovisioner.class.inc index f0cf20459fb..0ec61d74032 100644 --- a/modules/user_accounts/php/useraccountrowprovisioner.class.inc +++ b/modules/user_accounts/php/useraccountrowprovisioner.class.inc @@ -20,7 +20,14 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione // Get user $factory = \NDB_Factory::singleton(); $user = $factory->user(); - + if (!$user->hasPermission('user_accounts_multisite')) { + $siteLimitQuery = "LEFT JOIN user_psc_rel upsr2 + ON (upsr2.CenterID=upsr.CenterID) + WHERE upsr2.UserID=:uid"; + $params = ['uid' => $user->getId()]; + } else { + $params = []; + } parent::__construct( "SELECT GROUP_CONCAT(DISTINCT upsr.CenterID) as centerIds, GROUP_CONCAT(DISTINCT uprr.ProjectID) as projectIds, @@ -33,11 +40,10 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione FROM users u LEFT JOIN user_psc_rel upsr ON (upsr.UserID=u.ID) LEFT JOIN user_project_rel uprr ON (uprr.UserID=u.ID) - LEFT JOIN user_psc_rel upsr2 ON (upsr2.CenterID=upsr.CenterID) - WHERE upsr2.UserID=:uid + $siteLimitQuery GROUP BY u.ID ORDER BY u.UserID", - ['uid' => $user->getId()] + $params ); } From 8be0013bf874f7f572d4ef46a524b71667e1f031 Mon Sep 17 00:00:00 2001 From: CamilleBeau Date: Tue, 27 Feb 2024 16:57:47 -0500 Subject: [PATCH 4/6] Code cleanliness --- modules/user_accounts/php/useraccountrowprovisioner.class.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/user_accounts/php/useraccountrowprovisioner.class.inc b/modules/user_accounts/php/useraccountrowprovisioner.class.inc index 0ec61d74032..c5ea724c33b 100644 --- a/modules/user_accounts/php/useraccountrowprovisioner.class.inc +++ b/modules/user_accounts/php/useraccountrowprovisioner.class.inc @@ -24,7 +24,7 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione $siteLimitQuery = "LEFT JOIN user_psc_rel upsr2 ON (upsr2.CenterID=upsr.CenterID) WHERE upsr2.UserID=:uid"; - $params = ['uid' => $user->getId()]; + $params = ['uid' => $user->getId()]; } else { $params = []; } From 1fd754a46d38fbe03791d74477d17be4e9f1d3ab Mon Sep 17 00:00:00 2001 From: CamilleBeau Date: Fri, 1 Mar 2024 15:11:55 -0500 Subject: [PATCH 5/6] Add more user site functions & trying something with test --- .../user_accounts/php/user_accounts.class.inc | 13 ++------ .../php/useraccountrowprovisioner.class.inc | 3 +- .../user_accounts/test/user_accountsTest.php | 16 ++++++++++ php/libraries/User.class.inc | 31 +++++++++++++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/modules/user_accounts/php/user_accounts.class.inc b/modules/user_accounts/php/user_accounts.class.inc index f6b1e4daa39..f44bb18d253 100644 --- a/modules/user_accounts/php/user_accounts.class.inc +++ b/modules/user_accounts/php/user_accounts.class.inc @@ -82,16 +82,9 @@ class User_Accounts extends \DataFrameworkMenu $user = $factory->user(); // Without user_acounts_multisite permission, only show user sites - if ($user->hasPermission('user_accounts_multisite')) { - $sites = \Utility::getSiteList(false); - } else { - $sites = []; - foreach (\Utility::getSiteList(false) AS $centerID => $centerName) { - if ($user->hasCenter(new \CenterID($centerID))) { - $sites[$centerID] = $centerName; - } - } - } + $sites = $user->hasPermission('user_accounts_multisite') ? + \Utility::getSiteList(false) + : $user->getSiteNamesList(); return [ 'sites' => $sites, diff --git a/modules/user_accounts/php/useraccountrowprovisioner.class.inc b/modules/user_accounts/php/useraccountrowprovisioner.class.inc index c5ea724c33b..6f6ee895bc0 100644 --- a/modules/user_accounts/php/useraccountrowprovisioner.class.inc +++ b/modules/user_accounts/php/useraccountrowprovisioner.class.inc @@ -26,7 +26,8 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione WHERE upsr2.UserID=:uid"; $params = ['uid' => $user->getId()]; } else { - $params = []; + $siteLimitQuery = ''; + $params = []; } parent::__construct( "SELECT GROUP_CONCAT(DISTINCT upsr.CenterID) as centerIds, diff --git a/modules/user_accounts/test/user_accountsTest.php b/modules/user_accounts/test/user_accountsTest.php index cdb3753aeb7..b6d246480e7 100644 --- a/modules/user_accounts/test/user_accountsTest.php +++ b/modules/user_accounts/test/user_accountsTest.php @@ -60,6 +60,12 @@ public function setUp(): void { parent::setUp(); $password = new \Password($this->validPassword); + + $permID = $this->DB->pselectOne( + "SELECT permID FROM permissions WHERE code='user_accounts_multisite'", + [] + ); + $this->DB->insert( "users", [ @@ -93,6 +99,15 @@ public function setUp(): void 'ProjectID' => 1, ] ); + + $this->DB->insert( + "user_perm_rel", + [ + 'userID' => 999995, + 'permID' => $permID, + ] + ); + } /** @@ -506,6 +521,7 @@ function tearDown(): void $this->DB->delete("user_psc_rel", ["UserID" => 999995]); $this->DB->delete("user_project_rel", ["UserID" => 999995]); $this->DB->delete("users", ["UserID" => 'UnitTesterTwo']); + $this->DB->delete("user_perm_rel", ["userID" => 999995]); parent::tearDown(); } } diff --git a/php/libraries/User.class.inc b/php/libraries/User.class.inc index 46a5a3dba8b..a477d7d674d 100644 --- a/php/libraries/User.class.inc +++ b/php/libraries/User.class.inc @@ -351,6 +351,37 @@ class User extends UserPermissions implements return $site_list; } + /** + * Returns all user's sites + * + * @return array + */ + function getSites(): array + { + return array_map( + function ($centerID) { + return \Site::singleton($centerID); + }, + $this->getCenterIDs() + ); + } + + /** + * Returns all user's sites in associative + * array (CenterID => CenterName) + * + * @return array + */ + function getSiteNamesList(): array + { + $sites = []; + foreach ($this->getSites() as $site) { + $sites[$site->getCenterID()->__toString()] = $site->getCenterName(); + } + return $sites; + } + + /** * Returns all user's sites that are StudySites * From c40c2e7244b9543373ae4f29487a64a3ee388360 Mon Sep 17 00:00:00 2001 From: CamilleBeau Date: Tue, 12 Mar 2024 14:55:36 -0400 Subject: [PATCH 6/6] Get rid of user_accounts changes --- .../user_accounts/php/user_accounts.class.inc | 10 +--------- .../php/useraccountrowprovisioner.class.inc | 15 +-------------- modules/user_accounts/test/TestPlan.md | 4 ++-- modules/user_accounts/test/user_accountsTest.php | 16 ---------------- 4 files changed, 4 insertions(+), 41 deletions(-) diff --git a/modules/user_accounts/php/user_accounts.class.inc b/modules/user_accounts/php/user_accounts.class.inc index f44bb18d253..d4db5401c85 100644 --- a/modules/user_accounts/php/user_accounts.class.inc +++ b/modules/user_accounts/php/user_accounts.class.inc @@ -77,17 +77,9 @@ class User_Accounts extends \DataFrameworkMenu 'Y' => 'Yes', 'N' => 'No', ]; - // Get user - $factory = \NDB_Factory::singleton(); - $user = $factory->user(); - - // Without user_acounts_multisite permission, only show user sites - $sites = $user->hasPermission('user_accounts_multisite') ? - \Utility::getSiteList(false) - : $user->getSiteNamesList(); return [ - 'sites' => $sites, + 'sites' => \Utility::getSiteList(false), 'projects' => \Utility::getProjectList(), 'actives' => $yesNoOptions, 'pendingApprovals' => $yesNoOptions, diff --git a/modules/user_accounts/php/useraccountrowprovisioner.class.inc b/modules/user_accounts/php/useraccountrowprovisioner.class.inc index 6f6ee895bc0..382a70b8673 100644 --- a/modules/user_accounts/php/useraccountrowprovisioner.class.inc +++ b/modules/user_accounts/php/useraccountrowprovisioner.class.inc @@ -17,18 +17,6 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione */ function __construct() { - // Get user - $factory = \NDB_Factory::singleton(); - $user = $factory->user(); - if (!$user->hasPermission('user_accounts_multisite')) { - $siteLimitQuery = "LEFT JOIN user_psc_rel upsr2 - ON (upsr2.CenterID=upsr.CenterID) - WHERE upsr2.UserID=:uid"; - $params = ['uid' => $user->getId()]; - } else { - $siteLimitQuery = ''; - $params = []; - } parent::__construct( "SELECT GROUP_CONCAT(DISTINCT upsr.CenterID) as centerIds, GROUP_CONCAT(DISTINCT uprr.ProjectID) as projectIds, @@ -41,10 +29,9 @@ class UserAccountRowProvisioner extends \LORIS\Data\Provisioners\DBRowProvisione FROM users u LEFT JOIN user_psc_rel upsr ON (upsr.UserID=u.ID) LEFT JOIN user_project_rel uprr ON (uprr.UserID=u.ID) - $siteLimitQuery GROUP BY u.ID ORDER BY u.UserID", - $params + [] ); } diff --git a/modules/user_accounts/test/TestPlan.md b/modules/user_accounts/test/TestPlan.md index c1ba6680f9b..8083e718e51 100644 --- a/modules/user_accounts/test/TestPlan.md +++ b/modules/user_accounts/test/TestPlan.md @@ -41,8 +41,8 @@ When creating or editing a user: (subtest: edit_user) 13. Check that if generating a new password for a user an email is sent to that user containing the new password (requires email server). 14. Check that when editing a user account it is not possible to set the password to its actual value (i.e. it needs to change). [Automated] -15. Verify that if the editor does not have permission 'User Accounts: View/Create/Edit User Accounts - All Sites' then the multi-select - site field in both edit user page and the filters for the data table are populated with the editor's associated sites, otherwise all sites are displayed. +15. Verify that if the editor does not have permission 'Across all sites add and edit users' then the site drop-down list is populated with + the editor's associated sites, otherwise all sites are displayed. 16. Check that if the 'Additional user information' entry is set to false in the Configuration module, fields Degree, Academic Position, Institution, Department, Street Address, City, State/Province, Country and FAX are not shown. 17. Check that the 'Examiner At:' and 'Examiner Status' sections are available only if you have the 'Across all sites add and certify examiners'. diff --git a/modules/user_accounts/test/user_accountsTest.php b/modules/user_accounts/test/user_accountsTest.php index b6d246480e7..cdb3753aeb7 100644 --- a/modules/user_accounts/test/user_accountsTest.php +++ b/modules/user_accounts/test/user_accountsTest.php @@ -60,12 +60,6 @@ public function setUp(): void { parent::setUp(); $password = new \Password($this->validPassword); - - $permID = $this->DB->pselectOne( - "SELECT permID FROM permissions WHERE code='user_accounts_multisite'", - [] - ); - $this->DB->insert( "users", [ @@ -99,15 +93,6 @@ public function setUp(): void 'ProjectID' => 1, ] ); - - $this->DB->insert( - "user_perm_rel", - [ - 'userID' => 999995, - 'permID' => $permID, - ] - ); - } /** @@ -521,7 +506,6 @@ function tearDown(): void $this->DB->delete("user_psc_rel", ["UserID" => 999995]); $this->DB->delete("user_project_rel", ["UserID" => 999995]); $this->DB->delete("users", ["UserID" => 'UnitTesterTwo']); - $this->DB->delete("user_perm_rel", ["userID" => 999995]); parent::tearDown(); } }