Skip to content

Commit

Permalink
Bug: 15022 Fix regression in sending event change notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
mrubinsk committed Jul 12, 2020
1 parent bcdb1ca commit 9484e1d
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions lib/Kronolith.php
Expand Up @@ -2137,16 +2137,18 @@ public static function sendNotification($event, $action)

$owner = $share->get('owner');
if ($owner) {
$recipients[$owner] = self::_notificationPref($owner, 'owner');
$recipients[$owner]['private'] = $event->isPrivate($owner);
if ($notificationPrefs = self::_notificationPref($owner, 'owner')) {
$recipients[$owner] = $notificationPrefs;
$recipients[$owner]['private'] = $event->isPrivate($owner);
}
}

$senderIdentity = $injector->getInstance('Horde_Core_Factory_Identity')
->create($registry->getAuth() ?: $event->creator ?: $owner);

foreach ($share->listUsers(Horde_Perms::READ) as $user) {
if (empty($recipients[$user])) {
$recipients[$user] = self::_notificationPref($user, 'read', $calendar);
if (empty($recipients[$user]) && ($notificationPrefs = self::_notificationPref($user, 'read', $calendar))) {
$recipients[$user] = $notificationPrefs;
$recipients[$user]['private'] = $event->isPrivate($user);
}
}
Expand All @@ -2160,8 +2162,8 @@ public static function sendNotification($event, $action)
}

foreach ($group_users as $user) {
if (empty($recipients[$user])) {
$recipients[$user] = self::_notificationPref($user, 'read', $calendar);
if (empty($recipients[$user]) && ($notificationPrefs = self::_notificationPref($user, 'read', $calendar))) {
$recipients[$user] = $notificationPrefs;
$recipients[$user]['private'] = $event->isPrivate($user);
}
}
Expand Down

1 comment on commit 9484e1d

@dpetrov67
Copy link

@dpetrov67 dpetrov67 commented on 9484e1d Jul 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code could be made simpler. There is not need to add 'private" element and non-empty check in 3 different places.

Two possible solutions:

  1. Pass $event as parameter to _notificationPref() and add 'private" element in there

  2. Even better - check if event is private inside of the recipients loop. This then would result in minimal changes (all the changes in this commit could then be reverted):

      $addresses = array();
      foreach ($recipients as $user => $vals) {
          if (!$vals) {
              continue;
          }
    
          $identity = $injector->getInstance('Horde_Core_Factory_Identity')->create($user);
          $email = $identity->getValue('from_addr');
          if (strpos($email, '@') === false) {
              continue;
          }
    
          $private = $event->isPrivate($user);
    
          if (!isset($addresses[$vals['lang']][$vals['tf']][$vals['df']][$private])) {
              $addresses[$vals['lang']][$vals['tf']][$vals['df']][$private]] = array();
          }
          $tmp = new Horde_Mail_Rfc822_Address($email);
          $tmp->personal = $identity->getValue('fullname');
          $addresses[$vals['lang']][$vals['tf']][$vals['df']][$private][] = strval($tmp);
      }
    

I can create a patch, if needed.

Please sign in to comment.