MDL-59159 upgrade: remove unused functions/settings from upgradelib
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 4 Dec 2017 00:36:57 +0000 (01:36 +0100)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 4 Dec 2017 00:42:01 +0000 (01:42 +0100)
All these functions were used only by deleted upgrade steps
so it's safe to proceed with straight deletion, considering
them internal. Deletion has been documented in corresponding
upgrade.txt files:

- mod_feedback_upgrade_delete_duplicate_values()
- mod_feedback_upgrade_courseid()

These have been kept because they continue being used:

- @ install: make_competence_scale()
- @ restore: upgrade_course_letter_boundary()

mod/feedback/db/upgrade.php
mod/feedback/db/upgradelib.php [deleted file]
mod/feedback/tests/upgradelib_test.php [deleted file]
mod/feedback/upgrade.txt

index 6687355..46e9696 100644 (file)
@@ -38,7 +38,6 @@ defined('MOODLE_INTERNAL') || die();
 
 function xmldb_feedback_upgrade($oldversion) {
     global $CFG, $DB;
-    require_once($CFG->dirroot . '/mod/feedback/db/upgradelib.php');
 
     $dbman = $DB->get_manager(); // Loads ddl manager and xmldb classes.
 
diff --git a/mod/feedback/db/upgradelib.php b/mod/feedback/db/upgradelib.php
deleted file mode 100644 (file)
index 35fe93f..0000000
+++ /dev/null
@@ -1,89 +0,0 @@
-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * Upgrade helper functions
- *
- * @package   mod_feedback
- * @copyright 2016 Marina Glancy
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-defined('MOODLE_INTERNAL') || die();
-
-/**
- * Fill new field courseid in tables feedback_completed or feedback_completedtmp
- *
- * @param bool $tmp use for temporary table
- */
-function mod_feedback_upgrade_courseid($tmp = false) {
-    global $DB;
-    $suffix = $tmp ? 'tmp' : '';
-
-    // Part 1. Ensure that each completed record has associated values with only one courseid.
-    $sql = "SELECT c.id
-        FROM {feedback_completed$suffix} c, {feedback_value$suffix} v
-        WHERE c.id = v.completed
-        GROUP by c.id
-        having count(DISTINCT v.course_id) > 1";
-    $problems = $DB->get_fieldset_sql($sql);
-    foreach ($problems as $problem) {
-        $courses = $DB->get_fieldset_sql("SELECT DISTINCT course_id "
-                . "FROM {feedback_value$suffix} WHERE completed = ?", array($problem));
-        $firstcourse = array_shift($courses);
-        $record = $DB->get_record('feedback_completed'.$suffix, array('id' => $problem));
-        unset($record->id);
-        $DB->update_record('feedback_completed'.$suffix, ['id' => $problem, 'courseid' => $firstcourse]);
-        foreach ($courses as $courseid) {
-            $record->courseid = $courseid;
-            $completedid = $DB->insert_record('feedback_completed'.$suffix, $record);
-            $DB->execute("UPDATE {feedback_value$suffix} SET completed = ? WHERE completed = ? AND course_id = ?",
-                    array($completedid, $problem, $courseid));
-        }
-    }
-
-    // Part 2. Update courseid in the completed table.
-    if ($DB->get_dbfamily() !== 'mysql') {
-        $sql = "UPDATE {feedback_completed$suffix} "
-            . "SET courseid = (SELECT COALESCE(MIN(v.course_id), 0) "
-            . "FROM {feedback_value$suffix} v "
-            . "WHERE v.completed = {feedback_completed$suffix}.id)";
-        $DB->execute($sql);
-    } else {
-        $sql = "UPDATE {feedback_completed$suffix} c, {feedback_value$suffix} v "
-            . "SET c.courseid = v.course_id "
-            . "WHERE v.completed = c.id AND v.course_id <> 0";
-        $DB->execute($sql);
-    }
-}
-
-/**
- * Ensure tables feedback_value and feedback_valuetmp have unique entries for each pair (completed,item).
- *
- * @param bool $tmp use for temporary table
- */
-function mod_feedback_upgrade_delete_duplicate_values($tmp = false) {
-    global $DB;
-    $suffix = $tmp ? 'tmp' : '';
-
-    $sql = "SELECT MIN(id) AS id, completed, item, course_id " .
-            "FROM {feedback_value$suffix} GROUP BY completed, item, course_id HAVING count(id)>1";
-    $records = $DB->get_records_sql($sql);
-    foreach ($records as $record) {
-        $DB->delete_records_select("feedback_value$suffix",
-            "completed = :completed AND item = :item AND course_id = :course_id AND id > :id", (array)$record);
-    }
-}
diff --git a/mod/feedback/tests/upgradelib_test.php b/mod/feedback/tests/upgradelib_test.php
deleted file mode 100644 (file)
index b4a470a..0000000
+++ /dev/null
@@ -1,301 +0,0 @@
-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * Tests for functions in db/upgradelib.php
- *
- * @package   mod_feedback
- * @copyright 2016 Marina Glancy
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-defined('MOODLE_INTERNAL') || die();
-
-global $CFG;
-require_once($CFG->dirroot . '/mod/feedback/db/upgradelib.php');
-
-/**
- * Tests for functions in db/upgradelib.php
- *
- * @package    mod_feedback
- * @copyright  2016 Marina Glancy
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-class mod_feedback_upgradelib_testcase extends advanced_testcase {
-
-    /** @var string  */
-    protected $testsql = "SELECT COUNT(v.id) FROM {feedback_completed} c, {feedback_value} v
-            WHERE c.id = v.completed AND c.courseid <> v.course_id";
-    /** @var string  */
-    protected $testsqltmp = "SELECT COUNT(v.id) FROM {feedback_completedtmp} c, {feedback_valuetmp} v
-            WHERE c.id = v.completed AND c.courseid <> v.course_id";
-    /** @var int */
-    protected $course1;
-    /** @var int */
-    protected $course2;
-    /** @var stdClass */
-    protected $feedback;
-    /** @var stdClass */
-    protected $user;
-
-    /**
-     * Sets up the fixture
-     * This method is called before a test is executed.
-     */
-    public function setUp() {
-        parent::setUp();
-        $this->resetAfterTest(true);
-
-        $this->course1 = $this->getDataGenerator()->create_course();
-        $this->course2 = $this->getDataGenerator()->create_course();
-        $this->feedback = $this->getDataGenerator()->create_module('feedback', array('course' => SITEID));
-
-        $this->user = $this->getDataGenerator()->create_user();
-    }
-
-    public function test_upgrade_courseid_completed() {
-        global $DB;
-
-        // Case 1. No errors in the data.
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 2, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // We have errors!
-        mod_feedback_upgrade_courseid(true); // Running script for temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // Nothing changed.
-        mod_feedback_upgrade_courseid();
-        $this->assertCount(1, $DB->get_records('feedback_completed')); // Number of records is the same.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsql)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_completed_with_errors() {
-        global $DB;
-
-        // Case 2. Errors in data (same feedback_completed has values for different courses).
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-            'item' => 1, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // We have errors!
-        mod_feedback_upgrade_courseid(true); // Running script for temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // Nothing changed.
-        mod_feedback_upgrade_courseid();
-        $this->assertCount(2, $DB->get_records('feedback_completed')); // Extra record inserted.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsql)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_completedtmp() {
-        global $DB;
-
-        // Case 1. No errors in the data.
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 2, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // We have errors!
-        mod_feedback_upgrade_courseid(); // Running script for non-temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // Nothing changed.
-        mod_feedback_upgrade_courseid(true);
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp')); // Number of records is the same.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsqltmp)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_completedtmp_with_errors() {
-        global $DB;
-
-        // Case 2. Errors in data (same feedback_completed has values for different courses).
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-            'item' => 1, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // We have errors!
-        mod_feedback_upgrade_courseid(); // Running script for non-temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // Nothing changed.
-        mod_feedback_upgrade_courseid(true);
-        $this->assertCount(2, $DB->get_records('feedback_completedtmp')); // Extra record inserted.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsqltmp)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_empty_completed() {
-        global $DB;
-
-        // Record in 'feedback_completed' does not have corresponding values.
-        $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $record1 = $DB->get_record('feedback_completed', []);
-        mod_feedback_upgrade_courseid();
-        $this->assertCount(1, $DB->get_records('feedback_completed')); // Number of records is the same.
-        $record2 = $DB->get_record('feedback_completed', []);
-        $this->assertEquals($record1, $record2);
-    }
-
-    public function test_upgrade_remove_duplicates_no_duplicates() {
-        global $DB;
-
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 2, 'value' => 2]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value'));
-        mod_feedback_upgrade_delete_duplicate_values();
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value')); // Same number of records, no changes made.
-    }
-
-    public function test_upgrade_remove_duplicates() {
-        global $DB;
-
-        // Remove the index that was added in the upgrade.php AFTER running mod_feedback_upgrade_delete_duplicate_values().
-        $dbman = $DB->get_manager();
-        $table = new xmldb_table('feedback_value');
-        $index = new xmldb_index('completed_item', XMLDB_INDEX_UNIQUE, array('completed', 'item', 'course_id'));
-        $dbman->drop_index($table, $index);
-
-        // Insert duplicated values.
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 2]); // This is a duplicate with another value.
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]); // This is not a duplicate because course id is different.
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value'));
-        mod_feedback_upgrade_delete_duplicate_values(true); // Running script for temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value')); // Nothing changed.
-        mod_feedback_upgrade_delete_duplicate_values();
-        $this->assertCount(1, $DB->get_records('feedback_completed')); // Number of records is the same.
-        $this->assertEquals(3, $DB->count_records('feedback_value')); // Duplicate was deleted.
-        $this->assertEquals(1, $DB->get_field('feedback_value', 'value', ['item' => 1]));
-
-        $dbman->add_index($table, $index);
-    }
-
-    public function test_upgrade_remove_duplicates_no_duplicates_tmp() {
-        global $DB;
-
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 2, 'value' => 2]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp'));
-        mod_feedback_upgrade_delete_duplicate_values(true);
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp')); // Same number of records, no changes made.
-    }
-
-    public function test_upgrade_remove_duplicates_tmp() {
-        global $DB;
-
-        // Remove the index that was added in the upgrade.php AFTER running mod_feedback_upgrade_delete_duplicate_values().
-        $dbman = $DB->get_manager();
-        $table = new xmldb_table('feedback_valuetmp');
-        $index = new xmldb_index('completed_item', XMLDB_INDEX_UNIQUE, array('completed', 'item', 'course_id'));
-        $dbman->drop_index($table, $index);
-
-        // Insert duplicated values.
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 2]); // This is a duplicate with another value.
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]); // This is not a duplicate because course id is different.
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp'));
-        mod_feedback_upgrade_delete_duplicate_values(); // Running script for non-temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp')); // Nothing changed.
-        mod_feedback_upgrade_delete_duplicate_values(true);
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp')); // Number of records is the same.
-        $this->assertEquals(3, $DB->count_records('feedback_valuetmp')); // Duplicate was deleted.
-        $this->assertEquals(1, $DB->get_field('feedback_valuetmp', 'value', ['item' => 1]));
-
-        $dbman->add_index($table, $index);
-    }
-}
\ No newline at end of file
index bba064b..f5d4733 100644 (file)
@@ -1,3 +1,10 @@
+=== 3.5 ===
+
+* The following functions, previously used (exclusively) by upgrade steps are not available
+  anymore because of the upgrade cleanup performed for this version. See MDL-59159 for more info:
+    - mod_feedback_upgrade_delete_duplicate_values()
+    - mod_feedback_upgrade_courseid()
+
 === 3.3.2 ===
 
 * feedback_refresh_events() Now takes two additional parameters to refine the update to a specific instance. This function