Tickets :: Comment on [#8226] Change notifications are not sent to owner

* Your Email Address
* Spam protection	Enter the letters below: .__ .__ \ /.___.__. [ __\| \ >< [__ \| \| [_./\|__// \[___\|__\
Comment	>>> In cases 'read' and 'show', $mode should also be allowed to be 'owner'. > >> No, because that's what 'read' and 'show' mean. If we'd change it > >> like you do, the user would get a notification if he is the owner of > >> a calendar even if he doesn't show the calendar and has set in his > >> preference that he only want to be notified about displayed calendars. > > > > I don't agree. This would not happen because there still is a check > for 'display_cals' pref. However, one would need to pass the third > parameter to the method to make it actually work. It is easy to > confuse $mode with the 'event_notification' pref. > > > > But let's go through all possible cases: > > > > Case 1: $mode == 'owner' > > The user $user is the owner of the current callendar. > > We should send the owner a notification when his event_notification > pref is set to 'owner'. But also, when it is set to 'read' and we > should do the show check when it is set to 'show'. If we don't do it, > the first call with $mode == 'owner' will return false, resulting in > isset(recipients[$user]) later in sendNotification(). This means that > _notificationPref() will not be called for this user again and the > user won't get a notification for his own calendar he has read access > to. > > > > Case 2: $mode == 'read' > > In case the 'event_notification' pref is set to owner, we need to > return "false" because the owner of the calendar has been alread > handled with the first invocation of _notificationPref() and it is > not called for a user twice. Therefore, the user cannot be the owner. > For 'event_notification' being equal to 'read' return $vals and for > 'show' do the check. > > > > ------ > > The easiest solution is to remove the check for $mode in case > 'event_notification' is 'read' and return $vals unconditionally. Also > remove the check in the 'show' case, but of course leave the > display_cals check intact. For this to work with $mode == 'owner', > we also need to modify the first call in sendNotification to also > include $calendar as third parameter. > > > > Thus, the switch should be > > > > switch ($prefs->getValue('event_notification')) { > > case 'owner': > > // do not notify unless owner > > return $mode == 'owner' ? $vals : false; > > > > case 'read': > > // notify about changes in all calendars user has read > access to which is > > // true for all users this method is called for (see > sendNotification) > > return $vals; > > > > case 'show': > > // notify about changes in shown calendars only. > > $display_calendars = > unserialize($prefs->getValue('display_cals')); > > return in_array($calendar, $display_calendars) ? $vals : false; > > } > > > > and the call in sendNotification > > > > $recipients[$owner] = Kronolith::_notificationPref($owner, > 'owner', $calendar); > > > > The above is true under the assumption that the owner has always read > access to his own calendars.
Attachment
Watch this ticket

Saved Queries