From 7b3a6afa7c13ab77c353b30c50bdd7406b4c9d85 Mon Sep 17 00:00:00 2001 From: Renaat Debleu Date: Fri, 5 Apr 2024 07:07:39 +0000 Subject: [PATCH] code review --- classes/condition.php | 26 +++++++++---------- classes/frontend.php | 2 -- classes/privacy/provider.php | 1 - tests/advanced_test.php | 7 ++--- tests/basic_test.php | 1 - .../behat_availability_coursecompleted.php | 3 +-- tests/behat_test.php | 1 - tests/coverage.php | 8 ++++++ tests/privacy/privacy_test.php | 1 - 9 files changed, 26 insertions(+), 24 deletions(-) diff --git a/classes/condition.php b/classes/condition.php index 987392f..626d1d8 100755 --- a/classes/condition.php +++ b/classes/condition.php @@ -40,7 +40,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class condition extends \core_availability\condition { - /** @var string coursecompleted 0 => No, 1 => Yes */ protected $coursecompleted; @@ -56,7 +55,7 @@ public function __construct($structure) { } else if (is_string($structure->id)) { $this->coursecompleted = $structure->id; } else { - throw new coding_exception('Invalid value for course completed condition'); + new coding_exception('Invalid value for course completed condition'); } } @@ -134,7 +133,7 @@ public function get_description($full, $not, info $info) { * @return string Text representation of parameters */ protected function get_debug_string() { - return $this->coursecompleted ? '#' . 'True' : 'False'; + return get_string($this->coursecompleted ? 'true' : 'false', 'mod_quiz'); } /** @@ -162,7 +161,8 @@ public function filter_user_list( array $users, $not, \core_availability\info $info, - \core_availability\capability_checker $checker) { + \core_availability\capability_checker $checker + ) { global $DB; @@ -186,15 +186,15 @@ public function filter_user_list( // Always include users with access to completion report. if (array_key_exists($id, $adusers)) { $result[$id] = $user; - continue; - } - // Other users are included or not based on course completion. - $allow = array_key_exists($id, $compusers); - if ($not) { - $allow = !$allow; - } - if ($allow) { - $result[$id] = $user; + } else { + // Other users are included or not based on course completion. + $allow = array_key_exists($id, $compusers); + if ($not) { + $allow = !$allow; + } + if ($allow) { + $result[$id] = $user; + } } } return $result; diff --git a/classes/frontend.php b/classes/frontend.php index 959fa7b..03ac141 100755 --- a/classes/frontend.php +++ b/classes/frontend.php @@ -39,8 +39,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class frontend extends \core_availability\frontend { - - /** * Decides whether this plugin should be available in a given course. The * plugin can do this depending on course or system settings. diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index ad9b64a..4d08710 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -34,7 +34,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class provider implements \core_privacy\local\metadata\null_provider { - /** * Get the language string identifier with the component's language * file to explain why this plugin stores no data. diff --git a/tests/advanced_test.php b/tests/advanced_test.php index 0fb4c3b..a2bc019 100644 --- a/tests/advanced_test.php +++ b/tests/advanced_test.php @@ -41,7 +41,6 @@ * @coversDefaultClass \availability_coursecompleted */ final class advanced_test extends \advanced_testcase { - /** @var stdClass course. */ private $course; @@ -208,7 +207,9 @@ public function test_page(): void { $this->assertFalse($cond->is_available_for_all()); $this->assertFalse($cond->update_dependency_id(null, 1, 2)); $this->assertEquals($cond->__toString(), '{coursecompleted:False}'); - $this->assertEquals($cond->get_standalone_description(true, true, $info), - 'Not available unless: You completed this course.'); + $this->assertEquals( + $cond->get_standalone_description(true, true, $info), + 'Not available unless: You completed this course.' + ); } } diff --git a/tests/basic_test.php b/tests/basic_test.php index 6538738..0b2ca2d 100644 --- a/tests/basic_test.php +++ b/tests/basic_test.php @@ -41,7 +41,6 @@ * @coversDefaultClass \availability_coursecompleted */ final class basic_test extends \basic_testcase { - /** * Tests the constructor including error conditions. * @covers \availability_coursecompleted\condition diff --git a/tests/behat/behat_availability_coursecompleted.php b/tests/behat/behat_availability_coursecompleted.php index 5c56eec..297632c 100644 --- a/tests/behat/behat_availability_coursecompleted.php +++ b/tests/behat/behat_availability_coursecompleted.php @@ -30,7 +30,7 @@ require_once(__DIR__ . '/../../../../../lib/behat/behat_base.php'); // @codeCoverageIgnoreEnd -use Behat\Mink\Exception\ElementNotFoundException as ElementNotFoundException; +use Behat\Mink\Exception\ElementNotFoundException; /** * Step definitions related to mark user complete. @@ -41,7 +41,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class behat_availability_coursecompleted extends behat_base { - /** * Complete user in a course * @Then /^I mark course "(?P[^"]*)" completed for user "(?P[^"]*)"$/ diff --git a/tests/behat_test.php b/tests/behat_test.php index eb5cadc..ccd94d5 100644 --- a/tests/behat_test.php +++ b/tests/behat_test.php @@ -35,7 +35,6 @@ * @coversDefaultClass \availability_coursecompleted */ final class behat_test extends \advanced_testcase { - /** * Test behat funcs * @covers \behat_availability_coursecompleted diff --git a/tests/coverage.php b/tests/coverage.php index 380fb48..a46db72 100644 --- a/tests/coverage.php +++ b/tests/coverage.php @@ -25,6 +25,14 @@ defined('MOODLE_INTERNAL') || die(); +/** + * Unit tests for the coursecompleted condition. + * + * @package availability_coursecompleted + * @copyright 2022 iplusacademy (www.iplusacademy.org) + * @author Renaat Debleu + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ return new class extends phpunit_coverage_info { /** @var array The list of folders relative to the plugin root to include in coverage generation. */ protected $includelistfolders = ['classes']; diff --git a/tests/privacy/privacy_test.php b/tests/privacy/privacy_test.php index da24b64..f4bcbea 100644 --- a/tests/privacy/privacy_test.php +++ b/tests/privacy/privacy_test.php @@ -38,7 +38,6 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ final class privacy_test extends provider_testcase { - /** * Test returning metadata. * @covers \availability_coursecompleted\privacy\provider