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

[Bug]: No calendar event email/notification when event content is updated #41084

Open
6 of 8 tasks
flortsch opened this issue Oct 24, 2023 · 6 comments · Fixed by #44017
Open
6 of 8 tasks

[Bug]: No calendar event email/notification when event content is updated #41084

flortsch opened this issue Oct 24, 2023 · 6 comments · Fixed by #44017
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 27-feedback bug feature: caldav Related to CalDAV internals

Comments

@flortsch
Copy link

⚠️ This issue respects the following points: ⚠️

Bug description

When updating content fields of a calendar event (such as the summary, location, link, description, etc.), no email/notification with the updated event is sent to the event participants. As a user, I expect that participants of my event receive an email/notification, when I, e.g., change the location of the event, or when I update the link/url of an event that is supposed to be an online call.

The only way which works and where I can trigger an email/notification is when I update the date/time of the event (e.g. by making the event a minute longer), in addition to updating the content of the event. Another workaround is deleting the event and creating a new event with the updated content. However, this might confuse event participants, because they first receive an email/notification that the original event is cancelled.

Steps to reproduce

  1. Create a calendar event with a participant --> invite email/notification is sent to participant.
  2. Update a content field of the event (such as the location or link/url --> no update email/notification is sent to participant.

Expected behavior

  1. As the participant, receive an email/notification with the invitation to the calendar event.
  2. Receive an email/notification with the updated calendar event.

Installation method

Community Docker image

Nextcloud Server version

27

Operating system

None

PHP engine version

None

Web server

Nginx

Database engine version

MariaDB

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "version": "27.1.2.1",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "ldapIgnoreNamingRules": false,
        "ldapProviderFactory": "OCA\\User_LDAP\\LDAPProviderFactory",
        "maintenance": false,
        "loglevel": 2,
        "app_install_overwrite": [
            "groupfolders",
            "passwords"
        ],
        "theme": "",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "dbtype": "mysql",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "password": "***REMOVED SENSITIVE VALUE***",
            "port": 6379
        },
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "smtp",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "25",
        "mail_smtpsecure": "tls",
        "mail_smtpauth": false,
        "mail_smtpauthtype": "PLAIN",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***"
    }
}

List of activated Apps

Enabled:
  - activity: 2.19.0
  - admin_audit: 1.17.0
  - auto_groups: 1.5.2
  - bruteforcesettings: 2.7.0
  - calendar: 4.5.2
  - cloud_federation_api: 1.10.0
  - contacts: 5.4.2
  - dav: 1.27.0
  - federatedfilesharing: 1.17.0
  - files: 1.22.0
  - files_pdfviewer: 2.8.0
  - files_reminders: 1.0.0
  - files_rightclick: 1.6.0
  - files_sharing: 1.19.0
  - files_trashbin: 1.17.0
  - files_versions: 1.20.0
  - groupfolders: 15.3.1
  - impersonate: 1.14.0
  - logreader: 2.12.0
  - lookup_server_connector: 1.15.0
  - mail: 3.4.3
  - notifications: 2.15.0
  - oauth2: 1.15.1
  - passwords: 2023.10.30
  - provisioning_api: 1.17.0
  - recommendations: 1.6.0
  - richdocuments: 8.2.1
  - settings: 1.9.0
  - spreed: 17.1.1
  - text: 3.8.0
  - theming: 2.2.0
  - twofactor_backupcodes: 1.16.0
  - updatenotification: 1.17.0
  - user_ldap: 1.17.0
  - viewer: 2.1.0
  - workflowengine: 2.9.0
Disabled:
  - circles: 27.0.1 (installed 26.0.0)
  - comments: 1.17.0 (installed 1.16.0)
  - contactsinteraction: 1.8.0 (installed 1.7.0)
  - dashboard: 7.7.0 (installed 7.6.0)
  - encryption: 2.15.0
  - federation: 1.17.0 (installed 1.16.0)
  - files_external: 1.19.0 (installed 1.19.0)
  - firstrunwizard: 2.16.0 (installed 2.15.0)
  - nextcloud_announcements: 1.16.0 (installed 1.15.0)
  - password_policy: 1.17.0 (installed 1.16.0)
  - photos: 2.3.0 (installed 2.2.0)
  - privacy: 1.11.0 (installed 1.10.0)
  - related_resources: 1.2.0 (installed 1.1.0-alpha1)
  - serverinfo: 1.17.0 (installed 1.16.0)
  - sharebymail: 1.17.0 (installed 1.16.0)
  - support: 1.10.0 (installed 1.9.0)
  - survey_client: 1.15.0 (installed 1.14.0)
  - suspicious_login: 5.0.0
  - systemtags: 1.17.0 (installed 1.16.0)
  - twofactor_totp: 9.0.0
  - user_status: 1.7.0 (installed 1.6.0)
  - weather_status: 1.7.0 (installed 1.6.0)

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

No response

@flortsch flortsch added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Oct 24, 2023
@flortsch
Copy link
Author

flortsch commented Nov 8, 2023

Found the same issue in nextcloud/calendar#4428

@joshtrichards joshtrichards added the feature: caldav Related to CalDAV internals label Nov 11, 2023
rcwschaller added a commit to rcwschaller/nextcloud-server that referenced this issue Mar 6, 2024
…etween two events.

Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparision works 'by-label' individually.
Refer to php manual https://www.php.net/manual/en/function.array-diff.php

Partly fixes nextcloud#41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Plus formating fix.

Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
@rcwschaller
Copy link
Contributor

After some digging, I found that this a two part issue:

  1. Some calendar event updates were prevented by a simply wrong array comparison. The fix is provided above, should be easy to review, apply and backport.
  2. Other calendar event updates are not sent because the requirements of https://datatracker.ietf.org/doc/html/rfc5546#section-2.1.4 were not fully implemented. I am working on a fix, that I believe will also resolve Event updates via mail only sent when date/time changes calendar#4428 , email notification after data event update, not only dates [$40] calendar#848 and Send invitation mail for room changes #5222 , but it needs some more testing. In this context I don't think adding more properties to the significantChanges list is the right way. Also I don't think that it is an upstream issue of ITip\Broker. As far as I can tell it does exactly what is specified in the RFC, however its usage here in the nextcloud server is incomplete as the RFC not only requires to observe the listed significantChanges for new notifications to be sent out but also gives the organiser the right to overrule this minimum list of properties by incrementing the SEQUENCE number with any change he thinks is relevant enough.

@flortsch
Copy link
Author

flortsch commented Mar 6, 2024

Nice! The array comparison fix would be a first step. Maybe that also fixes some other open bugs I have seen where users complained about receiving no update notifications.

Regarding 2., you are right, the significantChanges list in ITip\Broker simply represents the fields strictly listed in the RFC and the overrule logic / sequence incrementing when changing other fields should be handled in Nextcloud and not in the upstream parsing library. How are you thinking about fixing this (asking purely out of interest)? I can help with testing.

Cheers, Florian

rcwschaller added a commit to rcwschaller/nextcloud-server that referenced this issue Mar 18, 2024
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
rcwschaller added a commit to rcwschaller/nextcloud-server that referenced this issue Mar 18, 2024
…tic significance evaluation

fixes nextcloud#41084 , nextcloud/calendar#4428 , nextcloud/calendar#848 (presumably closed prematurely), nextcloud#5222 and nextcloud/calendar#5744

Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
@rcwschaller
Copy link
Contributor

How are you thinking about fixing this (asking purely out of interest)?

As trivial as this answer may sound, by implementing the (correct) overrule logic. It is (almost) all there already. Check out the schedule function of apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

Check out master...rcwschaller:nextcloud-server:fix/caldav/respect-event-organizers-increment-of-sequence

I'll add a test for it and put it in a pull request when I find the time.

BTW, good job finding even more open issues with probably the same cause.

@flortsch
Copy link
Author

Ahh, I see! Thought it would be much more complicated to fix. That's very nice!

rcwschaller added a commit to rcwschaller/nextcloud-server that referenced this issue Mar 25, 2024
Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparison works 'by-label' individually.

Partly fixes nextcloud#41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
rcwschaller added a commit to rcwschaller/nextcloud-server that referenced this issue Mar 25, 2024
Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparison works 'by-label' individually.

Partly fixes nextcloud#41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
miaulalala pushed a commit to rcwschaller/nextcloud-server that referenced this issue Mar 25, 2024
Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparison works 'by-label' individually.

Partly fixes nextcloud#41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
backportbot bot pushed a commit that referenced this issue Mar 26, 2024
Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparison works 'by-label' individually.

Partly fixes #41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
backportbot bot pushed a commit that referenced this issue Mar 26, 2024
Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparison works 'by-label' individually.

Partly fixes #41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
backportbot bot pushed a commit that referenced this issue Mar 26, 2024
Old comparison implementation compares each element of the array against each other with no respect for the associated array label, which leads to wrongful removals because one value is accidentally present in a completely different label. New comparison works 'by-label' individually.

Partly fixes #41084 because changes between 'SEQUENCE' not present, 'SEQUENCE:0' and 'SEQUENCE:1' were not detected in the old implementation and thus no email update sent.

Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Robert C. Schaller <gtbc_robert.schaller@rsxc.de>
@rcwschaller
Copy link
Contributor

Please reopen this issue. As mentioned earlier #44017 is only part of the solution and I am still working on the second part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 27-feedback bug feature: caldav Related to CalDAV internals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants