diff --git a/CRM/Mailing/BAO/MailingJob.php b/CRM/Mailing/BAO/MailingJob.php index c52b84721480..0857c950ef27 100644 --- a/CRM/Mailing/BAO/MailingJob.php +++ b/CRM/Mailing/BAO/MailingJob.php @@ -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) { @@ -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 * diff --git a/tests/phpunit/CRM/Mailing/BAO/MailingJobTest.php b/tests/phpunit/CRM/Mailing/BAO/MailingJobTest.php index 85e9b1f1bb72..626a7417d6ad 100644 --- a/tests/phpunit/CRM/Mailing/BAO/MailingJobTest.php +++ b/tests/phpunit/CRM/Mailing/BAO/MailingJobTest.php @@ -16,6 +16,7 @@ * @group headless */ class CRM_Mailing_BAO_MailingJobTest extends CiviUnitTestCase { + use \Civi\Test\Api4TestTrait; /** * Tests CRM_Mailing_BAO_MailingJob::isTemporaryError() method. @@ -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'); + } + }