Let existing goals be edited from the planner and calendar - #46
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds editing for existing study goals in the planner and calendar, with shared validation and checklist preservation.
Changes:
- Added planner and calendar Edit actions.
- Centralized goal edit fields and validation.
- Updated project documentation.
File summaries
| File | Description |
|---|---|
| src/main/java/com/studysync/presentation/ui/components/TaskManagementPanel.java | Updated as part of this pull request. |
| src/main/java/com/studysync/presentation/ui/components/StudyPlannerPanel.java | Updated as part of this pull request. |
| src/main/java/com/studysync/presentation/ui/components/GoalEditFields.java | Updated as part of this pull request. |
| src/main/java/com/studysync/presentation/ui/components/CalendarViewPanel.java | Updated as part of this pull request. |
| AGENTS.md | Updated as part of this pull request. |
Review details
Suppressed comments (1)
src/main/java/com/studysync/presentation/ui/components/GoalEditFields.java:17
- This contract is not true for all goal forms:
StudyPlannerPanel.showAddGoalDialogandCalendarViewPanel.showAddGoalDialogstill build their own description, checklist, and date controls and validate them independently. Those copies can diverge fromGoalEditFieldsas validation evolves; reuse this component there (with the task selector/date-specific behavior around it) or narrow the documentation to edit dialogs.
/**
* The fields every goal form shares: description, the "done when" checklist
* and the planned date. Build one, drop {@link #view()} into any dialog, then
* call {@link #save} or {@link #create} from its confirm button.
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… pane when its date moves
There was a problem hiding this comment.
🔵 Needs a closer look
One or more issues must be addressed before approval.
Review details
Suppressed comments (1)
src/main/java/com/studysync/presentation/ui/components/CalendarViewPanel.java:1266
StudyGoal.findByIdprefers a pending attempt over achieved/missed attempts, so this reload is not guaranteed to represent the attempt shown ingoalBox. For example, editing an achieved attempt after a later retry is planned leaves the old attempt on this date, butupdated.getDate()is the retry date and this code removes the still-valid box (and can redraw the wrong attempt when dates coincide). Reload the attempt by itsattemptIdon the original date and remove the box only when that specific attempt no longer exists there.
StudyGoal.findById(goal.getId()).ifPresent(updated -> {
VBox parent = (VBox) goalBox.getParent();
if (updated.getDate().equals(goal.getDate())) {
parent.getChildren().set(parent.getChildren().indexOf(goalBox), createStudyGoalBox(updated));
} else {
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
…empt findById prefers
There was a problem hiding this comment.
🔵 Needs a closer look
A moderate past-date validation issue remains, and shared dialog paths lack JavaFX-thread coverage.
Review details
Suppressed comments (2)
src/main/java/com/studysync/presentation/ui/components/GoalEditFields.java:49
- This shared date picker leaves past days selectable. Goal planning elsewhere rejects dates before the current day (
StudyService.planGoalAttemptand the planner's add-goal picker), whilegetGoalsForDateruns the overdue sweep that marks pending attempts before today as MISSED. Editing a pending goal to yesterday therefore silently changes its outcome on the next refresh instead of just rescheduling it; reject past dates in the service and disable them in this shared editor.
date = new DatePicker(isNew ? defaultDate : goal.getDate());
date.setMaxWidth(Double.MAX_VALUE);
date.setDisable(!canEditDate);
src/main/java/com/studysync/presentation/ui/components/GoalEditFields.java:85
- This new helper is the shared execution path for all three edit dialogs, but the current tests only exercise
StudyService.updateStudyGoalDetails; none constructs the JavaFX fields or verifies that blank descriptions/null pending dates keep the dialog open and that a pending date is passed through. A regression here would break planner, calendar, and task-history editing together. Add a JavaFX-thread test for these validation and save paths, similar toSessionVisibilityTest.
/** Applies the fields to an existing goal; throws with a user-facing message when invalid. */
void save(StudyService studyService, StudyGoal goal) {
studyService.updateStudyGoalDetails(goal.getId(), validDescription(),
canEditDate ? validDate() : null, criteria.getText());
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
Existing goals can now be edited wherever they are shown, so a "done when" checklist can be added to goals created before 0.1.8 or extended later.
GoalEditFieldsclass builds and validates the description, checklist, and planned date; the three panels dropped their own copies.Validation:
./gradlew testpasses (79 tests). UI not exercised interactively.