Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more specific bounce handling #75

Merged
merged 4 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion bounces.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);

Expand All @@ -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();
3 changes: 1 addition & 2 deletions classes/check/bounces.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
58 changes: 58 additions & 0 deletions classes/event/bounce_count_reset.php
Original file line number Diff line number Diff line change
@@ -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 ([email protected])
* @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.";
}
}
9 changes: 3 additions & 6 deletions classes/event/notification_received.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
*/
namespace tool_emailutils\event;

use tool_emailutils;

/**
* Event
*/
Expand All @@ -34,18 +32,17 @@ 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';
}

/**
* Returns localised event name.
*
* @return string
*/
public static function get_name() {
public static function get_name(): string {
return get_string('event:notificationreceived', 'tool_emailutils');
}

Expand All @@ -54,7 +51,7 @@ public static function get_name() {
*
* @return string
*/
public function get_description() {
public function get_description(): mixed {
return $this->other;
}
}
57 changes: 57 additions & 0 deletions classes/event/over_bounce_threshold.php
Original file line number Diff line number Diff line change
@@ -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 ([email protected])
* @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.";
}
}
140 changes: 138 additions & 2 deletions classes/helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

namespace tool_emailutils;

use tool_emailutils\event\bounce_count_reset;

/**
* Helper class for tool_emailutils.
*
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -91,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);
}
}
}
}
2 changes: 1 addition & 1 deletion classes/hook_callbacks.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
));
Expand Down
Loading
Loading