Conversation
Co-authored-by: MasterMarcoHD <57987974+MasterMarcoHD@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Kanban feature (domain, infra, presentation, DI, routing, UI, tests), Kanban user preferences, settings UI, slot improvements (duplication, mappings, tabbed master), numerous generated-model JsonKey annotation updates, Echidna licensing removal, dependency/SDK bumps, and related repository/datasource changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UI as KanbanScreen
participant KR as KanbanRepository
participant DS as KanbanDatasource
participant API as ApiService (Moodle)
U->>UI: Open /kanban/
UI->>KR: build(trigger)
KR->>DS: getBoard(token, backlogCandidates)
alt Moodle datasource
DS->>API: local_lbplanner_kanban_get_board
API-->>DS: Board JSON
DS-->>KR: KanbanBoard
else Local datasource
DS-->>KR: KanbanBoard (static)
end
KR-->>UI: State with KanbanBoard
UI-->>U: Render columns (optional backlog)
sequenceDiagram
autonumber
actor U as User (drag)
participant UI as KanbanColumnWidget
participant KR as KanbanRepository
participant DS as KanbanDatasource
participant A as Analytics
participant S as Sentry
U->>UI: Drag task to column
UI->>KR: move(taskId, to)
KR->>S: startSpan("kanban.move")
KR->>KR: Optimistically update board
KR->>DS: move(token, taskId, to)
DS-->>KR: ok
KR->>A: event(move_task)
KR-->>UI: refreshed state
KR->>S: finishSpan
sequenceDiagram
autonumber
participant KR as KanbanRepository
participant UR as UserRepository
participant TR as MoodleTasksRepository
participant DS as KanbanDatasource
participant S as Sentry
KR->>UR: read auto-move prefs
KR->>TR: get tasks by status
KR->>S: startSpan("kanban.automove")
loop per source column
KR->>KR: derive target mapping
KR->>DS: move(token, taskId, to) (batch/parallel)
end
KR->>S: finishSpan
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Analysis Report for 4978af7
Click to see the full report |
1 similar comment
Analysis Report for 4978af7
Click to see the full report |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 74
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (25)
lib/src/theming/domain/services/theme_generator_service.dart (2)
23-26:left/right/top/bottomparams are unused—either remove or make them effective.They currently have no effect and can mislead callers.
Option A (implement side gating):
SmoothRectangleBorder squircle({ BorderSide side = BorderSide.none, double radius = 10, bool topLeft = true, bool topRight = true, bool bottomLeft = true, bool bottomRight = true, bool left = true, bool right = true, bool top = true, bool bottom = true, BorderAlign borderAlign = BorderAlign.inside, }) { final _radius = SmoothRadius( cornerRadius: radius, cornerSmoothing: 0.9, ); + final tl = (topLeft && left && top) ? _radius : SmoothRadius.zero; + final tr = (topRight && right && top) ? _radius : SmoothRadius.zero; + final bl = (bottomLeft && left && bottom) ? _radius : SmoothRadius.zero; + final br = (bottomRight && right && bottom) ? _radius : SmoothRadius.zero; return SmoothRectangleBorder( side: side, borderAlign: borderAlign, borderRadius: SmoothBorderRadius.only( - topLeft: topLeft ? _radius : SmoothRadius.zero, - topRight: topRight ? _radius : SmoothRadius.zero, - bottomLeft: bottomLeft ? _radius : SmoothRadius.zero, - bottomRight: bottomRight ? _radius : SmoothRadius.zero, + topLeft: tl, + topRight: tr, + bottomLeft: bl, + bottomRight: br, ), ); }Option B (simplify API): drop the side booleans if not needed.
Also applies to: 37-42
3-13: Do not hide Flutter's Theme; rename the generic type to TThemeHiding
Themeprevents callingTheme.of(context)and creates ambiguity — rename the generic type parameter and restore the normal Material import.-import 'package:flutter/material.dart' hide Theme; +import 'package:flutter/material.dart'; -/// A service that generates a theme of type [Theme] based on a [ThemeBase]. +/// A service that generates a theme of type [TTheme] based on a [ThemeBase]. -abstract class ThemeGeneratorService<Theme> extends Service { +abstract class ThemeGeneratorService<TTheme> extends Service { @override String get name => 'ThemeGenerator'; /// Generates a theme based on the provided [themeBase]. - Theme generateTheme(ThemeBase themeBase); + TTheme generateTheme(ThemeBase themeBase); }lib/src/theming/infra/services/material_theme_generator_service.dart (1)
46-51: Fix type: use CardTheme, not CardThemeData (compile-time error).ThemeData.cardTheme expects a CardTheme. CardThemeData is not a Flutter SDK type for cards and will fail analysis/compile.
Apply this diff:
- cardTheme: CardThemeData( + cardTheme: CardTheme( color: themeBase.primaryColor, elevation: 6, margin: EdgeInsets.zero, shape: squircle(), ),lib/src/calendar/presentation/repositories/calendar_plan_repository.dart (6)
340-351: Date-only vs DateTime comparisons can silently mis-filter. Normalize input dates._start/_end are truncated to day, but start/end/betweenStart/betweenEnd aren’t. Passing a DateTime with time-of-day will fail equality/bounds checks unexpectedly.
Apply normalization and use the normalized values in comparisons:
final now = DateTime(DateTime.now().year, DateTime.now().month, DateTime.now().day); + // Normalize incoming filters to date-only to avoid time-of-day mismatches + final DateTime? s = start != null ? DateTime(start.year, start.month, start.day) : null; + final DateTime? e = end != null ? DateTime(end.year, end.month, end.day) : null; + final DateTime? bs = betweenStart != null ? DateTime(betweenStart.year, betweenStart.month, betweenStart.day) : null; + final DateTime? be = betweenEnd != null ? DateTime(betweenEnd.year, betweenEnd.month, betweenEnd.day) : null; @@ - if (start != null && _start != start) return false; - if (end != null && _end != end) return false; + if (s != null && _start != s) return false; + if (e != null && _end != e) return false; @@ - if (betweenStart != null && _start.isBefore(betweenStart)) return false; - if (betweenEnd != null && _end.isAfter(betweenEnd)) return false; + if (bs != null && _start.isBefore(bs)) return false; + if (be != null && _end.isAfter(be)) return false;Also applies to: 347-351
365-368: Possible crash: firstWhere throws if the current user isn’t in members.Use a safe lookup and default to read.
- PlanMemberAccessType get accessType => state.hasData && _user.state.hasData - ? state.requireData.members.firstWhere((member) => member.id == _user.state.requireData.id).accessType - : PlanMemberAccessType.read; + PlanMemberAccessType get accessType { + if (!(state.hasData && _user.state.hasData)) return PlanMemberAccessType.read; + final idx = state.requireData.members.indexWhere((m) => m.id == _user.state.requireData.id); + return idx != -1 ? state.requireData.members[idx].accessType : PlanMemberAccessType.read; + }
370-371: Typo in public API: canModifiy. Add canModify alias and deprecate old.Keeps backward compatibility while fixing the name.
- bool get canModifiy => accessType == PlanMemberAccessType.write || accessType == PlanMemberAccessType.owner; + @Deprecated('Use canModify') + bool get canModifiy => accessType == PlanMemberAccessType.write || accessType == PlanMemberAccessType.owner; + bool get canModify => accessType == PlanMemberAccessType.write || accessType == PlanMemberAccessType.owner;
61-61: Log typo.“chached” → “cached”.
- log('Cannot load plan: No internet connection. Skipping to preserve chached data.'); + log('Cannot load plan: No internet connection. Skipping to preserve cached data.');
50-59: Guard for missing lb_planner_api token.tokens.isEmpty doesn’t guarantee the specific key exists; calling getPlan with null/empty can fail.
- final tokens = waitForData(_auth); + final tokens = waitForData(_auth); + final token = tokens[Webservice.lb_planner_api]; @@ - if (tokens.isEmpty) { - log('Cannot load plan: No tokens provided.'); + if (tokens.isEmpty || token == null || (token is String && token.isEmpty)) { + log('Cannot load plan: Missing token for ${Webservice.lb_planner_api}.'); return; } @@ - () => _plan.getPlan(tokens[Webservice.lb_planner_api]), + () => _plan.getPlan(token as String),Also applies to: 68-75
118-124: Validate date range when setting a deadline.Prevent inverted ranges early.
final transaction = startTransaction('setDeadline'); try { + if (end.isBefore(start)) { + log('Cannot set deadline: end is before start.'); + return; + } final deadline = PlanDeadline( id: taskId, start: start, end: end, );lib/src/slots/presentation/widgets/number_spinner.dart (6)
139-147: Clamping without setState leaves UI out-of-syncWhen clamping to min/max you update
valuebut not the field, leading to mismatch. Update UI and move caret to end.Apply this outside the changed hunk:
if (widget.max != null && number > widget.max!) { final clamped = widget.max!; setState(() { value = clamped; final text = clamped.toString(); controller.value = TextEditingValue( text: text, selection: TextSelection.collapsed(offset: text.length), ); }); return; } if (widget.min != null && number < widget.min!) { final clamped = widget.min!; setState(() { value = clamped; final text = clamped.toString(); controller.value = TextEditingValue( text: text, selection: TextSelection.collapsed(offset: text.length), ); }); return; }
63-69: Generic default0 as Tcan crash for T=double
_value = widget.initialValue ?? widget.min ?? 0 as T;throws whenTisdouble(int→double cast). Initialize by type.@override void initState() { super.initState(); if (widget.initialValue != null) { _value = widget.initialValue!; } else if (widget.min != null) { _value = widget.min!; } else if (T == double) { _value = 0.0 as T; } else { _value = 0 as T; // int/num } }
51-59: Dispose the controllerLeak risk without disposing
TextEditingController.@override void dispose() { controller.dispose(); super.dispose(); }
20-25: Doc comments mention a “controller” prop that doesn’t existThese comments are misleading for API users.
- /// The controller to use for the spinner. If null, the default behavior is to create a new controller. + /// Initial value for the spinner. If null, defaults to `min` or 0. final T? initialValue; - /// The controller to use for the spinner. If null, the default behavior is to create a new controller. + /// Callback invoked when the value changes. final void Function(T)? onChanged;
110-127: Use the computedenabledconsistentlyYou compute
enabledbut passwidget.enabledtoTextField. Use the same boolean for consistency.- enabled: widget.enabled, + enabled: enabled,
157-158: Formatter may block decimals/negatives despite T extends num
FilteringTextInputFormatter.digitsOnlyforbids-and.; ifTcan bedoubleor negativeint, input becomes impossible.Options:
- Provide formatters based on
T(allow-and.fordouble).- Document that only non-negative integers are supported here.
lib/src/app/presentation/widgets/title_bar.dart (1)
199-205: Replace large commented blocks with a feature flag or helper.Long commented sections reduce readability. Gate the license badge behind a constant/config or extract
buildLicenseBadge()that returnsSizedBox.shrink()when disabled.Apply this minimal pattern:
+const bool kLicenseBadgeEnabled = false; ... - // if (showLicenseBadge) ... + if (kLicenseBadgeEnabled) ...[ + Spacing.smallHorizontal(), + /* badge widget */ + Spacing.smallHorizontal(), + ],Also applies to: 234-255, 352-355, 387-409
lib/src/slots/domain/models/reservation.freezed.dart (1)
34-36: Doc comment reads incorrectly for userId.“The id of this reservation is for.” → likely “The id of the user this reservation is for.”
Please fix in the source model (reservation.dart), not the generated file. For example:
/// The id of the user this reservation is for. @JsonKey(name: 'userid') final int userId;lib/src/moodle/domain/models/moodle_course.freezed.dart (1)
38-41: Grammar nit in docstring.“Whether the user want's the app…” → “Whether the user wants the app…”
Please update in moodle_course.dart so regenerated docs are correct.
lib/src/moodle/domain/models/moodle_task.freezed.dart (1)
27-30: Grammar nit in docstring.“within it's parent” → “within its parent”.
Please change in moodle_task.dart (source) so generation reflects it.
l10n/de.arb (2)
552-555: Typo: “Abonament” → “Abonnement”.User-facing text; fix before release.
Apply:
- "settings_general_manageSubscription": "Abonament verwalten", + "settings_general_manageSubscription": "Abonnement verwalten",
718-726: Grammar: “Desktop geräte” → “Desktopgeräte”.Compound noun spelling; user-facing.
Apply:
- "mobile": "Sorry, {appname} ist derzeit nur für Desktop geräte verfügbar. Bitte öffne die App auf einem Desktop-Gerät, um fortzufahren.", + "mobile": "Sorry, {appname} ist derzeit nur für Desktopgeräte verfügbar. Bitte öffne die App auf einem Desktop-Gerät, um fortzufahren.",lib/src/settings/presentation/screens/settings_screen.dart (1)
71-74: Optional: add Kanban settings to mobile for parity.Consider surfacing KanbanSettings on mobile behind the same capability gate.
Apply after ThemesSettings:
SizedBox(child: const GeneralSettings().stretch()), const ThemesSettings().stretch(), + if (capabilities.hasStudent) + SizedBox(child: const KanbanSettings().stretch()),lib/src/auth/presentation/repositories/user_repository.dart (1)
91-115: Optimistic update lacks rollback on failure.If the server update fails, state remains patched. Capture the previous state and revert on error.
Apply this diff:
Future<void> _updateUser(User patch, ISentrySpan span) async { - log('Updating user to $patch'); + log('Updating user to $patch'); if (!state.hasData) { log('User is not loaded yet. Aborting.'); return; } - data(patch); + final prev = state.requireData; + data(patch); final transaction = span.startChild('updateUser'); await guard( () => _userDatasource.updateUser( _auth.state.requireData[Webservice.lb_planner_api], patch, ), onError: (e, s) { log('Failed to update user', e, s); transaction.internalError(e); + // rollback + data(prev); }, onData: (user) => log('User updated successfully'), ); await transaction.commit(); }lib/src/settings/presentation/widgets/general_settings.dart (1)
20-35: Remove unused state and stubs (dead code).
checkingUpdates/clearingCache/deletingProfileand their methods aren’t used in UI.Apply this diff:
-class _GeneralSettingsState extends State<GeneralSettings> { - bool checkingUpdates = false; - bool clearingCache = false; - bool deletingProfile = false; - - Future<void> checkUpdates() async { - if (checkingUpdates) { - return; - } - checkingUpdates = true; - //checkUpdates - checkingUpdates = false; - } - - Future<void> clearCache() async { - if (clearingCache) { - return; - } - clearingCache = true; - //clearCache - clearingCache = false; - } - - Future<void> deleteProfile() async { - if (deletingProfile) { - return; - } - final user = context.read<UserRepository>(); - deletingProfile = true; - await user.deleteUser(); - deletingProfile = false; - } +class _GeneralSettingsState extends State<GeneralSettings> {Also applies to: 37-47, 49-61
lib/src/moodle/presentation/repositories/moodle_tasks_repository.dart (1)
44-60: Docs drift: include courseIds/taskIds/cmids in the filter docs.The signature supports more filters than the bullets list mentions.
Example doc tweak:
- Add bullets for
courseIds,taskIds, andcmids.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
lib/gen/l10n/app_localizations.dartis excluded by!**/gen/**lib/gen/l10n/app_localizations_de.dartis excluded by!**/gen/**lib/gen/l10n/app_localizations_en.dartis excluded by!**/gen/**pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (85)
.fvmrc(1 hunks).vscode/settings.json(1 hunks)build.yaml(1 hunks)l10n.yaml(0 hunks)l10n/de.arb(5 hunks)l10n/en.arb(5 hunks)lib/config/echidna.dart(1 hunks)lib/eduplanner.dart(1 hunks)lib/main.dart(1 hunks)lib/src/app/app.dart(2 hunks)lib/src/app/presentation/widgets/sidebar.dart(3 hunks)lib/src/app/presentation/widgets/sidebar_target.dart(3 hunks)lib/src/app/presentation/widgets/title_bar.dart(4 hunks)lib/src/app/utils/animate_utils.dart(3 hunks)lib/src/auth/auth.dart(2 hunks)lib/src/auth/domain/models/token.freezed.dart(3 hunks)lib/src/auth/domain/models/user.dart(1 hunks)lib/src/auth/domain/models/user.freezed.dart(12 hunks)lib/src/auth/domain/models/user.g.dart(2 hunks)lib/src/auth/infra/datasources/moodle_user_datasource.dart(1 hunks)lib/src/auth/presentation/repositories/echidna_user_repository.dart(1 hunks)lib/src/auth/presentation/repositories/user_repository.dart(8 hunks)lib/src/auth/utils/kanban_column_converter_utils.dart(1 hunks)lib/src/auth/utils/utils.dart(1 hunks)lib/src/calendar/domain/models/calendar_plan.freezed.dart(4 hunks)lib/src/calendar/domain/models/calendar_plan.g.dart(1 hunks)lib/src/calendar/domain/models/plan_deadline.freezed.dart(3 hunks)lib/src/calendar/domain/models/plan_invite.freezed.dart(3 hunks)lib/src/calendar/domain/models/plan_member.freezed.dart(3 hunks)lib/src/calendar/presentation/repositories/calendar_plan_repository.dart(1 hunks)lib/src/calendar/presentation/widgets/plan_popup.dart(1 hunks)lib/src/kanban/domain/datasources/datasources.dart(1 hunks)lib/src/kanban/domain/datasources/kanban_datasource.dart(1 hunks)lib/src/kanban/domain/domain.dart(1 hunks)lib/src/kanban/domain/models/kanban_board.dart(1 hunks)lib/src/kanban/domain/models/kanban_board.freezed.dart(1 hunks)lib/src/kanban/domain/models/kanban_board.g.dart(1 hunks)lib/src/kanban/domain/models/models.dart(1 hunks)lib/src/kanban/domain/services/services.dart(1 hunks)lib/src/kanban/infra/datasources/datasources.dart(1 hunks)lib/src/kanban/infra/datasources/local_kanban_datasource.dart(1 hunks)lib/src/kanban/infra/datasources/moodle_kanban_datasource.dart(1 hunks)lib/src/kanban/infra/infra.dart(1 hunks)lib/src/kanban/infra/services/services.dart(1 hunks)lib/src/kanban/kanban.dart(1 hunks)lib/src/kanban/presentation/guards/guards.dart(1 hunks)lib/src/kanban/presentation/presentation.dart(1 hunks)lib/src/kanban/presentation/repositories/kanban_repository.dart(1 hunks)lib/src/kanban/presentation/repositories/repositories.dart(1 hunks)lib/src/kanban/presentation/screens/kanban_screen.dart(1 hunks)lib/src/kanban/presentation/screens/screens.dart(1 hunks)lib/src/kanban/presentation/widgets/kanban_card.dart(1 hunks)lib/src/kanban/presentation/widgets/kanban_column_widget.dart(1 hunks)lib/src/kanban/presentation/widgets/widgets.dart(1 hunks)lib/src/kanban/utils/utils.dart(1 hunks)lib/src/moodle/domain/models/moodle_course.freezed.dart(3 hunks)lib/src/moodle/domain/models/moodle_task.freezed.dart(3 hunks)lib/src/moodle/presentation/repositories/moodle_courses_repository.dart(2 hunks)lib/src/moodle/presentation/repositories/moodle_tasks_repository.dart(6 hunks)lib/src/notifications/domain/models/notification.freezed.dart(3 hunks)lib/src/settings/presentation/screens/settings_screen.dart(1 hunks)lib/src/settings/presentation/widgets/feedback_widget.dart(0 hunks)lib/src/settings/presentation/widgets/general_settings.dart(3 hunks)lib/src/settings/presentation/widgets/generic_settings.dart(1 hunks)lib/src/settings/presentation/widgets/kanban_settings.dart(1 hunks)lib/src/settings/presentation/widgets/widgets.dart(1 hunks)lib/src/slots/domain/models/course_to_slot.freezed.dart(3 hunks)lib/src/slots/domain/models/reservation.freezed.dart(3 hunks)lib/src/slots/domain/models/slot.freezed.dart(4 hunks)lib/src/slots/domain/models/slot.g.dart(1 hunks)lib/src/slots/presentation/repositories/slot_master_slots_repository.dart(1 hunks)lib/src/slots/presentation/screens/slot_master_screen.dart(3 hunks)lib/src/slots/presentation/widgets/edit_slot_dialog.dart(11 hunks)lib/src/slots/presentation/widgets/number_spinner.dart(4 hunks)lib/src/slots/presentation/widgets/slot_master_widget.dart(2 hunks)lib/src/statistics/domain/models/chart_value.freezed.dart(3 hunks)lib/src/statistics/domain/models/status_aggregate.freezed.dart(3 hunks)lib/src/statistics/domain/models/task_aggregate.freezed.dart(3 hunks)lib/src/statistics/domain/models/task_aggregate.g.dart(1 hunks)lib/src/statistics/domain/models/type_aggregate.freezed.dart(3 hunks)lib/src/theming/domain/models/theme_base.freezed.dart(3 hunks)lib/src/theming/domain/services/theme_generator_service.dart(1 hunks)lib/src/theming/infra/services/material_theme_generator_service.dart(2 hunks)pubspec.yaml(5 hunks)test/kanban_module_test.dart(1 hunks)
💤 Files with no reviewable changes (2)
- lib/src/settings/presentation/widgets/feedback_widget.dart
- l10n.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Testing 🧪
- GitHub Check: Analysis 🔍
- GitHub Check: Testing 🧪
- GitHub Check: Analysis 🔍
🔇 Additional comments (82)
lib/src/moodle/presentation/repositories/moodle_courses_repository.dart (2)
1-1: Import forfirstWhereOrNullis correct.Required to use the iterable extension; no issues spotted.
134-145: Approve: callers handle null; no unsafe usages found.Only call site is lib/src/kanban/presentation/widgets/kanban_card.dart — it checks
course == nulland usescourse ?? MoodleCourse.skeleton(), with no!or direct member access aftergetById.lib/src/theming/infra/services/material_theme_generator_service.dart (1)
72-81: Keep TabBarThemeData and splashBorderRadius — original suggestion is incorrect.ThemeData.tabBarTheme is TabBarThemeData (post component-theme normalization) and TabBarThemeData includes splashBorderRadius. Do NOT change TabBarThemeData → TabBarTheme or remove splashBorderRadius. For ripple tuning use overlayColor (MaterialStateProperty/WidgetStateProperty) or splashFactory (InteractiveInkFeatureFactory/NoSplash) instead.
Locations: lib/src/theming/infra/services/material_theme_generator_service.dart (tabBarTheme ~line 72); see also lib/src/calendar/presentation/widgets/plan_popup.dart (uses TabBarTheme(data: const TabBarThemeData())).
Likely an incorrect or invalid review comment.
lib/src/calendar/presentation/repositories/calendar_plan_repository.dart (1)
346-346: Resolve — taskIds is an include filter and call sites use it that way.Call sites pass ID sets to include (kanban_card.dart:29; course_overview_screen.dart:72; calendar/plan_cell.dart:45; dashboard/presentation/widgets/todays_tasks.dart:19,61). No breaking semantic change detected. Optional: add the doc line clarifying inclusive behavior.
lib/src/slots/presentation/widgets/edit_slot_dialog.dart (13)
14-15: LGTM! Constructor signature update handles new parameters correctly.The addition of
startUnitandduplicateparameters with proper default values and assertion logic is well implemented.
20-22: LGTM! New public fields are properly documented.The new
startUnitandduplicatefields are well-documented and follow the existing code style.Also applies to: 26-28
49-49: LGTM! State management evolution from simple list to structured elements.The introduction of
courseMappingsas a list ofMappingElementobjects provides better encapsulation and management of course mapping state.
57-57: LGTM! Edit mode logic correctly handles duplication.The logic properly distinguishes between editing and duplicating modes by checking both the slot existence and the duplicate flag.
63-63: LGTM! Proper fallback for start unit initialization.The initialization correctly uses the slot's start unit first, then falls back to the new
startUnitparameter.
69-70: LGTM! Proper initialization of course mappings with fallback.The code correctly transforms existing mappings into
MappingElementobjects and ensures at least one mapping exists.
88-89: LGTM! Enhanced validation using structured mapping elements.The validation logic correctly uses the new
MappingElementstructure and ensures all mappings are submittable.
106-109: LGTM! Proper reconstruction of mappings before submission.The code correctly clears the mappings list and rebuilds it from the current
courseMappingsstate, ensuring consistency.
135-136: LGTM! Duplication logic creates new slot with proper ID.The duplication path correctly creates a new slot by setting ID to -1, which is appropriate for new slot creation.
197-208: LGTM! Mapping creation logic handles both new and existing mappings.The method properly distinguishes between creating new mappings (no ID) and updating existing ones based on the
mappingIdpresence.
332-362: Enhanced room autocomplete with hover effects looks great.The implementation of hover effects and improved styling for the autocomplete options significantly enhances the user experience.
456-456: Icon choice improvement for supervisor removal.Using
Icons.remove_circle_outline_roundedinstead of a delete icon provides better semantic meaning for removing supervisors from the list.
472-568: Excellent dynamic course mappings UI implementation.The transformation from static dropdowns to a dynamic list of mapping elements with individual controls is well-executed. The UI provides:
- Per-element course and vintage selection
- Individual delete buttons for each mapping
- Add button to append new mappings
- Proper state management for each element
.fvmrc (1)
2-2: LGTM! Flutter version upgrade looks good.Flutter 3.35 has landed and brings performance boost and stateful hot reload on the web, making this a worthwhile upgrade. The version update aligns with the VS Code settings change.
lib/src/kanban/presentation/repositories/repositories.dart (1)
1-1: LGTM! Clean repository barrel export.The export statement properly re-exports the Kanban repository for use by the presentation layer and higher-level modules.
lib/src/kanban/domain/datasources/datasources.dart (1)
1-1: LGTM! Clean datasource barrel export.The export statement properly re-exports the Kanban datasource interface for use by the domain layer and dependent modules.
.vscode/settings.json (1)
5-5: LGTM! VS Code Flutter SDK path updated correctly.The Flutter SDK path has been updated to match the version bump from 3.29.0 to 3.35.0, which aligns with the .fvmrc change.
lib/src/kanban/presentation/guards/guards.dart (1)
1-1: No action required — empty Kanban guards file is harmless.lib/src/kanban/presentation/guards/guards.dart is an intentionally empty placeholder (exported by kanban/presentation.dart) and there are no Kanban-specific guards referenced by the app routes (routes use Auth/Capability/HasCourses guards from auth/moodle). An empty export file will not cause import errors; optional: add a one-line comment like
// intentionally emptyto make intent explicit.lib/src/theming/domain/models/theme_base.freezed.dart (1)
61-63: Freezed JSON ignore updates look good.
@JsonKey(ignore: true)oncopyWithaligns with current Freezed conventions and avoids polluting (de)serialization.Also applies to: 443-448, 525-527
lib/src/statistics/domain/models/type_aggregate.freezed.dart (1)
39-41: Consistent@JsonKey(ignore: true)application.Matches the project‑wide pattern; safe for (de)serialization.
Also applies to: 204-214, 256-259
lib/src/auth/auth.dart (1)
18-19: Echidna removal — no active runtime references found; only commented/config remnantsrg output: only commented usages and config constants remain:
- lib/main.dart:106 — commented initializeEchidnaApi(...)
- lib/src/app/presentation/widgets/title_bar.dart:199, 352 — commented LicenseRepository watchers
- lib/src/auth/auth.dart:18, 30 — EchidnaModule / initializeLicenseRepo commented
- lib/src/auth/presentation/repositories/echidna_user_repository.dart:11, 15 — commented class
- lib/config/echidna.dart:6, 10, 14 — kEchidnaClientKey / kEchidnaClientID / kEchidnaHost
- CHANGELOG.md:80 — changelog entry
Conclusion: no live references found — safe to drop the module. Remove the commented lines and unused config constants if you want a full cleanup.
lib/src/slots/presentation/widgets/slot_master_widget.dart (1)
55-61: EditSlotDialog: confirm duplication semanticsConstructor exposes a
duplicatenamed parameter (duplicate = false). Verify that whenduplicate == truethe dialog:
- creates a new Slot instance (clears/resets the ID) and does not mutate the original,
- adjusts/validates start/end times as required,
- uses the create/save flow (not the update path).
File: lib/src/slots/presentation/widgets/edit_slot_dialog.dart
pubspec.yaml (2)
24-27: Confirm echidna_flutter removal side effects.
- Residual occurrences found: lib/config/echidna.dart; lib/src/auth/presentation/repositories/echidna_user_repository.dart (import/commented echidna code); lib/src/auth/presentation/repositories/repositories.dart (exports echidna_user_repository.dart).
- Action: either remove/clean these files/exports (and update callers) or restore echidna_flutter in pubspec — otherwise CI may fail.
13-13: Verify breaking changes for awesome_extensions, flutter_sticky_header, skeletonizerrg output shows direct usages of withValues(), Skeletonizer, and SliverStickyHeader — review package changelogs, run widget/unit/integration tests, and update callsites if APIs changed.
Representative callsites:
- withValues(): lib/main.dart:360; lib/src/slots/presentation/widgets/slot_widget.dart:108,112,233; lib/src/app/utils/animate_utils.dart:103–104
- Skeletonizer: lib/src/kanban/presentation/widgets/kanban_column_widget.dart:100; lib/src/kanban/presentation/widgets/kanban_card.dart:57; lib/src/notifications/presentation/widgets/notification.dart:155,215,241,255,274
- SliverStickyHeader: lib/src/slots/presentation/screens/slot_reservation_screen.dart:101,114; lib/src/calendar/presentation/screens/tasks_overview_screen.dart:77
Run full test suite and a quick manual smoke of Kanban, Slots, Calendar, and Notifications screens; address any failing callsites or API changes.
build.yaml (1)
7-12: Remove or widengenerate_forfor source_gen:combining_builder and freezed, or verify no annotations outside the pathConstraining these builders to lib/src/*/domain/models/** can skip files annotated with @freezed/@unfreezed. File: build.yaml (lines 7–12). Verify locally:
rg -n -S '@freezed|@unfreezed' lib -g '!**/*.freezed.dart' -g '!**/*.g.dart' | rg -v '^lib/src/[^/]+/domain/models/'No output = safe; any matches require removing or expanding the
generate_forentries.lib/src/app/presentation/widgets/sidebar_target.dart (2)
103-110: Transformer wrapping on desktop path: OK.Correctly preserves active state styling while allowing injection.
120-127: Transformer wrapping on mobile path: OK.Matches desktop behavior; consistent API use.
lib/src/statistics/domain/models/status_aggregate.freezed.dart (1)
36-38: Approve — file is generated by Freezed/build_runner; no action required.
Found "GENERATED CODE - DO NOT MODIFY BY HAND" andpart of 'status_aggregate.dart'in the .freezed.dart, @freezed in the source, and freezed_annotation/json_serializable in pubspec; @jsonkey(ignore: true) on copyWith/hashCode is expected.lib/src/slots/presentation/screens/slot_master_screen.dart (1)
28-29: TabController has noanimationDurationparameter — compile error.Remove the argument; control animation via animateTo when needed.
- _tabController = TabController(length: Weekday.values.length, vsync: this, animationDuration: 500.ms); + _tabController = TabController(length: Weekday.values.length, vsync: this);Likely an incorrect or invalid review comment.
lib/src/kanban/infra/datasources/local_kanban_datasource.dart (1)
34-47: I’ve requested the exact enum definition to confirm whether the enum value is namedinprogressorinProgress. Once we have that, I’ll update my comment accordingly.lib/src/statistics/domain/models/chart_value.freezed.dart (2)
31-33: Migration to JsonKey(ignore: true) for copyWith — looks goodSwitching from includeFromJson/includeToJson to ignore: true is consistent with current json_serializable guidance and keeps copyWith out of JSON. No functional changes to the model surface.
Also applies to: 178-183, 210-213
1-214: Generator/tooling alignment verified — no legacy annotations found.rg scan results: no @jsonkey(includeFromJson/includeToJson) in .freezed.dart; no "copyWith" or "hashCode" keys emitted in .g.dart. pubspec shows freezed_annotation ^2.4.4, json_annotation ^4.9.0, build_runner ^2.4.11; build targets generate_for: lib/src//domain/models/* (covers lib/src/statistics/domain/models).
lib/src/statistics/domain/models/task_aggregate.freezed.dart (2)
30-33: Uniform ignore on copyWith/hashCode — approvedApplying @jsonkey(ignore: true) to copyWith and hashCode matches the rest of the migration and won’t affect (de)serialization of status/type.
Also applies to: 166-175, 202-205
1-206: Sanity‑check: JSON footprint unchangedVerified lib/src/statistics/domain/models/task_aggregate.g.dart — fromJson reads 'status' and 'type'; toJson emits 'status' and 'type'; no 'copyWith'/'hashCode' serialized keys present.
lib/src/calendar/domain/models/plan_member.freezed.dart (2)
32-35: PlanMember: ignore copyWith/hashCode — OKAnnotations are consistent; existing JsonKey name mappings (userid/accesstype) remain intact.
Also applies to: 153-162, 192-195
1-196: Verify generated files are source-of-truthlib/src/calendar/domain/models/plan_member.dart declares @freezed and has the part directives (plan_member.freezed.dart / plan_member.g.dart); generated file exists. Ensure CI runs build_runner to regenerate artifacts (e.g. flutter pub run build_runner build --delete-conflicting-outputs) so generated files remain in sync across environments.
lib/src/slots/domain/models/slot.freezed.dart (2)
58-60: Slot: ignore annotations for copyWith/hashCode — approvedNo changes to field semantics. DeepCollectionEquality remains for list fields; unmodifiable views are preserved.
Also applies to: 342-356, 357-362, 433-435
1-437: Guard against accidental JSON key changes and list default regressionsAutomated verification failed in the sandbox (/dev/fd/63); manual check required.
- Check lib/src/slots/domain/models/slot.g.dart: toJson must contain exactly these keys — "id", "startunit", "duration", "weekday", "room", "size", "fullness", "forcuruser", "filters" — and must not serialize copyWith or hashCode.
- Confirm generated defaults for supervisors and mappings remain const [] (slot.freezed.dart shows defaults) and that fromJson/toJson preserve empty-list behavior.
- If any serialization keys or defaults changed, revert or add a serialization-compatibility test.
lib/src/notifications/domain/models/notification.freezed.dart (2)
54-57: Notification: consistent @jsonkey(ignore: true) application — goodConverters and JsonKey name mappings remain unchanged; only ignore semantics updated.
Also applies to: 276-286, 349-352
1-353: Converters present and exported via eduplanner barrelUnixTimestampConverter and BoolConverter are defined in lib/src/app/utils/json_serializable_utils.dart and re-exported via lib/src/app/utils/utils.dart -> lib/src/app/app.dart -> lib/eduplanner.dart; lib/src/notifications/domain/models/notification.dart imports package:eduplanner/eduplanner.dart so the converters are available for code generation.
lib/src/calendar/domain/models/plan_deadline.freezed.dart (2)
38-41: PlanDeadline: ignore copyWith/hashCode — approvedNo behavior change; JSON keys (moduleid/deadlinestart/deadlineend) preserved.
Also applies to: 183-192, 233-236
1-237: Missing generated .g.dart — cannot verify toJson unchangedlib/**/plan_deadline*.g.dart not present; cannot confirm whether toJson keys/structure changed. Regenerate and commit the generated .g.dart (or paste the produced toJson output) so I can re-check.
lib/src/slots/domain/models/reservation.freezed.dart (1)
43-45: LGTM: copyWith/hashCode excluded from JSON.Using @jsonkey(ignore: true) here is correct and keeps the JSON surface stable.
Also applies to: 220-229, 277-279
lib/src/moodle/domain/models/moodle_course.freezed.dart (1)
43-45: LGTM: JSON ignore alignment.Consistently marking copyWith/hashCode with @jsonkey(ignore: true) is appropriate.
Also applies to: 220-229, 276-279
lib/src/moodle/domain/models/moodle_task.freezed.dart (1)
50-52: LGTM: JSON ignore standardization.copyWith/hashCode hidden from JSON is correct and consistent.
Also applies to: 271-280, 340-342
lib/src/auth/domain/models/user.freezed.dart (2)
550-571: LGTM: equality/hashCode/copyWith updated.New fields included in ==/hashCode and copyWith ignored in JSON. Looks correct.
Also applies to: 572-576
69-91: Kanban user prefs: defaults OK; BoolConverterNullable not applied — verify handling
- showColumnColors: generated fromJson uses
json['showcolumncolors'] as bool? ?? true(lib/src/auth/domain/models/user.g.dart) — missing/null → default true. Note: the field is annotated with @BoolConverterNullable(), but the generated code does not call the converter, so integer values (0/1) from the backend would not be converted and will cause a cast error.- KanbanColumnConverter: deserialize uses
KanbanColumn.values.asNameMap()[data]and the generated helper only calls the converter when JSON != null, so unknown/invalid strings → null (safe). (lib/src/auth/utils/kanban_column_converter_utils.dart + lib/src/auth/domain/models/user.g.dart)Action: if the backend can return 0/1 for booleans, either ensure it returns true/false or adjust the JSON converter/annotations so json_serializable actually applies the BoolConverterNullable on these fields.
lib/src/kanban/presentation/screens/screens.dart (1)
1-1: Barrel export LGTM.Publicly exposing KanbanScreen via the screens barrel is correct.
lib/src/kanban/domain/models/models.dart (1)
1-1: Models barrel export LGTM.Exposing KanbanBoard through the domain models barrel is appropriate.
lib/src/settings/presentation/screens/settings_screen.dart (1)
32-35: Desktop: gated KanbanSettings integration LGTM.Conditional Expanded with stagger matches existing layout patterns.
lib/src/settings/presentation/widgets/widgets.dart (1)
4-4: Barrel export LGTM.Exporting KanbanSettings through the widgets barrel aligns with usage in SettingsScreen.
lib/src/kanban/presentation/widgets/widgets.dart (1)
1-2: Kanban widgets barrel exports LGTM.Exposes KanbanCard and KanbanColumnWidget as intended.
lib/src/auth/utils/utils.dart (1)
1-1: Layering: remove Kanban-specific export from auth utilslib/src/auth/utils/utils.dart currently exports a Kanban-only converter (
export 'kanban_column_converter_utils.dart';) — this couples auth → kanban. Move the converter to a neutral/shared serializers module, or keep it inside the Kanban feature and have auth store the raw value to avoid cross-feature coupling and potential cycles.lib/eduplanner.dart (1)
6-6: Public API surface expanded with Kanban module — transitive exports enumeratedTransitive exports (reachable via lib/src/kanban/kanban.dart):
- lib/src/kanban/kanban.dart -> domain/domain.dart, presentation/presentation.dart, utils/utils.dart
- lib/src/kanban/domain/domain.dart -> datasources/datasources.dart, models/models.dart, services/services.dart
- lib/src/kanban/domain/datasources/datasources.dart -> kanban_datasource.dart
- lib/src/kanban/domain/models/models.dart -> kanban_board.dart
- lib/src/kanban/presentation/presentation.dart -> guards/guards.dart, repositories/repositories.dart, screens/screens.dart, widgets/widgets.dart
- lib/src/kanban/presentation/repositories/repositories.dart -> kanban_repository.dart
- lib/src/kanban/presentation/screens/screens.dart -> kanban_screen.dart
- lib/src/kanban/presentation/widgets/widgets.dart -> kanban_card.dart, kanban_column_widget.dart
- lib/src/kanban/utils/utils.dart (no further exports listed)
Note: lib/src/kanban/infra/* files (infra.dart -> datasources/services; infra/datasources -> local_kanban_datasource.dart, moodle_kanban_datasource.dart) exist but are not re-exported by kanban.dart.
Confirm whether domain/datasources (kanban_datasource.dart) and domain/services should be part of the public API; if they are internal, stop re-exporting them from domain/domain.dart.
lib/src/kanban/infra/infra.dart (1)
1-2: Barrel looks good.Exports are minimal and clear.
lib/src/kanban/domain/domain.dart (1)
1-3: Domain barrel is fine.No issues spotted.
lib/src/kanban/infra/datasources/datasources.dart (1)
1-2: Infra datasources barrel is fine.Keeps imports tidy.
lib/src/app/app.dart (1)
34-35: Module import OK.KanbanModule added to imports aligns with other modules.
lib/src/auth/infra/datasources/moodle_user_datasource.dart (1)
91-103: Filter nulls and use a const Set for O(1) membership.Replace the inline list/allocation and avoid sending null fields.
File: lib/src/auth/infra/datasources/moodle_user_datasource.dart — change:
- user.toJson().entries.where( - (e) => const <String>[ - 'theme', - 'colorblindness', - 'displaytaskcount', - 'ekenabled', - 'showcolumncolors', - 'automoveoverduetasks', - 'automovesubmittedtasks', - 'automovecompletedtasks', - ].contains(e.key), - ), + user.toJson().entries.where( + (e) => _kUserMutableKeys.contains(e.key) && e.value != null, + ),Add near the top of the file (outside the function):
// Allowed mutable user fields for update payloads. const _kUserMutableKeys = <String>{ 'theme', 'colorblindness', 'displaytaskcount', 'ekenabled', 'showcolumncolors', 'automoveoverduetasks', 'automovesubmittedtasks', 'automovecompletedtasks', };Confirm the Moodle webservice (local_lbplanner_user_update_user) accepts these keys in your target deployment to avoid 400s.
lib/src/kanban/presentation/presentation.dart (1)
1-4: Barrel exports look good.No issues spotted; exports align with the new presentation surface.
lib/src/app/presentation/widgets/sidebar.dart (1)
19-19: Theme surface change LGTM.Switch to
context.theme.colorScheme.surfaceis consistent with M3 theming.Also applies to: 92-92
lib/src/settings/presentation/widgets/kanban_settings.dart (2)
31-40: ConfirmEnumSettingsItem<T?>supportsnullvalues.You pass
values: [null, ...]andvaluemay benull. Ensure the widget handles null equality/selection and doesn’t assert on non‑null generics.
60-64: Default forshowColumnColorsshould match repository/domain default.You default to
truewhensettingsis null. Verify this aligns with the persisted/default value to avoid UI desync on first load.lib/src/kanban/kanban.dart (1)
33-35: Route LGTM.Child route mounts
KanbanScreenat module root as expected.lib/src/kanban/domain/datasources/kanban_datasource.dart (1)
5-8: Interface design LGTM.Clear naming and documentation; extends existing infra patterns.
lib/src/slots/domain/models/course_to_slot.freezed.dart (1)
38-41: LGTM: standardized on @jsonkey(ignore: true) for copyWith/hashCode.Consistent with other models; avoids leaking internals into JSON.
Also applies to: 195-204, 243-246
lib/src/auth/domain/models/token.freezed.dart (1)
30-31: LGTM: JSON-ignore annotations align with Freezed conventions.
No behavior change; serialization surface is correct.Also applies to: 140-149, 175-177
lib/src/auth/presentation/repositories/echidna_user_repository.dart (1)
1-31: Delete or gate the commented-out EchidnaUserRepository.Commented-out code at lib/src/auth/presentation/repositories/echidna_user_repository.dart adds maintenance noise; remove it (Git history preserves) or gate it behind a compile-time feature flag that compiles cleanly when disabled.
Verification — previous ripgrep run returned "No files were searched" (search was inconclusive). Run one of the commands below locally and paste the output to confirm no remaining references:
# fast, searches tracked files git grep -n -E '\bEchidnaUserRepository\b|echidna_' || true # recursive search (includes untracked/hidden files); avoids type filters that can skip files rg -n --hidden -S -C2 '\bEchidnaUserRepository\b|echidna_' -g 'lib/**' -g '!**/*.g.dart' -g '!**/*.freezed.dart' || trueIf both commands produce no matches, delete the file or convert it to a feature-gated implementation; if matches exist, update or remove callers before deleting.
lib/config/echidna.dart (1)
5-18: Deprecations OK — remove leftover commented usages & export.
- Search found no active runtime uses of the kEchidna* constants; only commented references and an exported (but commented) repository remain:
- lib/config/echidna.dart — deprecated consts (definitions).
- lib/main.dart — commented initializeEchidnaApi(... kEchidnaHost, kEchidnaClientKey, kEchidnaClientID) (line ~106).
- lib/src/auth/auth.dart — commented // EchidnaModule() (line ~18) and // ..initializeLicenseRepo(EchidnaUserRepository.new); (line ~30).
- lib/src/auth/presentation/repositories/repositories.dart — export 'echidna_user_repository.dart' (line ~2).
- lib/src/auth/presentation/repositories/echidna_user_repository.dart — file present but its echidna code is commented out.
- Action: remove the commented initialization and the export (or delete/cleanup the echidna_user_repository file) to prevent accidental runtime usage — these constants default to ''/0 if used.
lib/src/auth/domain/models/user.dart (1)
51-62: Drop redundant @default(null) on nullable fields and fix a doc typo.
- Remove the unnecessary @default(null) from the nullable Kanban fields; fix capitalization/punctuation and change the overdue-field doc to "overdue tasks".
- /// Whether to show column colors in the kanban view. - @Default(true) @JsonKey(name: 'showcolumncolors') @BoolConverterNullable() bool showColumnColors, + /// Whether to show column colors in the Kanban view. + @Default(true) @JsonKey(name: 'showcolumncolors') @BoolConverterNullable() bool showColumnColors, - /// The column to auto-move completed tasks to - @Default(null) @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo, + /// The column to auto-move completed tasks to. + @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo, - /// The column to auto-move submitted tasks to - @Default(null) @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo, + /// The column to auto-move submitted tasks to. + @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo, - /// The column to auto-move in-progress tasks to - @Default(null) @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo, + /// The column to auto-move overdue tasks to. + @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo,Confirmed: BoolConverterNullable.fromJson returns null for json == null (lib/src/app/utils/json_serializable_utils.dart), and the generated mapping in lib/src/auth/domain/models/user.g.dart uses json['showcolumncolors'] as bool? ?? true — so missing/null JSON will not be coerced to false and the default true is preserved.
l10n/en.arb (1)
749-814: LGTM: Kanban strings and placeholders look consistent.Keys match usage (due/planned placeholders, screen/setting labels). No structural issues spotted.
lib/src/auth/domain/models/user.g.dart (1)
22-35: LGTM: New fields serialize/deserialize correctly.Defaults and the helper
_$JsonConverterFromJsonhandle nulls properly; converter usage is consistent.Also applies to: 52-59, 62-67
lib/src/calendar/domain/models/calendar_plan.freezed.dart (1)
37-39: LGTM: JSON ignore annotations align with Freezed conventions.Ignoring copyWith/hashCode for JSON is correct and non-breaking.
Also applies to: 209-223, 261-263
lib/src/moodle/presentation/repositories/moodle_tasks_repository.dart (2)
92-105: LGTM: Added getters for cmid/id are safe and efficient.Use of
firstWhereOrNullwith ahasDataguard is correct.
130-170: Filter: cmid check is fine; consider null cmid behavior explicitly.If tasks may have
cmid == null, the current condition excludes them whencmidsis provided. Confirm that’s desired.lib/src/kanban/domain/models/kanban_board.freezed.dart (1)
139-151: LGTM: model and JSON mapping (inprogress key) align with usage.lib/src/calendar/domain/models/plan_invite.freezed.dart (1)
46-48: Approve — JsonKey(ignore: true) on copyWith/hashCode; verify codegen locally.
Matches Freezed/JsonSerializable patterns; ignoring copyWith/hashCode is correct. Sandbox build failed ('dart: command not found'); run locally: dart run build_runner build -d and confirm no generated-file drift. Also applies to lines 243–249 and 305–307.lib/src/settings/presentation/widgets/generic_settings.dart (1)
166-173: BooleanSettingsItem: make onChanged non-nullable and safely map Checkbox's nullable callback.
Change the field tofinal ValueChanged<bool> onChanged;, keepsuper(onPressed: () => onChanged(!value));, and in buildSuffix useCheckbox(value: value, onChanged: (v) => onChanged(v ?? value))(leave Switch as-is).
File: lib/src/settings/presentation/widgets/generic_settings.dart — constructor & buildSuffix (~lines 166–191).
Analysis Report for 4978af7
Click to see the full report |
1 similar comment
Analysis Report for 4978af7
Click to see the full report |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Analysis Report for aa52fba
Click to see the full report |
Analysis Report for 63beecc
Click to see the full report |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Analysis Report for d35c340
Click to see the full report |
Analysis Report for 93bf70c
Click to see the full report |
Analysis Report for 156d704
Click to see the full report |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Analysis Report for 010bf19
Click to see the full report |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/auth/presentation/repositories/user_repository.dart (1)
91-115: Optimistic update lacks rollback and ignores canonical server response.
- No rollback on error → UI may get stuck in an invalid state.
onDataonly logs; it should sync state with the server-returneduser.Apply this diff to revert on failure and adopt the canonical user on success:
/// Optimistically updates the user with [patch]. Future<void> _updateUser(User patch, ISentrySpan span) async { log('Updating user to $patch'); if (!state.hasData) { log('User is not loaded yet. Aborting.'); return; } - data(patch); + final prev = state.requireData; + data(patch); final transaction = span.startChild('updateUser'); await guard( () => _userDatasource.updateUser( _auth.state.requireData[Webservice.lb_planner_api], patch, ), onError: (e, s) { log('Failed to update user', e, s); + // revert optimistic state + data(prev); transaction.internalError(e); }, - onData: (user) => log('User updated successfully'), + onData: (user) { + log('User updated successfully'); + // sync with canonical server state + data(user); + }, ); await transaction.commit(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
lib/src/app/app.dart(2 hunks)lib/src/app/utils/animate_utils.dart(3 hunks)lib/src/auth/presentation/repositories/user_repository.dart(8 hunks)lib/src/slots/presentation/widgets/slot_master_widget.dart(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analysis 🔍
- GitHub Check: Testing 🧪
🔇 Additional comments (17)
lib/src/slots/presentation/widgets/slot_master_widget.dart (2)
55-61: Duplicate flow wiring — LGTM.Opening EditSlotDialog with duplicate: true is correct.
173-181: Disable Delete and rebuild on isDeleting; current gating won’t update.
- Delete button still enabled; please gate it like Duplicate.
- isDeleting is mutated without setState, so the Duplicate button’s null onPressed won’t take effect at runtime.
Apply this diff for the buttons:
TextButton( - onPressed: deleteSlot, + onPressed: isDeleting ? null : deleteSlot, style: TextButton.styleFrom(foregroundColor: context.theme.colorScheme.error), child: Text(context.t.global_delete), ), TextButton( onPressed: isDeleting ? null : duplicateSlot, child: Text(context.t.global_duplicate), ),Then update deleteSlot to trigger rebuilds:
Future<void> deleteSlot() async { if (isDeleting) return; final slots = context.read<SlotMasterSlotsRepository>(); if (!slots.state.hasData) return; final confirmed = await showConfirmationDialog( context, title: context.t.slots_slotmaster_deleteSlot_title( widget.slot.room, widget.slot.startUnit.humanReadable(), widget.slot.endUnit.humanReadable(), ), message: context.t.slots_slotmaster_deleteSlot_message, ); if (!confirmed || !mounted) return; setState(() => isDeleting = true); try { await slots.deleteSlot(widget.slot.id); } finally { if (mounted) setState(() => isDeleting = false); } }Optional: also gate Edit while deleting for consistency.
lib/src/app/utils/animate_utils.dart (3)
29-29: LGTM: assertion message matches the condition.
60-62: Fix typo: keyPrefex → keyPrefix.Undefined identifier
keyPrefexbreaks compilation. Use the declaredkeyPrefix.- .animate( - key: keyPrefex != null ? ValueKey('$keyPrefex-$i') : null, - ) + .animate( + key: keyPrefix != null ? ValueKey('$keyPrefix-$i') : null, + )
91-94: Avoid ambiguity with Widget.key: rename parameter to animKey.Reduces confusion at call sites; safe rename (no known callers passing
key).- Key? key, + Key? animKey,- return animate(key: key).scale(duration: duration, delay: stagger?.add() ?? delay, curve: Curves.easeOutCubic); + return animate(key: animKey) + .scale(duration: duration, delay: stagger?.add() ?? delay, curve: Curves.easeOutCubic);lib/src/app/app.dart (3)
73-78: LGTM: Kanban route configured consistently.Route uses
customTransition: defaultTransitionand mirrors guard order semantics. Thanks for addressing the earlier nitpick.
79-83: Remove commented-out FeatureGuard; if needed, gate via a runtime flag.Commented code tends to drift. Also, the constant references Calendar; use a Kanban-specific ID when you enable it.
Apply to remove the commented line:
- // FeatureGuard([kCalendarPlanFeatureID], redirectTo: '/settings/'),Example pattern if you intend to feature‑gate:
guards: [ if (kFeatures.kanbanEnabled) FeatureGuard([kKanbanFeatureID], redirectTo: '/settings/'), CapabilityGuard({UserCapability.student}, redirectTo: '/slots/'), AuthGuard(redirectTo: '/auth/'), ],
90-90: Avoid committing commented-out guards.Either remove it or gate via a runtime flag as shown above for consistency.
- // FeatureGuard([kCalendarPlanFeatureID], redirectTo: '/settings/'),lib/src/auth/presentation/repositories/user_repository.dart (9)
4-4: Consolidated import and Sentry integration — LGTM.Unified
eduplanner.dartimport and explicit Sentry dependency look good.Also applies to: 8-8
72-76: Identify payload now serializes enums as strings — LGTM.Using
.name(and null-aware) prevents PostHog JSON serialization issues.
129-137: Await_updateUserto preserve Sentry span/transaction ordering.Returning the future lets
finallycommit the parent before the child span finishes.Apply this diff:
- return _updateUser( + await _updateUser( state.requireData.copyWith(themeName: theme), transaction, );
186-197: Good: awaited update and event capture.This path now guards state presence and awaits the update.
206-240: Guards + awaited update + logging — LGTM.Null-checks and telemetry wiring are consistent.
245-276: New setter (showColumnColors) — implementation looks correct.Early guard, defaulting to true, awaited update, and telemetry are consistent.
331-355: Same issues as above: guard missing and "null" string in analytics.Apply this diff:
/// Sets [User.autoMoveOverdueTasksTo] to [column] for the current user. Future<void> setAutoMoveOverdueTasksTo(KanbanColumn? column) async { + if (!state.hasData) { + log('Cannot set auto move overdue tasks: No user loaded.'); + return; + } final patch = state.requireData.copyWith( autoMoveOverdueTasksTo: column, ); final transaction = startTransaction('setAutoMoveOverdueTasksTo'); log('Setting auto move overdue tasks to ${column?.name}'); try { await _updateUser( patch, transaction, ); - await captureEvent('auto_move_overdue_tasks', properties: {'column': '${column?.name}'}); + await captureEvent('auto_move_overdue_tasks', properties: {'column': column?.name}); log('Set auto move overdue tasks to ${column?.name}');
306-328: Same issues as above: guard missing and "null" string in analytics.Apply this diff:
/// Sets [User.autoMoveSubmittedTasksTo] to [column] for the current user. Future<void> setAutoMoveSubmittedTasksTo(KanbanColumn? column) async { + if (!state.hasData) { + log('Cannot set auto move submitted tasks: No user loaded.'); + return; + } final patch = state.requireData.copyWith( autoMoveSubmittedTasksTo: column, ); log('Setting auto move submitted tasks to ${column?.name}'); final transaction = startTransaction('setAutoMoveSubmittedTasksTo'); try { await _updateUser( patch, transaction, ); - await captureEvent('auto_move_submitted_tasks', properties: {'column': '${column?.name}'}); + await captureEvent('auto_move_submitted_tasks', properties: {'column': column?.name});
279-303: Add user-loaded guard and avoid sending "null" string to analytics.
state.requireDatacan throw if user not loaded.'${column?.name}'serializes null as the literal "null".Apply this diff:
/// Sets [User.autoMoveCompletedTasksTo] to [column] for the current user. Future<void> setAutoMoveCompletedTasksTo(KanbanColumn? column) async { + if (!state.hasData) { + log('Cannot set auto move completed tasks: No user loaded.'); + return; + } final patch = state.requireData.copyWith( autoMoveCompletedTasksTo: column, ); final transaction = startTransaction('setAutoMoveCompletedTasksTo'); log('Setting auto move completed tasks to ${column?.name}'); try { await _updateUser( patch, transaction, ); - await captureEvent('auto_move_completed_tasks', properties: {'column': '${column?.name}'}); + await captureEvent('auto_move_completed_tasks', properties: {'column': column?.name}); log('Set auto move completed tasks to ${column?.name}');
Analysis Report for b3ab979
Click to see the full report |
Analysis Report for b3ab979
Click to see the full report |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/src/slots/presentation/widgets/edit_slot_dialog.dart (2)
81-90: Simplify boolean checkNo need for “== true”.
- courseMappings.isNotEmpty && - courseMappings.every((element) => element.isSubmitable() == true); + courseMappings.isNotEmpty && + courseMappings.every((e) => e.isSubmitable());
54-79: Dispose controllers and FocusNode (and per‑element controllers)Missing dispose causes leaks. Dispose TextEditingControllers, FocusNode, and MappingElement controllers.
Add this method to the State class:
@override void dispose() { supervisorController.dispose(); roomController.dispose(); roomFocusNode.dispose(); for (final e in courseMappings) { e.courseController.dispose(); e.vintageController.dispose(); } super.dispose(); }lib/src/slots/presentation/widgets/slot_master_widget.dart (1)
44-52: Use setState for isDeleting to update UIWithout setState, buttons might not disable/enable during deletion.
- isDeleting = true; + setState(() => isDeleting = true); try { await slots.deleteSlot(widget.slot.id); } catch (e) { rethrow; } finally { - isDeleting = false; + if (mounted) { + setState(() => isDeleting = false); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
lib/gen/l10n/app_localizations.dartis excluded by!**/gen/**lib/gen/l10n/app_localizations_de.dartis excluded by!**/gen/**lib/gen/l10n/app_localizations_en.dartis excluded by!**/gen/**
📒 Files selected for processing (11)
l10n/de.arb(5 hunks)l10n/en.arb(5 hunks)lib/src/app/utils/animate_utils.dart(3 hunks)lib/src/auth/domain/models/user.dart(1 hunks)lib/src/auth/domain/models/user.freezed.dart(12 hunks)lib/src/kanban/kanban.dart(1 hunks)lib/src/settings/presentation/widgets/kanban_settings.dart(1 hunks)lib/src/slots/presentation/repositories/slot_master_slots_repository.dart(1 hunks)lib/src/slots/presentation/screens/slot_master_screen.dart(3 hunks)lib/src/slots/presentation/widgets/edit_slot_dialog.dart(12 hunks)lib/src/slots/presentation/widgets/slot_master_widget.dart(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analysis 🔍
- GitHub Check: Testing 🧪
🔇 Additional comments (29)
lib/src/app/utils/animate_utils.dart (4)
25-25: LGTM: keyPrefix parameter additionThe addition of the optional
keyPrefixparameter allows callers to provide stable keys for animated widgets, which is good for animation identity management.
27-27: Assertion message correctly updatedThe assertion message has been properly corrected from "positive" to "non-negative" to match the condition
limit >= 0.
58-60: Index-based keys remain unstable for reorderingWhile the typo fix is good, the fundamental issue remains: using index
iinValueKey('$keyPrefix-$i')makes widget identity unstable when list order changes, causing animation recycling on reorders/filtering.Consider the suggested approach from the previous review to add a key factory parameter:
- String? keyPrefix, + String? keyPrefix, + Key? Function(int index, Widget widget)? keyOf,And use it in the animation:
- .animate( - key: keyPrefix != null ? ValueKey('$keyPrefix-$i') : null, - ) + .animate( + key: keyOf?.call(i, this[i]) ?? + (this[i].key ?? (keyPrefix != null ? ValueKey('$keyPrefix-$i') : null)), + )
89-91: Parameter naming: key vs animKeyThe
keyparameter name inAnimateX.showcould be confused withWidget.keyat call sites, as mentioned in the previous review.Consider renaming to
animKeyfor clarity:- Key? key, + Key? animKey,- return animate(key: key).scale(duration: duration, delay: stagger?.add() ?? delay, curve: Curves.easeOutCubic); + return animate(key: animKey).scale(duration: duration, delay: stagger?.add() ?? delay, curve: Curves.easeOutCubic);lib/src/slots/presentation/repositories/slot_master_slots_repository.dart (1)
349-361: LGTM: Efficient O(n) grouping with groupListsBy.Addresses the earlier O(n²) concern; implementation is clean and correct.
lib/src/kanban/kanban.dart (5)
25-25: LGTM: TitleBuilder is localized.Good switch to context.t.kanban_title; addresses prior feedback.
24-24: Ensure auth tokens never reach logs/traces.KanbanDatasource takes a token; verify no logs/prints include it and redact if necessary.
Run:
#!/bin/bash set -euo pipefail # 1) Find Kanban datasources and token parameters rg -nP --type=dart -C2 '(abstract\s+class\s+KanbanDatasource|class\s+\w+KanbanDatasource)\b' rg -nP --type=dart -C2 '\b(token|authToken|authorization)\b' lib | sed -n '1,200p' # 2) Look for potential logging of tokens rg -nP --type=dart -C2 '(log\(|logger\.(debug|info|warn|error|trace)\(|print\()' lib \ | rg -nP -C1 '\b(token|Authorization)\b' || true
8-10: Public export surface looks good.Domain/presentation/utils only; infra remains internal.
24-26: Prefer a resilient composite datasource (Moodle primary + Local fallback).Bind LocalKanbanDatasource and expose KanbanDatasource as a composite that falls back on network errors/timeouts. Improves offline/resilience without touching callers.
Apply this diff:
- ..add<KanbanDatasource>(MoodleKanbanDatasource.new) + ..add<LocalKanbanDatasource>(LocalKanbanDatasource.new) + ..add<MoodleKanbanDatasource>(MoodleKanbanDatasource.new) + ..add<KanbanDatasource>(() => CompositeKanbanDatasource( + i.get<MoodleKanbanDatasource>(), + i.get<LocalKanbanDatasource>(), + ))If not present yet, add a composite in infra (skeleton):
class CompositeKanbanDatasource implements KanbanDatasource { final MoodleKanbanDatasource primary; final LocalKanbanDatasource fallback; CompositeKanbanDatasource(this.primary, this.fallback); // For each method: // try { return await primary.method(...); } // on TimeoutException catch (_) { return fallback.method(...); } // on SocketException catch (_) { return fallback.method(...); } // (optionally cache writes for later sync) }
32-35: No change required — KanbanModule is mounted under '/kanban/'lib/src/app/app.dart registers ModuleRoute('/kanban/', module: KanbanModule()), so r.child('/', ...) inside KanbanModule maps to the /kanban root and won't clash with the app root.
lib/src/settings/presentation/widgets/kanban_settings.dart (5)
42-44: Good: settings title is localized.Thanks for switching to a localized key.
13-15: Verify availability of context.watch().Ensure provider extensions are available via imports/re-exports (e.g., provider.dart) from eduplanner.dart, otherwise import provider explicitly.
16-29: Centralize KanbanColumn → label mapping (duplication).This switch appears in multiple widgets. Extract to a shared i18n helper and reuse.
Apply this change here (remove the local helper):
- String translateColumn(KanbanColumn? column) { - switch (column) { - case KanbanColumn.backlog: - return context.t.kanban_screen_backlog; - case KanbanColumn.todo: - return context.t.kanban_screen_toDo; - case KanbanColumn.inprogress: - return context.t.kanban_screen_inProgress; - case KanbanColumn.done: - return context.t.kanban_screen_done; - case null: - return context.t.kanban_settings_disabled; - } - }Then use the centralized function in itemBuilder (see next comment).
Helper (new file suggestion):
// lib/src/common/i18n/kanban_translations.dart import 'package:eduplanner/eduplanner.dart'; import 'package:flutter/widgets.dart'; String translateKanbanColumn(BuildContext context, KanbanColumn? column) { switch (column) { case KanbanColumn.backlog: return context.t.kanban_screen_backlog; case KanbanColumn.todo: return context.t.kanban_screen_toDo; case KanbanColumn.inprogress: return context.t.kanban_screen_inProgress; case KanbanColumn.done: return context.t.kanban_screen_done; case null: return context.t.kanban_settings_disabled; } }
36-37: Use the shared translator helper in itemBuilder.- itemBuilder: (context, value) => translateColumn(value), + itemBuilder: (context, value) => translateKanbanColumn(context, value),
38-39: Avoid fixed dropdown width; prevents truncation in longer locales.Let it size intrinsically.
- dropDownWidth: 125, + // Let the dropdown size intrinsically to avoid truncation in i18n.lib/src/auth/domain/models/user.dart (1)
51-53: Field addition LGTM: showColumnColors with sane default.Annotation and default align with existing boolean fields.
lib/src/auth/domain/models/user.freezed.dart (1)
478-501: Generated surface for Kanban prefs looks correct.Fields, keys, converters, and defaults are wired through; no manual edits needed.
lib/src/slots/presentation/widgets/edit_slot_dialog.dart (2)
294-306: Weekday dropdown icon consistency resolvedLeading and entry icons now both use FontAwesome5Solid.calendar_check.
584-614: Static ID counter persists across dialogs — optional changeStatic counter grows app‑wide; consider per‑dialog IDs or UniqueKey/UUID.
lib/src/slots/presentation/widgets/slot_master_widget.dart (1)
173-184: Action buttons disable during delete — goodDelete, Duplicate, and Edit correctly disable when deleting.
lib/src/slots/presentation/screens/slot_master_screen.dart (3)
33-38: Good lifecycle managementBoth TabController and searchController are disposed.
119-127: Sort key nullability — verify room non-nullIf Slot.room can be null, sortedBy((s) => s.room) may throw. If it’s non-null in the model, ignore.
69-77: Consider wrapping TabBar in AppBar for consistent theming/elevationScaffold.appBar usually expects an AppBar; using AppBar(bottom: TabBar(...)) restores standard theming and affordances.
- appBar: TabBar( - controller: _tabController, - tabs: [ - for (final weekday in Weekday.values) - Tab( - text: weekday.translate(context), - ), - ], - ), + appBar: AppBar( + bottom: TabBar( + controller: _tabController, + tabs: [ + for (final weekday in Weekday.values) + Tab(text: weekday.translate(context)), + ], + ), + ),l10n/en.arb (3)
51-54: New key: global_duplicateConsistent with new Duplicate button usage.
604-612: “Size” → “Students” alignmentUpdated labels match UI changes.
Also applies to: 660-663
749-818: Kanban keys added — placeholders and descriptions look correctKeys for titles, columns, settings, and card metadata are well-formed.
l10n/de.arb (3)
51-54: Neuer Schlüssel: global_duplicateStimmt mit der neuen Duplizieren-Schaltfläche überein.
602-610: „Größe“ → „Schüler“ angepasstEntspricht den UI-Änderungen.
Also applies to: 658-661
741-810: Kanban-Übersetzungen — korrekt und konsistentBacklog „ausblenden/einblenden“ und „Spaltenfarben“ sind idiomatisch.
| /// The column to auto-move completed tasks to | ||
| @Default(null) @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo, | ||
|
|
||
| /// The column to auto-move submitted tasks to | ||
| @Default(null) @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo, | ||
|
|
||
| /// The column to auto-move overdue tasks to | ||
| @Default(null) @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo, | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick
Drop redundant @default(null) on nullable fields.
For KanbanColumn? properties, null is the implicit default; removing @default(null) reduces noise.
- /// The column to auto-move completed tasks to
- @Default(null) @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo,
+ /// The column to auto-move completed tasks to
+ @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo,
- /// The column to auto-move submitted tasks to
- @Default(null) @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo,
+ /// The column to auto-move submitted tasks to
+ @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo,
- /// The column to auto-move overdue tasks to
- @Default(null) @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo,
+ /// The column to auto-move overdue tasks to
+ @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// The column to auto-move completed tasks to | |
| @Default(null) @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo, | |
| /// The column to auto-move submitted tasks to | |
| @Default(null) @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo, | |
| /// The column to auto-move overdue tasks to | |
| @Default(null) @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo, | |
| /// The column to auto-move completed tasks to | |
| @JsonKey(name: 'automovecompletedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveCompletedTasksTo, | |
| /// The column to auto-move submitted tasks to | |
| @JsonKey(name: 'automovesubmittedtasks') @KanbanColumnConverter() KanbanColumn? autoMoveSubmittedTasksTo, | |
| /// The column to auto-move overdue tasks to | |
| @JsonKey(name: 'automoveoverduetasks') @KanbanColumnConverter() KanbanColumn? autoMoveOverdueTasksTo, |
🤖 Prompt for AI Agents
In lib/src/auth/domain/models/user.dart around lines 54 to 62, the nullable
KanbanColumn? fields use redundant @Default(null) annotations; remove the
@Default(null) from autoMoveCompletedTasksTo, autoMoveSubmittedTasksTo, and
autoMoveOverdueTasksTo so their implicit null default is used, leaving the
@JsonKey and @KanbanColumnConverter() annotations intact.
| ); | ||
| } | ||
|
|
||
| /// Groups all slots by their weekday and startunit. |
There was a problem hiding this comment.
🧹 Nitpick
Nit: Doc typo/consistency.
Use “startUnit” (or “start unit”) for consistency with the model field.
- /// Groups all slots by their weekday and startunit.
+ /// Groups all slots by weekday and startUnit.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Groups all slots by their weekday and startunit. | |
| /// Groups all slots by weekday and startUnit. |
🤖 Prompt for AI Agents
In lib/src/slots/presentation/repositories/slot_master_slots_repository.dart
around line 349, the doc comment uses “startunit” which is inconsistent with the
model field naming; update the comment to use “startUnit” (or “start unit” if
you prefer spaced words) so it matches the model field and maintain consistency
across the codebase.
| start = widget.slot?.startUnit ?? widget.startUnit; | ||
| end = widget.slot?.endUnit; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick
Auto-select end when startUnit is provided
Pre-populate end to start + 1 for better UX when creating/duplicating.
- start = widget.slot?.startUnit ?? widget.startUnit;
- end = widget.slot?.endUnit;
+ start = widget.slot?.startUnit ?? widget.startUnit;
+ end = widget.slot?.endUnit;
+ if (end == null && start != null) {
+ end = start! + 1;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| start = widget.slot?.startUnit ?? widget.startUnit; | |
| end = widget.slot?.endUnit; | |
| start = widget.slot?.startUnit ?? widget.startUnit; | |
| end = widget.slot?.endUnit; | |
| if (end == null && start != null) { | |
| end = start! + 1; | |
| } |
🤖 Prompt for AI Agents
In lib/src/slots/presentation/widgets/edit_slot_dialog.dart around lines 63 to
65, the end unit is not initialized when a startUnit is passed in; update
initialization so that end is set to the slot's endUnit if present, otherwise if
widget.startUnit is non-null set end to widget.startUnit + 1 (ensuring you
handle nullable ints), and only leave end null when neither value exists. Ensure
you preserve existing behavior when slot provides endUnit and handle null-safety
appropriately.
| if (editing) { | ||
| await repo.updateSlot(slot); | ||
| } else if (widget.duplicate) { | ||
| await repo.createSlot(slot.copyWith(id: -1)); | ||
| } else { | ||
| await repo.createSlot(slot); | ||
| } |
There was a problem hiding this comment.
Duplication creates mappings that still reference the original slot and reuse mapping IDs
When duplicating, you’re creating mappings with the original slotId and possibly reusing existing mapping IDs. This can update the original mappings or be rejected downstream. For duplication/new creation, mappings should use slotId = -1 and omit mappingId.
Apply this diff (paired with the next comment’s change) so duplication/new creation never reuses mapping IDs and never references the old slot:
- if (editing) {
+ if (editing) {
await repo.updateSlot(slot);
- } else if (widget.duplicate) {
- await repo.createSlot(slot.copyWith(id: -1));
+ } else if (widget.duplicate) {
+ await repo.createSlot(slot.copyWith(id: -1));
} else {
await repo.createSlot(slot);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/src/slots/presentation/widgets/edit_slot_dialog.dart around lines 133 to
139, the duplication branch currently calls createSlot with the slot (or
slot.copyWith(id: -1)) but leaves mapping objects referencing the original
slotId and retaining mapping IDs; update the code that prepares the Slot for
creation (both duplicate and new cases) to deep-copy its mappings so each
mapping has slotId set to -1 and mappingId omitted/null (or removed) before
passing to repo.createSlot, ensuring new mappings are treated as new records and
do not reference or overwrite the original slot's mappings.
| void addMapping(MappingElement element) { | ||
| final CourseToSlot mapping; | ||
| final slotId = widget.slot?.id ?? -1; | ||
|
|
||
| void setCourse(MoodleCourse? course) { | ||
| setState(() { | ||
| this.course = course; | ||
| }); | ||
| } | ||
| if (element.mappingId == null) { | ||
| mapping = CourseToSlot.noId(courseId: element.courseId!, slotId: slotId, vintage: element.vintage!); | ||
| } else { | ||
| mapping = CourseToSlot(id: element.mappingId!, courseId: element.courseId!, slotId: slotId, vintage: element.vintage!); | ||
| } | ||
|
|
||
| void setVintage(Vintage? vintage) { | ||
| setState(() { | ||
| this.vintage = vintage; | ||
| }); | ||
| mappings.add(mapping); | ||
| } |
There was a problem hiding this comment.
Fix mapping construction for create/duplicate flows
Ensure slotId = -1 for new/duplicate and never reuse mappingId unless editing. This prevents cross-linking to the original slot and accidental updates.
Apply:
void addMapping(MappingElement element) {
- final CourseToSlot mapping;
- final slotId = widget.slot?.id ?? -1;
-
- if (element.mappingId == null) {
- mapping = CourseToSlot.noId(courseId: element.courseId!, slotId: slotId, vintage: element.vintage!);
- } else {
- mapping = CourseToSlot(id: element.mappingId!, courseId: element.courseId!, slotId: slotId, vintage: element.vintage!);
- }
-
- mappings.add(mapping);
+ final CourseToSlot mapping;
+ // Only retain the existing slotId when editing. For create/duplicate, use -1.
+ final slotId = editing ? widget.slot!.id : -1;
+
+ // Only reuse mappingId when editing; for create/duplicate always create a new mapping.
+ if (editing && element.mappingId != null) {
+ mapping = CourseToSlot(
+ id: element.mappingId!,
+ courseId: element.courseId!,
+ slotId: slotId,
+ vintage: element.vintage!,
+ );
+ } else {
+ mapping = CourseToSlot.noId(
+ courseId: element.courseId!,
+ slotId: slotId,
+ vintage: element.vintage!,
+ );
+ }
+
+ mappings.add(mapping);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void addMapping(MappingElement element) { | |
| final CourseToSlot mapping; | |
| final slotId = widget.slot?.id ?? -1; | |
| void setCourse(MoodleCourse? course) { | |
| setState(() { | |
| this.course = course; | |
| }); | |
| } | |
| if (element.mappingId == null) { | |
| mapping = CourseToSlot.noId(courseId: element.courseId!, slotId: slotId, vintage: element.vintage!); | |
| } else { | |
| mapping = CourseToSlot(id: element.mappingId!, courseId: element.courseId!, slotId: slotId, vintage: element.vintage!); | |
| } | |
| void setVintage(Vintage? vintage) { | |
| setState(() { | |
| this.vintage = vintage; | |
| }); | |
| mappings.add(mapping); | |
| } | |
| void addMapping(MappingElement element) { | |
| final CourseToSlot mapping; | |
| // Only retain the existing slotId when editing. For create/duplicate, use -1. | |
| final slotId = editing ? widget.slot!.id : -1; | |
| // Only reuse mappingId when editing; for create/duplicate always create a new mapping. | |
| if (editing && element.mappingId != null) { | |
| mapping = CourseToSlot( | |
| id: element.mappingId!, | |
| courseId: element.courseId!, | |
| slotId: slotId, | |
| vintage: element.vintage!, | |
| ); | |
| } else { | |
| mapping = CourseToSlot.noId( | |
| courseId: element.courseId!, | |
| slotId: slotId, | |
| vintage: element.vintage!, | |
| ); | |
| } | |
| mappings.add(mapping); | |
| } |
🤖 Prompt for AI Agents
In lib/src/slots/presentation/widgets/edit_slot_dialog.dart around lines 197 to
208, the mapping construction currently risks reusing the original
slotId/mappingId for create/duplicate flows; ensure that for new or duplicate
flows you always set slotId to -1 and do not reuse mappingId (leave it null) so
a new mapping is created, and only preserve mappingId when the user is actively
editing an existing mapping. Modify the logic to detect create/duplicate context
and force slotId = -1 and mappingId = null in those cases, otherwise construct
CourseToSlot with the existing id when genuinely editing.
| height: (options.length * 50.0).clamp(0, 200), | ||
| child: Card( | ||
| elevation: 8, | ||
| color: context.theme.scaffoldBackgroundColor, | ||
| child: Column( | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| for (final option in options) | ||
| MenuItemButton( | ||
| child: Text(option), | ||
| onPressed: () => onSelected(option), | ||
| ), | ||
| ], | ||
| child: ListView.builder( |
There was a problem hiding this comment.
Clamp returns num; SizedBox.height expects double
This won’t type-check in Dart 3 without a cast or double bounds.
Use toDouble():
- height: (options.length * 50.0).clamp(0, 200),
+ height: (options.length * 50.0).clamp(0, 200).toDouble(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| height: (options.length * 50.0).clamp(0, 200), | |
| child: Card( | |
| elevation: 8, | |
| color: context.theme.scaffoldBackgroundColor, | |
| child: Column( | |
| mainAxisSize: MainAxisSize.min, | |
| children: [ | |
| for (final option in options) | |
| MenuItemButton( | |
| child: Text(option), | |
| onPressed: () => onSelected(option), | |
| ), | |
| ], | |
| child: ListView.builder( | |
| height: (options.length * 50.0).clamp(0, 200).toDouble(), | |
| child: Card( | |
| elevation: 8, | |
| color: context.theme.scaffoldBackgroundColor, | |
| child: ListView.builder( |
🤖 Prompt for AI Agents
In lib/src/slots/presentation/widgets/edit_slot_dialog.dart around lines 332 to
336, the expression (options.length * 50.0).clamp(0, 200) yields a num but
SizedBox.height expects a double; change the clamp call to produce a double
(e.g. call .toDouble() on the result or supply double bounds like clamp(0.0,
200.0)) so the height expression is a double and the code type-checks in Dart 3.
2 participants
This pull request introduces support for a Kanban board feature, updates localization for new and existing UI elements, and upgrades the project's Flutter SDK version. The changes also include adjustments to code generation settings and minor code cleanups. Below are the most important updates grouped by theme:
Kanban Board Feature Integration
lib/eduplanner.dartto expose Kanban functionality throughout the app.l10n/en.arbandl10n/de.arb, as well as corresponding getters inlib/gen/l10n/app_localizations.dart. This covers labels, settings, and UI text for the Kanban board. [1] [2] [3]Localization Improvements
Flutter SDK and Tooling Updates
.fvmrcand updated the VSCode Dart SDK path accordingly. [1] [2]Code Generation and Build Configuration
build.yamlto restrict code generation (json_serializable, source_gen, freezed) to domain model directories for better performance and organization.synthetic-packagefield froml10n.yamlfor localization configuration.Code Cleanup and Maintenance
@Deprecatedannotations to all Echidna licensing-related constants inlib/config/echidna.dartto indicate licensing is currently disabled.These changes collectively add Kanban board support, improve clarity and consistency in localization, and keep the project up-to-date with the latest Flutter tooling.