Skip to content

Fix Calendar sends duplicate notifications if there is an additional attendee (#21370) - #30980

Closed
kominoshja wants to merge 3 commits into
nextcloud:masterfrom
kominoshja:patch-1
Closed

Fix Calendar sends duplicate notifications if there is an additional attendee (#21370)#30980
kominoshja wants to merge 3 commits into
nextcloud:masterfrom
kominoshja:patch-1

Conversation

@kominoshja

Copy link
Copy Markdown

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.

@szaimenszaimen added 3. to review Waiting for reviews bug labels Feb 2, 2022
@szaimenszaimen added this to the Nextcloud 24 milestone Feb 2, 2022
@tcitworldtcitworld 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, come-nc, icewind1991 and nickvergessen and removed request for a teamMarch 14, 2022 07:21
->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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

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

Copy link
Copy Markdown

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

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

Copy link
Copy Markdown
Member

Fixed already by #34909

@skjnldsvskjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developingWork in progressbugfeature: caldavRelated to CalDAV internals

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@kominoshja@miaulalala@Irdiism@skjnldsv@nickvergessen@ChristophWurst@blizzz@tcitworld@szaimen