From 4d211732477dd0746179adf488676e4c2bc415ec Mon Sep 17 00:00:00 2001
From: Mark Nelson <markn@moodle.com>
Date: Sun, 21 May 2017 10:16:45 +0800
Subject: [PATCH] #111 Fixed task creating duplicate rows in issue table

---
 classes/task/email_certificate_task.php | 18 +++++++++++------
 db/upgrade.php                          | 26 +++++++++++++++++++++++++
 tests/email_certificate_task_test.php   | 11 +++++++++++
 version.php                             |  4 ++--
 4 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/classes/task/email_certificate_task.php b/classes/task/email_certificate_task.php
index a86d490..17ac75d 100644
--- a/classes/task/email_certificate_task.php
+++ b/classes/task/email_certificate_task.php
@@ -86,16 +86,14 @@ class email_certificate_task extends \core\task\scheduled_task {
                 $info->coursename = $coursename;
                 $info->certificatename = $certificatename;
 
-                // Get a list of issues that have not yet been emailed.
+                // Get a list of all the issues.
                 $userfields = get_all_user_name_fields(true, 'u');
-                $sql = "SELECT u.id, u.username, $userfields, u.email, ci.id as issueid
+                $sql = "SELECT u.id, u.username, $userfields, u.email, ci.id as issueid, ci.emailed
                           FROM {customcert_issues} ci
                           JOIN {user} u
                             ON ci.userid = u.id
-                         WHERE ci.customcertid = :customcertid
-                           AND emailed = :emailed";
-                $issuedusers = $DB->get_records_sql($sql, array('customcertid' => $customcert->id,
-                    'emailed' => 0));
+                         WHERE ci.customcertid = :customcertid";
+                $issuedusers = $DB->get_records_sql($sql, array('customcertid' => $customcert->id));
 
                 // Now, get a list of users who can access the certificate but have not yet.
                 $enrolledusers = get_enrolled_users(\context_course::instance($customcert->courseid), 'mod/customcert:view');
@@ -137,9 +135,17 @@ class email_certificate_task extends \core\task\scheduled_task {
 
                     // Add them to the array so we email them.
                     $enroluser->issueid = $issueid;
+                    $enroluser->emailed = 0;
                     $issuedusers[] = $enroluser;
                 }
 
+                // Remove all the users who have already been emailed.
+                foreach ($issuedusers as $key => $issueduser) {
+                    if ($issueduser->emailed) {
+                        unset($issuedusers[$key]);
+                    }
+                }
+
                 // Now, email the people we need to.
                 if ($issuedusers) {
                     foreach ($issuedusers as $user) {
diff --git a/db/upgrade.php b/db/upgrade.php
index d88d235..8d94b9d 100644
--- a/db/upgrade.php
+++ b/db/upgrade.php
@@ -80,5 +80,31 @@ function xmldb_customcert_upgrade($oldversion) {
         upgrade_mod_savepoint(true, 2016120505, 'customcert');
     }
 
+    if ($oldversion < 2016120507) {
+        // Remove any duplicate rows from customcert issue table.
+        // This SQL fetches the id of those records which have duplicate customcert issues.
+        // This doesn't return the first issue.
+        $fromclause = "FROM (
+                             SELECT min(id) AS minid, userid, customcertid
+                               FROM {customcert_issues}
+                           GROUP BY userid, customcertid
+                            ) minid
+                       JOIN {customcert_issues} ci
+                         ON ci.userid = minid.userid
+                        AND ci.customcertid = minid.customcertid
+                        AND ci.id > minid.minid";
+
+        // Get the records themselves.
+        $getduplicatessql = "SELECT ci.id $fromclause ORDER BY minid";
+        if ($records = $DB->get_records_sql($getduplicatessql)) {
+            // Delete them.
+            $ids = implode(',', array_keys($records));
+            $DB->delete_records_select('customcert_issues', "id IN ($ids)");
+        }
+
+        // Savepoint reached.
+        upgrade_mod_savepoint(true, 2016120507, 'customcert');
+    }
+
     return true;
 }
diff --git a/tests/email_certificate_task_test.php b/tests/email_certificate_task_test.php
index abbfcf3..972baec 100644
--- a/tests/email_certificate_task_test.php
+++ b/tests/email_certificate_task_test.php
@@ -110,6 +110,17 @@ class mod_customcert_task_email_certificate_task_testcase extends advanced_testc
         $this->assertContains(fullname($user3), $emails[1]->header);
         $this->assertEquals($CFG->noreplyaddress, $emails[1]->from);
         $this->assertEquals($user2->email, $emails[1]->to);
+
+        // Now, run the task again and ensure we did not issue any more certificates.
+        $sink = $this->redirectEmails();
+        $task = new \mod_customcert\task\email_certificate_task();
+        $task->execute();
+        $emails = $sink->get_messages();
+
+        $issues = $DB->get_records('customcert_issues');
+
+        $this->assertCount(2, $issues);
+        $this->assertCount(0, $emails);
     }
 
     /**
diff --git a/version.php b/version.php
index c47a8c9..879ea7e 100644
--- a/version.php
+++ b/version.php
@@ -24,10 +24,10 @@
 
 defined('MOODLE_INTERNAL') || die('Direct access to this script is forbidden.');
 
-$plugin->version   = 2016120506; // The current module version (Date: YYYYMMDDXX).
+$plugin->version   = 2016120507; // The current module version (Date: YYYYMMDDXX).
 $plugin->requires  = 2016120500; // Requires this Moodle version (3.2).
 $plugin->cron      = 0; // Period for cron to check this module (secs).
 $plugin->component = 'mod_customcert';
 
 $plugin->maturity  = MATURITY_STABLE;
-$plugin->release   = "3.2 release (Build: 2016120506)"; // User-friendly version number.
+$plugin->release   = "3.2 release (Build: 2016120507)"; // User-friendly version number.