Store notifications in emailutils log

catalyst · Nov 28, 2024 · ea84bd8 · ea84bd8
1 parent fea50e6
commit ea84bd8
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 16 deletions.
diff --git a/classes/helper.php b/classes/helper.php
@@ -151,4 +151,80 @@ public static function get_bounce_config(): array {
             self::get_bounce_ratio(),
         ];
     }
+
+    /**
+     * Get the most recent bounce recorded for an email address
+     *
+     * @param string $email
+     * @return \stdClass|false
+     */
+    public static function get_most_recent_bounce(string $email): mixed {
+        global $DB;
+
+        $params = [
+            'email' => $email,
+        ];
+        $sql = "SELECT *
+                  FROM {tool_emailutils_log}
+                 WHERE type = 'Bounce' AND email = :email
+                 ORDER BY time DESC
+                 LIMIT 1";
+        return $DB->get_record_sql($sql, $params);
+    }
+
+    /**
+     * Gets the sum of the send count for multiple users
+     * @param array $users array of users or userids
+     * @return int sum of send count
+     */
+    public static function get_sum_send_count(array $users): int {
+        $sendcount = 0;
+        foreach ($users as $user) {
+            $sendcount += get_user_preferences('email_send_count', 0, $user);
+        }
+        return $sendcount;
+    }
+
+    /**
+     * Gets the max bounce count from a group of users
+     * @param array $users array of users or userids
+     * @return int max bounce count
+     */
+    public static function get_max_bounce_count(array $users): int {
+        $maxbounces = 0;
+        foreach ($users as $user) {
+            $maxbounces = max($maxbounces, get_user_preferences('email_bounce_count', 0, $user));
+        }
+        return $maxbounces;
+    }
+
+    /**
+     * Checks whether bounces are likely to be consecutive, and if not reset the bounce count.
+     * This is a fallback in case delivery notifications are not enabled.
+     *
+     * @param string $email
+     * @param array $users
+     * @return void
+     */
+    public static function check_consecutive_bounces(string $email, array $users): void {
+        $recentbounce = self::get_most_recent_bounce($email);
+        if (empty($recentbounce)) {
+            // No data on the previous bounce.
+            return;
+        }
+
+        $sendcount = self::get_sum_send_count($users);
+        $prevsendcount = $recentbounce->sendcount ?? 0;
+        $sincelastbounce = $sendcount - $prevsendcount;
+
+        // The only things we can compare are the previous send count and time.
+        // A direct comparison in sendcount isn't accurate because notifications may be delayed, so use a buffer.
+        // Minbounces is ideal since future notifications would push it over the threshold.
+        $buffer = min(self::get_min_bounces(), 5);
+        if ($sincelastbounce >= $buffer) {
+            foreach ($users as $user) {
+                self::reset_bounce_count($user);
+            }
+        }
+    }
 }
diff --git a/classes/sns_notification.php b/classes/sns_notification.php
@@ -285,8 +285,7 @@ protected function process_bounce_notification(\stdClass $user): void {
 
         if ($this->should_block_immediately()) {
             // User should only be able to recover from this if they change their email or have their bounces reset.
-            // This sets the bounce ratio to 1 to improve visibility when something is a hard bounce.
-            $bouncecount = max($sendcount, helper::get_min_bounces());
+            $bouncecount = helper::get_min_bounces();
             set_user_preference('email_bounce_count', $bouncecount, $user);
         } else if ($this->should_block_softly()) {
             // Swap back to set_bounce_count($user) once MDL-73798 is integrated.
@@ -344,12 +343,26 @@ public function process_notification(): void {
 
         if ($this->is_bounce()) {
             // Ideally bounce handling would be tracked per email instead of user.
+            if (helper::use_consecutive_bounces()) {
+                helper::check_consecutive_bounces($this->get_destination(), $users);
+            }
             foreach ($users as $user) {
                 $this->process_bounce_notification($user);
             }
         }
 
         // TODO: Implement complaint handling.
+
+        // Save to emailutils log. This should be done last as processing may increase send count.
+        // Time should represent when this was processed - send time will be in message if needed.
+        $DB->insert_record('tool_emailutils_log', [
+            'time' => time(),
+            'type' => $this->get_type(),
+            'subtypes' => $this->get_subtypes(),
+            'email' => $this->get_destination(),
+            'message' => $this->messageraw,
+            'sendcount' => helper::get_sum_send_count($users),
+        ]);
     }
 
     /**

diff --git a/db/install.xml b/db/install.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="admin/tool/emailutils/db" VERSION="20241114" COMMENT="XMLDB file for plugin admin/tool/emailutils"
+<XMLDB PATH="admin/tool/emailutils/db" VERSION="20241128" COMMENT="XMLDB file for plugin admin/tool/emailutils"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../../lib/xmldb/xmldb.xsd"
 >
@@ -9,8 +9,10 @@
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
         <FIELD NAME="time" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
         <FIELD NAME="type" TYPE="char" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
+        <FIELD NAME="subtypes" TYPE="char" LENGTH="32" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="email" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="message" TYPE="text" NOTNULL="true" SEQUENCE="false"/>
+        <FIELD NAME="sendcount" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>

diff --git a/db/upgrade.php b/db/upgrade.php
@@ -78,5 +78,29 @@ function xmldb_tool_emailutils_upgrade($oldversion) {
         // Emailutils savepoint reached.
         upgrade_plugin_savepoint(true, 2024111800, 'tool', 'emailutils');
     }
+
+    if ($oldversion < 2024112801) {
+
+        // Define field subtypes to be added to tool_emailutils_log.
+        $table = new xmldb_table('tool_emailutils_log');
+        $field = new xmldb_field('subtypes', XMLDB_TYPE_CHAR, '32', null, null, null, null, 'type');
+
+        // Conditionally launch add field subtypes.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Define field sendcount to be added to tool_emailutils_log.
+        $field = new xmldb_field('sendcount', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'message');
+
+        // Conditionally launch add field sendcount.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Emailutils savepoint reached.
+        upgrade_plugin_savepoint(true, 2024112801, 'tool', 'emailutils');
+    }
+
     return true;
 }
diff --git a/tests/sns_client_test.php b/tests/sns_client_test.php
@@ -145,22 +145,14 @@ private function get_mock_bounce_notification(string $bouncetype, string $bounce
     public function bounce_processing_provider(): array {
         // To be tested with minbounces of 3 and bounceratio of -1.
         return [
-            'Block immediately with low send count' => [
+            'Block immediately' => [
                 'type' => 'Permanent',
                 'subtype' => 'General',
                 'notifications' => 1,
                 'sendcount' => 1,
                 'expectedbounces' => 3,
                 'overthreshold' => true,
             ],
-            'Block immediately with high send count' => [
-                'type' => 'Permanent',
-                'subtype' => 'General',
-                'notifications' => 1,
-                'sendcount' => 100,
-                'expectedbounces' => 100,
-                'overthreshold' => true,
-            ],
             'Block softly' => [
                 'type' => 'Transient',
                 'subtype' => 'General',
@@ -218,7 +210,7 @@ public function bounce_processing_provider(): array {
      **/
     public function test_bounce_processing(string $type, string $subtype, int $notifications, int $sendcount,
             int $expectedbounces, bool $overthreshold): void {
-        global $CFG;
+        global $CFG, $DB;
 
         // Setup config and users.
         $this->resetAfterTest();
@@ -249,6 +241,10 @@ public function test_bounce_processing(string $type, string $subtype, int $notif
         $bounceevents = 2 * (int) $overthreshold;
         $this->assertCount($notifications + $bounceevents, $events);
 
+        // Confirm bounce notification stored in emailutils log table.
+        $records = $DB->get_records('tool_emailutils_log', null, 'id ASC');
+        $this->assertCount($notifications, $records);
+
         // Confirm that shared email addresses have the same status.
         $this->assertSame($overthreshold, over_bounce_threshold($user2));
     }
@@ -259,7 +255,7 @@ public function test_bounce_processing(string $type, string $subtype, int $notif
      * @covers \tool_emailutils\sns_notification::process_notification()
      **/
     public function test_delivery_processing(): void {
-        global $CFG;
+        global $CFG, $DB;
 
         // Setup config and users.
         $this->resetAfterTest();
@@ -297,6 +293,10 @@ public function test_delivery_processing(): void {
         // This also confirms the third user didn't have their count reset.
         $this->assertCount(2, $events);
 
+        // Confirm delivery notification isn't stored in emailutils log table.
+        $records = $DB->get_records('tool_emailutils_log');
+        $this->assertCount(0, $records);
+
         // Ensure bounces aren't reset when bounce ratio config is positive.
         $CFG->bounceratio = 0.5;
         $this->assertFalse(helper::use_consecutive_bounces());

diff --git a/version.php b/version.php
@@ -25,8 +25,8 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version  = 2024112200;
-$plugin->release  = 2024112200;
+$plugin->version  = 2024112801;
+$plugin->release  = 2024112801;
 $plugin->requires = 2024042200;
 $plugin->component = 'tool_emailutils';
 $plugin->maturity  = MATURITY_STABLE;