🐛 fix bug: QueueHelper::storeEmail(): $email null given

Civi/Mailinglistsync/ADGroupMailingList.php

@@ -340,8 +340,7 @@ class ADGroupMailingList extends GroupMailingList {
    * @throws \Civi\Mailinglistsync\Exceptions\MailinglistException
    */
   public
-  static function syncContacts($recipients
+  static function syncContacts($recipients): array {
-  ): array {
     $results = [];
 
     foreach ($recipients as $contact) {
@@ -471,12 +470,13 @@ class ADGroupMailingList extends GroupMailingList {
    * @return array|array[]
    */
   private
-  function addRecipient(int $contactId
+  function addRecipient(int $contactId): array {
-  ): array {
     $result = [];
     // Add the contact to the group
     try {
-      $this->addContactToGroup($contactId);
+      $contactEmail = MailingListRecipient::getContactById($contactId)
+        ->getEmailId(self::LOCATION_TYPE);
+      $this->addContactToGroup($contactId, $contactEmail);
       $result['added'] = TRUE;
     }
     catch (MailinglistException $e) {
@@ -496,8 +496,7 @@ class ADGroupMailingList extends GroupMailingList {
    * @return array
    */
   public
-  function removeRecipient(MailingListRecipient $recipient
+  function removeRecipient(MailingListRecipient $recipient): array {
-  ): array {
     $result = [
       'sid' => $recipient->getSid(),
       'contact_id' => $recipient->getContactId(),

Civi/Mailinglistsync/BaseMailingList.php

@@ -95,7 +95,7 @@ abstract class BaseMailingList
    */
   static protected function getCustomFields(): array
   {
-    return CustomField::get()
+    return CustomField::get(FALSE)
       ->addSelect('*')
       ->addWhere('custom_group_id:name', '=', static::CUSTOM_GROUP_NAME)
       ->execute()

Civi/Mailinglistsync/EventMailingList.php

@@ -25,7 +25,7 @@ class EventMailingList extends BaseMailingList {
    * @return array
    */
   protected function getEntity(): array {
-    return $this->event ?? NULL;
+    return $this->event;
   }
 
   /**
@@ -39,6 +39,12 @@ class EventMailingList extends BaseMailingList {
     $this->event = $value;
   }
 
+  /**
+   * Get a list of recipients indexed by email address.
+   *
+   * @return array List of recipients (MailListRecipient)
+   * @throws \Civi\Mailinglistsync\Exceptions\MailinglistException
+   */
   public function getRecipients(): array {
     try {
       $recipientData = Contact::get()
@@ -47,7 +53,7 @@ class EventMailingList extends BaseMailingList {
         ->addJoin('Participant AS participant', 'INNER',
           ['id', '=', 'participant.contact_id'],
           ['participant.event_id', '=', $this->event['id']],
-          ['participant.status', 'IN', self::getEnabledParticipantStatus()],
+          ['participant.status_id', 'IN', self::getEnabledParticipantStatus()],
         )
         ->addGroupBy('id')
         ->execute()
@@ -72,7 +78,7 @@ class EventMailingList extends BaseMailingList {
         );
       } catch (\Exception $e) {
         throw new MailinglistException(
-          "Could not create recipient object for contact with id '{$recipient['id']}'\n$e",
+          "Could not create recipient object for contact with id '{$recipient['id']}': $e",
           MailinglistException::ERROR_CODE_GET_RECIPIENTS_FAILED
         );
       }
@@ -87,6 +93,6 @@ class EventMailingList extends BaseMailingList {
    * @return ?array
    */
   public static function getEnabledParticipantStatus(): ?array {
-    return MailingListSettings::get(E::SHORT_NAME . '_participant_status');
+    return MailingListSettings::get('participant_status');
   }
 }

Civi/Mailinglistsync/Exceptions/MailinglistException.php

@@ -41,5 +41,6 @@ class MailinglistException extends BaseException {
   public const ERROR_CODE_UPDATE_MAILING_LIST_FAILED = 'update_mailing_list_failed';
   public const ERROR_CODE_UPDATE_SUBSCRIBERS_FAILED = 'update_subscribers_failed';
   public const ERROR_CODE_DELETE_MAILING_LIST_FAILED = 'delete_mailing_list_failed';
+  public const ERROR_CODE_GET_EMAIL_ID_FAILED = 'get_email_id_failed';
 }
 

Civi/Mailinglistsync/GroupMailingList.php

@@ -110,7 +110,7 @@ class GroupMailingList extends BaseMailingList {
     }
     catch (\Exception $e) {
       throw new MailinglistException(
-        "Could not get recipients for group with id '{$this->group['id']}'\n$e",
+        "Could not get recipients for group with id '{$this->group['id']}': {$e->getMessage()}",
         MailinglistException::ERROR_CODE_GET_RECIPIENTS_FAILED
       );
     }
@@ -172,16 +172,20 @@ class GroupMailingList extends BaseMailingList {
    * Add a contact to this mailing lists related group.
    *
    * @param int $contactId
+   * @param ?int $contactEmailId
    *
    * @return void
    * @throws \Civi\Mailinglistsync\Exceptions\MailinglistException
    */
-  protected function addContactToGroup(int $contactId): void {
+  protected function addContactToGroup(int $contactId, int $contactEmailId = NULL): void {
     try {
-      \Civi\Api4\GroupContact::create()
+      $query = \Civi\Api4\GroupContact::create()
         ->addValue('contact_id', $contactId)
-        ->addValue('group_id', $this->group['id'])
+        ->addValue('group_id', $this->group['id']);
-        ->execute();
+        if ($contactEmailId) {
+          $query->addValue('email_id', $contactEmailId);
+        }
+        $query->execute();
     }
     catch (\Exception $e) {
       if ($e->getMessage() === 'DB Error: already exists') {

Civi/Mailinglistsync/MailingListRecipient.php

@@ -128,7 +128,7 @@ class MailingListRecipient {
    * @return MailingListRecipient
    * @throws \Civi\Mailinglistsync\Exceptions\MailinglistException
    */
-  public static function getContactIdEmail(string $email): MailingListRecipient {
+  public static function getContactByEmail(string $email): MailingListRecipient {
     try {
       $recipientData = Contact::get()
         ->addSelect('id', 'first_name', 'last_name', 'email.email', 'Active_Directory.SID')
@@ -163,6 +163,49 @@ class MailingListRecipient {
     return $recipient;
   }
 
+  /**
+   * Get a recipient by its id.
+   *
+   * @param int $id
+   *
+   * @return MailingListRecipient
+   * @throws \Civi\Mailinglistsync\Exceptions\MailinglistException
+   */
+  public static function getContactById(int $id): MailingListRecipient {
+    try {
+      $recipientData = Contact::get()
+        ->addSelect('id', 'first_name', 'last_name', 'email.email', 'Active_Directory.SID')
+        ->addJoin('Email AS email', 'LEFT', ['id', '=', 'email.contact_id'])
+        ->addWhere('id', '=', $id)
+        ->addGroupBy('id')
+        ->execute()
+        ->first();
+    }
+    catch (\Exception $e) {
+      throw new MailinglistException(
+        "Could not get recipient by id '{$id}': {$e->getMessage()}",
+        MailinglistException::ERROR_CODE_GET_RECIPIENTS_FAILED
+      );
+    }
+
+    try {
+      $recipient = new MailingListRecipient(
+        sid: $recipientData['Active_Directory.SID'],
+        contact_id: $recipientData['id'],
+        first_name: $recipientData['first_name'],
+        last_name: $recipientData['last_name'],
+        email: $recipientData['email.email'],
+      );
+    } catch (\Exception $e) {
+      throw new MailinglistException(
+        "Could not create recipient object for contact with id '{$recipientData['id']}': {$e->getMessage()}",
+        MailinglistException::ERROR_CODE_GET_RECIPIENTS_FAILED
+      );
+    }
+
+    return $recipient;
+  }
+
   /**
    * Get a list of group mailing lists the contact is subscribed to.
    *
@@ -307,6 +350,32 @@ class MailingListRecipient {
     return $this->email;
   }
 
+  /**
+   * Get the email id.
+   *
+   * @param string $locationType
+   *
+   * @return int|NULL
+   * @throws \Civi\Mailinglistsync\Exceptions\MailinglistException
+   */
+  public function getEmailId(string $locationType): ?int {
+    try {
+      return \Civi\Api4\Email::get()
+        ->addSelect('id')
+        ->addWhere('contact_id', '=', $this->getContactId())
+        ->addWhere('email', '=', $this->email)
+        ->addWhere('location_type_id:name', '=', $locationType)
+        ->execute()
+        ->first()['id'] ?? NULL;
+    }
+    catch (\Exception $e) {
+      throw new MailinglistException(
+        "Failed to get email id: {$e->getMessage()}",
+        MailinglistException::ERROR_CODE_GET_EMAIL_ID_FAILED,
+      );
+    }
+  }
+
   /**
    * Get the SID of the contact.
    *

Civi/Mailinglistsync/QueueHelper.php

@@ -29,21 +29,21 @@ class QueueHelper {
       'type' => 'SqlParallel',
       'is_autorun' => FALSE,
       'reset' => FALSE,
-      'error' => 'drop',
+      'error' => 'abort',
     ]);
     $this->eventQueue = \Civi::queue(
       'propeace-mailinglist-event-queue', [
       'type' => 'SqlParallel',
       'is_autorun' => FALSE,
       'reset' => FALSE,
-      'error' => 'drop',
+      'error' => 'abort',
     ]);
     $this->emailQueue = \Civi::queue(
       'propeace-mailinglist-email-queue', [
       'type' => 'SqlParallel',
       'is_autorun' => FALSE,
       'reset' => FALSE,
-      'error' => 'drop',
+      'error' => 'abort',
     ]);
 
     $this->groups = [];
@@ -106,12 +106,14 @@ class QueueHelper {
    * Stores an email address in the queue helper singleton.
    *
    * @param \CRM_Queue_TaskContext $context
-   * @param string $email
+   * @param string|null $email
    *
    * @return bool
    */
-  public static function storeEmail(\CRM_Queue_TaskContext $context, string $email): bool {
+  public static function storeEmail(\CRM_Queue_TaskContext $context, ?string $email): bool {
+    if (!empty($email)) {
       self::getInstance()->addToEmails($email);
+    }
     return TRUE;
   }
 

LICENSE.txt

@@ -1,5 +1,5 @@
 Package: de.propeace.mailinglistsync
-Copyright (C) 2025, FIXME <FIXME>
+Copyright (C) 2025, Pro Peace <info@propeace.de>
 Licensed under the GNU Affero Public License 3.0 (below).
 
 -------------------------------------------------------------------------------

api/v3/Mailinglistsync/Mlmmjsync.php

@@ -110,7 +110,7 @@ function civicrm_api3_mailinglistsync_Mlmmjsync($params) {
       $mailingListsToSync[$event->getId()] = $event;
     }
     foreach ($emails as $email) {
-      $recipient = MailingListRecipient::getContactIdEmail($email);
+      $recipient = MailingListRecipient::getContactByEmail($email);
       $emailGroups = $recipient->getMailingLists();
       $emailEvents = $recipient->getEventMailingLists();
       foreach ($emailGroups as $group) {
@@ -122,6 +122,7 @@ function civicrm_api3_mailinglistsync_Mlmmjsync($params) {
     }
 
     foreach ($mailingListsToSync as $mailingList) {
+      /* @var \Civi\Mailinglistsync\BaseMailingList $mailingList */
       $results['mailing_lists'][$mailingList->getEmailAddress()] = $mailingList->sync(); // TODO: re-add failed task to queue
     }
 

info.xml

@@ -7,12 +7,12 @@
   <authors>
     <author>
       <name>Marc Koch</name>
-      <email>koch@forumZFD.de</email>
+      <email>marc.koch@propeace.de</email>
       <role>Maintainer</role>
     </author>
   </authors>
   <urls>
-    <!--    <url desc="Main Extension Page">https://FIXME</url>-->
+    <url desc="Main Extension Page">https://git.propeace.de/ProPeace/de.propeace.mailinglistsync</url>-->
     <!--    <url desc="Documentation">https://FIXME</url>-->
     <!--    <url desc="Support">https://FIXME</url>-->
     <url desc="Licensing">https://www.gnu.org/licenses/agpl-3.0.html</url>

mailinglistsync.php

@@ -129,7 +129,7 @@ function mailinglistsync_civicrm_validateForm($formName, &$fields, &$files, &$fo
             'type' => 'SqlParallel',
             'is_autorun' => FALSE,
             'reset' => FALSE,
-            'error' => 'drop',
+            'error' => 'abort',
           ]);
           $queue->createItem(new CRM_Queue_Task(
           // callback
@@ -145,7 +145,7 @@ function mailinglistsync_civicrm_validateForm($formName, &$fields, &$files, &$fo
             'type' => 'SqlParallel',
             'is_autorun' => FALSE,
             'reset' => FALSE,
-            'error' => 'drop',
+            'error' => 'abort',
           ]);
           $queue->createItem(new CRM_Queue_Task(
           // callback
@@ -174,26 +174,6 @@ function mailinglistsync_civicrm_validateForm($formName, &$fields, &$files, &$fo
   }
 }
 
-
-
-function mailinglistsync_civicrm_pre($op, $objectName, $objectId, &$params) {
-  if ($op === 'delete' || $op === 'edit') {
-
-    if ($objectName === 'Group' || $objectName === 'Event') {
-      $mailingList = $objectName === 'Group'
-        ? new GroupMailingList($objectId)
-        : new EventMailingList($objectId);
-      if ($mailingList->isEnabled()) {
-
-        // If email has changed, delete the mailing list and create a new one
-        if ($mailingList->getEmailAddress() !== $params['email']) {
-          //
-        }
-      }
-    }
-  }
-}
-
 /**
  * Implements hook_civicrm_post() to check on permissions to alter mailing list
  * groups and sync group with mlmmj.
@@ -243,10 +223,12 @@ function mailinglistsync_civicrm_post(string $op, string $objectName, int $objec
         ->addWhere('id', '=', $objectId)
         ->execute()
         ->first()['event_id'];
-      $mailingList = new GroupMailingList($eventId);
+      $mailingList = new EventMailingList($eventId);
 
-      // Check permission to alter event mailing list
+      // Check permission to alter event mailing list if it is not a deletion
-      if ($mailingList->isEnabled() && !CRM_Core_Permission::check('manage_event_mailinglists')) {
+      if ($op !== 'delete'
+        && $mailingList->isEnabled()
+        && !CRM_Core_Permission::check('manage_event_mailinglists')) {
         CRM_Core_Session::setStatus(
           E::ts('You do not have permission to manage event mailing lists.'),
           E::ts('Permission Denied'),
@@ -344,7 +326,7 @@ function mailinglistsync_civicrm_postCommit(string $op, string $objectName, int
         'type' => 'SqlParallel',
         'is_autorun' => FALSE,
         'reset' => FALSE,
-        'error' => 'drop',
+        'error' => 'abort',
       ]);
       $queue->createItem(new CRM_Queue_Task(
       // callback
@@ -365,7 +347,7 @@ function mailinglistsync_civicrm_postCommit(string $op, string $objectName, int
         'type' => 'SqlParallel',
         'is_autorun' => FALSE,
         'reset' => FALSE,
-        'error' => 'drop',
+        'error' => 'abort',
       ]);
       $queue->createItem(new CRM_Queue_Task(
       // callback
@@ -392,7 +374,7 @@ function mailinglistsync_civicrm_postCommit(string $op, string $objectName, int
         'type' => 'SqlParallel',
         'is_autorun' => FALSE,
         'reset' => FALSE,
-        'error' => 'drop',
+        'error' => 'abort',
       ]);
       $queue->createItem(new CRM_Queue_Task(
       // callback
@@ -413,7 +395,7 @@ function mailinglistsync_civicrm_postCommit(string $op, string $objectName, int
         'type' => 'SqlParallel',
         'is_autorun' => FALSE,
         'reset' => FALSE,
-        'error' => 'drop',
+        'error' => 'abort',
       ]);
       $queue->createItem(new CRM_Queue_Task(
       // callback
@@ -434,7 +416,7 @@ function mailinglistsync_civicrm_postCommit(string $op, string $objectName, int
         'type' => 'SqlParallel',
         'is_autorun' => FALSE,
         'reset' => FALSE,
-        'error' => 'drop',
+        'error' => 'abort',
       ]);
       $queue->createItem(new CRM_Queue_Task(
       // callback
@@ -469,7 +451,7 @@ function mailinglistsync_civicrm_postCommit(string $op, string $objectName, int
         'type' => 'SqlParallel',
         'is_autorun' => FALSE,
         'reset' => FALSE,
-        'error' => 'drop',
+        'error' => 'abort',
       ]);
       $queue->createItem(new CRM_Queue_Task(
       // callback