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 sends duplicate notifications if there is an additional attendee (#21370) #30980

Closed
wants to merge 3 commits into from

Conversation

kominoshja
Copy link

The bug disclosed on #21370 still occurs on Nextcloud 23. The patch provided by @EhiOnime fixes this issue, but they never made a PR, so it continued to happen.

@szaimen szaimen added 3. to review Waiting for reviews bug labels Feb 2, 2022
@szaimen szaimen added this to the Nextcloud 24 milestone Feb 2, 2022
@tcitworld tcitworld added the feature: caldav Related to CalDAV internals label Feb 4, 2022
@miaulalala

This comment was marked as outdated.

@kominoshja

This comment was marked as outdated.

@miaulalala

This comment was marked as outdated.

@miaulalala miaulalala requested review from a team, nickvergessen, icewind1991 and come-nc and removed request for a team March 14, 2022 07:21
@@ -68,7 +68,8 @@ public function getRemindersToProcess():array {
->from('calendar_reminders', 'cr')
->where($query->expr()->lte('cr.notification_date', $query->createNamedParameter($this->timeFactory->getTime())))
->join('cr', 'calendarobjects', 'co', $query->expr()->eq('cr.object_id', 'co.id'))
->join('cr', 'calendars', 'c', $query->expr()->eq('cr.calendar_id', 'c.id'));
->join('cr', 'calendars', 'c', $query->expr()->eq('cr.calendar_id', 'c.id'))
->groupBy(['cr.event_hash','cr.type','cr.uid']);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

good to know. I guess I have to do the grouping in memory.

Copy link
Member

Choose a reason for hiding this comment

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

Select those columns on L67 as well, then you can still run this on the DB

Copy link
Member

Choose a reason for hiding this comment

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

Select those columns on L67 as well

That's not right it. It's the other way around: All items that are selected need to be aggregated.
But I agree, if we previously did not group on db level, just group manually in php.

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
This was referenced Apr 7, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
@come-nc come-nc removed their request for review May 5, 2022 14:48
This was referenced Aug 12, 2022
@blizzz blizzz mentioned this pull request Aug 24, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@miaulalala miaulalala self-assigned this Sep 26, 2022
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@Irdiism
Copy link

Irdiism commented Oct 4, 2023

This problem persists on NC25. Any plans to merge this patch? Thank you!

@tcitworld tcitworld added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 4, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv
Copy link
Member

Fixed already by #34909

@skjnldsv skjnldsv closed this Feb 27, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug feature: caldav Related to CalDAV internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants