Feature/calendar #69

noe.trevino · 2026-02-14T03:07:04-06:00

noe.trevino commented

2026-02-14 03:07:04 -06:00

No description provided.

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:08:11 -06:00

internal/tui/renderers/date_picker.go

					
				@ -0,0 +99,4 @@

				func lastActiveWeek(grid [][]int) int {

					for i := len(grid) - 1; i >= 0; i-- {

						for _, day := range grid[i] {

							if day > 0 {

DRY violation: This exact calculation time.Date(newYear, newMonth+1, 0, 0, 0, 0, 0, time.Local).Day() is duplicated in PrevMonth() at line 130. Extract this into a helper function:

func getLastDayOfMonth(year int, month time.Month) int {
    return time.Date(year, month+1, 0, 0, 0, 0, 0, time.Local).Day()
}

Then both functions can use: lastDayOfNewMonth := getLastDayOfMonth(newYear, newMonth)

**DRY violation:** This exact calculation `time.Date(newYear, newMonth+1, 0, 0, 0, 0, 0, time.Local).Day()` is duplicated in `PrevMonth()` at line 130. Extract this into a helper function: ```go func getLastDayOfMonth(year int, month time.Month) int { return time.Date(year, month+1, 0, 0, 0, 0, 0, time.Local).Day() } ``` Then both functions can use: `lastDayOfNewMonth := getLastDayOfMonth(newYear, newMonth)`

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:08:14 -06:00

internal/tui/renderers/date_picker.go

					
				@ -0,0 +103,4 @@

								return i

							}

						}

					}

DRY violation: This clamping logic (lines 102-106) is duplicated in PrevMonth() (lines 130-134). After extracting the getLastDayOfMonth helper, extract the clamping logic:

func clampDayToMonth(desiredDay int, year int, month time.Month) int {
    lastDay := getLastDayOfMonth(year, month)
    if desiredDay > lastDay {
        return lastDay
    }
    return desiredDay
}

**DRY violation:** This clamping logic (lines 102-106) is duplicated in `PrevMonth()` (lines 130-134). After extracting the `getLastDayOfMonth` helper, extract the clamping logic: ```go func clampDayToMonth(desiredDay int, year int, month time.Month) int { lastDay := getLastDayOfMonth(year, month) if desiredDay > lastDay { return lastDay } return desiredDay } ```

github-actions[bot] commented

2026-02-14 03:08:15 -06:00

(Migrated from github.com)

PR Summary (auto-generated)

Added due date functionality to tasks with database migration (00010_add_task_due_date.sql) adding a nullable due_date timestamp column to the tasks table across both SQLite and PostgreSQL
Implemented a complete date picker UI component with calendar navigation, keyboard shortcuts (ctrl+d for quick-edit), and Vim-style cursor memory pattern that preserves day selection when navigating between months
Mapped DueDate field through all layers of the application including database adapters (postgres/sqlite), converters, service layer, and TUI components with comprehensive test coverage
Added AGENTS.md documentation file containing tech stack overview, coding practices, and development guidelines for the Paso project
Enhanced detail panel and task components to display and edit due dates with improved styling including color theming for calendar UI
Fixed test parallelization issues by removing t.Parallel() from subtests that share a single database instance in gitlink integration tests

**PR Summary** _(auto-generated)_ - Added due date functionality to tasks with database migration (00010_add_task_due_date.sql) adding a nullable `due_date` timestamp column to the tasks table across both SQLite and PostgreSQL - Implemented a complete date picker UI component with calendar navigation, keyboard shortcuts (ctrl+d for quick-edit), and Vim-style cursor memory pattern that preserves day selection when navigating between months - Mapped DueDate field through all layers of the application including database adapters (postgres/sqlite), converters, service layer, and TUI components with comprehensive test coverage - Added AGENTS.md documentation file containing tech stack overview, coding practices, and development guidelines for the Paso project - Enhanced detail panel and task components to display and edit due dates with improved styling including color theming for calendar UI - Fixed test parallelization issues by removing t.Parallel() from subtests that share a single database instance in gitlink integration tests

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:08:19 -06:00

internal/tui/update_pickers.go

					
				@ -907,0 +975,4 @@

							m.Pickers.DatePicker.CurrentYear,

							m.Pickers.DatePicker.CurrentMonth,

							m.Pickers.DatePicker.CursorDay,

							0, 0, 0, 0,

DRY violation: This entire block (clearing due date and updating database) is duplicated at lines 986-1001 (setting due date). The only difference is the date value and notification message. Extract to a helper:

func (m *Model) updateDueDateIfEditing(dueDate *time.Time, successMsg string) {
    if m.Forms.Form.EditingTaskID == 0 {
        m.UI.Notification.Add(state.LevelInfo, successMsg)
        return
    }
    
    ctx, cancel := m.DBContext()
    defer cancel()
    
    err := m.App.TaskService.UpdateTaskDueDate(ctx, m.Forms.Form.EditingTaskID, dueDate)
    if err != nil {
        slog.Error("failed to update task due date", "error", err)
        m.UI.Notification.Add(state.LevelError, "Failed to update due date")
    } else {
        m.UI.Notification.Add(state.LevelInfo, successMsg)
        m.reloadCurrentColumnTasks()
    }
}

Then call: m.updateDueDateIfEditing(nil, "Due date cleared") and m.updateDueDateIfEditing(&selectedDate, "Due date set to "+selectedDate.Format("Jan 2, 2006"))

**DRY violation:** This entire block (clearing due date and updating database) is duplicated at lines 986-1001 (setting due date). The only difference is the date value and notification message. Extract to a helper: ```go func (m *Model) updateDueDateIfEditing(dueDate *time.Time, successMsg string) { if m.Forms.Form.EditingTaskID == 0 { m.UI.Notification.Add(state.LevelInfo, successMsg) return } ctx, cancel := m.DBContext() defer cancel() err := m.App.TaskService.UpdateTaskDueDate(ctx, m.Forms.Form.EditingTaskID, dueDate) if err != nil { slog.Error("failed to update task due date", "error", err) m.UI.Notification.Add(state.LevelError, "Failed to update due date") } else { m.UI.Notification.Add(state.LevelInfo, successMsg) m.reloadCurrentColumnTasks() } } ``` Then call: `m.updateDueDateIfEditing(nil, "Due date cleared")` and `m.updateDueDateIfEditing(&selectedDate, "Due date set to "+selectedDate.Format("Jan 2, 2006"))`

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:09:00 -06:00

internal/tui/update_pickers.go

					
				@ -907,0 +975,4 @@

							m.Pickers.DatePicker.CurrentYear,

							m.Pickers.DatePicker.CurrentMonth,

							m.Pickers.DatePicker.CursorDay,

							0, 0, 0, 0,

DRY violation: This block (clearing due date + database update) is duplicated at lines 986-1004. Extract to a helper method:

func (m *Model) saveDueDateChange(dueDate *time.Time, successMsg string) {
    m.Forms.Form.FormDueDate = dueDate
    
    if m.Forms.Form.EditingTaskID != 0 {
        ctx, cancel := m.DBContext()
        defer cancel()
        
        if err := m.App.TaskService.UpdateTaskDueDate(ctx, m.Forms.Form.EditingTaskID, dueDate); err != nil {
            slog.Error("failed to update task due date", "error", err)
            m.UI.Notification.Add(state.LevelError, "Failed to update due date")
            return
        }
        m.reloadCurrentColumnTasks()
    }
    m.UI.Notification.Add(state.LevelInfo, successMsg)
}

**DRY violation:** This block (clearing due date + database update) is duplicated at lines 986-1004. Extract to a helper method: ```go func (m *Model) saveDueDateChange(dueDate *time.Time, successMsg string) { m.Forms.Form.FormDueDate = dueDate if m.Forms.Form.EditingTaskID != 0 { ctx, cancel := m.DBContext() defer cancel() if err := m.App.TaskService.UpdateTaskDueDate(ctx, m.Forms.Form.EditingTaskID, dueDate); err != nil { slog.Error("failed to update task due date", "error", err) m.UI.Notification.Add(state.LevelError, "Failed to update due date") return } m.reloadCurrentColumnTasks() } m.UI.Notification.Add(state.LevelInfo, successMsg) } ```

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:09:03 -06:00

internal/services/task/service.go

DRY violation: This null time conversion pattern is duplicated in the new UpdateTaskDueDate method. Add a helper to internal/database/types/nullable.go:

func NullTimeFromPointer(ptr *time.Time) NullTime {
    if ptr == nil {
        return NullTime{}
    }
    return NullTime{Time: *ptr, Valid: true}
}

Then use: dueDate := types.NullTimeFromPointer(req.DueDate)

**DRY violation:** This null time conversion pattern is duplicated in the new `UpdateTaskDueDate` method. Add a helper to `internal/database/types/nullable.go`: ```go func NullTimeFromPointer(ptr *time.Time) NullTime { if ptr == nil { return NullTime{} } return NullTime{Time: *ptr, Valid: true} } ``` Then use: `dueDate := types.NullTimeFromPointer(req.DueDate)`

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:09:10 -06:00

internal/tui/state/date_picker_state.go

					
				@ -0,0 +109,4 @@

					s.CurrentMonth = newMonth

					s.CursorDay = newDay

					// Do NOT update cursorWant - keep the desired position for future navigation

				}

DRY violation: NextMonth() and PrevMonth() (lines 114-140) contain duplicated logic. Both functions:

Calculate new month/year with wrapping
Get last day of new month via time.Date(newYear, newMonth+1, 0, ...)
Clamp cursorWant to valid day range
Update state fields

Refactor to eliminate duplication:

func (s *DatePickerState) changeMonth(delta int) {
    s.CurrentMonth += time.Month(delta)
    s.CurrentYear += int(s.CurrentMonth-1) / 12  // Handle year changes
    s.CurrentMonth = ((s.CurrentMonth-1)%12 + 12) % 12 + 1  // Wrap month to 1-12
    
    lastDay := time.Date(s.CurrentYear, s.CurrentMonth+1, 0, 0, 0, 0, 0, time.Local).Day()
    s.CursorDay = min(s.cursorWant, lastDay)
}

Then NextMonth() becomes s.changeMonth(1) and PrevMonth() becomes s.changeMonth(-1).

**DRY violation:** `NextMonth()` and `PrevMonth()` (lines 114-140) contain duplicated logic. Both functions: 1. Calculate new month/year with wrapping 2. Get last day of new month via `time.Date(newYear, newMonth+1, 0, ...)` 3. Clamp cursorWant to valid day range 4. Update state fields Refactor to eliminate duplication: ```go func (s *DatePickerState) changeMonth(delta int) { s.CurrentMonth += time.Month(delta) s.CurrentYear += int(s.CurrentMonth-1) / 12 // Handle year changes s.CurrentMonth = ((s.CurrentMonth-1)%12 + 12) % 12 + 1 // Wrap month to 1-12 lastDay := time.Date(s.CurrentYear, s.CurrentMonth+1, 0, 0, 0, 0, 0, time.Local).Day() s.CursorDay = min(s.cursorWant, lastDay) } ``` Then `NextMonth()` becomes `s.changeMonth(1)` and `PrevMonth()` becomes `s.changeMonth(-1)`.

claude[bot] (Migrated from github.com) reviewed

2026-02-14 03:09:51 -06:00

internal/converters/task.go

					
				@ -61,1 +61,4 @@

					}

					if t.DueDate.Valid {

						task.DueDate = &t.DueDate.Time

					}

Minor improvement: This null-to-pointer conversion pattern is used 4 times in this file (lines 53-64). Consider extracting to a helper in the types package:

// In internal/database/types/nullable.go
func (n NullTime) ToPointer() *time.Time {
    if !n.Valid {
        return nil
    }
    t := n.Time
    return &t
}

Then: task.DueDate = t.DueDate.ToPointer()

Same for NullString and NullInt64.

**Minor improvement:** This null-to-pointer conversion pattern is used 4 times in this file (lines 53-64). Consider extracting to a helper in the types package: ```go // In internal/database/types/nullable.go func (n NullTime) ToPointer() *time.Time { if !n.Valid { return nil } t := n.Time return &t } ``` Then: `task.DueDate = t.DueDate.ToPointer()` Same for `NullString` and `NullInt64`.

claude[bot] commented

2026-02-14 03:10:09 -06:00

(Migrated from github.com)

Code Review Summary

This PR adds due date functionality to tasks. The implementation is functional but contains several refactoring opportunities to reduce code duplication.

Key Issues Found

1. DRY Violations - Date Picker State (High Priority)

NextMonth() and PrevMonth() in date_picker_state.go contain nearly identical logic (22 lines duplicated)
Month boundary calculations and day clamping logic are repeated
Impact: Maintenance burden, potential for divergence in behavior

2. DRY Violations - Renderer Helpers (Medium Priority)

date_picker.go duplicates last-day-of-month calculation in two places
Clamping logic appears twice identically
Impact: Harder to maintain calendar logic

3. DRY Violations - Update Handlers (Medium Priority)

update_pickers.go duplicates the "update database if editing" pattern for clearing/setting due dates
This same pattern appears across estimate, assignee, priority, type, and due date updates
Impact: 5+ functions with near-identical database update logic

4. Missing Helper Methods (Low Priority)

Null-to-pointer conversions (types.NullTime → *time.Time) lack utility methods
Pattern appears multiple times: creating NullTime{Time: *ptr, Valid: true}
Impact: Minor verbosity, opportunity for DRYer code

Test Coverage

Due date service tests are comprehensive (happy path + edge cases + error cases)
Date picker state tests cover month navigation edge cases
Component tests verify relative date formatting

Security & Performance

No security issues identified
No N+1 query problems introduced
Database migrations properly handle nullable columns

Recommendation

Approve with suggestions for follow-up refactoring. The functionality works correctly and is well-tested. The DRY violations should be addressed in a follow-up PR to improve maintainability, but they don't block this feature.

## Code Review Summary This PR adds due date functionality to tasks. The implementation is functional but contains several refactoring opportunities to reduce code duplication. ### Key Issues Found #### 1. **DRY Violations - Date Picker State (High Priority)** - `NextMonth()` and `PrevMonth()` in `date_picker_state.go` contain nearly identical logic (22 lines duplicated) - Month boundary calculations and day clamping logic are repeated - **Impact:** Maintenance burden, potential for divergence in behavior #### 2. **DRY Violations - Renderer Helpers (Medium Priority)** - `date_picker.go` duplicates last-day-of-month calculation in two places - Clamping logic appears twice identically - **Impact:** Harder to maintain calendar logic #### 3. **DRY Violations - Update Handlers (Medium Priority)** - `update_pickers.go` duplicates the "update database if editing" pattern for clearing/setting due dates - This same pattern appears across estimate, assignee, priority, type, and due date updates - **Impact:** 5+ functions with near-identical database update logic #### 4. **Missing Helper Methods (Low Priority)** - Null-to-pointer conversions (`types.NullTime → *time.Time`) lack utility methods - Pattern appears multiple times: creating `NullTime{Time: *ptr, Valid: true}` - **Impact:** Minor verbosity, opportunity for DRYer code ### Test Coverage - Due date service tests are comprehensive (happy path + edge cases + error cases) - Date picker state tests cover month navigation edge cases - Component tests verify relative date formatting ### Security & Performance - No security issues identified - No N+1 query problems introduced - Database migrations properly handle nullable columns ### Recommendation **Approve with suggestions for follow-up refactoring.** The functionality works correctly and is well-tested. The DRY violations should be addressed in a follow-up PR to improve maintainability, but they don't block this feature.

Sign in to join this conversation.

No reviewers