Fix: Start date remains unset unless explicitly selected by user #517
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #515 by preventing backend auto-generated start dates from appearing for new tasks. The fix ensures that start dates remain null by default unless explicitly selected by the user in the Edit view, and persist correctly after saving and reopening.
Key Changes:
- Added tracking mechanism (
startEditedflag) to detect when users explicitly select a start date - Implemented logic to filter out backend auto-generated start dates (where start == entry)
- Added exposure of
originaltask in the Modify class for comparison purposes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
lib/app/utils/taskfunctions/modify.dart |
Exposes original task getter to enable comparison between original and draft task states |
lib/app/utils/taskfunctions/draft.dart |
Removes leading empty line (formatting fix) |
lib/app/modules/detailRoute/controllers/detail_route_controller.dart |
Implements core fix with startEdited flag, auto-start detection logic, and conditional start date handling in initValues() and saveChanges() methods; includes minor refactoring of tutorial tour conditional |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final originalStart = modify.original.start; | ||
| final originalEntry = modify.original.entry; | ||
|
|
||
| final backendAutoStart = (originalStart != null && | ||
| originalStart.isAtSameMomentAs(originalEntry)); | ||
|
|
||
| // START DATE LOGIC (THE FIX) | ||
| if (startEdited) { | ||
| startValue.value = modify.draft.start; | ||
| } else if (backendAutoStart) { | ||
| startValue.value = null; // Do not show backend auto start | ||
| } else { | ||
| startValue.value = modify.draft.start; // Existing meaningful start | ||
| } |
There was a problem hiding this comment.
The new logic for handling the start date is complex and undocumented. Consider adding a comment explaining:
- What
backendAutoStartmeans (start date was auto-generated by backend when it equals entry date) - Why we hide it from the UI when not explicitly edited by the user
- The three scenarios being handled (user-edited, backend auto-generated, and meaningful existing start)
This will help future maintainers understand the intent behind this fix.
| Future<void> saveChanges() async { | ||
| // If start was never edited AND backend auto-generated it (start == entry) | ||
| if (!startEdited && | ||
| modify.original.start != null && | ||
| modify.original.start!.isAtSameMomentAs(modify.original.entry)) { | ||
| modify.set('start', null); // remove auto start | ||
| } |
There was a problem hiding this comment.
The new behavior for handling start dates lacks test coverage. Given that tests exist for other task functions (e.g., test/taskfunctions/draft_test.dart), consider adding tests to verify:
- Start date remains null for new tasks with backend auto-generated start (start == entry)
- Start date is preserved when explicitly set by user
- Start date persists correctly after save and reopen
- The
startEditedflag behaves correctly across different scenarios
This will prevent regressions and document the expected behavior.
|
|
||
| if (name == 'start') { | ||
| debugPrint('Start Value Changed to $newValue'); | ||
| startEdited = true; // MARK AS USER-SELECTED |
There was a problem hiding this comment.
[nitpick] The comment "// MARK AS USER-SELECTED" uses non-standard comment style. The codebase appears to use standard Dart comment conventions. Consider using a more descriptive comment like:
// Mark that user explicitly selected a start dateThis is clearer and follows typical Dart commenting practices.
| startEdited = true; // MARK AS USER-SELECTED | |
| startEdited = true; // Mark that user explicitly selected a start date |
| // ignore: avoid_print | ||
| print('User has seen this page'), | ||
| } | ||
| if (!value) {tutorialCoachMark.show(context: context)} |
There was a problem hiding this comment.
[nitpick] The refactored conditional expression is missing proper formatting. While the logic is correct, it should be formatted more clearly for readability:
if (!value) {
tutorialCoachMark.show(context: context);
}The current single-line block without braces, while valid Dart, is inconsistent with the codebase style and less maintainable. If additional logic needs to be added later, it could lead to bugs.
| // Track whether user explicitly selected a start date | ||
| bool startEdited = false; |
There was a problem hiding this comment.
The startEdited flag is not reset when the controller is reused. If the user edits a task, saves it, and then opens another task detail view (or the same task again), the flag will still be true from the previous interaction. This could cause the start date logic to behave incorrectly.
Consider resetting startEdited = false in the onInit() method or creating a new instance of the controller for each task.
| // If start was never edited AND backend auto-generated it (start == entry) | ||
| if (!startEdited && | ||
| modify.original.start != null && | ||
| modify.original.start!.isAtSameMomentAs(modify.original.entry)) { |
There was a problem hiding this comment.
This logic has a potential issue: it checks if modify.original.start is not null and equals modify.original.entry, but it doesn't verify whether modify.draft.start is still the same value. If the user explicitly sets the start date to match the entry date (an unlikely but valid scenario), this code would incorrectly remove it.
Consider also checking that modify.draft.start equals modify.original.start to ensure you're only removing the backend-generated value, not a user's intentional selection.
| modify.original.start!.isAtSameMomentAs(modify.original.entry)) { | |
| modify.original.start!.isAtSameMomentAs(modify.original.entry) && | |
| modify.draft.start != null && | |
| modify.draft.start!.isAtSameMomentAs(modify.original.start!)) { |
1 participant
Description
Prevent backend/auto-generated start from appearing for new tasks. Start now:
Fixes #515
Screenshots
after.515.refix.mp4
Checklist