From c95a0839fc0bd7c060881a7f0769926013b5bfda Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Fri, 29 Nov 2024 14:27:55 +1000 Subject: [PATCH] Clean up bounce check and add breakdown #74 --- classes/check/bounces.php | 77 +++++++++++++++++++++----------- lang/en/tool_emailutils.php | 8 ++++ templates/bounce_config.mustache | 46 ++++++++++++++----- version.php | 4 +- 4 files changed, 95 insertions(+), 40 deletions(-) diff --git a/classes/check/bounces.php b/classes/check/bounces.php index 30c8ee6..62086b0 100644 --- a/classes/check/bounces.php +++ b/classes/check/bounces.php @@ -62,53 +62,76 @@ public function get_result() : result { [$handlebounces, $minbounces, $bounceratio] = helper::get_bounce_config(); if (empty($handlebounces)) { $status = result::OK; - $summary = "Moodle is not configured to handle bounces."; + $summary = get_string('check:bounces:disabled', 'tool_emailutils'); $details = $summary; } else { - $sql = "SELECT up.userid, up.value AS bouncecount, up2.value AS sendcount + $domainsql = $DB->sql_substr('LOWER(u.email)', $DB->sql_position("'@'", 'u.email') . ' + 1'); + $sql = "SELECT up.userid, up.value AS bouncecount, up2.value AS sendcount, $domainsql AS domain FROM {user_preferences} up LEFT JOIN {user_preferences} up2 ON up2.name = 'email_send_count' AND up.userid = up2.userid + JOIN {user} u ON u.id = up.userid WHERE up.name = 'email_bounce_count' AND CAST(up.value AS INTEGER) > :threshold"; // Start with a threshold of 1 as that was a historical default for manually created users. - $bounces = $DB->get_records_sql($sql, ['threshold' => 1]); + $bounces = $DB->get_records_sql($sql, ['threshold' => 0]); $userswithbounces = count($bounces); // Split bounces into 3 groups based on whether they meet bounce threshold criteria. - $overthreshold = 0; - $overbouncereq = 0; - $underbouncereq = 0; + $breakdown = []; + $total = (object) [ + 'overthreshold' => 0, + 'overbouncereq' => 0, + 'underbouncereq' => 0, + ]; foreach ($bounces as $bounce) { $bouncereq = $bounce->bouncecount >= $minbounces; - $ratioreq = !empty($bounce->sendcount) && ($bounce->bouncecount / $bounce->sendcount >= $bounceratio); + // If there's no send count due to MDL-73798, treat it as 1. + $sendcount = !empty($bounce->sendcount) ? $bounce->sendcount : 1; + $ratioreq = $bounce->bouncecount / $sendcount >= $bounceratio; + + if (!isset($breakdown[$bounce->domain])) { + $breakdown[$bounce->domain] = (object) [ + 'domain' => $bounce->domain, + 'overthreshold' => 0, + 'overbouncereq' => 0, + 'underbouncereq' => 0, + ]; + } if ($bouncereq && $ratioreq) { - $overthreshold++; + $total->overthreshold++; + $breakdown[$bounce->domain]->overthreshold++; } else if (!$ratioreq) { - $overbouncereq++; + $total->overbouncereq++; + $breakdown[$bounce->domain]->overbouncereq++; } else { - $underbouncereq++; + $total->underbouncereq++; + $breakdown[$bounce->domain]->underbouncereq++; } } - $messages = []; + // Order breakdown by users over threshold, overbouncereq, then underbouncereq. + usort($breakdown, function ($a, $b) { + if ($a->overthreshold != $b->overthreshold) { + return $b->overthreshold - $a->overthreshold; + } + if ($a->overbouncereq != $b->overbouncereq) { + return $b->overbouncereq - $a->overbouncereq; + } + return $b->underbouncereq - $a->underbouncereq; + }); + if (!$userswithbounces) { $status = result::OK; - $summary = "No users have had emails rejected."; - } else if (!$overthreshold) { + $summary = get_string('check:bounces:none', 'tool_emailutils'); + } else if (!$total->overthreshold) { $status = result::OK; - $summary = "No users are over the Moodle bounce threshold, but $userswithbounces have had emails rejected"; + $summary = get_string('check:bounces:underthreshold', 'tool_emailutils'); } else { $status = result::WARNING; - $summary = "Found $overthreshold users over the Moodle bounce threshold"; - $messages[] = "$overthreshold user(s) have at least $minbounces email rejections with a bounce ratio over $bounceratio"; - } - - if ($overbouncereq) { - $messages[] = "$overbouncereq user(s) have at least $minbounces email rejections with a bounce ratio under $bounceratio"; - } - - if ($underbouncereq) { - $allowedbounces = $minbounces - 1; - $messages[] = "$underbouncereq user(s) have between 1 and $allowedbounces email rejections"; + $summary = get_string('check:bounces:overthreshold', 'tool_emailutils', [ + 'count' => $total->overthreshold, + 'minbounces' => $minbounces, + 'bounceratio' => $bounceratio, + ]); } // Render config used for calculating threshold. @@ -116,8 +139,8 @@ public function get_result() : result { 'handlebounces' => $handlebounces, 'minbounces' => $minbounces, 'bounceratio' => $bounceratio, - 'breakdown' => true, - 'messages' => $messages, + 'breakdown' => $breakdown, + 'total' => $total, ]); } diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php index 5bf399f..c1a65d7 100644 --- a/lang/en/tool_emailutils.php +++ b/lang/en/tool_emailutils.php @@ -35,6 +35,7 @@ $string['aws_suppressionlist_taskdisabled'] = 'The scheduled task to update this list is disabled.'; $string['aws_suppressionlist_tasknever'] = 'The scheduled task to update this list has never been run successfully.'; $string['aws_suppressionlist_taskupdated'] = 'The scheduled task to update this list was last run {$a} ago.'; +$string['bouncebreakdown'] = 'Breakdown of users with bounces'; $string['bouncecheckfull'] = 'Are you absolutely sure you want to reset the bounce count for {$a} ?'; $string['bouncecount'] = 'Bounce count'; $string['bounceconfig'] = 'Bounce handling is enabled. Emails will not be sent to addresses with over {$a->minbounces} bounces and a bounce ratio above {$a->bounceratio}. These values can be changed in config.php.'; @@ -48,6 +49,13 @@ $string['checkdnsnoreply'] = 'DNS Email noreply shape check'; $string['checkdnspostmastertools'] = 'Check Post master tools'; $string['checkdnsspf'] = 'DNS Email SPF check'; +$string['check:bounces:breakdown:overthreshold'] = 'Over threshold'; +$string['check:bounces:breakdown:overminbounces'] = 'Over minbounces, under ratio'; +$string['check:bounces:breakdown:underminbounces'] = 'Under minbounces'; +$string['check:bounces:disabled'] = 'The site is not configured to handle bounces.'; +$string['check:bounces:none'] = 'No users have recorded bounces.'; +$string['check:bounces:overthreshold'] = 'Found {$a->count} users over the Moodle bounce threshold.'; +$string['check:bounces:underthreshold'] = 'No users are over the Moodle bounce threshold.'; $string['complaints'] = 'For a list of complaints, search for ".c.invalid"'; $string['configmissing'] = 'Bounce handling is not enabled as $CFG->handlebounces is not set. Please review config-dist.php for more information.'; $string['dkimmanager'] = 'SPF & DKIM manager'; diff --git a/templates/bounce_config.mustache b/templates/bounce_config.mustache index fe8b946..3603777 100644 --- a/templates/bounce_config.mustache +++ b/templates/bounce_config.mustache @@ -27,11 +27,15 @@ "handlebounces": true, "minbounces": 10, "bounceratio": 0.2, - "breakdown": true, - "messages": [] + "breakdown": [], + "total": { + "overthreshold": 5, + "overbouncereq": 3, + "underbouncereq": 2 + } } }} -
Config
+
{{#str}} configuration {{/str}}
-{{#breakdown}} -
Breakdown
- -{{/breakdown}} \ No newline at end of file + +
{{#str}} bouncebreakdown, tool_emailutils {{/str}}
+ + + + + + + + + + + {{#breakdown}} + + + + + + + {{/breakdown}} + + + + + + + +
{{#str}} domain, tool_emailutils {{/str}}{{#str}} check:bounces:breakdown:overthreshold, tool_emailutils{{/str}}{{#str}} check:bounces:breakdown:overminbounces, tool_emailutils{{/str}}{{#str}} check:bounces:breakdown:underminbounces, tool_emailutils{{/str}}
{{ domain }}{{ overthreshold }}{{ overbouncereq }}{{ underbouncereq }}
{{#str}} total {{/str}}{{ total.overthreshold }}{{ total.overbouncereq }}{{ total.underbouncereq }}
\ No newline at end of file diff --git a/version.php b/version.php index 335cf76..ca188ea 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024112801; -$plugin->release = 2024112801; +$plugin->version = 2024112900; +$plugin->release = 2024112900; $plugin->requires = 2024042200; $plugin->component = 'tool_emailutils'; $plugin->maturity = MATURITY_STABLE;