From 4e2e54eed07d4537e65e4790fee1f6a335109ed7 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 6 Sep 2016 12:37:44 +0800 Subject: [PATCH] MDL-55866 mod_data: Remember editor enabled state for templates Some templates can legitimately contain invalid HTML (e.g. mismatched tags) because the templates operate in pairs. In these instances we should not use an editor because the nature of the editor (content editable) means that the browser automatically corrects all HTML supplied to it, thus breaking the template entirely. Therefore we need to disable HTML editors for some templates, and do so in a way tied to the instance of the activity, rather than to a specific user. This patch adds a new 'config' field, with matching setters and getters, to allow such per-instance values to be stored. --- .../backup/moodle2/backup_data_stepslib.php | 2 +- mod/data/db/install.xml | 3 +- mod/data/db/upgrade.php | 15 ++ mod/data/lang/en/data.php | 3 +- mod/data/lib.php | 46 ++++ mod/data/templates.php | 62 ++--- mod/data/tests/lib_test.php | 213 ++++++++++++++++++ mod/data/version.php | 2 +- 8 files changed, 316 insertions(+), 30 deletions(-) diff --git a/mod/data/backup/moodle2/backup_data_stepslib.php b/mod/data/backup/moodle2/backup_data_stepslib.php index fd6eb520e74..45f85a8d5ec 100644 --- a/mod/data/backup/moodle2/backup_data_stepslib.php +++ b/mod/data/backup/moodle2/backup_data_stepslib.php @@ -45,7 +45,7 @@ class backup_data_activity_structure_step extends backup_activity_structure_step 'addtemplate', 'rsstemplate', 'rsstitletemplate', 'csstemplate', 'jstemplate', 'asearchtemplate', 'approval', 'manageapproved', 'scale', 'assessed', 'assesstimestart', 'assesstimefinish', 'defaultsort', - 'defaultsortdir', 'editany', 'notification', 'timemodified')); + 'defaultsortdir', 'editany', 'notification', 'timemodified', 'config')); $fields = new backup_nested_element('fields'); diff --git a/mod/data/db/install.xml b/mod/data/db/install.xml index e069bffeaf2..f3a45d3d986 100644 --- a/mod/data/db/install.xml +++ b/mod/data/db/install.xml @@ -1,5 +1,5 @@ - @@ -41,6 +41,7 @@ + diff --git a/mod/data/db/upgrade.php b/mod/data/db/upgrade.php index 27abdab14d9..65d5011a3d9 100644 --- a/mod/data/db/upgrade.php +++ b/mod/data/db/upgrade.php @@ -82,5 +82,20 @@ function xmldb_data_upgrade($oldversion) { // Moodle v3.1.0 release upgrade line. // Put any upgrade step following this. + if ($oldversion < 2016090600) { + + // Define field config to be added to data. + $table = new xmldb_table('data'); + $field = new xmldb_field('config', XMLDB_TYPE_TEXT, null, null, null, null, null, 'timemodified'); + + // Conditionally launch add field config. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Data savepoint reached. + upgrade_mod_savepoint(true, 2016090600, 'data'); + } + return true; } diff --git a/mod/data/lang/en/data.php b/mod/data/lang/en/data.php index 4b0e332b58b..e60e3796a27 100644 --- a/mod/data/lang/en/data.php +++ b/mod/data/lang/en/data.php @@ -124,6 +124,7 @@ $string['editordisable'] = 'Disable editor'; $string['editorenable'] = 'Enable editor'; $string['emptyadd'] = 'The Add template is empty, generating a default form...'; $string['emptyaddform'] = 'You did not fill out any fields!'; +$string['enabletemplateeditorcheck'] = 'Are you sure you want to enable the editor? This may result in content being altered when the template is saved.'; $string['eventfieldcreated'] = 'Field created'; $string['eventfielddeleted'] = 'Field deleted'; $string['eventfieldupdated'] = 'Field updated'; @@ -376,4 +377,4 @@ $string['namepicture'] = 'Picture field'; $string['nameradiobutton'] = 'Radio button field'; $string['nametext'] = 'Text field'; $string['nametextarea'] = 'Textarea field'; -$string['nameurl'] = 'URL field'; \ No newline at end of file +$string['nameurl'] = 'URL field'; diff --git a/mod/data/lib.php b/mod/data/lib.php index 01ddaa9cafb..d318a8e2bf6 100644 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -4045,3 +4045,49 @@ function data_refresh_events($courseid = 0) { } return true; } + +/** + * Fetch the configuration for this database activity. + * + * @param stdClass $database The object returned from the database for this instance + * @param string $key The name of the key to retrieve. If none is supplied, then all configuration is returned + * @param mixed $default The default value to use if no value was found for the specified key + * @return mixed The returned value + */ +function data_get_config($database, $key = null, $default = null) { + if (!empty($database->config)) { + $config = json_decode($database->config); + } else { + $config = new stdClass(); + } + + if ($key === null) { + return $config; + } + + if (property_exists($config, $key)) { + return $config->$key; + } + return $default; +} + +/** + * Update the configuration for this database activity. + * + * @param stdClass $database The object returned from the database for this instance + * @param string $key The name of the key to set + * @param mixed $value The value to set for the key + */ +function data_set_config(&$database, $key, $value) { + // Note: We must pass $database by reference because there may be subsequent calls to update_record and these should + // not overwrite the configuration just set. + global $DB; + + $config = data_get_config($database); + + if (!isset($config->$key) || $config->$key !== $value) { + $config->$key = $value; + $database->config = json_encode($config); + $DB->set_field('data', 'config', $database->config, ['id' => $database->id]); + } +} diff --git a/mod/data/templates.php b/mod/data/templates.php index 06d1a7b2829..77953162f84 100644 --- a/mod/data/templates.php +++ b/mod/data/templates.php @@ -29,8 +29,7 @@ require_once('lib.php'); $id = optional_param('id', 0, PARAM_INT); // course module id $d = optional_param('d', 0, PARAM_INT); // database id $mode = optional_param('mode', 'singletemplate', PARAM_ALPHA); -$disableeditor = optional_param('switcheditor', false, PARAM_RAW); -$enableeditor = optional_param('useeditor', false, PARAM_RAW); +$useeditor = optional_param('useeditor', null, PARAM_BOOL); $url = new moodle_url('/mod/data/templates.php'); if ($mode !== 'singletemplate') { @@ -69,6 +68,11 @@ require_login($course, false, $cm); $context = context_module::instance($cm->id); require_capability('mod/data:managetemplates', $context); +if ($useeditor !== null) { + // The useeditor param was set. Update the value for this template. + data_set_config($data, "editor_{$mode}", !!$useeditor); +} + if (!$DB->count_records('data_fields', array('dataid'=>$data->id))) { // Brand new database! redirect($CFG->wwwroot.'/mod/data/field.php?d='.$data->id); // Redirect to field entry } @@ -140,21 +144,18 @@ if (($mytemplate = data_submitted()) && confirm_sesskey()) { // Check for multiple tags, only need to check for add template. if ($mode != 'addtemplate' or data_tags_check($data->id, $newtemplate->{$mode})) { - // if disableeditor or enableeditor buttons click, don't save instance - if (empty($disableeditor) && empty($enableeditor)) { - $DB->update_record('data', $newtemplate); - echo $OUTPUT->notification(get_string('templatesaved', 'data'), 'notifysuccess'); + $DB->update_record('data', $newtemplate); + echo $OUTPUT->notification(get_string('templatesaved', 'data'), 'notifysuccess'); - // Trigger an event for saving the templates. - $event = \mod_data\event\template_updated::create(array( - 'context' => $context, - 'courseid' => $course->id, - 'other' => array( - 'dataid' => $data->id, - ) - )); - $event->trigger(); - } + // Trigger an event for saving the templates. + $event = \mod_data\event\template_updated::create(array( + 'context' => $context, + 'courseid' => $course->id, + 'other' => array( + 'dataid' => $data->id, + ) + )); + $event->trigger(); } } } else { @@ -172,15 +173,21 @@ if (empty($data->addtemplate) and empty($data->singletemplate) and } editors_head_setup(); -$format = FORMAT_HTML; -if ($mode === 'csstemplate' or $mode === 'jstemplate') { - $disableeditor = true; +// Determine whether to use HTML editors. +if (($mode === 'csstemplate') || ($mode === 'jstemplate')) { + // The CSS and JS templates aren't HTML. + $usehtmleditor = false; +} else { + $usehtmleditor = data_get_config($data, "editor_{$mode}", true); } -if ($disableeditor) { +if ($usehtmleditor) { + $format = FORMAT_HTML; +} else { $format = FORMAT_PLAIN; } + $editor = editors_get_preferred_editor($format); $strformats = format_text_menu(); $formats = $editor->get_supported_formats(); @@ -208,8 +215,6 @@ if (!$resettemplate) { echo $OUTPUT->box_start('generalbox boxaligncenter boxwidthwide'); echo ''; -/// Add the HTML editor(s). -$usehtmleditor = ($mode != 'csstemplate') && ($mode != 'jstemplate') && !$disableeditor; if ($mode == 'listtemplate'){ // Print the list template header. echo ''; @@ -298,11 +303,16 @@ if ($mode != 'csstemplate' and $mode != 'jstemplate') { echo '



'; echo '

'; if ($usehtmleditor) { - $switcheditor = get_string('editordisable', 'data'); - echo ''; + $switchlink = new moodle_url($PAGE->url, ['useeditor' => false]); + echo html_writer::link($switchlink, get_string('editordisable', 'data')); } else { - $switcheditor = get_string('editorenable', 'data'); - echo ''; + $switchlink = new moodle_url($PAGE->url, ['useeditor' => true]); + echo html_writer::link($switchlink, get_string('editorenable', 'data'), [ + 'id' => 'enabletemplateeditor', + ]); + $PAGE->requires->event_handler('#enabletemplateeditor', 'click', 'M.util.show_confirm_dialog', [ + 'message' => get_string('enabletemplateeditorcheck', 'data'), + ]); } } else { echo '



'; diff --git a/mod/data/tests/lib_test.php b/mod/data/tests/lib_test.php index c9fc50eb1c9..a6388305945 100644 --- a/mod/data/tests/lib_test.php +++ b/mod/data/tests/lib_test.php @@ -37,6 +37,23 @@ require_once($CFG->dirroot . '/mod/data/lib.php'); */ class mod_data_lib_testcase extends advanced_testcase { + /** + * @var moodle_database + */ + protected $DB = null; + + /** + * Tear Down to reset DB. + */ + public function tearDown() { + global $DB; + + if (isset($this->DB)) { + $DB = $this->DB; + $this->DB = null; + } + } + public function test_data_delete_record() { global $DB; @@ -666,4 +683,200 @@ class mod_data_lib_testcase extends advanced_testcase { } } } + + /** + * Data provider for tests of data_get_config. + * + * @return array + */ + public function data_get_config_provider() { + $initialdata = (object) [ + 'template_foo' => true, + 'template_bar' => false, + 'template_baz' => null, + ]; + + $database = (object) [ + 'config' => json_encode($initialdata), + ]; + + return [ + 'Return full dataset (no key/default)' => [ + [$database], + $initialdata, + ], + 'Return full dataset (no default)' => [ + [$database, null], + $initialdata, + ], + 'Return full dataset' => [ + [$database, null, null], + $initialdata, + ], + 'Return requested key only, value true, no default' => [ + [$database, 'template_foo'], + true, + ], + 'Return requested key only, value false, no default' => [ + [$database, 'template_bar'], + false, + ], + 'Return requested key only, value null, no default' => [ + [$database, 'template_baz'], + null, + ], + 'Return unknown key, value null, no default' => [ + [$database, 'template_bum'], + null, + ], + 'Return requested key only, value true, default null' => [ + [$database, 'template_foo', null], + true, + ], + 'Return requested key only, value false, default null' => [ + [$database, 'template_bar', null], + false, + ], + 'Return requested key only, value null, default null' => [ + [$database, 'template_baz', null], + null, + ], + 'Return unknown key, value null, default null' => [ + [$database, 'template_bum', null], + null, + ], + 'Return requested key only, value true, default 42' => [ + [$database, 'template_foo', 42], + true, + ], + 'Return requested key only, value false, default 42' => [ + [$database, 'template_bar', 42], + false, + ], + 'Return requested key only, value null, default 42' => [ + [$database, 'template_baz', 42], + null, + ], + 'Return unknown key, value null, default 42' => [ + [$database, 'template_bum', 42], + 42, + ], + ]; + } + + /** + * Tests for data_get_config. + * + * @dataProvider data_get_config_provider + * @param array $funcargs The args to pass to data_get_config + * @param mixed $expectation The expected value + */ + public function test_data_get_config($funcargs, $expectation) { + $this->assertEquals($expectation, call_user_func_array('data_get_config', $funcargs)); + } + + /** + * Data provider for tests of data_set_config. + * + * @return array + */ + public function data_set_config_provider() { + $basevalue = (object) ['id' => rand(1, 1000)]; + $config = [ + 'template_foo' => true, + 'template_bar' => false, + ]; + + $withvalues = clone $basevalue; + $withvalues->config = json_encode((object) $config); + + return [ + 'Empty config, New value' => [ + $basevalue, + 'etc', + 'newvalue', + true, + json_encode((object) ['etc' => 'newvalue']) + ], + 'Has config, New value' => [ + clone $withvalues, + 'etc', + 'newvalue', + true, + json_encode((object) array_merge($config, ['etc' => 'newvalue'])) + ], + 'Has config, Update value, string' => [ + clone $withvalues, + 'template_foo', + 'newvalue', + true, + json_encode((object) array_merge($config, ['template_foo' => 'newvalue'])) + ], + 'Has config, Update value, true' => [ + clone $withvalues, + 'template_bar', + true, + true, + json_encode((object) array_merge($config, ['template_bar' => true])) + ], + 'Has config, Update value, false' => [ + clone $withvalues, + 'template_foo', + false, + true, + json_encode((object) array_merge($config, ['template_foo' => false])) + ], + 'Has config, Update value, null' => [ + clone $withvalues, + 'template_foo', + null, + true, + json_encode((object) array_merge($config, ['template_foo' => null])) + ], + 'Has config, No update, value true' => [ + clone $withvalues, + 'template_foo', + true, + false, + $withvalues->config, + ], + ]; + } + + /** + * Tests for data_set_config. + * + * @dataProvider data_set_config_provider + * @param object $database The example row for the entry + * @param string $key The config key to set + * @param mixed $value The value of the key + * @param bool $expectupdate Whether we expected an update + * @param mixed $newconfigvalue The expected value + */ + public function test_data_set_config($database, $key, $value, $expectupdate, $newconfigvalue) { + global $DB; + + // Mock the database. + // Note: Use the actual test class here rather than the abstract because are testing concrete methods. + $this->DB = $DB; + $DB = $this->getMockBuilder(get_class($DB)) + ->setMethods(['set_field']) + ->getMock(); + + $DB->expects($this->exactly((int) $expectupdate)) + ->method('set_field') + ->with( + 'data', + 'config', + $newconfigvalue, + ['id' => $database->id] + ); + + // Perform the update. + data_set_config($database, $key, $value); + + // Ensure that the value was updated by reference in $database. + $config = json_decode($database->config); + $this->assertEquals($value, $config->$key); + } } diff --git a/mod/data/version.php b/mod/data/version.php index 5325b0a541e..17133a032b2 100644 --- a/mod/data/version.php +++ b/mod/data/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016071500; // The current module version (Date: YYYYMMDDXX) +$plugin->version = 2016090600; // The current module version (Date: YYYYMMDDXX) $plugin->requires = 2016051900; // Requires this Moodle version $plugin->component = 'mod_data'; // Full name of the plugin (used for diagnostics) $plugin->cron = 0;