fix(dav): fix event birthday alarms not being updated#39977

Open

tcitworld wants to merge 2 commits intomasterfrom

dav-fix-birthday-sync

Member

tcitworld commented Aug 19, 2023

In #33251 the default offset for a birthday event alarm was changed to 9AM on the day of the event.

However the birthdayEvenChanged method did not account for alarm changes, so it was never propagated to existing birthday events, which were kept on midnight.

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

tcitworld added bug 3. to review feature: caldav feature: carddav labels

tcitworld added this to the Nextcloud 28 milestone

tcitworld requested a review from a team

August 19, 2023 09:47

tcitworld requested review from ChristophWurst and miaulalala as code owners

August 19, 2023 09:47

Member Author

tcitworld commented Aug 19, 2023

/backport to stable27

backportbot-nextcloud bot added the backport-request label

Member Author

tcitworld commented Aug 19, 2023

/backport to stable26

Member Author

tcitworld commented Aug 19, 2023

/backport to stable25

github-advanced-security bot found potential problems

View reviewed changes

apps/dav/lib/CalDAV/BirthdayService.php

               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||
+              			($newCalendarData->VEVENT->VALARM && $existingBirthday->VEVENT->VALARM && $newCalendarData->VEVENT->VALARM->TRIGGER->getValue() !== $existingBirthday->VEVENT->VALARM->TRIGGER->getValue())

Check failure

Code scanning / Psalm

UndefinedPropertyFetch

Instance property Sabre\VObject\Property::$VALARM is not defined

apps/dav/lib/CalDAV/BirthdayService.php

               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||
+              			($newCalendarData->VEVENT->VALARM && $existingBirthday->VEVENT->VALARM && $newCalendarData->VEVENT->VALARM->TRIGGER->getValue() !== $existingBirthday->VEVENT->VALARM->TRIGGER->getValue())

Check failure

Code scanning / Psalm

UndefinedPropertyFetch

Instance property Sabre\VObject\Property::$VALARM is not defined

apps/dav/lib/CalDAV/BirthdayService.php

               		return (
               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||

Check notice

Code scanning / Psalm

PossiblyNullPropertyFetch

Cannot get property on possibly null variable $newCalendarData->VEVENT of type Sabre\VObject\Property|null

apps/dav/lib/CalDAV/BirthdayService.php

               		return (
               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||

Check notice

Code scanning / Psalm

UndefinedPropertyFetch

Instance property Sabre\VObject\Property::$SUMMARY is not defined

apps/dav/lib/CalDAV/BirthdayService.php

               		return (
               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getValue on possibly null value

apps/dav/lib/CalDAV/BirthdayService.php

               		return (
               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||

Check notice

Code scanning / Psalm

PossiblyNullPropertyFetch

Cannot get property on possibly null variable $existingBirthday->VEVENT of type Sabre\VObject\Property|null

apps/dav/lib/CalDAV/BirthdayService.php

               		return (
               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||

Check notice

Code scanning / Psalm

UndefinedPropertyFetch

Instance property Sabre\VObject\Property::$SUMMARY is not defined

apps/dav/lib/CalDAV/BirthdayService.php

               		return (
               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getValue on possibly null value

apps/dav/lib/CalDAV/BirthdayService.php

               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||
+              			($newCalendarData->VEVENT->VALARM && $existingBirthday->VEVENT->VALARM && $newCalendarData->VEVENT->VALARM->TRIGGER->getValue() !== $existingBirthday->VEVENT->VALARM->TRIGGER->getValue())

Check notice

Code scanning / Psalm

PossiblyNullPropertyFetch

Cannot get property on possibly null variable $newCalendarData->VEVENT of type Sabre\VObject\Property|null

apps/dav/lib/CalDAV/BirthdayService.php

               			$newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() ||
-              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue()
+              			$newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() ||
+              			($newCalendarData->VEVENT->VALARM && $existingBirthday->VEVENT->VALARM && $newCalendarData->VEVENT->VALARM->TRIGGER->getValue() !== $existingBirthday->VEVENT->VALARM->TRIGGER->getValue())

Check notice

Code scanning / Psalm

PossiblyNullPropertyFetch

Cannot get property on possibly null variable $existingBirthday->VEVENT of type Sabre\VObject\Property|null

nfebe approved these changes

View reviewed changes

Contributor

nfebe left a comment

LGTM

come-nc approved these changes

View reviewed changes

miaulalala force-pushed the dav-fix-birthday-sync branch from 03a300c to f3ccb5d Compare

August 28, 2023 10:35

tcitworld force-pushed the dav-fix-birthday-sync branch from f3ccb5d to 24e6ef5 Compare

October 2, 2023 09:33

Member Author

tcitworld commented Oct 2, 2023

Should we add a repair job just like RegenerateBirthdayCalendars to fix all existing events? Or just edit the app config value from regeneratedBirthdayCalendarsForYearFix to something like regeneratedBirthdayCalendarsForAlarmFix so that it's triggered again?

Contributor

miaulalala commented Oct 2, 2023

Should we add a repair job just like RegenerateBirthdayCalendars to fix all existing events? Or just edit the app config value from regeneratedBirthdayCalendarsForYearFix to something like regeneratedBirthdayCalendarsForAlarmFix so that it's triggered again?

Repair step sounds good to me.

tcitworld force-pushed the dav-fix-birthday-sync branch 2 times, most recently from 3ba9335 to c2c2c8b Compare

October 2, 2023 15:12

miaulalala approved these changes

View reviewed changes

blizzz force-pushed the dav-fix-birthday-sync branch from c2c2c8b to 87c8581 Compare

October 11, 2023 13:33

skjnldsv mentioned this pull request

chore: 28.0.0 beta 1 #41228

Merged

This was referenced Nov 6, 2023

28.0.0 beta 2 #41314

Merged

28.0.0 beta 3 #41379

Merged

This was referenced Nov 14, 2023

28.0.0 beta 4 #41477

Merged

28.0.0 RC1 #41621

Merged

blizzz modified the milestones: Nextcloud 28, Nextcloud 29

Altahrim mentioned this pull request

29.0.0 beta 6 #44449

Merged

skjnldsv mentioned this pull request

29.0.0 RC1 #44552

Merged

81 tasks

skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30

This was referenced Jul 30, 2024

30.0.0 beta 2 #46870

Merged

30.0.0 beta 3 #46946

Merged

This was referenced Aug 5, 2024

30.0.0 beta 4 #47028

Merged

30.0.0 beta 5 #47100

Merged

skjnldsv mentioned this pull request

30.0.0 RC1 #47190

Merged

skjnldsv removed this from the Nextcloud 30 milestone

tcitworld mentioned this pull request

Birthday event alert should happen in the morning, not at midnight #28747

Open

skjnldsv added this to the Nextcloud 32 milestone

This was referenced Aug 22, 2025

32.0.0 beta 2 #54581

Merged

32.0.0 beta 3 #54606

Merged

32.0.0 beta 4 #54688

Merged

This was referenced Sep 2, 2025

32.0.0 beta 5 #54796

Merged

[stable32] 32.0.0 RC1 #54864

Merged

nextcloud-bot mentioned this pull request

[stable32] 32.0.0 RC2 #55000

Merged

nextcloud-bot mentioned this pull request

[stable32] 32.0.0 RC3 #55162

Merged

This was referenced Sep 25, 2025

[stable32] 32.0.0 RC4 #55298

Merged

[stable32] 32.0.0 #55354

Merged

skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33

This was referenced Jan 7, 2026

33.0.0 beta 2 #57398

Merged

33.0.0 beta 3 #57441

Merged

This was referenced Jan 14, 2026

33.0.0 beta 4 #57545

Merged

33.0.0 beta 5 #57651

Merged

nextcloud-bot mentioned this pull request

[stable33] 33.0.0 RC1 #57724

Merged

This was referenced Jan 29, 2026

[stable33] 33.0.0 RC2 #57899

Merged

[stable33] 33.0.0 RC3 #58047

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing backport-request bug feature: caldav feature: carddav