filters: MDL-18879 filter data not deleted when a context is deleted

Missed when implementing MDL-7336

Also, fix minor issues with deleting filters

diff --git a/admin/filters.php b/admin/filters.php
index d5d35fe6649fd1f704ae53a0e77b56a8a03a511b..f7de2823e735ba4865c91638eabc5bfe1135bfa4 100644 (file)
--- a/admin/filters.php
+++ b/admin/filters.php
@@ -112,7 +112,7 @@
             $a = new stdClass;
             $a->filter = $filtername;
             $a->module = get_string('modulename', $mod);
-            print_error('cannotdeletemodfilter', 'admin', admin_url('qtypes.php'), $a);
+            print_error('cannotdeletemodfilter', 'admin', $returnurl, $a);
         }
 
         // If not yet confirmed, display a confirmation message.
@@ -120,8 +120,8 @@
             $title = get_string('deletefilterareyousure', 'admin', $filtername);
             admin_externalpage_print_header();
             print_heading($title);
-            notice_yesno(get_string('deletefilterareyousuremessage', 'admin', $filtername), $CFG->wwwroot . '/' . $CFG->admin .
-                    '/filters.php?action=delete&filterpath=' . $filterpath . '&confirm=1&sesskey=' . sesskey(),
+            notice_yesno(get_string('deletefilterareyousuremessage', 'admin', $filtername), $returnurl . 
+                    '?action=delete&filterpath=' . $filterpath . '&confirm=1&sesskey=' . sesskey(),
                     $returnurl, NULL, NULL, 'post', 'get');
             admin_externalpage_print_footer();
             exit;
@@ -133,7 +133,7 @@
         print_heading($title);
 
         // Delete all data for this plugin.
-        filter_delete_all_data($filterpath);
+        filter_delete_all_for_filter($filterpath);
 
         $a = new stdClass;
         $a->filter = $filtername;
@@ -274,15 +274,19 @@ function get_table_row($filterinfo, $isfirstrow, $islastactive, $applytostrings)
             $filterinfo->active == TEXTFILTER_DISABLED);
 
     // Settings link, if required
-    $settings = '';
     if (filter_has_global_settings($filter)) {
-        $settings = '<a href="' . $CFG->wwwroot . '/' . $CFG->admin . '/settings.php?section=filtersetting' .
+        $row[] = '<a href="' . $CFG->wwwroot . '/' . $CFG->admin . '/settings.php?section=filtersetting' .
                 str_replace('/', '',$filter) . '">' . get_string('settings') . '</a>';
+    } else {
+        $row[] = '';
     }
-    $row[] = $settings;
 
     // Delete
-    $row[] = '<a href="' . action_url($filter, 'delete') . '">' . get_string('delete') . '</a>';
+    if (substr($filter, 0, 4) != 'mod/') {
+        $row[] = '<a href="' . action_url($filter, 'delete') . '">' . get_string('delete') . '</a>';
+    } else {
+        $row[] = '';
+    }
 
     return $row;
 }

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 17b37cb4e4f9f264b00b2d3e957fae6e3f8559a3..21125fa93280bc02daf8dba7d7e56edd3ab615aa 100755 (executable)
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -2236,6 +2236,8 @@ function delete_context($contextlevel, $instanceid) {
         unset($ACCESSLIB_PRIVATE->contexts[$contextlevel][$instanceid]);
         unset($ACCESSLIB_PRIVATE->contextsbyid[$context->id]);
 
+        filter_delete_all_for_context($context->id);
+
         return $result;
     } else {
 

diff --git a/lib/filterlib.php b/lib/filterlib.php
index 02a530cd39e731c906ff78aa18334a1c3f0b3f48..c37f26cc80595d13952d696047f50985f5f00fc3 100644 (file)
--- a/lib/filterlib.php
+++ b/lib/filterlib.php
@@ -746,7 +746,7 @@ function filter_get_global_states() {
  * Delete all the data in the database relating to a filter, prior to deleting it.
  * @param string $filter The filter name, for example 'filter/tex' or 'mod/glossary'.
  */
-function filter_delete_all_data($filter) {
+function filter_delete_all_for_filter($filter) {
     global $DB;
     if (substr($filter, 0, 7) == 'filter/') {
         unset_all_config_for_plugin('filter_' . basename($filter));
@@ -755,6 +755,16 @@ function filter_delete_all_data($filter) {
     $DB->delete_records('filter_config', array('filter' => $filter));
 }
 
+/**
+ * Delete all the data in the database relating to a context, used when contexts are deleted.
+ * @param integer $contextid The id of the context being deleted.
+ */
+function filter_delete_all_for_context($contextid) {
+    global $DB;
+    $DB->delete_records('filter_active', array('contextid' => $contextid));
+    $DB->delete_records('filter_config', array('contextid' => $contextid));
+}
+
 /**
  * Does this filter have a global settings page in the admin tree?
  * (The settings page for a filter must be called, for example,

diff --git a/lib/simpletest/testfilterconfig.php b/lib/simpletest/testfilterconfig.php
index 7f14440bd6f11092d02acc94799d8834772f1544..fac66374e3746c27b25ca8868e99d679a39814f5 100644 (file)
--- a/lib/simpletest/testfilterconfig.php
+++ b/lib/simpletest/testfilterconfig.php
@@ -547,26 +547,29 @@ class filter_get_active_available_in_context_test extends UnitTestCaseUsingDatab
     }
 }
 
-class filter_delete_all_data_test extends UnitTestCaseUsingDatabase {
+class filter_delete_config_test extends UnitTestCaseUsingDatabase {
+    protected $syscontext;
+
     public function setUp() {
         parent::setUp();
 
+        $this->syscontext = get_context_instance(CONTEXT_SYSTEM);
+
         // Create the table we need and switch to test DB.
         $this->create_test_tables(array('filter_active', 'filter_config', 'config', 'config_plugins'), 'lib');
         $this->switch_to_test_db();
     }
 
-    public function test_filter_delete_all_data_filter() {
+    public function test_filter_delete_all_for_filter() {
         // Setup fixture.
-        $syscontext = get_context_instance(CONTEXT_SYSTEM);
         filter_set_global_state('filter/name', TEXTFILTER_ON);
         filter_set_global_state('filter/other', TEXTFILTER_ON);
-        filter_set_local_config('filter/name', $syscontext->id, 'settingname', 'A value');
-        filter_set_local_config('filter/other', $syscontext->id, 'settingname', 'Other value');
+        filter_set_local_config('filter/name', $this->syscontext->id, 'settingname', 'A value');
+        filter_set_local_config('filter/other', $this->syscontext->id, 'settingname', 'Other value');
         set_config('configname', 'A config value', 'filter_name');
         set_config('configname', 'Other config value', 'filter_other');
         // Exercise SUT.
-        filter_delete_all_data('filter/name');
+        filter_delete_all_for_filter('filter/name');
         // Validate.
         $this->assertEqual(1, $this->testdb->count_records('filter_active'));
         $this->assertTrue($this->testdb->record_exists('filter_active', array('filter' => 'filter/other')));
@@ -577,6 +580,22 @@ class filter_delete_all_data_test extends UnitTestCaseUsingDatabase {
         $this->assertEqual($expectedconfig, get_config('filter_other'));
         $this->assertFalse(get_config('filter_name'));
     }
+
+    public function test_filter_delete_all_for_context() {
+        // Setup fixture.
+        filter_set_global_state('filter/name', TEXTFILTER_ON);
+        filter_set_local_state('filter/name', 123, TEXTFILTER_OFF);
+        filter_set_local_config('filter/name', 123, 'settingname', 'A value');
+        filter_set_local_config('filter/other', 123, 'settingname', 'Other value');
+        filter_set_local_config('filter/other', 122, 'settingname', 'Other value');
+        // Exercise SUT.
+        filter_delete_all_for_context(123);
+        // Validate.
+        $this->assertEqual(1, $this->testdb->count_records('filter_active'));
+        $this->assertTrue($this->testdb->record_exists('filter_active', array('contextid' => $this->syscontext->id)));
+        $this->assertEqual(1, $this->testdb->count_records('filter_config'));
+        $this->assertTrue($this->testdb->record_exists('filter_config', array('filter' => 'filter/other')));
+    }
 }
 
 class filter_filter_set_applies_to_strings extends UnitTestCaseUsingDatabase {