From 8e70465aa8074dbfa5f1984f9e80d740b030e11c Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Wed, 11 May 2016 13:38:01 +0800 Subject: [PATCH] MDL-54108 mod_feedback: add unique index on values for completed items --- mod/feedback/db/install.xml | 4 +- mod/feedback/db/upgrade.php | 44 +++++++-- mod/feedback/db/upgradelib.php | 18 ++++ mod/feedback/tests/upgradelib_test.php | 130 ++++++++++++++++++++++++- mod/feedback/version.php | 2 +- 5 files changed, 188 insertions(+), 10 deletions(-) diff --git a/mod/feedback/db/install.xml b/mod/feedback/db/install.xml index 8c810b07fb9..ab8e56eddfd 100644 --- a/mod/feedback/db/install.xml +++ b/mod/feedback/db/install.xml @@ -1,5 +1,5 @@ - @@ -119,6 +119,7 @@ + @@ -136,6 +137,7 @@ +
diff --git a/mod/feedback/db/upgrade.php b/mod/feedback/db/upgrade.php index 9ca3d7b51fd..4d6d36548bb 100644 --- a/mod/feedback/db/upgrade.php +++ b/mod/feedback/db/upgrade.php @@ -75,7 +75,39 @@ function xmldb_feedback_upgrade($oldversion) { upgrade_mod_savepoint(true, 2016040100, 'feedback'); } - if ($oldversion < 2016040300) { + if ($oldversion < 2016051103) { + + // Define index completed_item (unique) to be added to feedback_value. + $table = new xmldb_table('feedback_value'); + $index = new xmldb_index('completed_item', XMLDB_INDEX_UNIQUE, array('completed', 'item', 'course_id')); + + // Conditionally launch add index completed_item. + if (!$dbman->index_exists($table, $index)) { + mod_feedback_upgrade_delete_duplicate_values(); + $dbman->add_index($table, $index); + } + + // Feedback savepoint reached. + upgrade_mod_savepoint(true, 2016051103, 'feedback'); + } + + if ($oldversion < 2016051104) { + + // Define index completed_item (unique) to be added to feedback_valuetmp. + $table = new xmldb_table('feedback_valuetmp'); + $index = new xmldb_index('completed_item', XMLDB_INDEX_UNIQUE, array('completed', 'item', 'course_id')); + + // Conditionally launch add index completed_item. + if (!$dbman->index_exists($table, $index)) { + mod_feedback_upgrade_delete_duplicate_values(true); + $dbman->add_index($table, $index); + } + + // Feedback savepoint reached. + upgrade_mod_savepoint(true, 2016051104, 'feedback'); + } + + if ($oldversion < 2016051105) { // Define field courseid to be added to feedback_completed. $table = new xmldb_table('feedback_completed'); @@ -84,6 +116,8 @@ function xmldb_feedback_upgrade($oldversion) { // Conditionally launch add field courseid. if (!$dbman->field_exists($table, $field)) { $dbman->add_field($table, $field); + // Run upgrade script to fill the new field courseid with the data from feedback_value table. + mod_feedback_upgrade_courseid(false); } // Define field courseid to be added to feedback_completedtmp. @@ -93,6 +127,8 @@ function xmldb_feedback_upgrade($oldversion) { // Conditionally launch add field courseid. if (!$dbman->field_exists($table, $field)) { $dbman->add_field($table, $field); + // Run upgrade script to fill the new field courseid with the data from feedback_valuetmp table. + mod_feedback_upgrade_courseid(true); } // Define table feedback_tracking to be dropped. @@ -103,12 +139,8 @@ function xmldb_feedback_upgrade($oldversion) { $dbman->drop_table($table); } - // Run upgrade script to fill the new field courseid with the data from feedback_value* tables. - mod_feedback_upgrade_courseid(false); - mod_feedback_upgrade_courseid(true); - // Feedback savepoint reached. - upgrade_mod_savepoint(true, 2016040300, 'feedback'); + upgrade_mod_savepoint(true, 2016051105, 'feedback'); } return true; diff --git a/mod/feedback/db/upgradelib.php b/mod/feedback/db/upgradelib.php index bff41339dbb..d0716c4a70a 100644 --- a/mod/feedback/db/upgradelib.php +++ b/mod/feedback/db/upgradelib.php @@ -62,3 +62,21 @@ function mod_feedback_upgrade_courseid($tmp = false) { . "WHERE v.completed = {feedback_completed$suffix}.id)"; $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 index de395420c51..b4a470ac4f7 100644 --- a/mod/feedback/tests/upgradelib_test.php +++ b/mod/feedback/tests/upgradelib_test.php @@ -77,7 +77,7 @@ class mod_feedback_upgradelib_testcase extends advanced_testcase { 'item' => 1, 'value' => 1]); $DB->insert_record('feedback_value', ['completed' => $completed1, 'course_id' => $this->course1->id, - 'item' => 1, 'value' => 2]); + 'item' => 2, 'value' => 2]); $this->assertCount(1, $DB->get_records('feedback_completed')); $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // We have errors! @@ -123,7 +123,7 @@ class mod_feedback_upgradelib_testcase extends advanced_testcase { 'item' => 1, 'value' => 1]); $DB->insert_record('feedback_valuetmp', ['completed' => $completed1, 'course_id' => $this->course1->id, - 'item' => 1, 'value' => 2]); + 'item' => 2, 'value' => 2]); $this->assertCount(1, $DB->get_records('feedback_completedtmp')); $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // We have errors! @@ -172,4 +172,130 @@ class mod_feedback_upgradelib_testcase extends advanced_testcase { $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/version.php b/mod/feedback/version.php index 6c511e2c12b..7b02ce3f3d7 100644 --- a/mod/feedback/version.php +++ b/mod/feedback/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016040300; // The current module version (Date: YYYYMMDDXX) +$plugin->version = 2016051105; // The current module version (Date: YYYYMMDDXX) $plugin->requires = 2015111000; // Requires this Moodle version $plugin->component = 'mod_feedback'; // Full name of the plugin (used for diagnostics) $plugin->cron = 0;