Skip to content

Commit 6772b04

Browse files
bwalkerlbrendanheywood
bwalkerl
authored and
brendanheywood
committed
Clean up bounce check and add breakdown #74 
1 parent e2ea86b commit 6772b04

File tree

4 files changed

+95
-40
lines changed

4 files changed

+95
-40
lines changed

‎classes/check/bounces.php

+50-27
Original file line numberDiff line numberDiff line change
@@ -62,62 +62,85 @@ public function get_result() : result {
6262
[$handlebounces, $minbounces, $bounceratio] = helper::get_bounce_config();
6363
if (empty($handlebounces)) {
6464
$status = result::OK;
65-
$summary = "Moodle is not configured to handle bounces.";
65+
$summary = get_string('check:bounces:disabled', 'tool_emailutils');
6666
$details = $summary;
6767
} else {
68-
$sql = "SELECT up.userid, up.value AS bouncecount, up2.value AS sendcount
68+
$domainsql = $DB->sql_substr('LOWER(u.email)', $DB->sql_position("'@'", 'u.email') . ' + 1');
69+
$sql = "SELECT up.userid, up.value AS bouncecount, up2.value AS sendcount, $domainsql AS domain
6970
FROM {user_preferences} up
7071
LEFT JOIN {user_preferences} up2 ON up2.name = 'email_send_count' AND up.userid = up2.userid
72+
JOIN {user} u ON u.id = up.userid
7173
WHERE up.name = 'email_bounce_count' AND CAST(up.value AS INTEGER) > :threshold";
7274
// Start with a threshold of 1 as that was a historical default for manually created users.
73-
$bounces = $DB->get_records_sql($sql, ['threshold' => 1]);
75+
$bounces = $DB->get_records_sql($sql, ['threshold' => 0]);
7476
$userswithbounces = count($bounces);
7577

7678
// Split bounces into 3 groups based on whether they meet bounce threshold criteria.
77-
$overthreshold = 0;
78-
$overbouncereq = 0;
79-
$underbouncereq = 0;
79+
$breakdown = [];
80+
$total = (object) [
81+
'overthreshold' => 0,
82+
'overbouncereq' => 0,
83+
'underbouncereq' => 0,
84+
];
8085
foreach ($bounces as $bounce) {
8186
$bouncereq = $bounce->bouncecount >= $minbounces;
82-
$ratioreq = !empty($bounce->sendcount) && ($bounce->bouncecount / $bounce->sendcount >= $bounceratio);
87+
// If there's no send count due to MDL-73798, treat it as 1.
88+
$sendcount = !empty($bounce->sendcount) ? $bounce->sendcount : 1;
89+
$ratioreq = $bounce->bouncecount / $sendcount >= $bounceratio;
90+
91+
if (!isset($breakdown[$bounce->domain])) {
92+
$breakdown[$bounce->domain] = (object) [
93+
'domain' => $bounce->domain,
94+
'overthreshold' => 0,
95+
'overbouncereq' => 0,
96+
'underbouncereq' => 0,
97+
];
98+
}
8399
if ($bouncereq && $ratioreq) {
84-
$overthreshold++;
100+
$total->overthreshold++;
101+
$breakdown[$bounce->domain]->overthreshold++;
85102
} else if (!$ratioreq) {
86-
$overbouncereq++;
103+
$total->overbouncereq++;
104+
$breakdown[$bounce->domain]->overbouncereq++;
87105
} else {
88-
$underbouncereq++;
106+
$total->underbouncereq++;
107+
$breakdown[$bounce->domain]->underbouncereq++;
89108
}
90109
}
91110

92-
$messages = [];
111+
// Order breakdown by users over threshold, overbouncereq, then underbouncereq.
112+
usort($breakdown, function ($a, $b) {
113+
if ($a->overthreshold != $b->overthreshold) {
114+
return $b->overthreshold - $a->overthreshold;
115+
}
116+
if ($a->overbouncereq != $b->overbouncereq) {
117+
return $b->overbouncereq - $a->overbouncereq;
118+
}
119+
return $b->underbouncereq - $a->underbouncereq;
120+
});
121+
93122
if (!$userswithbounces) {
94123
$status = result::OK;
95-
$summary = "No users have had emails rejected.";
96-
} else if (!$overthreshold) {
124+
$summary = get_string('check:bounces:none', 'tool_emailutils');
125+
} else if (!$total->overthreshold) {
97126
$status = result::OK;
98-
$summary = "No users are over the Moodle bounce threshold, but $userswithbounces have had emails rejected";
127+
$summary = get_string('check:bounces:underthreshold', 'tool_emailutils');
99128
} else {
100129
$status = result::WARNING;
101-
$summary = "Found $overthreshold users over the Moodle bounce threshold";
102-
$messages[] = "$overthreshold user(s) have at least $minbounces email rejections with a bounce ratio over $bounceratio";
103-
}
104-
105-
if ($overbouncereq) {
106-
$messages[] = "$overbouncereq user(s) have at least $minbounces email rejections with a bounce ratio under $bounceratio";
107-
}
108-
109-
if ($underbouncereq) {
110-
$allowedbounces = $minbounces - 1;
111-
$messages[] = "$underbouncereq user(s) have between 1 and $allowedbounces email rejections";
130+
$summary = get_string('check:bounces:overthreshold', 'tool_emailutils', [
131+
'count' => $total->overthreshold,
132+
'minbounces' => $minbounces,
133+
'bounceratio' => $bounceratio,
134+
]);
112135
}
113136

114137
// Render config used for calculating threshold.
115138
$details = $OUTPUT->render_from_template('tool_emailutils/bounce_config', [
116139
'handlebounces' => $handlebounces,
117140
'minbounces' => $minbounces,
118141
'bounceratio' => $bounceratio,
119-
'breakdown' => true,
120-
'messages' => $messages,
142+
'breakdown' => $breakdown,
143+
'total' => $total,
121144
]);
122145
}
123146

‎lang/en/tool_emailutils.php

+8
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
$string['aws_suppressionlist_taskdisabled'] = 'The scheduled task to update this list is disabled.';
3636
$string['aws_suppressionlist_tasknever'] = 'The scheduled task to update this list has never been run successfully.';
3737
$string['aws_suppressionlist_taskupdated'] = 'The scheduled task to update this list was last run {$a} ago.';
38+
$string['bouncebreakdown'] = 'Breakdown of users with bounces';
3839
$string['bouncecheckfull'] = 'Are you absolutely sure you want to reset the bounce count for {$a} ?';
3940
$string['bouncecount'] = 'Bounce count';
4041
$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 @@
4849
$string['checkdnsnoreply'] = 'DNS Email noreply shape check';
4950
$string['checkdnspostmastertools'] = 'Check Post master tools';
5051
$string['checkdnsspf'] = 'DNS Email SPF check';
52+
$string['check:bounces:breakdown:overthreshold'] = 'Over threshold';
53+
$string['check:bounces:breakdown:overminbounces'] = 'Over minbounces, under ratio';
54+
$string['check:bounces:breakdown:underminbounces'] = 'Under minbounces';
55+
$string['check:bounces:disabled'] = 'The site is not configured to handle bounces.';
56+
$string['check:bounces:none'] = 'No users have recorded bounces.';
57+
$string['check:bounces:overthreshold'] = 'Found {$a->count} users over the Moodle bounce threshold.';
58+
$string['check:bounces:underthreshold'] = 'No users are over the Moodle bounce threshold.';
5159
$string['complaints'] = 'For a list of complaints, search for ".c.invalid"';
5260
$string['configmissing'] = 'Bounce handling is not enabled as $CFG->handlebounces is not set. Please review config-dist.php for more information.';
5361
$string['dkimmanager'] = 'SPF & DKIM manager';

‎templates/bounce_config.mustache

+35-11
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,47 @@
2727
"handlebounces": true,
2828
"minbounces": 10,
2929
"bounceratio": 0.2,
30-
"breakdown": true,
31-
"messages": []
30+
"breakdown": [],
31+
"total": {
32+
"overthreshold": 5,
33+
"overbouncereq": 3,
34+
"underbouncereq": 2
35+
}
3236
}
3337
}}
34-
<h5>Config</h5>
38+
<h5>{{#str}} configuration {{/str}}</h5>
3539
<ul>
3640
<li>handlebounces: {{handlebounces}}</li>
3741
{{#handlebounces}}
3842
<li>minbounces: {{minbounces}}</li>
3943
<li>bounceratio: {{bounceratio}}</li>
4044
{{/handlebounces}}
4145
</ul>
42-
{{#breakdown}}
43-
<h5>Breakdown</h5>
44-
<ul>
45-
{{#messages}}
46-
<li>{{.}}</li>
47-
{{/messages}}
48-
</ul>
49-
{{/breakdown}}
46+
47+
<h5>{{#str}} bouncebreakdown, tool_emailutils {{/str}}</h5>
48+
<table class="admintable generaltable table-bordered table-sm w-auto">
49+
<thead>
50+
<tr>
51+
<th>{{#str}} domain, tool_emailutils {{/str}}</th>
52+
<th class="text-end">{{#str}} check:bounces:breakdown:overthreshold, tool_emailutils{{/str}}</th>
53+
<th class="text-end">{{#str}} check:bounces:breakdown:overminbounces, tool_emailutils{{/str}}</th>
54+
<th class="text-end">{{#str}} check:bounces:breakdown:underminbounces, tool_emailutils{{/str}}</th>
55+
</tr>
56+
</thead>
57+
<tbody>
58+
{{#breakdown}}
59+
<tr>
60+
<td>{{ domain }}</td>
61+
<td class="text-end">{{ overthreshold }}</td>
62+
<td class="text-end">{{ overbouncereq }}</td>
63+
<td class="text-end">{{ underbouncereq }}</td>
64+
</tr>
65+
{{/breakdown}}
66+
<tr class="font-weight-bold">
67+
<td>{{#str}} total {{/str}}</td>
68+
<td class="text-end">{{ total.overthreshold }}</td>
69+
<td class="text-end">{{ total.overbouncereq }}</td>
70+
<td class="text-end">{{ total.underbouncereq }}</td>
71+
</tr>
72+
</tbody>
73+
</table>

‎version.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525

2626
defined('MOODLE_INTERNAL') || die();
2727

28-
$plugin->version = 2024112801;
29-
$plugin->release = 2024112801;
28+
$plugin->version = 2024112900;
29+
$plugin->release = 2024112900;
3030
$plugin->requires = 2024042200;
3131
$plugin->component = 'tool_emailutils';
3232
$plugin->maturity = MATURITY_STABLE;

0 commit comments

Comments
 (0)