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