Feat: add Upcoming view - #40
Conversation
while here, fixed the tomorrow's view broken header handling note : this is getting out of hands, we need to find a better way to handle sections and so on
WalkthroughAdds an Upcoming view across storage, sync, UI core, sidebar, and task list. Introduces IconService::upcoming(), storage/sync APIs to retrieve upcoming tasks, a new SidebarSelection::Upcoming variant, sidebar navigation/rendering for Upcoming, and task list rendering logic for grouped upcoming sections. Minor today query changes to SELECT *. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks (3 passed)✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/ui/components/task_list_component.rs (1)
617-650: Wrong empty-state message for UpcomingShows “No tasks in this project” when viewing Upcoming. Use an Upcoming-specific message.
- let empty_message = if self.projects.is_empty() { + let empty_message = if self.projects.is_empty() { "No projects available. Press 'r' to sync or 'A' to create a project." } else if matches!(self.sidebar_selection, SidebarSelection::Today) { "No tasks due today. Press 'a' to create a task or 'r' to sync." } else if matches!(self.sidebar_selection, SidebarSelection::Tomorrow) { "No tasks due tomorrow. Press 'a' to create a task or 'r' to sync." + } else if matches!(self.sidebar_selection, SidebarSelection::Upcoming) { + "No upcoming tasks scheduled. Press 'a' to create a task or 'r' to sync." } else { "No tasks in this project. Press 'a' to create a task." };src/ui/components/sidebar_component.rs (1)
104-117: Fix wrap-around: Previous from Today should go to Upcoming (not Tomorrow) when no labels/projectsWith Upcoming inserted between Tomorrow and Labels, pressing “previous” on Today should wrap to Upcoming when there are no labels or projects. The current behavior creates a Today ↔ Tomorrow loop and skips Upcoming.
Apply:
SidebarSelection::Today => { if !self.projects.is_empty() { let sorted_projects = self.get_sorted_projects(); if let Some((original_index, _)) = sorted_projects.last() { SidebarSelection::Project(*original_index) } else { - SidebarSelection::Tomorrow + SidebarSelection::Upcoming } } else if !self.labels.is_empty() { SidebarSelection::Label(self.labels.len() - 1) } else { - SidebarSelection::Tomorrow + SidebarSelection::Upcoming } }
🧹 Nitpick comments (6)
src/storage.rs (3)
861-874: Prefer explicit column list overSELECT *for stability/perfAvoids fetching unused columns and reduces fragility if schema changes.
- SELECT * - FROM tasks + SELECT id, content, project_id, section_id, is_completed, is_deleted, priority, + due_date, due_datetime, is_recurring, deadline, duration, labels, description + FROM tasks
918-920: Avoid N+1 label lookups by batching once per listCurrently we issue one SELECT per task. Batch once to cut DB round-trips.
Apply to both today and upcoming flows:
- // Update label colors for all tasks - for task in &mut tasks { - self.update_task_labels(task).await?; - } + // Update label colors for all tasks in one go + self.update_labels_for_tasks(&mut tasks).await?;Add helper (outside this hunk):
pub async fn update_labels_for_tasks(&self, tasks: &mut [TaskDisplay]) -> Result<()> { use std::collections::{HashMap, HashSet}; let names: HashSet<String> = tasks.iter() .flat_map(|t| t.labels.iter().map(|l| l.name.clone())) .collect(); if names.is_empty() { return Ok(()); } let stored = self.get_labels_by_ids(&names.into_iter().collect::<Vec<_>>()).await?; let map: HashMap<_, _> = stored.into_iter().map(|l| (l.name, l.color)).collect(); for t in tasks { for ld in &mut t.labels { if let Some(c) = map.get(&ld.name) { ld.color = c.clone(); } } } Ok(()) }Also applies to: 1054-1056
238-329: Add indexes to keep Upcoming/Today queries fastFilters/sorts hit
is_completed,is_deleted,due_date,priority,order_index. Add composite indexes.// Create tasks table sqlx::query( r" CREATE TABLE IF NOT EXISTS tasks ( ... ) " ) .execute(&self.pool) .await?; + + // Helpful indexes for date-based views + sqlx::query( + r" + CREATE INDEX IF NOT EXISTS idx_tasks_due_flags + ON tasks(is_completed, is_deleted, due_date); + CREATE INDEX IF NOT EXISTS idx_tasks_sort + ON tasks(due_date, priority DESC, order_index ASC); + " + ).execute(&self.pool).await?;src/icons.rs (1)
293-300: LGTM:upcoming()theming helperMatches existing pattern. Suggest adding tests for it alongside today/tomorrow.
// tests/icons_test.rs @@ fn test_today_tomorrow_icons() { @@ } + +#[test] +fn test_upcoming_icons() { + let emoji = IconService::new(IconTheme::Emoji); + assert_eq!(emoji.upcoming(), "📊"); + let unicode = IconService::new(IconTheme::Unicode); + assert_eq!(unicode.upcoming(), "◎"); + let ascii = IconService::new(IconTheme::Ascii); + assert_eq!(ascii.upcoming(), ">"); +}src/ui/components/task_list_component.rs (1)
224-299: Use IconService instead of hard-coded emojis in headersKeeps theming consistent and avoids mixed icon sources.
- let date_header = if due_date == today { - "📅 Today".to_string() - } else if due_date == today + chrono::Duration::days(1) { - "📅 Tomorrow".to_string() - } else if due_date == today + chrono::Duration::days(2) { - "📊 Day After Tomorrow".to_string() - } else { - let weekday = due_date.format("%A").to_string(); - let formatted_date = due_date.format("%b %d").to_string(); - format!("📊 {} - {}", weekday, formatted_date) - }; + let date_header = if due_date == today { + format!("{} {}", self.icons.today(), "Today") + } else if due_date == today + chrono::Duration::days(1) { + format!("{} {}", self.icons.tomorrow(), "Tomorrow") + } else if due_date == today + chrono::Duration::days(2) { + format!("{} {}", self.icons.upcoming(), "Day After Tomorrow") + } else { + let weekday = due_date.format("%A").to_string(); + let formatted_date = due_date.format("%b %d").to_string(); + format!("{} {} - {}", self.icons.upcoming(), weekday, formatted_date) + };src/ui/components/sidebar_component.rs (1)
330-344: Render for Upcoming is correct; consider DRY-ing repeated styling codeToday/Tomorrow/Upcoming share identical selected/unselected style branches. A tiny helper would reduce duplication.
Example helper (outside the changed range):
fn nav_item(&self, selected: bool, icon: &str, label: &str) -> ListItem { let style = if selected { Style::default().fg(Color::Yellow).add_modifier(Modifier::BOLD) } else { Style::default().fg(Color::White) }; ListItem::new(Line::from(vec![ Span::styled(icon.to_string(), style), Span::styled(label.to_string(), style), ])) }Then here:
- all_items.push(ListItem::new(Line::from(vec![ - Span::styled(self.icons.upcoming().to_string(), upcoming_style), - Span::styled("Upcoming".to_string(), upcoming_style), - ]))); + all_items.push(self.nav_item(is_upcoming_selected, self.icons.upcoming(), "Upcoming"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/icons.rs(1 hunks)src/storage.rs(2 hunks)src/sync.rs(1 hunks)src/ui/app_component.rs(4 hunks)src/ui/components/sidebar_component.rs(5 hunks)src/ui/components/task_list_component.rs(4 hunks)src/ui/core/actions.rs(1 hunks)src/ui/core/task_manager.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/icons.rs (1)
tests/icons_test.rs (1)
test_today_tomorrow_icons(43-55)
src/sync.rs (1)
src/storage.rs (1)
get_tasks_for_upcoming(994-1059)
src/storage.rs (1)
src/sync.rs (1)
get_tasks_for_upcoming(123-126)
src/ui/components/task_list_component.rs (2)
src/ui/components/sidebar_component.rs (2)
new(29-39)default(23-25)src/todoist.rs (3)
from(50-59)from(63-70)from(74-109)
src/ui/components/sidebar_component.rs (1)
src/icons.rs (5)
default(20-22)default(80-82)new(88-90)icons(114-120)upcoming(294-300)
🔇 Additional comments (11)
src/ui/core/actions.rs (1)
9-11: LGTM: Upcoming selection addedEnum extension is clean and preserves default.
src/sync.rs (1)
122-127: LGTM: storage passthrough for UpcomingConsistent with Today/Tomorrow methods.
src/ui/core/task_manager.rs (1)
204-207: LGTM: Upcoming data-load branchParallels existing flows and uses unwrap_or_default defensively.
src/ui/app_component.rs (1)
231-235: LGTM: Guard rails for Upcoming and selection loggingConsistent UX with Today/Tomorrow special views.
Also applies to: 283-287, 384-385
src/ui/components/task_list_component.rs (3)
87-89: LGTM: Rendered-index routing includes UpcomingKeeps selection math consistent.
115-118: LGTM: Upcoming short-circuit in item creationPrevents fallthrough into generic rendering.
711-782: LGTM: Rendered-index math for Tomorrow/UpcomingMatches section/blank-line structure of the renderers.
src/ui/components/sidebar_component.rs (4)
49-63: Next selection flow from Tomorrow → Upcoming looks correctThe transition to Upcoming and the fallbacks into Labels/Projects/Today read well and match the new index order.
119-125: Good: Upcoming → Tomorrow and Label(0) → UpcomingThese preserve a consistent backward navigation order with the new section inserted.
142-143: Good: From first Project back to Upcoming when no labelsThis completes the cyclic nav path when labels are absent.
225-228: Index mapping correct; no stale “2-based” offsets foundindex 2 → Upcoming and labels begin at index 3 in src/ui/components/sidebar_component.rs (index_to_selection, lines 217–241). Repo-wide search returned no other index==2 or index±2 matches.
| /// Get tasks for upcoming from local storage (overdue + next 3 months) | ||
| pub async fn get_tasks_for_upcoming(&self) -> Result<Vec<TaskDisplay>> { | ||
| let rows = sqlx::query( | ||
| r" | ||
| SELECT * | ||
| FROM tasks | ||
| WHERE is_completed = false | ||
| AND is_deleted = false | ||
| AND due_date IS NOT NULL | ||
| AND due_date <= date('now', '+3 months') | ||
| ORDER BY | ||
| CASE | ||
| WHEN due_date < date('now') THEN 0 -- Overdue tasks first | ||
| WHEN due_date = date('now') THEN 1 -- Today's tasks second | ||
| ELSE 2 -- Future tasks third | ||
| END, | ||
| due_date ASC, -- Then chronological order | ||
| priority DESC, -- Then priority (high to low) | ||
| order_index ASC -- Finally by user's manual order | ||
| ", | ||
| ) | ||
| .fetch_all(&self.pool) | ||
| .await?; | ||
|
|
||
| let mut tasks = rows | ||
| .into_iter() | ||
| .map(|row| { | ||
| // Parse labels from JSON string | ||
| let label_names: Vec<String> = | ||
| serde_json::from_str(&row.get::<String, _>("labels")).unwrap_or_default(); | ||
|
|
||
| // Convert label names to LabelDisplay objects (colors will be filled in later) | ||
| let labels = label_names | ||
| .into_iter() | ||
| .map(|name| crate::todoist::LabelDisplay { | ||
| id: name.clone(), // Use name as ID for now | ||
| name, | ||
| color: "blue".to_string(), // Default color, will be updated from storage | ||
| }) | ||
| .collect(); | ||
|
|
||
| TaskDisplay { | ||
| id: row.get("id"), | ||
| content: row.get("content"), | ||
| project_id: row.get("project_id"), | ||
| section_id: row.get("section_id"), | ||
| is_completed: row.get("is_completed"), | ||
| is_deleted: row.get("is_deleted"), | ||
| priority: row.get("priority"), | ||
| due: row.get("due_date"), | ||
| due_datetime: row.get("due_datetime"), | ||
| is_recurring: row.get("is_recurring"), | ||
| deadline: row.get("deadline"), | ||
| duration: row.get("duration"), | ||
| labels, | ||
| description: row.get("description"), | ||
| } | ||
| }) | ||
| .collect::<Vec<TaskDisplay>>(); | ||
|
|
||
| // Update label colors for all tasks | ||
| for task in &mut tasks { | ||
| self.update_task_labels(task).await?; | ||
| } | ||
|
|
||
| Ok(tasks) | ||
| } |
There was a problem hiding this comment.
Label colors never update: querying by ID but passing names
update_task_labels extracts label names, but get_labels_by_ids queries WHERE id IN (...). Result: empty hits and labels stay "blue".
Minimal fix: make the helper query by name.
- /// Get labels by their IDs
- pub async fn get_labels_by_ids(&self, label_ids: &[String]) -> Result<Vec<LocalLabel>> {
- if label_ids.is_empty() {
+ /// Get labels by their names (matches TaskDisplay.label.name)
+ pub async fn get_labels_by_ids(&self, label_names: &[String]) -> Result<Vec<LocalLabel>> {
+ if label_names.is_empty() {
return Ok(Vec::new());
}
- let placeholders = label_ids.iter().map(|_| "?").collect::<Vec<_>>().join(",");
- let query = format!("SELECT * FROM labels WHERE id IN ({placeholders}) ORDER BY order_index");
+ let placeholders = label_names.iter().map(|_| "?").collect::<Vec<_>>().join(",");
+ let query = format!("SELECT * FROM labels WHERE name IN ({placeholders}) ORDER BY order_index");
let mut query_builder = sqlx::query_as::<_, LocalLabel>(&query);
- for id in label_ids {
- query_builder = query_builder.bind(id);
+ for name in label_names {
+ query_builder = query_builder.bind(name);
}Optionally rename method to get_labels_by_names for clarity (can update the single caller).
📝 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.
| /// Get tasks for upcoming from local storage (overdue + next 3 months) | |
| pub async fn get_tasks_for_upcoming(&self) -> Result<Vec<TaskDisplay>> { | |
| let rows = sqlx::query( | |
| r" | |
| SELECT * | |
| FROM tasks | |
| WHERE is_completed = false | |
| AND is_deleted = false | |
| AND due_date IS NOT NULL | |
| AND due_date <= date('now', '+3 months') | |
| ORDER BY | |
| CASE | |
| WHEN due_date < date('now') THEN 0 -- Overdue tasks first | |
| WHEN due_date = date('now') THEN 1 -- Today's tasks second | |
| ELSE 2 -- Future tasks third | |
| END, | |
| due_date ASC, -- Then chronological order | |
| priority DESC, -- Then priority (high to low) | |
| order_index ASC -- Finally by user's manual order | |
| ", | |
| ) | |
| .fetch_all(&self.pool) | |
| .await?; | |
| let mut tasks = rows | |
| .into_iter() | |
| .map(|row| { | |
| // Parse labels from JSON string | |
| let label_names: Vec<String> = | |
| serde_json::from_str(&row.get::<String, _>("labels")).unwrap_or_default(); | |
| // Convert label names to LabelDisplay objects (colors will be filled in later) | |
| let labels = label_names | |
| .into_iter() | |
| .map(|name| crate::todoist::LabelDisplay { | |
| id: name.clone(), // Use name as ID for now | |
| name, | |
| color: "blue".to_string(), // Default color, will be updated from storage | |
| }) | |
| .collect(); | |
| TaskDisplay { | |
| id: row.get("id"), | |
| content: row.get("content"), | |
| project_id: row.get("project_id"), | |
| section_id: row.get("section_id"), | |
| is_completed: row.get("is_completed"), | |
| is_deleted: row.get("is_deleted"), | |
| priority: row.get("priority"), | |
| due: row.get("due_date"), | |
| due_datetime: row.get("due_datetime"), | |
| is_recurring: row.get("is_recurring"), | |
| deadline: row.get("deadline"), | |
| duration: row.get("duration"), | |
| labels, | |
| description: row.get("description"), | |
| } | |
| }) | |
| .collect::<Vec<TaskDisplay>>(); | |
| // Update label colors for all tasks | |
| for task in &mut tasks { | |
| self.update_task_labels(task).await?; | |
| } | |
| Ok(tasks) | |
| } | |
| /// Get labels by their names (matches TaskDisplay.label.name) | |
| pub async fn get_labels_by_ids(&self, label_names: &[String]) -> Result<Vec<LocalLabel>> { | |
| if label_names.is_empty() { | |
| return Ok(Vec::new()); | |
| } | |
| let placeholders = label_names.iter().map(|_| "?").collect::<Vec<_>>().join(","); | |
| let query = format!("SELECT * FROM labels WHERE name IN ({}) ORDER BY order_index", placeholders); | |
| let mut query_builder = sqlx::query_as::<_, LocalLabel>(&query); | |
| for name in label_names { | |
| query_builder = query_builder.bind(name); | |
| } | |
| let rows = query_builder.fetch_all(&self.pool).await?; | |
| Ok(rows) | |
| } |
🤖 Prompt for AI Agents
In src/storage.rs around lines 993-1059, the label-color update fails because
update_task_labels extracts label names but calls a helper that queries labels
by id; change the lookup to match by name instead of id. Implement or rename the
helper to get_labels_by_names (or modify its SQL) so it executes WHERE name IN
(...) and return LabelDisplay rows by matching on the name field; update the
single caller in this file (update_task_labels) to call the new/updated function
and pass the Vec<String> of names so stored colors replace the default "blue".
1 participant
while here, fixed the tomorrow's view broken header handling
note : this is getting out of hands, we need to find a better way to handle sections and so on
Summary by CodeRabbit