From 9d262bbc4babbac8c58111e48aaac811d41a01c4 Mon Sep 17 00:00:00 2001
From: Benjamin Walker <benjaminwalker@catalyst-au.net>
Date: Fri, 22 Nov 2024 15:38:27 +1000
Subject: [PATCH 1/4] Add more specific bounce handling
---
classes/check/bounces.php | 3 +-
classes/event/bounce_count_reset.php | 58 ++++++
classes/event/notification_received.php | 9 +-
classes/event/over_bounce_threshold.php | 57 +++++
classes/helper.php | 64 +++++-
classes/sns_client.php | 22 +-
classes/sns_notification.php | 153 +++++++++++++-
client.php | 27 +--
lang/en/tool_emailutils.php | 2 +
reset_bounces.php | 3 +-
tests/sns_client_test.php | 266 ++++++++++++++++++++++++
version.php | 4 +-
12 files changed, 616 insertions(+), 52 deletions(-)
create mode 100644 classes/event/bounce_count_reset.php
create mode 100644 classes/event/over_bounce_threshold.php
diff --git a/classes/check/bounces.php b/classes/check/bounces.php
index c9c89b5..30c8ee6 100644
--- a/classes/check/bounces.php
+++ b/classes/check/bounces.php
@@ -69,8 +69,7 @@ public function get_result() : result {
FROM {user_preferences} up
LEFT JOIN {user_preferences} up2 ON up2.name = 'email_send_count' AND up.userid = up2.userid
WHERE up.name = 'email_bounce_count' AND CAST(up.value AS INTEGER) > :threshold";
- // Start with a threshold of 1 as that is the default for manually created users.
- // TODO: Update when core is fixed.
+ // Start with a threshold of 1 as that was a historical default for manually created users.
$bounces = $DB->get_records_sql($sql, ['threshold' => 1]);
$userswithbounces = count($bounces);
diff --git a/classes/event/bounce_count_reset.php b/classes/event/bounce_count_reset.php
new file mode 100644
index 0000000..4a13817
--- /dev/null
+++ b/classes/event/bounce_count_reset.php
@@ -0,0 +1,58 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Bounce count reset event
+ *
+ * @package tool_emailutils
+ * @author Benjamin Walker (benjaminwalker@catalyst-au.net)
+ * @copyright 2024 Catalyst IT
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+namespace tool_emailutils\event;
+
+/**
+ * Event
+ */
+class bounce_count_reset extends \core\event\base {
+
+ /**
+ * Initialise required event data properties.
+ */
+ protected function init(): void {
+ $this->data['crud'] = 'u';
+ $this->data['edulevel'] = self::LEVEL_OTHER;
+ }
+
+ /**
+ * Returns localised event name.
+ *
+ * @return string
+ */
+ public static function get_name(): string {
+ return get_string('event:bouncecountreset', 'tool_emailutils');
+ }
+
+ /**
+ * Returns non-localised description of what happened.
+ *
+ * @return string
+ */
+ public function get_description(): string {
+ $reason = empty($this->userid) ? 'because an email was delivered successfully' : 'manually';
+ return "The user with id '$this->relateduserid' had their email bounce count reset $reason.";
+ }
+}
diff --git a/classes/event/notification_received.php b/classes/event/notification_received.php
index ab5c153..b385ac2 100644
--- a/classes/event/notification_received.php
+++ b/classes/event/notification_received.php
@@ -24,8 +24,6 @@
*/
namespace tool_emailutils\event;
-use tool_emailutils;
-
/**
* Event
*/
@@ -34,10 +32,9 @@ class notification_received extends \core\event\base {
/**
* Initialise required event data properties.
*/
- protected function init() {
+ protected function init(): void {
$this->data['crud'] = 'u';
$this->data['edulevel'] = self::LEVEL_OTHER;
- $this->data['objecttable'] = 'user';
}
/**
@@ -45,7 +42,7 @@ protected function init() {
*
* @return string
*/
- public static function get_name() {
+ public static function get_name(): string {
return get_string('event:notificationreceived', 'tool_emailutils');
}
@@ -54,7 +51,7 @@ public static function get_name() {
*
* @return string
*/
- public function get_description() {
+ public function get_description(): mixed {
return $this->other;
}
}
diff --git a/classes/event/over_bounce_threshold.php b/classes/event/over_bounce_threshold.php
new file mode 100644
index 0000000..e2a7f05
--- /dev/null
+++ b/classes/event/over_bounce_threshold.php
@@ -0,0 +1,57 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Over bounce threshold event
+ *
+ * @package tool_emailutils
+ * @author Benjamin Walker (benjaminwalker@catalyst-au.net)
+ * @copyright 2024 Catalyst IT
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+namespace tool_emailutils\event;
+
+/**
+ * Event
+ */
+class over_bounce_threshold extends \core\event\base {
+
+ /**
+ * Initialise required event data properties.
+ */
+ protected function init(): void {
+ $this->data['crud'] = 'u';
+ $this->data['edulevel'] = self::LEVEL_OTHER;
+ }
+
+ /**
+ * Returns localised event name.
+ *
+ * @return string
+ */
+ public static function get_name(): string {
+ return get_string('event:overbouncethreshold', 'tool_emailutils');
+ }
+
+ /**
+ * Returns non-localised description of what happened.
+ *
+ * @return string
+ */
+ public function get_description(): string {
+ return "The user with id '$this->relateduserid' is now over the bounce threshold.";
+ }
+}
diff --git a/classes/helper.php b/classes/helper.php
index cb397e9..30a7637 100644
--- a/classes/helper.php
+++ b/classes/helper.php
@@ -26,6 +26,8 @@
namespace tool_emailutils;
+use tool_emailutils\event\bounce_count_reset;
+
/**
* Helper class for tool_emailutils.
*
@@ -67,7 +69,8 @@ public static function get_username_fields(string $tablealias = ''): string {
*/
public static function get_min_bounces(): int {
global $CFG;
- return $CFG->minbounce ?? self::DEFAULT_MIN_BOUNCES;
+ // The core check for using the default uses empty().
+ return empty($CFG->minbounces) ? self::DEFAULT_MIN_BOUNCES : $CFG->minbounces;
}
/**
@@ -76,7 +79,64 @@ public static function get_min_bounces(): int {
*/
public static function get_bounce_ratio(): float {
global $CFG;
- return $CFG->bounceratio ?? self::DEFAULT_BOUNCE_RATIO;
+ // The core check for using the default uses empty().
+ return empty($CFG->bounceratio) ? self::DEFAULT_BOUNCE_RATIO : $CFG->bounceratio;
+ }
+
+ /**
+ * Is a positive bounce ratio being used?
+ * @return bool use bounce ratio?
+ */
+ public static function use_bounce_ratio(): bool {
+ if (self::get_bounce_ratio() > 0) {
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Does the config imply the use of consecutive bounce count?
+ *
+ * Consecutive bounce counts should be used when possible as it's best practice.
+ * This requires a non-positive bounce ratio to ignore send count and pass threshold checks.
+ * Delivery notifications may also need to be set up to accurately track successful deliveries.
+ *
+ * @return bool use consecutive bounce count?
+ */
+ public static function use_consecutive_bounces(): bool {
+ return !self::use_bounce_ratio();
+ }
+
+
+ /**
+ * Resets the bounce count of a user and fires off an event.
+ * @param \stdClass $resetuser user who will have their bounce count reset
+ * @return void
+ */
+ public static function reset_bounce_count(\stdClass $resetuser): void {
+ global $USER;
+
+ // If bounce ratio is being used, the send count also needs to be reset.
+ $resetsendcount = self::use_bounce_ratio();
+
+ $bouncecount = get_user_preferences('email_bounce_count', null, $resetuser);
+ $sendcount = get_user_preferences('email_send_count', null, $resetuser);
+ if (!isset($bouncecount) && (!$resetsendcount || !isset($sendcount))) {
+ return;
+ }
+
+ // Swap to set_bounce_count($resetuser, true) once MDL-73798 is integrated.
+ unset_user_preference('email_bounce_count', $resetuser);
+ if ($resetsendcount) {
+ unset_user_preference('email_send_count', $resetuser);
+ }
+
+ $event = bounce_count_reset::create([
+ 'userid' => $USER->id,
+ 'relateduserid' => $resetuser->id,
+ 'context' => \context_system::instance(),
+ ]);
+ $event->trigger();
}
/**
diff --git a/classes/sns_client.php b/classes/sns_client.php
index b92bc54..367cfc1 100644
--- a/classes/sns_client.php
+++ b/classes/sns_client.php
@@ -64,6 +64,9 @@ class sns_client {
/** Bounce */
const BOUNCE_TYPE = 'Bounce';
+ /** Delivery */
+ const DELIVERY_TYPE = 'Delivery';
+
/**
* SNS Message Object
* @var \Aws\Sns\Message
@@ -182,25 +185,28 @@ public function is_authorised() {
* Validates the incoming message and sets up the sns_notification message.
* This accepts message types of 'Notification'.
*
+ * @param Message|null $message message injector for unit testing
* @return bool Successful validtion
*/
- public function process_message() {
+ public function process_message(?Message $message = null): bool {
$this->validator = new MessageValidator();
$this->client = new Client();
$this->notification = new sns_notification();
// Get the message from the POST data.
- $this->message = Message::fromRawPostData();
+ $this->message = PHPUNIT_TEST && isset($message) ? $message : Message::fromRawPostData();
$isvalidmessage = false;
// Validate the incoming message.
- try {
- $this->validator->validate($this->message);
- } catch (InvalidSnsMessageException $e) {
- // Message not valid!
- http_response_code(400); // Bad request.
- return $isvalidmessage;
+ if (!PHPUNIT_TEST) {
+ try {
+ $this->validator->validate($this->message);
+ } catch (InvalidSnsMessageException $e) {
+ // Message not valid!
+ http_response_code(400); // Bad request.
+ return $isvalidmessage;
+ }
}
// Process the message depending on its type.
diff --git a/classes/sns_notification.php b/classes/sns_notification.php
index b497f92..bfd4b41 100644
--- a/classes/sns_notification.php
+++ b/classes/sns_notification.php
@@ -25,6 +25,9 @@
namespace tool_emailutils;
+use tool_emailutils\event\notification_received;
+use tool_emailutils\event\over_bounce_threshold;
+
/**
* Amazon SNS Notification Class
*
@@ -32,6 +35,21 @@
*/
class sns_notification {
+ /** Bounce subtypes that should be blocked immediately */
+ const BLOCK_IMMEDIATELY = [
+ 'Permanent:General',
+ 'Permanent:NoEmail',
+ 'Permanent:Suppressed',
+ 'Permanent:OnAccountSuppressionList',
+ ];
+
+ /** Bounce subtypes that should be blocked after a few failures */
+ const BLOCK_SOFTLY = [
+ 'Undetermined:Undetermined',
+ 'Transient:General',
+ 'Transient:MailboxFull',
+ ];
+
/**
* SNS Message
* @var mixed
@@ -173,9 +191,9 @@ public function get_complaintsubtype(): string {
/**
* Returns all the message subtypes as an array
- * @return string subtypes as a array
+ * @return string subtypes as a string, split by ':'
*/
- protected function get_subtypes(): array {
+ protected function get_subtypes(): string {
$subtypes = [];
if ($this->is_complaint()) {
$subtypes = [
@@ -188,7 +206,7 @@ protected function get_subtypes(): array {
$this->get_bouncesubtype(),
];
}
- return array_filter($subtypes);
+ return implode(':', array_filter($subtypes));
}
/**
@@ -207,6 +225,133 @@ public function is_bounce(): bool {
return $this->get_type() === sns_client::BOUNCE_TYPE;
}
+ /**
+ * Is the message about a delivery?
+ * @return bool Is delivery?
+ */
+ public function is_delivery(): bool {
+ return $this->get_type() === sns_client::DELIVERY_TYPE;
+ }
+
+ /**
+ * Does this bounce type imply this should be blocked immediately?
+ * @return bool block immediately?
+ */
+ public function should_block_immediately(): bool {
+ return in_array($this->get_subtypes(), self::BLOCK_IMMEDIATELY);
+ }
+
+ /**
+ * Does this bounce type imply this should be blocked softly?
+ * @return bool block softly?
+ */
+ public function should_block_softly(): bool {
+ return in_array($this->get_subtypes(), self::BLOCK_SOFTLY);
+ }
+
+ /**
+ * Processes a delivery notification
+ * @return void
+ */
+ protected function process_delivery_notification(): void {
+ global $DB;
+
+ // Only need to process notifications if consecutive bounce count is being used.
+ if (!helper::use_consecutive_bounces()) {
+ return;
+ }
+
+ $users = $DB->get_records('user', ['email' => $this->get_destination()], 'id ASC', 'id, email');
+ foreach ($users as $user) {
+ // Clear bounces on successful delivery when using consecutive bounce counts.
+ if (!empty(get_user_preferences('email_bounce_count', 0, $user))) {
+ helper::reset_bounce_count($user);
+ }
+ }
+ }
+
+ /**
+ * Processes a bounce notification based on the subtype
+ * @param \stdClass $user
+ * @return void
+ */
+ protected function process_bounce_notification(\stdClass $user): void {
+ if (over_bounce_threshold($user)) {
+ // Can occur if multiple notifications are received close together. No action required.
+ return;
+ }
+ $sendcount = get_user_preferences('email_send_count', 0, $user);
+ $bouncecount = get_user_preferences('email_bounce_count', 0, $user);
+
+ 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());
+ 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.
+ $bouncecount++;
+ set_user_preference('email_bounce_count', $bouncecount, $user);
+ }
+
+ // If send count isn't set (bug prior to MDL-73798) we need to set a placeholder.
+ if (empty($sendcount) && !empty($bouncecount)) {
+ set_user_preference('email_send_count', $bouncecount, $user);
+ }
+
+ if (over_bounce_threshold($user)) {
+ $event = over_bounce_threshold::create([
+ 'relateduserid' => $user->id,
+ 'context' => \context_system::instance(),
+ ]);
+ $event->trigger();
+ }
+ }
+
+ /**
+ * Processes a notification based on the type
+ * @return void
+ */
+ public function process_notification(): void {
+ global $DB;
+
+ if ($this->is_delivery()) {
+ $this->process_delivery_notification();
+ return;
+ }
+
+ if (!$this->is_complaint() && !$this->is_bounce()) {
+ // Invalid request. We should never be here.
+ http_response_code(400);
+ return;
+ }
+
+ // Allow for shared emails. Only create a notification for the first user, but process all.
+ $users = $DB->get_records('user', ['email' => $this->get_destination()], 'id ASC', 'id, email');
+ $user = reset($users);
+
+ // Log all bounces and complaints as an event, even if user is invalid.
+ $event = notification_received::create([
+ 'relateduserid' => $user->id ?? null,
+ 'context' => \context_system::instance(),
+ 'other' => $this->get_messageasstring(),
+ ]);
+ $event->trigger();
+
+ if (empty($users)) {
+ return;
+ }
+
+ if ($this->is_bounce()) {
+ // Ideally bounce handling would be tracked per email instead of user.
+ foreach ($users as $user) {
+ $this->process_bounce_notification($user);
+ }
+ }
+
+ // TODO: Implement complaint handling.
+ }
+
/**
* Return the message as a string
* Eg. "Type about x from y"
@@ -215,7 +360,7 @@ public function is_bounce(): bool {
public function get_messageasstring(): string {
if ($this->is_complaint() || $this->is_bounce()) {
$subtypes = $this->get_subtypes();
- $subtypestring = !empty($subtypes) ? ' (' . implode(':', $subtypes) . ')' : '';
+ $subtypestring = !empty($subtypes) ? " ($subtypes)" : '';
$type = $this->get_type() . $subtypestring;
return $type . ' about ' . $this->get_source_email() . ' from ' . $this->get_destination();
} else {
diff --git a/client.php b/client.php
index a374d83..a7d2c7e 100644
--- a/client.php
+++ b/client.php
@@ -28,7 +28,6 @@
*/
use tool_emailutils\sns_client;
-use tool_emailutils\event\notification_received;
define('NO_MOODLE_COOKIES', true);
@@ -46,30 +45,6 @@
}
if ($client->process_message() && $client->is_notification()) {
- global $DB;
-
$notification = $client->get_notification();
-
- $user = $DB->get_record('user', ['email' => $notification->get_destination()], 'id, email');
-
- if ($user) {
- if ($notification->is_complaint()) {
- $type = 'c';
- } else if ($notification->is_bounce()) {
- $type = 'b';
- } else {
- http_response_code(400); // Invalid request.
- exit;
- }
-
- // Increment the user preference email_bounce_count.
- set_bounce_count($user);
-
- $event = notification_received::create([
- 'relateduserid' => $user->id,
- 'context' => context_system::instance(),
- 'other' => $notification->get_messageasstring(),
- ]);
- $event->trigger();
- }
+ $notification->process_notification();
}
diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php
index 64712be..91dcafa 100644
--- a/lang/en/tool_emailutils.php
+++ b/lang/en/tool_emailutils.php
@@ -73,7 +73,9 @@
$string['enabled_help'] = 'Allow the plugin to process incoming messages';
$string['enable_suppression_list'] = 'Enable email suppression list';
$string['enable_suppression_list_desc'] = 'When enabled, the system will maintain a suppression list of email addresses that should not receive emails.';
+$string['event:bouncecountreset'] = 'Bounce count reset';
$string['event:notificationreceived'] = 'AWS SNS notification received';
+$string['event:overbouncethreshold'] = 'Over bounce threshold';
$string['header'] = 'Header';
$string['header_help'] = 'HTTP Basic Auth Header';
$string['incorrect_access'] = 'Incorrect access detected. For use only by AWS SNS.';
diff --git a/reset_bounces.php b/reset_bounces.php
index 98d6c60..9bee4f9 100644
--- a/reset_bounces.php
+++ b/reset_bounces.php
@@ -44,8 +44,7 @@
list($in, $params) = $DB->get_in_or_equal($SESSION->bulk_users);
$rs = $DB->get_recordset_select('user', "id $in", $params, '', 'id, ' . \tool_emailutils\helper::get_username_fields());
foreach ($rs as $user) {
- // Reset the user bounce count.
- set_bounce_count($user, true);
+ \tool_emailutils\helper::reset_bounce_count($user);
}
$rs->close();
echo $OUTPUT->box_start('generalbox', 'notice');
diff --git a/tests/sns_client_test.php b/tests/sns_client_test.php
index 279fe31..f04dc11 100644
--- a/tests/sns_client_test.php
+++ b/tests/sns_client_test.php
@@ -16,6 +16,12 @@
namespace tool_emailutils;
+defined('MOODLE_INTERNAL') || die();
+
+require_once(__DIR__ . '/../lib/aws-sns-message-validator/src/Message.php');
+
+use Aws\Sns\Message;
+
/**
* Tests for SNS client.
*
@@ -25,6 +31,9 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class sns_client_test extends \advanced_testcase {
+ /**
+ * Test email for user in unit test */
+ const TEST_EMAIL = 'user@example.com';
/**
* Test required libs are installed.
@@ -38,4 +47,261 @@ public function test_lib(): void {
$client = new \tool_emailutils\sns_client('', '', '');
$client->process_message();
}
+
+ /**
+ * Creates a mock message
+ * @param string $message message contents
+ * @return string string json encoded message
+ */
+ private function get_mock_message(string $message): string {
+ // @codingStandardsIgnoreStart
+ return json_encode([
+ "Type" => "Notification",
+ "MessageId" => "165545c9-2a5c-472c-8df2-7ff2be2b3b1b",
+ "TopicArn" => "arn:aws:sns:us-west-2:123456789012:MyTopic",
+ "Message" => $message,
+ "Timestamp" => "2012-04-26T20:45:04.751Z",
+ "SignatureVersion" => "1",
+ "Signature" => "EXAMPLEpH+DcEwjAPg8O9mY8dReBSwksfg2S7WKQcikcNKWLQjwu6A4VbeS0QHVCkhRS7fUQvi2egU3N858fiTDN6bkkOxYDVrY0Ad8L10Hs3zH81mtnPk5uvvolIC1CXGu43obcgFxeL3khZl8IKvO61GWB6jI9b5+gLPoBc1Q=",
+ "SigningCertURL" => "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-f3ecfb7224c7233fe7bb5f59f96de52f.pem",
+ "UnsubscribeURL" => "https://sns.us-west-2.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:us-west-2:123456789012:MyTopic:c9135db0-26c4-47ec-8998-413945fb5a96"
+ ]);
+ // @codingStandardsIgnoreEnd
+ }
+
+ /**
+ * Creates a mock delivery notification
+ * @return \tool_emailutils\sns_notification
+ */
+ private function get_mock_delivery_notification(): sns_notification {
+ $client = new sns_client('', '', '');
+
+ // Create mock delivery notification using data from AWS example docs.
+ $mockdelivery = json_encode([
+ "notificationType" => "Delivery",
+ "delivery" => [
+ "timestamp" => "2012-05-25T14:59:35.605Z",
+ "processingTimeMillis" => 546,
+ "recipients" => [self::TEST_EMAIL],
+ "smtpResponse" => "250 ok: Message 64111812 accepted",
+ "reportingMTA" => "a8-70.smtp-out.amazonses.com",
+ "remoteMtaIp" => "127.0.2.0",
+ ],
+ "mail" => [
+ "timestamp" => "2012-05-25T14:59:35.605Z",
+ "source" => "sender@example.com",
+ "destination" => [self::TEST_EMAIL]
+ ],
+ ]);
+
+ $mockmessage = $this->get_mock_message($mockdelivery);
+ $client->process_message(Message::fromJsonString($mockmessage));
+ return $client->get_notification();
+ }
+
+ /**
+ * Creates a mock bounce notification.
+ * @param string $bouncetype
+ * @param string $bouncesubtype
+ * @return \tool_emailutils\sns_notification mock bounce notification
+ */
+ private function get_mock_bounce_notification(string $bouncetype, string $bouncesubtype): sns_notification {
+ $client = new sns_client('', '', '');
+
+ // Create mock bounces using data from AWS example docs.
+ $mockbounce = json_encode([
+ "notificationType" => "Bounce",
+ "bounce" => [
+ "bounceType" => $bouncetype,
+ "bounceSubType" => $bouncesubtype,
+ "bouncedRecipients" => [
+ [
+ "status" => "5.0.0",
+ "action" => "failed",
+ "diagnosticCode" => "smtp; 550 user unknown",
+ "emailAddress" => self::TEST_EMAIL,
+ ],
+ ],
+ "timestamp" => "2012-05-25T14:59:38.605Z",
+ "feedbackId" => "000001378603176d-5a4b5ad9-6f30-4198-a8c3-b1eb0c270a1d-000000"
+ ],
+ "mail" => [
+ "timestamp" => "2012-05-25T14:59:35.605Z",
+ "source" => "sender@example.com",
+ "destination" => [self::TEST_EMAIL]
+ ],
+ ]);
+
+ $mockmessage = $this->get_mock_message($mockbounce);
+ $client->process_message(Message::fromJsonString($mockmessage));
+ return $client->get_notification();
+ }
+
+ /**
+ * Data provider for {@see test_email_bounce_threshold}
+ *
+ * @return array
+ */
+ public function bounce_processing_provider(): array {
+ // To be tested with minbounces of 3 and bounceratio of -1.
+ return [
+ 'Block immediately with low send count' => [
+ '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',
+ 'notifications' => 1,
+ 'sendcount' => 1,
+ 'expectedbounces' => 1,
+ 'overthreshold' => false,
+ ],
+ 'Multiple block softly' => [
+ 'type' => 'Transient',
+ 'subtype' => 'General',
+ 'notifications' => 3,
+ 'sendcount' => 3,
+ 'expectedbounces' => 3,
+ 'overthreshold' => true,
+ ],
+ 'Extra block softly' => [
+ 'type' => 'Transient',
+ 'subtype' => 'General',
+ 'notifications' => 4,
+ 'sendcount' => 4,
+ 'expectedbounces' => 3,
+ 'overthreshold' => true,
+ ],
+ 'Do nothing' => [
+ 'type' => 'Transient',
+ 'subtype' => 'AttachmentRejected',
+ 'notifications' => 1,
+ 'sendcount' => 1,
+ 'expectedbounces' => 0,
+ 'overthreshold' => false,
+ ],
+ 'Divide by 0 sendcount' => [
+ 'type' => 'Permanent',
+ 'subtype' => 'General',
+ 'notifications' => 1,
+ 'sendcount' => 0,
+ 'expectedbounces' => 3,
+ 'overthreshold' => true,
+ ],
+ ];
+ }
+
+ /**
+ * Tests the email bounce thresholds
+ *
+ * @dataProvider bounce_processing_provider
+ * @param string $type bounce type
+ * @param string $subtype bounce subtype
+ * @param int $notifications the number of notifications to process
+ * @param int $sendcount the email send count
+ * @param int $expectedbounces expected bounce conut
+ * @param bool $overthreshold expected to be over the threshold
+ * @covers \tool_emailutils\sns_notification::process_notification()
+ **/
+ public function test_bounce_processing(string $type, string $subtype, int $notifications, int $sendcount,
+ int $expectedbounces, bool $overthreshold): void {
+ global $CFG;
+
+ // Setup config and users.
+ $this->resetAfterTest();
+ $CFG->handlebounces = true;
+ $CFG->minbounces = 3;
+ $CFG->bounceratio = -1;
+ $CFG->allowaccountssameemail = true;
+
+ $user1 = $this->getDataGenerator()->create_user(['email' => self::TEST_EMAIL]);
+ $user2 = $this->getDataGenerator()->create_user(['email' => self::TEST_EMAIL]);
+ set_user_preference('email_send_count', $sendcount, $user1->id);
+ set_user_preference('email_send_count', $sendcount, $user2->id);
+
+ // Process notification and store events.
+ $sink = $this->redirectEvents();
+ $notification = $this->get_mock_bounce_notification($type, $subtype);
+ for ($i = 0; $i < $notifications; $i++) {
+ $notification->process_notification();
+ }
+ $events = $sink->get_events();
+ $sink->close();
+
+ // Confirm bouncecount and over threshold.
+ $this->assertEquals($expectedbounces, get_user_preferences('email_bounce_count', 0, $user1));
+ $this->assertSame($overthreshold, over_bounce_threshold($user1));
+
+ // There should be one event for each bounce notification plus one threshold event per user over the threshold.
+ $bounceevents = 2 * (int) $overthreshold;
+ $this->assertCount($notifications + $bounceevents, $events);
+
+ // Confirm that shared email addresses have the same status.
+ $this->assertSame($overthreshold, over_bounce_threshold($user2));
+ }
+
+ /**
+ * Tests the email delivery processing
+ *
+ * @covers \tool_emailutils\sns_notification::process_notification()
+ **/
+ public function test_delivery_processing(): void {
+ global $CFG;
+
+ // Setup config and users.
+ $this->resetAfterTest();
+ $CFG->handlebounces = true;
+ $CFG->minbounces = 3;
+ $CFG->bounceratio = -1;
+ $CFG->allowaccountssameemail = true;
+
+ $user1 = $this->getDataGenerator()->create_user(['email' => self::TEST_EMAIL]);
+ $user2 = $this->getDataGenerator()->create_user(['email' => self::TEST_EMAIL]);
+ $user3 = $this->getDataGenerator()->create_user(['email' => self::TEST_EMAIL]);
+
+ // Testing 3 scenarios with a single notification - above minbounces, below minbounces and no bounces.
+ set_user_preference('email_bounce_count', 5, $user1->id);
+ set_user_preference('email_bounce_count', 2, $user2->id);
+ set_user_preference('email_bounce_count', 0, $user3->id);
+
+ $this->assertTrue(helper::use_consecutive_bounces());
+
+ // Process notification and store events.
+ $sink = $this->redirectEvents();
+ $notification = $this->get_mock_delivery_notification();
+ $notification->process_notification();
+
+ $events = $sink->get_events();
+ $sink->close();
+
+ // Confirm both were reset.
+ $this->assertEquals(0, get_user_preferences('email_bounce_count', 0, $user1->id));
+ $this->assertEquals(0, get_user_preferences('email_bounce_count', 0, $user2->id));
+ $this->assertFalse(over_bounce_threshold($user1));
+ $this->assertFalse(over_bounce_threshold($user2));
+
+ // There should be one event for each user who had their bounce count reset.
+ // This also confirms the third user didn't have their count reset.
+ $this->assertCount(2, $events);
+
+ // Ensure bounces aren't reset when bounce ratio config is positive.
+ $CFG->bounceratio = 0.5;
+ $this->assertFalse(helper::use_consecutive_bounces());
+ set_user_preference('email_bounce_count', 5, $user1->id);
+ $notification->process_notification();
+ $this->assertEquals(5, get_user_preferences('email_bounce_count', 0, $user1->id));
+ }
}
diff --git a/version.php b/version.php
index ce1d0e1..1e075a7 100644
--- a/version.php
+++ b/version.php
@@ -25,8 +25,8 @@
defined('MOODLE_INTERNAL') || die();
-$plugin->version = 2024111801;
-$plugin->release = 2024111801;
+$plugin->version = 2024112200;
+$plugin->release = 2024112200;
$plugin->requires = 2024042200;
$plugin->component = 'tool_emailutils';
$plugin->maturity = MATURITY_STABLE;
From 215f9c2644eb7a56466cdc1074d769b178225db9 Mon Sep 17 00:00:00 2001
From: Benjamin Walker <benjaminwalker@catalyst-au.net>
Date: Wed, 27 Nov 2024 10:37:47 +1000
Subject: [PATCH 2/4] Add reset bounces action to bounces report
---
bounces.php | 24 +++++++++++-
classes/hook_callbacks.php | 2 +-
.../local/systemreports/email_bounces.php | 28 +++++++++++++
lang/en/tool_emailutils.php | 5 ++-
reset_bounces.php | 39 +++++++++++++------
5 files changed, 83 insertions(+), 15 deletions(-)
diff --git a/bounces.php b/bounces.php
index e8c3688..f73655a 100644
--- a/bounces.php
+++ b/bounces.php
@@ -30,6 +30,8 @@
require(__DIR__.'/../../../config.php');
require_once($CFG->libdir.'/adminlib.php');
+require_once($CFG->dirroot.'/'.$CFG->admin.'/user/lib.php');
+require_once($CFG->dirroot.'/'.$CFG->admin.'/user/user_bulk_forms.php');
admin_externalpage_setup('tool_emailutils_bounces', '', [], '', ['pagelayout' => 'report']);
@@ -47,8 +49,28 @@
]), 'info');
}
-$report = system_report_factory::create(email_bounces::class, context_system::instance());
+echo html_writer::start_div('', ['data-region' => 'report-user-list-wrapper']);
+
+// Exclude all actions besides reset bounces.
+$actionames = array_keys((new user_bulk_action_form())->get_actions());
+$excludeactions = array_diff($actionames, ['tool_emailutils_reset_bounces']);
+
+$bulkactions = new user_bulk_action_form(new moodle_url('/admin/user/user_bulk.php'),
+ ['excludeactions' => $excludeactions, 'passuserids' => true, 'hidesubmit' => true],
+ 'post', '',
+ ['id' => 'user-bulk-action-form']);
+$bulkactions->set_data(['returnurl' => $PAGE->url->out_as_local_url(false)]);
+
+$report = system_report_factory::create(email_bounces::class, context_system::instance(),
+ parameters: ['withcheckboxes' => $bulkactions->has_bulk_actions()]);
echo $report->output();
+if ($bulkactions->has_bulk_actions()) {
+ $PAGE->requires->js_call_amd('core_admin/bulk_user_actions', 'init');
+ $bulkactions->display();
+}
+
+echo html_writer::end_div();
+
echo $OUTPUT->footer();
diff --git a/classes/hook_callbacks.php b/classes/hook_callbacks.php
index 87b399f..4279a14 100644
--- a/classes/hook_callbacks.php
+++ b/classes/hook_callbacks.php
@@ -33,7 +33,7 @@ class hook_callbacks {
*/
public static function extend_bulk_user_actions(\core_user\hook\extend_bulk_user_actions $hook): void {
if (has_capability('moodle/site:config', \context_system::instance())) {
- $hook->add_action('tool_ses_reset_bounces', new \action_link(
+ $hook->add_action('tool_emailutils_reset_bounces', new \action_link(
new \moodle_url('/admin/tool/emailutils/reset_bounces.php'),
get_string('resetbounces', 'tool_emailutils')
));
diff --git a/classes/reportbuilder/local/systemreports/email_bounces.php b/classes/reportbuilder/local/systemreports/email_bounces.php
index 721a38d..6854519 100644
--- a/classes/reportbuilder/local/systemreports/email_bounces.php
+++ b/classes/reportbuilder/local/systemreports/email_bounces.php
@@ -20,6 +20,7 @@
use tool_emailutils\reportbuilder\local\entities\email_bounce;
use core_reportbuilder\system_report;
use core_reportbuilder\local\entities\user;
+use core_reportbuilder\local\report\action;
/**
* Email bounces report class implementation.
@@ -44,6 +45,16 @@ protected function initialise(): void {
$this->add_entity($entitymain);
$this->add_base_condition_simple("{$entitymainalias}.name", 'email_bounce_count');
+ // Any columns required by actions should be defined here to ensure they're always available.
+ $this->add_base_fields("{$entitymainalias}.userid");
+
+ if ($this->get_parameter('withcheckboxes', false, PARAM_BOOL)) {
+ $canviewfullnames = has_capability('moodle/site:viewfullnames', context_system::instance());
+ $this->set_checkbox_toggleall(static function(\stdClass $row) use ($canviewfullnames): array {
+ return [$row->userid, fullname($row, $canviewfullnames)];
+ });
+ }
+
// We can join the "user" entity to our "main" entity using standard SQL JOIN.
$entityuser = new user();
$entityuseralias = $entityuser->get_table_alias('user');
@@ -54,6 +65,7 @@ protected function initialise(): void {
// Now we can call our helper methods to add the content we want to include in the report.
$this->add_columns();
$this->add_filters();
+ $this->add_actions();
// Set if report can be downloaded.
$this->set_downloadable(true, get_string('reportbounces', 'tool_emailutils'));
@@ -107,4 +119,20 @@ protected function add_filters(): void {
$this->add_filters_from_entities($filters);
}
+
+ /**
+ * Add the system report actions. An extra column will be appended to each row, containing all actions added here
+ *
+ * Note the use of ":id" placeholder which will be substituted according to actual values in the row
+ */
+ protected function add_actions(): void {
+ // Action to reset the bounce count.
+ $this->add_action((new action(
+ new \moodle_url('/admin/tool/emailutils/reset_bounces.php', ['id' => ':userid']),
+ new \pix_icon('i/reload', ''),
+ [],
+ false,
+ new \lang_string('resetbounces', 'tool_emailutils'),
+ )));
+ }
}
diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php
index 91dcafa..14a0162 100644
--- a/lang/en/tool_emailutils.php
+++ b/lang/en/tool_emailutils.php
@@ -93,8 +93,9 @@
$string['privacy:metadata:tool_emailutils_list'] = 'Information.';
$string['privacy:metadata:tool_emailutils_list:updatedid'] = 'The ID of updated user.';
$string['privacy:metadata:tool_emailutils_list:userid'] = 'The ID of the user.';
-$string['reportbounces'] = 'Email bounces report';
-$string['resetbounces'] = 'Reset the number of bounces';
+$string['reportbounces'] = 'Email bounces';
+$string['resetbounceratio'] = 'A bounce ratio is being used, so resetting the bounce count will also reset the send count.';
+$string['resetbounces'] = 'Reset bounce count';
$string['selectoractivate'] = 'Activate key pair';
$string['selectoractivateconfirm'] = 'This will set $CFG->emaildkimselector to this selector and it will be used for signing outgoing emails.';
$string['selectoractive'] = 'Active selector';
diff --git a/reset_bounces.php b/reset_bounces.php
index 9bee4f9..9d2625a 100644
--- a/reset_bounces.php
+++ b/reset_bounces.php
@@ -26,14 +26,17 @@
require_once(__DIR__ . '/../../../config.php');
require_once($CFG->libdir . '/adminlib.php');
-admin_externalpage_setup('userbulk');
+admin_externalpage_setup('tool_emailutils_bounces');
require_capability('moodle/user:update', context_system::instance());
$confirm = optional_param('confirm', 0, PARAM_BOOL);
+$userid = optional_param('id', null, PARAM_INT);
+$returnurl = optional_param('returnurl', null, PARAM_LOCALURL);
-$return = new moodle_url('/admin/user/user_bulk.php');
+$users = empty($userid) ? $SESSION->bulk_users : [$userid];
+$return = new moodle_url($returnurl ?? '/admin/tool/emailutils/bounces.php');
-if (empty($SESSION->bulk_users)) {
+if (empty($users)) {
redirect($return);
}
@@ -41,7 +44,7 @@
echo $OUTPUT->heading(get_string('resetbounces', 'tool_emailutils'));
if ($confirm && confirm_sesskey()) {
- list($in, $params) = $DB->get_in_or_equal($SESSION->bulk_users);
+ list($in, $params) = $DB->get_in_or_equal($users);
$rs = $DB->get_recordset_select('user', "id $in", $params, '', 'id, ' . \tool_emailutils\helper::get_username_fields());
foreach ($rs as $user) {
\tool_emailutils\helper::reset_bounce_count($user);
@@ -50,16 +53,30 @@
echo $OUTPUT->box_start('generalbox', 'notice');
echo $OUTPUT->notification(get_string('bouncesreset', 'tool_emailutils'), 'notifysuccess');
- $continue = new single_button(new moodle_url($return), get_string('continue'), 'post');
+ $continue = new single_button(new moodle_url($return), get_string('continue'));
echo $OUTPUT->render($continue);
echo $OUTPUT->box_end();
} else {
- list($in, $params) = $DB->get_in_or_equal($SESSION->bulk_users);
+ list($in, $params) = $DB->get_in_or_equal($users);
$userlist = $DB->get_records_select_menu('user', "id $in", $params, 'fullname', 'id,'.$DB->sql_fullname().' AS fullname');
- $usernames = implode(', ', $userlist);
- echo $OUTPUT->heading(get_string('confirmation', 'admin'));
- $formcontinue = new single_button(new moodle_url('reset_bounces.php', ['confirm' => 1]), get_string('yes'));
- $formcancel = new single_button(new moodle_url('/admin/user/user_bulk.php'), get_string('no'), 'get');
- echo $OUTPUT->confirm(get_string('bouncecheckfull', 'tool_emailutils', $usernames), $formcontinue, $formcancel);
+
+ if (empty($userlist)) {
+ echo $OUTPUT->notification(get_string('invaliduserid', 'error'));
+ $continue = new single_button(new moodle_url($return), get_string('continue'));
+ echo $OUTPUT->render($continue);
+ } else {
+ if (\tool_emailutils\helper::use_bounce_ratio()) {
+ echo $OUTPUT->notification(get_string('resetbounceratio', 'tool_emailutils'), 'warning');
+ }
+ $usernames = implode(', ', $userlist);
+ $params = array_filter([
+ 'confirm' => 1,
+ 'id' => $userid,
+ 'returnurl' => $returnurl,
+ ]);
+ $formcontinue = new single_button(new moodle_url('reset_bounces.php', $params), get_string('yes'));
+ $formcancel = new single_button($return, get_string('no'));
+ echo $OUTPUT->confirm(get_string('bouncecheckfull', 'tool_emailutils', $usernames), $formcontinue, $formcancel);
+ }
}
echo $OUTPUT->footer();
From 9d81c12f4bedac59fbf9f3c327a1fc2dd0886429 Mon Sep 17 00:00:00 2001
From: Benjamin Walker <benjaminwalker@catalyst-au.net>
Date: Thu, 28 Nov 2024 16:08:56 +1000
Subject: [PATCH 3/4] Store notifications in emailutils log
---
classes/helper.php | 76 ++++++++++++++++++++++++++++++++++++
classes/sns_notification.php | 17 +++++++-
db/install.xml | 4 +-
db/upgrade.php | 24 ++++++++++++
tests/sns_client_test.php | 22 +++++------
version.php | 4 +-
6 files changed, 131 insertions(+), 16 deletions(-)
diff --git a/classes/helper.php b/classes/helper.php
index 30a7637..c74947b 100644
--- 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
index bfd4b41..2536900 100644
--- 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
index 3d98ded..fc05989 100644
--- 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
index 2fbeefe..b049d5c 100644
--- 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
index f04dc11..d324c49 100644
--- a/tests/sns_client_test.php
+++ b/tests/sns_client_test.php
@@ -145,7 +145,7 @@ 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,
@@ -153,14 +153,6 @@ public function bounce_processing_provider(): array {
'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
index 1e075a7..335cf76 100644
--- 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;
From 872cd93ccbdbdb1d2186ca3f32c6c2af7c415176 Mon Sep 17 00:00:00 2001
From: Benjamin Walker <benjaminwalker@catalyst-au.net>
Date: Thu, 28 Nov 2024 16:13:26 +1000
Subject: [PATCH 4/4] Add bounce types to bounces table
---
.../local/entities/notification_log.php | 192 ++++++++++++++++++
.../local/systemreports/email_bounces.php | 17 ++
lang/en/tool_emailutils.php | 3 +
3 files changed, 212 insertions(+)
create mode 100644 classes/reportbuilder/local/entities/notification_log.php
diff --git a/classes/reportbuilder/local/entities/notification_log.php b/classes/reportbuilder/local/entities/notification_log.php
new file mode 100644
index 0000000..1f2fc29
--- /dev/null
+++ b/classes/reportbuilder/local/entities/notification_log.php
@@ -0,0 +1,192 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+namespace tool_emailutils\reportbuilder\local\entities;
+
+use lang_string;
+use core_reportbuilder\local\entities\base;
+use core_reportbuilder\local\filters\date;
+use core_reportbuilder\local\filters\text;
+use core_reportbuilder\local\helpers\format;
+use core_reportbuilder\local\report\column;
+use core_reportbuilder\local\report\filter;
+
+/**
+ * Notification log list entity class class implementation.
+ *
+ * Defines all the columns and filters that can be added to reports that use this entity.
+ *
+ * @package tool_emailutils
+ * @author Benjamin Walker <benjaminwalker@catalyst-au.net>
+ * @copyright Catalyst IT 2024
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ *
+ */
+class notification_log extends base {
+
+ /**
+ * Database tables that this entity uses
+ *
+ * @return string[]
+ */
+ protected function get_default_tables(): array {
+ return [
+ 'tool_emailutils_log',
+ ];
+ }
+
+ /**
+ * The default title for this entity
+ *
+ * @return lang_string
+ */
+ protected function get_default_entity_title(): lang_string {
+ return new lang_string('notificationlog', 'tool_emailutils');
+ }
+
+ /**
+ * Initialize the entity
+ *
+ * @return base
+ */
+ public function initialise(): base {
+ $columns = $this->get_all_columns();
+ foreach ($columns as $column) {
+ $this->add_column($column);
+ }
+
+ $filters = $this->get_all_filters();
+ foreach ($filters as $filter) {
+ $this->add_filter($filter);
+ $this->add_condition($filter);
+ }
+
+ return $this;
+ }
+
+ /**
+ * Returns list of all available columns
+ *
+ * @return column[]
+ */
+ protected function get_all_columns(): array {
+ $tablealias = $this->get_table_alias('tool_emailutils_log');
+
+ // Email column.
+ $columns[] = (new column(
+ 'email',
+ new lang_string('email'),
+ $this->get_entity_name()
+ ))
+ ->add_joins($this->get_joins())
+ ->set_type(column::TYPE_TEXT)
+ ->add_fields("{$tablealias}.email")
+ ->set_is_sortable(true);
+
+ // Type column.
+ $columns[] = (new column(
+ 'type',
+ new lang_string('type', 'tool_emailutils'),
+ $this->get_entity_name()
+ ))
+ ->add_joins($this->get_joins())
+ ->set_type(column::TYPE_TEXT)
+ ->add_fields("{$tablealias}.type")
+ ->set_is_sortable(true);
+
+ // Subtypes column.
+ $columns[] = (new column(
+ 'subtypes',
+ new lang_string('subtypes', 'tool_emailutils'),
+ $this->get_entity_name()
+ ))
+ ->add_joins($this->get_joins())
+ ->set_type(column::TYPE_TEXT)
+ ->add_fields("{$tablealias}.subtypes")
+ ->set_is_sortable(true);
+
+ // Time column.
+ $columns[] = (new column(
+ 'time',
+ new lang_string('time'),
+ $this->get_entity_name()
+ ))
+ ->add_joins($this->get_joins())
+ ->set_type(column::TYPE_TIMESTAMP)
+ ->add_fields("{$tablealias}.time")
+ ->set_is_sortable(true)
+ ->add_callback([format::class, 'userdate'], get_string('strftimedatetimeshortaccurate', 'core_langconfig'));
+
+ return $columns;
+ }
+
+ /**
+ * Return list of all available filters
+ *
+ * @return filter[]
+ */
+ protected function get_all_filters(): array {
+ $tablealias = $this->get_table_alias('tool_emailutils_log');
+
+ // Email filter.
+ $filters[] = (new filter(
+ text::class,
+ 'email',
+ new lang_string('email'),
+ $this->get_entity_name(),
+ "{$tablealias}.email"
+ ))
+ ->add_joins($this->get_joins());
+
+ // Type filter.
+ $filters[] = (new filter(
+ text::class,
+ 'type',
+ new lang_string('type', 'tool_emailutils'),
+ $this->get_entity_name(),
+ "{$tablealias}.type"
+ ))
+ ->add_joins($this->get_joins());
+
+ // Subtypes filter.
+ $filters[] = (new filter(
+ text::class,
+ 'subtypes',
+ new lang_string('subtypes', 'tool_emailutils'),
+ $this->get_entity_name(),
+ "{$tablealias}.subtypes"
+ ))
+ ->add_joins($this->get_joins());
+
+ // Time filter.
+ $filters[] = (new filter(
+ date::class,
+ 'time',
+ new lang_string('time'),
+ $this->get_entity_name(),
+ "{$tablealias}.time"
+ ))
+ ->add_joins($this->get_joins())
+ ->set_limited_operators([
+ date::DATE_ANY,
+ date::DATE_RANGE,
+ date::DATE_PREVIOUS,
+ date::DATE_CURRENT,
+ ]);
+
+ return $filters;
+ }
+}
diff --git a/classes/reportbuilder/local/systemreports/email_bounces.php b/classes/reportbuilder/local/systemreports/email_bounces.php
index 6854519..b5c3b0c 100644
--- a/classes/reportbuilder/local/systemreports/email_bounces.php
+++ b/classes/reportbuilder/local/systemreports/email_bounces.php
@@ -18,6 +18,7 @@
use context_system;
use tool_emailutils\reportbuilder\local\entities\email_bounce;
+use tool_emailutils\reportbuilder\local\entities\notification_log;
use core_reportbuilder\system_report;
use core_reportbuilder\local\entities\user;
use core_reportbuilder\local\report\action;
@@ -62,6 +63,21 @@ protected function initialise(): void {
->add_join("LEFT JOIN {user} {$entityuseralias} ON {$entityuseralias}.id = {$entitymainalias}.userid")
);
+ // Join with the latest entry in the notification log for each user.
+ $entitylog = new notification_log();
+ $entitylogalias = $entitylog->get_table_alias('tool_emailutils_log');
+ $this->add_entity($entitylog
+ ->add_join("LEFT JOIN (
+ SELECT l1.*
+ FROM {tool_emailutils_log} l1
+ WHERE l1.id = (
+ SELECT MAX(l2.id)
+ FROM {tool_emailutils_log} l2
+ WHERE l2.email = l1.email
+ )
+ ) {$entitylogalias} ON {$entitylogalias}.email = {$entityuseralias}.email")
+ );
+
// Now we can call our helper methods to add the content we want to include in the report.
$this->add_columns();
$this->add_filters();
@@ -94,6 +110,7 @@ protected function add_columns(): void {
'email_bounce:bounces',
'email_bounce:send',
'email_bounce:ratio',
+ 'notification_log:subtypes',
];
$this->add_columns_from_entities($columns);
diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php
index 14a0162..0f2a6b5 100644
--- a/lang/en/tool_emailutils.php
+++ b/lang/en/tool_emailutils.php
@@ -84,6 +84,7 @@
$string['mxrecords'] = 'MX records';
$string['mxtoolbox'] = 'MXtoolbox links';
$string['not_implemented'] = 'Not implemented yet. Search the user report for emails ending with ".b.invalid" and ".c.invalid".';
+$string['notificationlog'] = 'SNS notification log';
$string['password'] = 'Password';
$string['password_help'] = 'HTTP Basic Auth Password - Leave empty if you\'re not changing the password';
$string['pluginname'] = 'Email utilities';
@@ -114,9 +115,11 @@
$string['selectornotblank'] = 'Selector cannot be empty';
$string['sendcount'] = 'Send count';
$string['settings'] = 'AWS SES settings';
+$string['subtypes'] = 'Reason';
$string['suppressionreason'] = 'Reason';
$string['suppressioncreated'] = 'Created at';
$string['task_update_suppression_list'] = 'Update email suppression list';
+$string['type'] = 'Type';
$string['username'] = 'Username';
$string['username_help'] = 'HTTP Basic Auth Username';
$string['userdomains'] = 'Most common user email domains';