diff --git a/h5p/classes/framework.php b/h5p/classes/framework.php index 4818d861224..26c1f5a1f10 100644 --- a/h5p/classes/framework.php +++ b/h5p/classes/framework.php @@ -1259,28 +1259,38 @@ class framework implements \H5PFrameworkInterface { } /** - * Get the default behaviour for the display option defined. + * Get stored setting. * Implements getOption. * * @param string $name Identifier for the setting * @param string $default Optional default value if settings is not set - * @return mixed Return The default \H5PDisplayOptionBehaviour for this display option + * @return mixed Return Whatever has been stored as the setting */ public function getOption($name, $default = false) { - // TODO: Define the default behaviour for each display option. - // For now, all them are disabled by default, so only will be rendered when defined in the displayoptions DB field. - return \H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF; + if ($name == core::DISPLAY_OPTION_DOWNLOAD || $name == core::DISPLAY_OPTION_EMBED) { + // For now, the download and the embed displayoptions are disabled by default, so only will be rendered when + // defined in the displayoptions DB field. + // This check should be removed if they are added as new H5P settings, to let admins to define the default value. + return \H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF; + } + + $value = get_config('core_h5p', $name); + if ($value === false) { + return $default; + } + return $value; } /** * Stores the given setting. + * For example when did we last check h5p.org for updates to our libraries. * Implements setOption. * * @param string $name Identifier for the setting * @param mixed $value Data Whatever we want to store as the setting */ public function setOption($name, $value) { - // Currently not storing settings. + set_config($name, $value, 'core_h5p'); } /** diff --git a/h5p/tests/framework_test.php b/h5p/tests/framework_test.php index 099844ba4e1..8f72d80b548 100644 --- a/h5p/tests/framework_test.php +++ b/h5p/tests/framework_test.php @@ -1739,6 +1739,57 @@ class framework_testcase extends \advanced_testcase { $this->assertEquals($expected, $dynamicdependencies); } + /** + * Test the behaviour of getOption(). + */ + public function test_getOption(): void { + $this->resetAfterTest(); + + // Get value for display_option_download. + $value = $this->framework->getOption(\H5PCore::DISPLAY_OPTION_DOWNLOAD); + $expected = \H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF; + $this->assertEquals($expected, $value); + + // Get value for display_option_embed using default value (it should be ignored). + $value = $this->framework->getOption(\H5PCore::DISPLAY_OPTION_EMBED, \H5PDisplayOptionBehaviour::NEVER_SHOW); + $expected = \H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF; + $this->assertEquals($expected, $value); + + // Get value for unexisting setting without default. + $value = $this->framework->getOption('unexistingsetting'); + $expected = false; + $this->assertEquals($expected, $value); + + // Get value for unexisting setting with default. + $value = $this->framework->getOption('unexistingsetting', 'defaultvalue'); + $expected = 'defaultvalue'; + $this->assertEquals($expected, $value); + } + + /** + * Test the behaviour of setOption(). + */ + public function test_setOption(): void { + $this->resetAfterTest(); + + // Set value for 'newsetting' setting. + $name = 'newsetting'; + $value = $this->framework->getOption($name); + $this->assertEquals(false, $value); + $newvalue = 'value1'; + $this->framework->setOption($name, $newvalue); + $value = $this->framework->getOption($name); + $this->assertEquals($newvalue, $value); + + // Set value for display_option_download and then get it again. Check it hasn't changed. + $name = \H5PCore::DISPLAY_OPTION_DOWNLOAD; + $newvalue = \H5PDisplayOptionBehaviour::NEVER_SHOW; + $this->framework->setOption($name, $newvalue); + $value = $this->framework->getOption($name); + $expected = \H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF; + $this->assertEquals($expected, $value); + } + /** * Test the behaviour of updateContentFields(). */