Skip to content

CiviMail - Find and fix any missing recipients #36430

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.

Sign up for GitHub

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

Open
colemanw wants to merge 1 commit into
base: master
Choose a base branch
from
+170 −0
Open

CiviMail - Find and fix any missing recipients #36430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 73 additions & 0 deletions CRM/Mailing/BAO/MailingJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ public static function runJobs_post($mode = NULL) {

$anyChildLeft = CRM_Core_DAO::singleValueQuery($child_job_sql, $params);

if (!$anyChildLeft) {
$anyChildLeft = self::queueMissingRecipients((int) $job->mailing_id, (int) $job->id);
}

// all of the child jobs are complete, update
// the parent job as well as the mailing status
if (!$anyChildLeft) {
Expand All @@ -274,6 +278,75 @@ public static function runJobs_post($mode = NULL) {
}
}

/**
* Stopgap to find and enqueue any missing mailing recipients.
*
* This shouldn't be needed, but it's been reported that the initial mailing job sometimes misses recipients.
* This searches for any missing recipients and creates a child mailing job for them.
*
* @see https://lab.civicrm.org/dev/core/-/work_items/6678
*
* @param int $mailingID
* @param int $parentJobID
*
* @return bool True if a new child job was created for unqueued recipients, false otherwise.
*/
private static function queueMissingRecipients(int $mailingID, int $parentJobID): bool {
$sql = "
SELECT mr.email_id, mr.contact_id, mr.phone_id
FROM civicrm_mailing_recipients mr
LEFT JOIN civicrm_mailing_event_queue meq
ON meq.mailing_id = mr.mailing_id
AND meq.is_test = 0
AND (
(mr.email_id IS NOT NULL AND mr.email_id = meq.email_id)
OR (mr.phone_id IS NOT NULL AND mr.phone_id = meq.phone_id)
)
INNER JOIN civicrm_contact cc ON cc.id = mr.contact_id
WHERE mr.mailing_id = %1
AND meq.id IS NULL
AND (mr.email_id > 0 OR mr.phone_id > 0)
AND cc.is_opt_out = 0
";

$unread = CRM_Core_DAO::executeQuery($sql, [1 => [$mailingID, 'Integer']])->fetchAll();
if (count($unread) === 0) {
return FALSE;
}

$maxOffset = (int) CRM_Core_DAO::singleValueQuery("
SELECT MAX(job_offset) FROM civicrm_mailing_job WHERE mailing_id = %1 AND job_type = 'child'
", [1 => [$mailingID, 'Integer']]);

$newJob = MailingJob::create(FALSE)->setValues([
'mailing_id' => $mailingID,
'job_type' => 'child',
'parent_id' => $parentJobID,
'job_offset' => $maxOffset + 1000,
'job_limit' => count($unread),
'status' => 'Scheduled',
])->execute()->first();

$records = array_map(fn($row) => [
'job_id' => $newJob['id'],
'mailing_id' => $mailingID,
'is_test' => FALSE,
'email_id' => $row['email_id'] ? (int) $row['email_id'] : NULL,
'contact_id' => (int) $row['contact_id'],
'phone_id' => $row['phone_id'] ? (int) $row['phone_id'] : NULL,
], $unread);

CRM_Mailing_Event_BAO_MailingEventQueue::writeRecords($records);

Civi::log()->notice("MAILING RECIPIENT IRREGULARITY FIXED: Mailing #{mailing_id} missed {count} recipient(s) in its initial queue. Additional job #{job_id} has been created to ensure the missing recipient(s) receive the mailing. Please report this on https://lab.civicrm.org/dev/core/-/work_items/6678 to help us track down why these irregularities happen in the first place.", [
'mailing_id' => $mailingID,
'count' => count($records),
'job_id' => $newJob['id'],
]);

return TRUE;
}

/**
* before we run jobs, we need to split the jobs
*
Expand Down
97 changes: 97 additions & 0 deletions tests/phpunit/CRM/Mailing/BAO/MailingJobTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* @group headless
*/
class CRM_Mailing_BAO_MailingJobTest extends CiviUnitTestCase {
use \Civi\Test\Api4TestTrait;

/**
* Tests CRM_Mailing_BAO_MailingJob::isTemporaryError() method.
Expand All @@ -40,4 +41,100 @@ public function testIsTemporaryError(): void {
}
}

/**
* Tests CRM_Mailing_BAO_MailingJob::queueMissingRecipients() stopgap.
*/
public function testQueueMissingRecipients(): void {
$contact = $this->createTestRecord('Contact', [
'contact_type' => 'Individual',
'first_name' => 'Reconcile',
'last_name' => 'TestUser',
]);
$contactID = $contact['id'];

$email = $this->createTestRecord('Email', [
'contact_id' => $contactID,
'email' => 'reconcile_test@example.org',
'is_primary' => 1,
]);
$emailID = $email['id'];

// Create a dummy mailing
$mailing = $this->createTestRecord('Mailing', [
'name' => 'Reconciliation Test Mailing',
'subject' => 'Test Subject',
'body_text' => 'Test Body',
'status' => 'Running',
]);
$mailingID = $mailing['id'];

// Create parent job
$parentJob = $this->createTestRecord('MailingJob', [
'mailing_id' => $mailingID,
'status' => 'Running',
'is_test' => 0,
'job_type' => NULL,
]);
$parentJobID = $parentJob['id'];

// Create completed child job
$childJob = $this->createTestRecord('MailingJob', [
'mailing_id' => $mailingID,
'parent_id' => $parentJobID,
'status' => 'Complete',
'is_test' => 0,
'job_type' => 'child',
'job_offset' => 0,
'job_limit' => 10,
]);

// Insert a recipient record into civicrm_mailing_recipients without queueing in civicrm_mailing_event_queue
CRM_Core_DAO::executeQuery("
INSERT INTO civicrm_mailing_recipients (mailing_id, contact_id, email_id)
VALUES (%1, %2, %3)
", [
1 => [$mailingID, 'Integer'],
2 => [$contactID, 'Integer'],
3 => [$emailID, 'Integer'],
]);

// Initial check: recipient is not queued
$reconciled = Invasive::call(['CRM_Mailing_BAO_MailingJob', 'queueMissingRecipients'], [$mailingID, $parentJobID]);
$this->assertTrue($reconciled, 'queueMissingRecipients should return true when unqueued recipients are found');

// Verify a new child job was created
$childJobs = \Civi\Api4\MailingJob::get(FALSE)
->addWhere('parent_id', '=', $parentJobID)
->addWhere('job_type', '=', 'child')
->execute();
$this->assertCount(2, $childJobs, 'A new child job should be created for unqueued recipients');

// Get the new child job ID
$newJob = NULL;
foreach ($childJobs as $job) {
if ($job['id'] !== $childJob['id']) {
$newJob = $job;
break;
}
}
$this->assertNotNull($newJob, 'New child job found');
$this->assertEquals('Scheduled', $newJob['status']);

// Verify recipient was added to civicrm_mailing_event_queue for the new job
$queuedCount = CRM_Core_DAO::singleValueQuery("
SELECT COUNT(*)
FROM civicrm_mailing_event_queue
WHERE job_id = %1 AND email_id = %2 AND contact_id = %3
", [
1 => [$newJob['id'], 'Integer'],
2 => [$emailID, 'Integer'],
3 => [$contactID, 'Integer'],
]);
$this->assertEquals(1, $queuedCount, 'Unqueued recipient should now be present in event queue');

// Second check: no more unqueued recipients
$reconciledAgain = Invasive::call(['CRM_Mailing_BAO_MailingJob', 'queueMissingRecipients'], [$mailingID, $parentJobID]);
$this->assertFalse($reconciledAgain, 'queueMissingRecipients should return false when all recipients are queued');
}

}