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

Fix/calendar reminders #50867

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

matthias480
Copy link

@matthias480 matthias480 commented Feb 17, 2025

Summary

Following problems are fixed:

  • The Groupware setting "Send reminder notifications to calendar sharees as well" (sendEventRemindersToSharedUsers) behaved exactly the other way round.
    If it was activated, sharees din't receive a notification.
  • Members of groups with names that contain special characters (also spaces) didn't receive notifications.
  • Calendar principal user received reminder notification twice if he/she was also a sharee (emails not affected as EmailProvider filters duplicates)

Checklist

@matthias480
Copy link
Author

/backport to stable31

@matthias480
Copy link
Author

/backport to stable30

@matthias480
Copy link
Author

/backport to stable29

@SebastianKrupinski
Copy link
Contributor

Hi @matthias480

Thank you for the PR. I will test/review it when I get a min.

But I have some initial comments,

A. What is the best way to test this, can you give me a short step by step.
B. Once the changes are approved I will need to recreate this PR as our CI will not run properly on forked PR's
C. We will only backport this back to 29 as our policy is 3 version for backport unless they are security fixes.

@matthias480
Copy link
Author

Hi @SebastianKrupinski

Thank you!

B. OK. I made a fork, because I had no permission to push my branch.
C. OK, I removed 28 and 27.
A. How to test:

Preparation:

  • Have at least 3 users with email addresses.
  • Configure your instance to be able to send emails.
  • Enable Groupware setting "Send notifications for events".
  • Run php occ config:app:set dav sendEventRemindersMode --value occ in order to make sending of event reminders triggerable via occ command

Issue 1:

  1. Create new calendar with user 1.
  2. Share calendar with user 2 (write access).
  3. Enable Groupware setting "Send reminder notifications to calendar sharees as well".
  4. Create new event with one email reminder and one notification reminder.
  5. Save the event.
  6. Trigger sending when reminders are due via php occ dav:send-event-reminders.
  7. Observe:
    • Without this PR:
      • User 1 receives reminder mail and notification.
      • User 2 doesn't receive any reminder although "Send reminder notifications to calendar sharees as well" is enabled.
    • With this PR:
      • User 1 and user 2 receive reminder mail and notification.
  8. Disable Groupware setting "Send reminder notifications to calendar sharees as well".
  9. Repeat 4-6
  10. Observe:
    • Without this PR:
      • User 1 receives reminder mail and notification.
      • User 2 receives reminder mail and notification although "Send reminder notifications to calendar sharees as well" is disabled.
    • With this PR:
      • User 1 receives reminder mail and notification.
      • User 2 doesn't receive any reminder.

Issue 2 + 3:

  1. Create new group with name "Group1".
  2. Create new group with name "Group Two".
  3. Add user 1 and user 3 to "Group1", add user 2 to "Group Two".
  4. Create new calendar with user 1.
  5. Share calendar with both groups (write access).
  6. Enable Groupware setting "Send reminder notifications to calendar sharees as well". Without this PR: Disable setting since the behavior is the opposite..
  7. Create new event with one email reminder and one notification reminder.
  8. Save the event.
  9. Trigger sending when reminders are due via php occ dav:send-event-reminders.
  10. Observe:
    • Without this PR:
      • User 1 receives two notifications (because he is the owner/principal and member of Group1) and one mail (mail duplicates are filtered in EmailProvider).
      • User 2 doesn't receive any reminder because group name contains special character (space).
      • User 3 receives reminder mail and notification because group name doesn't contain special characters.
    • With this PR:
      • User 1, 2 and 3 receive one notification and one reminder mail.

Remark: I tested with cron instead of php occ dav:send-event-reminders.

@SebastianKrupinski
Copy link
Contributor

Hi @matthias480

Thank you for the very thoroughly step by step, it make it easier for us to reproduce the issues and test the fixes.

Please give me a few days to test this.

@AndyScherzinger AndyScherzinger added the 3. to review Waiting for reviews label Feb 19, 2025
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Calendar reminders not being sent if group name has special characters
3 participants