From ee3547306e8e8a30f54a4335790b778de1dd2ae6 Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Wed, 2 Jun 2021 10:38:32 +0200 Subject: [PATCH] MDL-71134 course: fix format get_output_classname The get_output_classname method is used to invoke overridden output components from course_formats. Now the method does not accept null output names anymore, and it controls the format class is extending the core one. --- course/classes/course_format.php | 13 +++- course/tests/courseformat_test.php | 63 +++++++++++++++++-- ...est_output_course_format_invalidoutput.php | 43 +++++++++++++ 3 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 course/tests/fixtures/format_theunittest_output_course_format_invalidoutput.php diff --git a/course/classes/course_format.php b/course/classes/course_format.php index 1549e41e07d..7dd81e7b6bc 100644 --- a/course/classes/course_format.php +++ b/course/classes/course_format.php @@ -33,6 +33,7 @@ use section_info; use context_course; use editsection_form; use moodle_exception; +use coding_exception; use moodle_url; use lang_string; use completion_info; @@ -1074,16 +1075,24 @@ abstract class course_format { /** * Returns instance of output compornent used by this plugin * + * @throws coding_exception if the format class does not extends the original core one. * @param string $outputname the element to render (section, activity...) * @return string the output component classname */ - public function get_output_classname(?string $outputname): string { + public function get_output_classname(string $outputname): string { + // The core output class. + $baseclass = "core_course\\output\\$outputname"; + // Check if there is a specific format class. $component = 'format_'. $this->get_format(); $outputclass = "$component\\output\\$outputname"; if (class_exists($outputclass)) { + // Check that the outputclass is a subclass of the base class. + if (!is_subclass_of($outputclass, $baseclass)) { + throw new coding_exception("The \"$outputclass\" must extend \"$baseclass\""); + } return $outputclass; } - return "core_course\\output\\$outputname"; + return $baseclass; } /** diff --git a/course/tests/courseformat_test.php b/course/tests/courseformat_test.php index 15cb2d723c2..06890d165d9 100644 --- a/course/tests/courseformat_test.php +++ b/course/tests/courseformat_test.php @@ -24,11 +24,19 @@ defined('MOODLE_INTERNAL') || die(); -global $CFG; -require_once($CFG->dirroot . '/course/lib.php'); -require_once($CFG->dirroot . '/course/tests/fixtures/format_theunittest.php'); +class courseformat_test extends advanced_testcase { + + /** + * Setup to ensure that fixtures are loaded. + */ + public static function setupBeforeClass(): void { + global $CFG; + require_once($CFG->dirroot . '/course/lib.php'); + require_once($CFG->dirroot . '/course/tests/fixtures/format_theunittest.php'); + require_once($CFG->dirroot . '/course/tests/fixtures/format_theunittest_output_course_format_state.php'); + require_once($CFG->dirroot . '/course/tests/fixtures/format_theunittest_output_course_format_invalidoutput.php'); + } -class core_course_courseformat_testcase extends advanced_testcase { public function test_available_hook() { global $DB; $this->resetAfterTest(); @@ -200,6 +208,53 @@ class core_course_courseformat_testcase extends advanced_testcase { $this->assertNotEmpty($format->get_view_url(1, ['navigation' => 1])); $this->assertNotEmpty($format->get_view_url(0, ['navigation' => 1])); } + + /** + * Test for get_output_classname method. + * + * @dataProvider get_output_classname_provider + * @param string $find the class to find + * @param string $result the expected result classname + * @param bool $exception if the method will raise an exception + */ + public function test_get_output_classname($find, $result, $exception) { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(['format' => 'theunittest']); + $courseformat = course_get_format($course); + + if ($exception) { + $this->expectException(coding_exception::class); + } + + $courseclass = $courseformat->get_output_classname($find); + $this->assertEquals($result, $courseclass); + } + + /** + * Data provider for test_get_output_classname. + * + * @return array the testing scenarios + */ + public function get_output_classname_provider(): array { + return [ + 'overridden class' => [ + 'find' => 'course_format\\state', + 'result' => 'format_theunittest\\output\\course_format\\state', + 'exception' => false, + ], + 'original class' => [ + 'find' => 'section_format\\state', + 'result' => 'core_course\\output\\section_format\\state', + 'exception' => false, + ], + 'invalid overridden class' => [ + 'find' => 'course_format\\invalidoutput', + 'result' => '', + 'exception' => true, + ], + ]; + } } /** diff --git a/course/tests/fixtures/format_theunittest_output_course_format_invalidoutput.php b/course/tests/fixtures/format_theunittest_output_course_format_invalidoutput.php new file mode 100644 index 00000000000..945d347def8 --- /dev/null +++ b/course/tests/fixtures/format_theunittest_output_course_format_invalidoutput.php @@ -0,0 +1,43 @@ +. + +namespace format_theunittest\output\course_format; + +use renderable; +use templatable; +use stdClass; + +/** + * Fixture for an invalid output for testing get_output_classname. + * + * @package core_course + * @copyright 2021 Ferran Recio (ferran@moodle.com) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class invalidoutput implements renderable, templatable { + + /** + * Export some data. + * + * @param renderer_base $output typically, the renderer that's calling this function + * @return stdClass data context for a mustache template + */ + public function export_for_template(\renderer_base $output): stdClass { + return (object)[ + 'something' => 'invalid', + ]; + } +}