From 7ec475e2d7053c8d29c9d7db472862d413cdfdad Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Mon, 4 Dec 2017 01:36:57 +0100 Subject: [PATCH] MDL-59159 upgrade: remove unused functions/settings from upgradelib 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 | 1 - mod/feedback/db/upgradelib.php | 89 -------- mod/feedback/tests/upgradelib_test.php | 301 ------------------------- mod/feedback/upgrade.txt | 7 + 4 files changed, 7 insertions(+), 391 deletions(-) delete mode 100644 mod/feedback/db/upgradelib.php delete mode 100644 mod/feedback/tests/upgradelib_test.php diff --git a/mod/feedback/db/upgrade.php b/mod/feedback/db/upgrade.php index 66873554268..46e96964048 100644 --- a/mod/feedback/db/upgrade.php +++ b/mod/feedback/db/upgrade.php @@ -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 index 35fe93f9363..00000000000 --- a/mod/feedback/db/upgradelib.php +++ /dev/null @@ -1,89 +0,0 @@ -. - -/** - * 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 index b4a470ac4f7..00000000000 --- a/mod/feedback/tests/upgradelib_test.php +++ /dev/null @@ -1,301 +0,0 @@ -. - -/** - * 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 diff --git a/mod/feedback/upgrade.txt b/mod/feedback/upgrade.txt index bba064bd949..f5d47331144 100644 --- a/mod/feedback/upgrade.txt +++ b/mod/feedback/upgrade.txt @@ -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