feat: Improve priority sorting by using due date as secondary key #528
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances task sorting functionality by introducing due date as a secondary sorting criterion when tasks have equal priority. Previously, tasks with the same priority were not ordered consistently. The implementation ensures that when priorities match, tasks are sorted by due date (earlier dates first), with tasks lacking due dates placed last.
Key Changes:
- Enhanced priority sorting with secondary due date sorting across all task views
- Applied consistent null handling for tasks without due dates
- Maintained existing code patterns by implementing the logic locally in each sorting location
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/app/utils/taskfunctions/comparator.dart |
Added secondary due date sorting logic to the Priority case in the compareTasks function |
lib/app/modules/home/views/show_tasks_replica.dart |
Implemented secondary due date sorting for Priority+ and Priority- cases in task replica view |
lib/app/modules/home/views/show_tasks.dart |
Implemented secondary due date sorting for Priority+ and Priority- cases with null-safe comparisons |
lib/app/modules/home/controllers/widget.controller.dart |
Implemented secondary due date sorting for Priority+ and Priority- cases in widget controller |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final priorityCompare = | ||
| (a.priority ?? '').compareTo(b.priority ?? ''); | ||
| if (priorityCompare != 0) return priorityCompare; | ||
| // If priorities are equal, sort by due date | ||
| return (a.due ?? '').compareTo(b.due ?? ''); | ||
| case 'Priority-': | ||
| return (b.priority ?? '').compareTo(a.priority ?? ''); | ||
| final priorityCompare = | ||
| (b.priority ?? '').compareTo(a.priority ?? ''); | ||
| if (priorityCompare != 0) return priorityCompare; | ||
| // If priorities are equal, sort by due date | ||
| return (a.due ?? '').compareTo(b.due ?? ''); |
There was a problem hiding this comment.
The secondary sort by due date uses string comparison instead of null-safe DateTime comparison. When 'a.due' or 'b.due' is null, the empty string '' will be compared, which may not produce the intended sorting order. Tasks without due dates should be placed after tasks with due dates, but string comparison of empty strings doesn't guarantee this behavior. Consider using explicit null checks as done in the other files (show_tasks.dart and widget.controller.dart) to ensure consistent sorting behavior.
| // If priorities are equal, sort by due date | ||
| if (result == 0) { | ||
| if (a.due == null && b.due == null) { | ||
| result = 0; | ||
| } else if (a.due == null) { | ||
| result = 1; | ||
| } else if (b.due == null) { | ||
| result = -1; | ||
| } else { | ||
| result = a.due!.compareTo(b.due!); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new secondary sorting logic for priority (sorting by due date when priorities are equal) lacks test coverage. The existing test for Priority in test/utils/taskfunctions/comparator_test.dart only tests the primary priority comparison and doesn't cover the scenario where two tasks have equal priority. Consider adding test cases that verify tasks with the same priority are correctly sorted by due date, including edge cases where one or both tasks have null due dates.
| final priorityCompare = b.priority!.compareTo(a.priority!); | ||
| if (priorityCompare != 0) return priorityCompare; | ||
| // If priorities are equal, sort by due date | ||
| if (a.due == null && b.due == null) return 0; | ||
| if (a.due == null) return 1; | ||
| if (b.due == null) return -1; | ||
| return a.due!.compareTo(b.due!); | ||
| case 'Priority+': | ||
| return a.priority!.compareTo(b.priority!); | ||
| final priorityCompare = a.priority!.compareTo(b.priority!); | ||
| if (priorityCompare != 0) return priorityCompare; | ||
| // If priorities are equal, sort by due date | ||
| if (a.due == null && b.due == null) return 0; | ||
| if (a.due == null) return 1; | ||
| if (b.due == null) return -1; | ||
| return a.due!.compareTo(b.due!); |
There was a problem hiding this comment.
This secondary sorting logic by due date is duplicated across multiple files (show_tasks.dart, show_tasks_replica.dart, widget.controller.dart, and comparator.dart). This code duplication makes maintenance harder and increases the risk of inconsistencies. Consider extracting this logic into a reusable helper function that can be shared across all these locations to ensure consistent behavior and easier maintenance.
1 participant
Description
This PR improves task sorting when sorting by priority.
Previously, tasks with the same priority were not further ordered, which could lead to confusing or inaccurate task lists.
This change introduces due date as a secondary sorting key when priorities are equal.
The logic is implemented locally in each sorting location to match existing patterns and avoid cross-layer refactors.
Fixes #527
Screenshots
Checklist