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.

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');
 

mod/data/db/install.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="mod/data/db" VERSION="20160303" COMMENT="XMLDB file for Moodle mod/data"
+<XMLDB PATH="mod/data/db" VERSION="20160906" COMMENT="XMLDB file for Moodle mod/data"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
 >
@@ -41,6 +41,7 @@
         <FIELD NAME="editany" TYPE="int" LENGTH="4" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
         <FIELD NAME="notification" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Notify people when things change"/>
         <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The time the settings for this database module instance were last modified."/>
+        <FIELD NAME="config" TYPE="text" NOTNULL="false" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>

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;
 }

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';

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]);
+    }
+}

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 '<table cellpadding="4" cellspacing="0" border="0">';
 
-/// Add the HTML editor(s).
-$usehtmleditor = ($mode != 'csstemplate') && ($mode != 'jstemplate') && !$disableeditor;
 if ($mode == 'listtemplate'){
     // Print the list template header.
     echo '<tr>';
@@ -298,11 +303,16 @@ if ($mode != 'csstemplate' and $mode != 'jstemplate') {
     echo '<br /><br /><br /><br /><input type="submit" name="defaultform" value="'.get_string('resettemplate','data').'" />';
     echo '<br /><br />';
     if ($usehtmleditor) {
-        $switcheditor = get_string('editordisable', 'data');
-        echo '<input type="submit" name="switcheditor" value="'.s($switcheditor).'" />';
+        $switchlink = new moodle_url($PAGE->url, ['useeditor' => false]);
+        echo html_writer::link($switchlink, get_string('editordisable', 'data'));
     } else {
-        $switcheditor = get_string('editorenable', 'data');
-        echo '<input type="submit" name="useeditor" value="'.s($switcheditor).'" />';
+        $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 '<br /><br /><br /><br /><input type="submit" name="defaultform" value="'.get_string('resettemplate','data').'" />';

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);
+    }
 }

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;