Merge branch 'MDL-54108-master' of git://github.com/marinaglancy/moodle

fladi · May 17, 2016 · 2a1cfd3 · 2a1cfd3
2 parents c3d079c + ea4176c
commit 2a1cfd3
Show file tree

Hide file tree

Showing 5 changed files with 197 additions and 12 deletions.
diff --git a/mod/feedback/db/install.xml b/mod/feedback/db/install.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="mod/feedback/db" VERSION="20160326" COMMENT="XMLDB file for Moodle mod/feedback"
+<XMLDB PATH="mod/feedback/db" VERSION="20160511" COMMENT="XMLDB file for Moodle mod/feedback"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
 >
@@ -119,6 +119,7 @@
       </KEYS>
       <INDEXES>
         <INDEX NAME="course_id" UNIQUE="false" FIELDS="course_id"/>
+        <INDEX NAME="completed_item" UNIQUE="true" FIELDS="completed, item, course_id"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="feedback_valuetmp" COMMENT="values of the completedstmp">
@@ -136,6 +137,7 @@
       </KEYS>
       <INDEXES>
         <INDEX NAME="course_id" UNIQUE="false" FIELDS="course_id"/>
+        <INDEX NAME="completed_item" UNIQUE="true" FIELDS="completed, item, course_id"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="feedback_sitecourse_map" COMMENT="feedback sitecourse map">

diff --git 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
@@ -56,9 +56,34 @@ function mod_feedback_upgrade_courseid($tmp = false) {
     }
 
     // Part 2. Update courseid in the completed table.
-    $sql = "UPDATE {feedback_completed$suffix} "
+    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);
+        $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
@@ -77,7 +77,7 @@ public function test_upgrade_courseid_completed() {
             '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 @@ public function test_upgrade_courseid_completedtmp() {
             '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 @@ public function test_upgrade_courseid_empty_completed() {
         $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);
+    }
 }
diff --git 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;