Skip to content
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.

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

Users should be able to choose whether reminders are generated for events in the birthday calendar #1505

Open
nursoda opened this issue Sep 25, 2016 · 67 comments
Labels
3. to review Waiting for reviews enhancement feature: caldav Related to CalDAV internals feature: dav

Comments

@nursoda
Copy link

nursoda commented Sep 25, 2016

I'm not sure if it's correct in "server" component - but it does not only affect calendar or contacts, so it's either both - or "server", thus I report it here. Feel free to point me to re-insert it somewhere else.

Steps to reproduce
Import a vcf file that contains data that contains birthday data.

Actual behaviour
All birthday events in calendar are created with reminder (valarm).

Expected behaviour
The admin should be able to set not to create any EVENT for contacts -or-
The user should be able to opt not to create any EVENT for imported contacts -or-
The admin should be able to set not to create any REMINDER for contacts -or-
The user should be able to opt not to create remindes (valarms) while importing (contacts).

Server configuration
Debian jessie fully patched, Apache 2.4.10 prefork, PHP 5.6.24, MySQL 5.5.52, Nextcloud 10.0 (stable) freshly installed via Webinstaller.

@nickvergessen
Copy link
Member

Well you can also just not sync that birthday calendar, to get rid of the events?

@nursoda
Copy link
Author

nursoda commented Sep 27, 2016

True, thanks!

However, that's not the point, is it? I like to be able to see birthdays on all devices, yet not be reminded.

At least not in the middle of the night ;-) But that's a more general request for enhancement to be able to set a fixed time or time offset for default alarms of all-day events. Which in turn could be a per installation, per account or a per calendar setting.

@nickvergessen
Copy link
Member

@jancborchardt can we have your input here?

@mnlipp
Copy link

mnlipp commented Oct 23, 2016

It's indeed extremely annoying (actually was, see workaround at the end of this comment). I moved from owncloud 9.x to nextcloud 10.0.1 yesterday. I didn't experience this problem before. The reminder pops up every time when the SOGO connector (Thunderbird/Lightning) synchronizes the birthday calendar that is automatically derived from the contacts.

I'm not sure whether this is related to importing contacts, though. I had all my contacts in the database already. So I would rephrase the requirement as "Users should be able to choose whether reminders are generated for events in the birthday calendar".

(To avoid unnecessary searching: the corresponding owncloud issue is owncloud/core#26118. It refers to the Thunderbird problem with read-only calendars https://bugzilla.mozilla.org/show_bug.cgi?id=356002. There I found that I can turn off reminders for a specific calendar in lightning -- wasn't aware of that --, which solved the problem for me.)

@nursoda nursoda changed the title Admin/User should be able to control event/reminder creation upon contacts import Users should be able to choose whether reminders are generated for events in the birthday calendar Oct 23, 2016
@nursoda
Copy link
Author

nursoda commented Oct 23, 2016

Yes, silencing a calendar in a user agent (here: TB) works, but it's the wrong way if you don't want/need reminders at all. Problem is that you need to silence ALL user agents. Using Business Calendar on Android for example, I can switch off the SOUND, but not the notification.

@nickvergessen
Copy link
Member

Yeah, that's why the issue is still open ;)

@jancborchardt
Copy link
Member

The user should be able to opt not to create remindes (valarms) while importing (contacts).

This seems to be the most sensible course of action. It’s definitely not an admin decision.

@georgehrke @raghunayyar @tcitworld we could have a default reminders setting for each calendar, including subscribed ones like birthdays?

@mnlipp
Copy link

mnlipp commented Oct 25, 2016

I'm not sure if a default setting for each calendar is a good idea. The problem here arises from the special birthday calendar that is generated by the server. And for this generated calendar, it is reasonable to allow the user to control if alarms should be generated along with the dates.

For "ordinary" user maintained calendars it is not the servers job to modify them. It should take and preserve whatever clients send to it. Any different behavior makes things harder to understand.

Of course, I have to admit that if the default setting is to not add alarms to dates without alarms, then the proposed feature would do no harm.

@georgehrke
Copy link
Member

georgehrke commented Oct 25, 2016

For "ordinary" user maintained calendars it is not the servers job to modify them. It should take and preserve whatever clients send to it. Any different behavior makes things harder to understand.

The server is never ever going to change any events sent by some clients.

There is a CalDAV extension providing some mechanisms for defining default alarms for allDay and timed events (see https://tools.ietf.org/id/draft-daboo-valarm-extensions-01.html#rfc.section.9)
This extension actually requires the server to add the default VAlarms, but it's only used by some Apple clients and also not implemented by the CalDAV server nextcloud uses.

We could use something similar for the birthday calendar though.
Users will be able to define a custom default reminder and the server will take that choice into account when generating the birthday calendar.

@nickvergessen
Copy link
Member

@jancborchardt so where should we put this setting? Personal page? Contacts app? Calendar app?

@georgehrke
Copy link
Member

I'd go for the calendar app.

@mnlipp
Copy link

mnlipp commented Oct 25, 2016

I'd go for the calendar app.

From a user's perspective, I agree. It's the only place where the contact birthday calendar currently appears in the user interface. (And considering the requests to allow modification of color for this calendar just as with the others, it needs an "edit" dialog on the long run anyway.)

@georgehrke
Copy link
Member

georgehrke commented Oct 25, 2016

Instead of providing default alarms for each calendar, which most clients don't expect anyway, we should simply add a simple select to the calendar settings, that provides the following options:

  • None
  • 9AM on day of birthday
  • 1 day before (9AM)
  • 2 days before (9AM)
  • 1 week before (9AM)

(This select will only influence the birthday calendar, having default alarms for ordinary events is a completely different topic)

@jancborchardt
Copy link
Member

@georgehrke sounds good!

@enoch85
Copy link
Member

enoch85 commented Nov 15, 2016

@georgehrke

(This select will only influence the birthday calendar, having default alarms for ordinary events is a completely different topic)

I would actually want this feature for ordinary calendars. Should I open a new issue or is there one already regarding this?

@georgehrke
Copy link
Member

I would actually want this feature for ordinary calendars. Should I open a new issue or is there one already regarding this?

What exact feature do you want? :)

I don't think we will go for a dedicated input for each calendar.
But I can very well imagine adding a user-based option for all (non-birthday) calendars.
One for all day and one for timed events.

This option should only affect events created in the calendar app though. As said above, the standard for adding alarms on the server is a very uncommon and used only by apple software (to my knowledge). Implementing it might cause very weird behavior in some clients.

@enoch85
Copy link
Member

enoch85 commented Nov 15, 2016

This is what I want:

None
9AM on day of birthday
1 day before (9AM)
2 days before (9AM)
1 week before (9AM)

Works for me. :)

But I can very well imagine adding a user-based option for all (non-birthday) calendars.

@georgehrke
Copy link
Member

Okay :)
Can you please open a feature request in the calendar bug tracker?

@nursoda
Copy link
Author

nursoda commented Mar 22, 2017

Unspecified: What shall happen if one already has birthday events and changes the general setting? Re-Create all birthday calendar alarms?

Anyone willing to set a milestone for this issue? ;-)

@nursoda
Copy link
Author

nursoda commented Dec 19, 2017

Sidenote in case someone stumbles upon the mute issue also discussed within this bug: Read-only calendars do not contain alarms since NC 13, see #6903. Mind that e.g. mobile clients cache existing appointments. After I removed and re-connected my account, an event that has an alarm for the "read-only-user" did not have an alarm on my device.

@CNeutron
Copy link

CNeutron commented Feb 7, 2018

Seems the issue is not mute, as 6903 only concerns shared calendars.
I hope that I'm not seeing something, could you please point me in the right direction on how to get rid of the annoying birthday alarms at midnight?

@georgehrke
Copy link
Member

This issue is still open -> you can't select it yet.

@SamuXzX

This comment has been minimized.

@tsvallender-zz
Copy link

Hi, just run into this issue myself. I was trying to take a look at fixing it but am struggling to work out where the calendar is actually being generated? Is it in the contacts app and if so where? Pointers appreciated.

@tcitworld
Copy link
Member

@tsvallender Hello,
Many thanks for being interested.
The part you're looking for is in the following file, I've highlighted where the alarm is being added:

$alarm = $vCal->createComponent('VALARM');
$alarm->add($vCal->createProperty('TRIGGER', '-PT0M', ['VALUE' => 'DURATION']));
$alarm->add($vCal->createProperty('ACTION', 'DISPLAY'));
$alarm->add($vCal->createProperty('DESCRIPTION', $vEvent->{'SUMMARY'}));
$vEvent->add($alarm);

My personal opinion would be for this config to be handled in the Groupware user settings alongside availability settings (new in Nextcloud 23), instead of the calendar settings like said above.

We could use the setting introduced in nextcloud/calendar#3001 (basic setting for all calendars) as a default, but I would prefer birthday calendars having their own setting.

Unrelated:
The default alarm setting over CalDAV was removed for some reason from the draft-daboo-valarm-extensions draft, which became RFC 9074, however some projects seem to use it anyway. I'm tracking this in the wiki if anyone's curious.

@tsvallender-zz
Copy link

@tcitworld Thank you for the help! I'm not a PHP guy but I'm going to set up a dev environment and see how far I can get with this.

@cneukom
Copy link

cneukom commented Jul 17, 2022

Hi all,

I have started working on this issue (related PR: #33251).

The next step is adding the configuration option to the UI.

My personal opinion would be for this config to be handled in the groupware user settings alongside availability settings (new in Nextcloud 23), instead of the calendar settings like said above.

In my opinion, such a setting should be located close to the "Enable birthday calendar" option. So, I would propose one of the following:

  1. Place the "Birthday reminder" select box in the calendar settings right below the "Enable birthday calendar" option.
  2. Provide both settings ("Enable birthday calendar" and "Birthday reminder") in the groupware user settings and link to them from the calendar app settings. Here we could either:
    1. keep the option "Enable birthday calendar" in the calendar settings to provide a quick way of en-/disabling it,
    2. or we could remove it there and only provide the link to the groupware user settings.

I have a slight preference for the option 2.ii. What do you guys think? @tcitworld

@nursoda
Copy link
Author

nursoda commented Jul 17, 2022

Thanks for your effort! I vote not to implement 2i as this leads to different howtos and users might expect it to be different and need to experiment instead of knowing straight away. I' also vote for 2ii, seems very clean. Also allows for (yet undeveloped) other apps to provide birthday calendar functionality. Only keeping it with the calendar app as in 1 seems wrong as it is a server feature. But there definitely must remain a link or at least an explanation that there is such feature and what may be configured where.

@insilmaril
Copy link

Suggestion looks good, thanks!

@b-pfl
Copy link

b-pfl commented Jul 18, 2022

Just as an additional thought: As it turns out, the reminders are now also sent for regular calendar events, which is annoying if you use CalDav synchronization on your devices. So if there is a way to disable reminders, this should ideally be possible for all different types of reminders.
I also tend more towards 2ii.
Thanks for starting your work on this issue!

@cneukom
Copy link

cneukom commented Jul 28, 2022

Hey all,

So I have implemented option 2ii (see #33397 and nextcloud/calendar#4402), hoping to resolve this issue :)

Just as an additional thought: As it turns out, the reminders are now also sent for regular calendar events, which is annoying if you use CalDav synchronization on your devices. So if there is a way to disable reminders, this should ideally be possible for all different types of reminders.

Hmm, I'm not sure, if I understand you correctly. You are probably referring to the feature discussed in nextcloud/calendar#629... But as I see, there is already the option "No reminder", and it's chosen by default... Also, as stated earlier in this thread, reminders for regular events are usually created by the client, so it might be the case, that one of your clients adds these reminders (and you could probably disable them there).

@nursoda
Copy link
Author

nursoda commented Jul 29, 2022

May all changes be made by just patching / without building? If not, I'll currently only be able to test a (test) release version.

@cneukom
Copy link

cneukom commented Jul 29, 2022

No, unfortunately, testing the changes in the calendar app requires you to build the calendar app. However, the only changes in the calendar app was the renaming of the link and the removal of the "Enable birthday calendar" checkbox in the settings panel. All the rest should be testable by checking out the feature branch of the server component (for checking the generated birthday events, you can use the latest version of the calendar app).

@Naginreed
Copy link

@tsvallender Hello, Many thanks for being interested. The part you're looking for is in the following file, I've highlighted where the alarm is being added:

$alarm = $vCal->createComponent('VALARM');
$alarm->add($vCal->createProperty('TRIGGER', '-PT0M', ['VALUE' => 'DURATION']));
$alarm->add($vCal->createProperty('ACTION', 'DISPLAY'));
$alarm->add($vCal->createProperty('DESCRIPTION', $vEvent->{'SUMMARY'}));
$vEvent->add($alarm);

Did i unterstand that correctly that the workaround so far is to:

  1. get rid of that 5 lines containing the $alarm string in the apps/dav/lib/CalDAV/BirthdayService.php
  2. save the BirthdayService.php file
  3. Go To "Calendar App" -> "Settings & Import" -> "Enable birthday calendar" and Uncheck the Box
  4. Check the "Enable birthday calendar" again to let the server newly build the Birthday Events without Alarms
  5. Resync on the other devices/apps which are read-only anyway for the birthday calendar? (therefore there should be no backsyncs or old entries left)

So far i got no errors or strange behavior on the server, caldav sync in outlook and DAVx5 Sync on Android
and all the events now dont have a reminder.
I get that this has to be done every update until the settings are permanently implemented, but anyone noticed any issues so far with a workaround like this?

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Aug 16, 2022
@fblz
Copy link

fblz commented Jun 15, 2023

It is almost one year since the patch was created by @cneukom.
The patch itself was not merged but part of it seems to have made it into the dav app as of NC25.
https://github.com/nextcloud/server/blob/v27.0.0/apps/dav/lib/CalDAV/BirthdayService.php#L455 now queries a user preference instead of being hard coded.
Despite this, I was unable to find a corresponding UI element to configure the birthdayCalendarReminderOffset preference.

Am I overlooking something here?

For the time being, I set the preference via SQL:
insert into oc_preferences (appid, configkey, configvalue, userid) values ('dav','birthdayCalendarReminderOffset','-PT0M','<USERNAME_HERE>');

With (some) possible values like this.

If you want to keep the alarm, but don't ring at 00:00 AM After every upgrade, in BirthdayService.php Change the line:

$alarm->add($vCal->createProperty('TRIGGER', '-PT0M', ['VALUE' => 'DURATION']));

To ring at 9AM on day of birthday: $alarm->add($vCal->createProperty('TRIGGER', 'PT9H', ['VALUE' => 'DURATION']));

To ring 1 day before (9AM): $alarm->add($vCal->createProperty('TRIGGER', '-PT15H', ['VALUE' => 'DURATION']));

2 days before (9AM) $alarm->add($vCal->createProperty('TRIGGER', '-P1DT15H', ['VALUE' => 'DURATION']));

1 week before (9AM) $alarm->add($vCal->createProperty('TRIGGER', '-P6DT15H', ['VALUE' => 'DURATION']));

@Gezzo42
Copy link

Gezzo42 commented Jul 15, 2023

I was able to remove the reminder by using the following command with Nextcloud 25:
sudo -u www-data php occ user:setting <username> dav birthdayCalendarReminderOffset ""
Perhaps it's a bit easier as the variant using directly the database.

@dgsiegel
Copy link

I was able to remove the reminder by using the following command with Nextcloud 25: sudo -u www-data php occ user:setting <username> dav birthdayCalendarReminderOffset "" Perhaps it's a bit easier as the variant using directly the database.

Works great even on Nextcloud 28. Be sure to recreate the Birthday calendar (e.g. disable and re-enable it), as existing entries won't be updated.

@fblz
Copy link

fblz commented May 24, 2024

The question is, when will the frontend component get the update?

@UlyssesZh
Copy link

UlyssesZh commented Oct 9, 2024

sudo -u www-data php occ user:setting <username> dav birthdayCalendarReminderOffset ""

This does not seem to work with Nextcloud 30.0.0? I re-enabled the birthday calendar after running this, but the reminder is still at 9:00 on every birthday.


Edit: Never mind. It still works. Somehow I have to refresh the webpage.

@Larsn-Rinnstroem
Copy link

After using that command and re-enabling, the birthday calendar is a shared calendar from me with myself instead of being a stand-alone on the list with the other calendars. Is that on purpose?

@UlyssesZh
Copy link

Just refresh the webpage, and the "shared" label will disappear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement feature: caldav Related to CalDAV internals feature: dav
Projects
None yet
Development

Successfully merging a pull request may close this issue.