Skip to content

fix: do not re-enable disabled default reminders because of legacy default reminder settings - #8817

Closed
odzhychko wants to merge 1 commit into
mainfrom
fix-do-not-re-enable-disabled-default-reminders-because-of-legacy-default-reminder-settings
Closed

fix: do not re-enable disabled default reminders because of legacy default reminder settings#8817
odzhychko wants to merge 1 commit into
mainfrom
fix-do-not-re-enable-disabled-default-reminders-because-of-legacy-default-reminder-settings

Conversation

@odzhychko

Copy link
Copy Markdown
Contributor

Resolves #8816

Given

  • php occ user:setting <userId> calendar defaultReminder 0
  • Screenshot From 2026-08-31 14-43-04

Before

Screenshot From 2026-08-31 14-43-15

After

Screenshot From 2026-08-31 14-43-33

…fault reminder settings

Resolves #8816

Signed-off-by: Oleksandr Dzhychko <hey@oleks.dev>
@odzhychko

Copy link
Copy Markdown
Contributor Author

/backport to stable6.6

@backportbot backportbot Bot added the backport-request A backport was requested for this pull request label Aug 31, 2026
@odzhychko

Copy link
Copy Markdown
Contributor Author

/backport to stable6.5

@SebastianKrupinski SebastianKrupinski left a comment

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.

This should be a database migration not a work around

@odzhychko

Copy link
Copy Markdown
Contributor Author

This should be a database migration not a work around

Looking at the existing code it seemed like we wanted to support default_reminder state for some reason. But I don't know why 🤷‍♂️

defaultReminderPartDay: loadState('calendar', 'default_reminder_part_day', loadState('calendar', 'default_reminder')),
defaultReminderFullDay: loadState('calendar', 'default_reminder_full_day', loadState('calendar', 'default_reminder')),

defaultReminderPartDay: null,
defaultReminderFullDay: null,
// Legacy fallback for users that have not saved separate part/full-day defaults yet.
defaultReminder: null,

@SebastianKrupinski

Copy link
Copy Markdown
Contributor

Looking at the existing code it seemed like we wanted to support default_reminder state for some reason. But I don't know why 🤷‍♂️

It was just something that was missed originally.

I have a fix, give me a few min

@SebastianKrupinski

Copy link
Copy Markdown
Contributor

Looking at the existing code it seemed like we wanted to support default_reminder state for some reason. But I don't know why 🤷‍♂️

Actually sorry, this was done because the alarm feature was NC34+ and we supported NC32+ in calendar.

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@odzhychko

Copy link
Copy Markdown
Contributor Author

Closing favor of #8818

@odzhychko odzhychko closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-request A backport was requested for this pull request bug Feature: Editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legacy default reminder settings override disabled default reminders

2 participants