feat(dav): store multiple default calendar alarms as JSON#61832
Open
rich0 wants to merge 1 commit into
Open
Conversation
Author
|
The checklist mentions documentation. A quick glance suggests the calendar screenshots are already dated after the initial feature was added in v34, so they definitely could use a refresh after this change as well. I do not know if a focused team typically does this or if this is solicited from contributors. Let me know what the usual approach is and I'll see if I can chip in if needed. The UI for the app was borrowed from the regular event notifications UI so it will be very consistent. |
This was referenced Jul 5, 2026
1cda052 to
1e99678
Compare
Add default_alarms_pday/fday TEXT columns and CalDAV properties default-alarms-part-day/full-day for alarm templates with trigger and DISPLAY|EMAIL action. Legacy integer columns remain in sync for NC34 clients. Includes migration from existing single-int defaults. Assisted-by: Grok:grok-4 Signed-off-by: Richard Freeman <rich@rich0.org>
1e99678 to
24485e2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is hand-written (other than the commit comment below, which was edited by me), but the code changes were created by AI.
I am a novice at PHP and new to Nextcloud development so I invite scrutiny. I noted questions about nextcloud dev conventions and the design below. Everything was reviewed and tested by me with both the stable server and the new app+lib, plus the stable app and the new server.
The intent of this feature is to extend the recent default calendar reminder feature by allowing multiple reminders to be set as well as the notification type for each. Only relative reminder intervals are supported as I didn't think absolute reminders made much sense in a template. This requires changes in server, cdav-library, and the calendar app.
The server revisions are backwards-compatible with the current stable app, and the app revisions are backwards-compatible with the current stable server. Within the server component, updates from legacy apps will update the notification time on the first defined alarm if there are multiple alarms, and legacy apps will be able to read the notification time on the first alarm. This should generally result in the new properties taking precedence if both are getting updated consistently.
The data migration itself does not have unit testing coverage, but I think this is the norm. As long as the schema changes are made, both the read and write paths in the server should migrate data as it is used if the migration did not complete.
The calendar app has a dependency on the updated cdav-library, which is not reflected in package.json as I'm not sure what the convention is for bumping these revisions in tandem.
Related to:
nextcloud/calendar#8567
nextcloud/cdav-library#1066
Commit comment:
Add default_alarms_pday/fday TEXT columns and CalDAV properties default-alarms-part-day/full-day for alarm templates with trigger and DISPLAY|EMAIL action. Legacy integer columns remain in sync for NC34 clients. Includes migration from existing single-int defaults.
Assisted-by: Grok:grok-4
Checklist
3. to review, feature component)stable32)AI (if applicable)