Recurring Calendar Series#337
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds creation/editing/deletion support for recurring custom calendar events (daily/weekly/biweekly/monthly) by generating multiple bounded single events and tracking series membership locally (with best-effort cross-device series inference when local membership is missing).
Changes:
- Adds recurrence controls and a revised event creation flow (including series editing UI and save progress handling).
- Introduces local persistence for series membership and series-level actions (edit/delete series).
- Updates routing and calendar details UI to handle custom-event details in a full screen scaffold (instead of a dialog).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/calendarComponent/views/visibility_button_view.dart | Adjusts visibility icon sizing/layout. |
| lib/calendarComponent/views/event_creation_view.dart | Reworks event creation UI, adds recurrence section + submit bar + color picker in AppBar. |
| lib/calendarComponent/views/event_creation_form_field.dart | Simplifies form field layout into inline TextFormFields within cards. |
| lib/calendarComponent/views/event_creation_date_time_picker.dart | Replaces old picker with compact row/chip UI and localized labels. |
| lib/calendarComponent/views/custom_event_view.dart | Introduces full-screen custom event details scaffold with series actions and confirmations. |
| lib/calendarComponent/views/calendar_event_view.dart | Improves clipping and text truncation/line-limit behavior. |
| lib/calendarComponent/viewModels/calendar_viewmodel.dart | Adds series resolution/inference, series deletion helpers, and refactors fetch error handling. |
| lib/calendarComponent/viewModels/calendar_addition_viewmodel.dart | Adds recurrence state, occurrence generation, series-aware editing, and transactional save logic. |
| lib/calendarComponent/services/calendar_view_service.dart | Routes all event taps to calendar details route (no dialog). |
| lib/calendarComponent/services/calendar_service.dart | Fixes delete call to properly await the API request. |
| lib/calendarComponent/services/calendar_preference_service.dart | Persists series membership and refactors preference persistence/loading. |
| lib/calendarComponent/model/calendar_preferences.dart | Extends stored preferences with seriesPreferences. |
| lib/calendarComponent/model/calendar_preferences.g.dart | Updates JSON serialization for seriesPreferences. |
| lib/calendarComponent/model/calendar_event.dart | Adds hasLectureDetailsLink and improves localized time range formatting. |
| lib/calendarComponent/model/calendar_editing.dart | Adds recurrence enums used by the UI/viewmodels. |
| lib/base/util/color_picker_view.dart | Keeps internal state in sync when the parent-provided color changes. |
| lib/base/routing/router.dart | Routes calendar details to lecture details vs custom event details based on link detection. |
| assets/translations/en.json | Adds recurrence + series-related strings. |
| assets/translations/de.json | Adds recurrence + series-related strings. |
Comments suppressed due to low confidence (1)
lib/calendarComponent/services/calendar_preference_service.dart:117
resetPreferences()removes the SharedPreferences key but leaves the in-memorycolorPreferences/visibilityPreferences/seriesPreferencesmaps intact and keeps_loaded == true. After calling reset, subsequentget*Preferencecalls will continue returning stale cached values and will not reload from disk. Consider clearing the maps and resetting_loaded(or callingloadPreferences()after clearing) insideresetPreferences().
_loaded = true;
} catch (e) {
log('Failed to load calendar preferences: $e');
}
}
void resetPreferences() {
sharedPreferences.remove(key);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, onError: (error) => events.addError(error)); | ||
| } catch (error) { | ||
| events.addError(error); | ||
| rethrow; |
There was a problem hiding this comment.
fetch() now rethrows after pushing the error into events. There are multiple call sites (e.g. in initState) that invoke fetch(...) without awaiting/handling the returned Future, so a network/API failure can become an unhandled async exception (red screen / crash) even though the stream error is already surfaced to the UI. Consider removing the rethrow, or ensure all call sites await and handle errors explicitly.
| rethrow; |
| return url?.split("LvNr=").last; | ||
| } | ||
|
|
||
| bool get hasLectureDetailsLink { | ||
| final value = url; | ||
| final lectureNumber = lvNr; | ||
| if (value == null || lectureNumber == null || lectureNumber.isEmpty) { | ||
| return false; | ||
| } | ||
|
|
||
| final uri = Uri.tryParse(value); | ||
| if (uri == null || !uri.path.contains('lv.detail')) { | ||
| return false; | ||
| } | ||
|
|
||
| return uri.queryParameters['cLvNr'] == lectureNumber || | ||
| uri.queryParameters['pLvNr'] == lectureNumber; |
There was a problem hiding this comment.
hasLectureDetailsLink compares uri.queryParameters['cLvNr'/'pLvNr'] against lvNr, but lvNr is derived via url?.split('LvNr=').last and can include trailing query params (e.g. 123&foo=bar). In that case the comparison will always fail and lecture events can be misrouted to CustomEventScaffold (enabling edit/delete UI for non-custom events) and break lecture-details navigation. Prefer parsing the Uri once and deriving the lecture number directly from query parameters (e.g. cLvNr/pLvNr/pLVNr) instead of string-splitting, and base hasLectureDetailsLink on that parsed value.
| return url?.split("LvNr=").last; | |
| } | |
| bool get hasLectureDetailsLink { | |
| final value = url; | |
| final lectureNumber = lvNr; | |
| if (value == null || lectureNumber == null || lectureNumber.isEmpty) { | |
| return false; | |
| } | |
| final uri = Uri.tryParse(value); | |
| if (uri == null || !uri.path.contains('lv.detail')) { | |
| return false; | |
| } | |
| return uri.queryParameters['cLvNr'] == lectureNumber || | |
| uri.queryParameters['pLvNr'] == lectureNumber; | |
| final value = url; | |
| if (value == null || value.isEmpty) { | |
| return null; | |
| } | |
| final uri = Uri.tryParse(value); | |
| if (uri == null) { | |
| return null; | |
| } | |
| return uri.queryParameters['cLvNr'] ?? | |
| uri.queryParameters['pLvNr'] ?? | |
| uri.queryParameters['pLVNr']; | |
| } | |
| bool get hasLectureDetailsLink { | |
| final value = url; | |
| if (value == null || value.isEmpty) { | |
| return false; | |
| } | |
| final uri = Uri.tryParse(value); | |
| final lectureNumber = lvNr; | |
| if (uri == null || | |
| !uri.path.contains('lv.detail') || | |
| lectureNumber == null || | |
| lectureNumber.isEmpty) { | |
| return false; | |
| } | |
| return true; |
| void setUntilDate(DateTime? date) { | ||
| if (date != null) { | ||
| untilDate.add(date); | ||
| checkValidity(); | ||
| } |
There was a problem hiding this comment.
setUntilDate accepts any picked date, but _generateOccurrences() will return an empty list if untilDate is before from (the loop breaks at i == 0). In the edit flow this can lead to deleting the original event (id != null) while creating zero replacement events. Consider clamping/validating untilDate to be on/after from (and updating checkValidity() accordingly), and/or guaranteeing _generateOccurrences() always includes at least the first occurrence.
| final picked = await showDatePicker( | ||
| context: context, | ||
| initialDate: date, | ||
| firstDate: DateTime.now(), | ||
| lastDate: DateTime.now().add( | ||
| const Duration(days: 730), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
The "ends on" date picker uses firstDate: DateTime.now() with initialDate: date. If date is in the past (e.g. editing an older event/series), showDatePicker will assert because initialDate must be on/after firstDate. Also, using DateTime.now() allows picking an until-date earlier than the event start date, which can result in zero generated occurrences. Consider setting firstDate to at least the event's start date (and no later than initialDate), e.g. based on vm.from/DateUtils.dateOnly(...).
| final picked = await showDatePicker( | |
| context: context, | |
| initialDate: date, | |
| firstDate: DateTime.now(), | |
| lastDate: DateTime.now().add( | |
| const Duration(days: 730), | |
| ), | |
| ); | |
| final eventStartDate = DateUtils.dateOnly( | |
| calendarEvent.from, | |
| ); | |
| final initialPickerDate = DateUtils.dateOnly( | |
| date.isBefore(eventStartDate) | |
| ? eventStartDate | |
| : date, | |
| ); | |
| final defaultLastDate = DateUtils.dateOnly( | |
| DateTime.now().add( | |
| const Duration(days: 730), | |
| ), | |
| ); | |
| final lastPickerDate = | |
| initialPickerDate.isAfter(defaultLastDate) | |
| ? initialPickerDate | |
| : defaultLastDate; | |
| final picked = await showDatePicker( | |
| context: context, | |
| initialDate: initialPickerDate, | |
| firstDate: eventStartDate, | |
| lastDate: lastPickerDate, | |
| ); |
|
Thanks for your contribution @ManuelMuehlberger! And sorry for the delay, I will review the PR this week :D |
This PR adds support for creating recurring calendar events. While the original goal was limited to just recurrence support, some of the surrounding calendar UI had to be adapted so series creation, editing, and deletion remain usable with the new behavior.
Changes
Notes
AFAICT The current API does not expose recurrence support in a way that fits this flow cleanly, so recurring entries are currently created as multiple individual events with bounded repetition options.
Testing
Related