From 33d04612cda4998595c276abfe014fe507364ab1 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 29 Aug 2023 10:39:58 +0700 Subject: [PATCH] MDL-78509 tool_mfa: Improve PHPUnit for factor_cohort and factor_role --- .../tool/mfa/factor/cohort/classes/factor.php | 6 ++-- .../mfa/factor/cohort/tests/factor_test.php | 1 + admin/tool/mfa/factor/role/classes/factor.php | 20 +++++------ .../mfa/factor/role/tests/factor_test.php | 34 +++++++++++++++---- 4 files changed, 41 insertions(+), 20 deletions(-) diff --git a/admin/tool/mfa/factor/cohort/classes/factor.php b/admin/tool/mfa/factor/cohort/classes/factor.php index 7a28bb61fbb..d145a4c7308 100644 --- a/admin/tool/mfa/factor/cohort/classes/factor.php +++ b/admin/tool/mfa/factor/cohort/classes/factor.php @@ -136,12 +136,12 @@ class factor extends object_factor_base { } /** - * Get array of the selected cohorts. + * Get cohorts information by given ids. * - * @param array $selectedcohorts + * @param array $selectedcohorts List of cohort ids. * @return array */ - public function get_cohorts(array $selectedcohorts) : array { + public function get_cohorts(array $selectedcohorts): array { global $DB; [$insql, $inparams] = $DB->get_in_or_equal($selectedcohorts); diff --git a/admin/tool/mfa/factor/cohort/tests/factor_test.php b/admin/tool/mfa/factor/cohort/tests/factor_test.php index df4c851a5e2..08bf79df398 100644 --- a/admin/tool/mfa/factor/cohort/tests/factor_test.php +++ b/admin/tool/mfa/factor/cohort/tests/factor_test.php @@ -47,6 +47,7 @@ class factor_test extends \advanced_testcase { $selectedcohorts = get_config('factor_cohort', 'cohorts'); $selectedcohorts = $cohortfactor->get_cohorts(explode(',', $selectedcohorts)); + $this->assertArrayHasKey($cohort->id, $selectedcohorts); $this->assertStringContainsString( implode(', ', $selectedcohorts), $cohortfactor->get_summary_condition() diff --git a/admin/tool/mfa/factor/role/classes/factor.php b/admin/tool/mfa/factor/role/classes/factor.php index 63d8d72d7ba..017aa661e62 100644 --- a/admin/tool/mfa/factor/role/classes/factor.php +++ b/admin/tool/mfa/factor/role/classes/factor.php @@ -154,29 +154,27 @@ class factor extends object_factor_base { } /** - * Get array of the selected role name. + * Get roles information by given ids. * - * @param array $selectedroles + * @param array $selectedroles List of role ids. * @return array */ - public function get_roles(array $selectedroles) : array { + public function get_roles(array $selectedroles): array { global $DB; - $roles = []; // Checks for admin role and gets its role name. - if (($key = array_search('admin', $selectedroles)) !== false) { + if (in_array('admin', $selectedroles)) { $roles[] = get_string('administrator'); - unset($selectedroles[$key]); } + $integerroles = array_map('intval', $selectedroles); // Gets role name for all non admin roles. - if (count($selectedroles) > 0) { - [$insql, $inparams] = $DB->get_in_or_equal($selectedroles); + if (!empty($integerroles)) { + [$insql, $inparams] = $DB->get_in_or_equal($integerroles); $otherroles = $DB->get_records_select('role', 'id ' . $insql, $inparams); - foreach ($otherroles as $role) { - $roles[] = role_get_name($role); - } + $otherrolenames = role_fix_names($otherroles, null, ROLENAME_ALIAS, true); + $roles = array_merge($roles, $otherrolenames); } return $roles; diff --git a/admin/tool/mfa/factor/role/tests/factor_test.php b/admin/tool/mfa/factor/role/tests/factor_test.php index 66c243722e4..ec89a97aced 100644 --- a/admin/tool/mfa/factor/role/tests/factor_test.php +++ b/admin/tool/mfa/factor/role/tests/factor_test.php @@ -37,58 +37,80 @@ class factor_test extends \advanced_testcase { $this->resetAfterTest(); + $managerrole = $DB->get_record('role', ['shortname' => 'manager']); + $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']); + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $adminrolename = get_string('administrator'); + $managerrolename = role_get_name($managerrole); + $teacherrolename = role_get_name($teacherrole); + $studentrolename = role_get_name($studentrole); + set_config('enabled', 1, 'factor_role'); $rolefactor = \tool_mfa\plugininfo\factor::get_factor('role'); // Admin is disabled by default in this factor. $selectedroles = get_config('factor_role', 'roles'); $selectedroles = $rolefactor->get_roles(explode(',', $selectedroles)); + $this->assertContains($adminrolename, $selectedroles); + $this->assertNotContains($managerrolename, $selectedroles); + $this->assertNotContains($teacherrolename, $selectedroles); + $this->assertNotContains($studentrolename, $selectedroles); $this->assertStringContainsString( implode(', ', $selectedroles), $rolefactor->get_summary_condition() ); // Disabled role factor for managers. - $managerrole = $DB->get_record('role', ['shortname' => 'manager']); set_config('roles', $managerrole->id, 'factor_role'); $selectedroles = get_config('factor_role', 'roles'); $selectedroles = $rolefactor->get_roles(explode(',', $selectedroles)); + $this->assertNotContains($adminrolename, $selectedroles); + $this->assertContains($managerrolename, $selectedroles); + $this->assertNotContains($teacherrolename, $selectedroles); + $this->assertNotContains($studentrolename, $selectedroles); $this->assertStringContainsString( implode(', ', $selectedroles), $rolefactor->get_summary_condition() ); // Disabled role factor for teachers. - $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']); set_config('roles', $teacherrole->id, 'factor_role'); $selectedroles = get_config('factor_role', 'roles'); $selectedroles = $rolefactor->get_roles(explode(',', $selectedroles)); + $this->assertNotContains($adminrolename, $selectedroles); + $this->assertNotContains($managerrolename, $selectedroles); + $this->assertContains($teacherrolename, $selectedroles); + $this->assertNotContains($studentrolename, $selectedroles); $this->assertStringContainsString( implode(', ', $selectedroles), $rolefactor->get_summary_condition() ); // Disabled role factor for students. - $studentrole = $DB->get_record('role', ['shortname' => 'student']); set_config('roles', $studentrole->id, 'factor_role'); $selectedroles = get_config('factor_role', 'roles'); $selectedroles = $rolefactor->get_roles(explode(',', $selectedroles)); + $this->assertNotContains($adminrolename, $selectedroles); + $this->assertNotContains($managerrolename, $selectedroles); + $this->assertNotContains($teacherrolename, $selectedroles); + $this->assertContains($studentrolename, $selectedroles); $this->assertStringContainsString( implode(', ', $selectedroles), $rolefactor->get_summary_condition() ); // Disabled role factor for admins, managers, teachers and students. - $managerrole = $DB->get_record('role', ['shortname' => 'manager']); - $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']); - $studentrole = $DB->get_record('role', ['shortname' => 'student']); set_config('roles', "admin,$managerrole->id,$teacherrole->id,$studentrole->id", 'factor_role'); $selectedroles = get_config('factor_role', 'roles'); $selectedroles = $rolefactor->get_roles(explode(',', $selectedroles)); + $this->assertContains($adminrolename, $selectedroles); + $this->assertContains($managerrolename, $selectedroles); + $this->assertContains($teacherrolename, $selectedroles); + $this->assertContains($studentrolename, $selectedroles); $this->assertStringContainsString( implode(', ', $selectedroles), $rolefactor->get_summary_condition()