fix: respect active filters when reloading tasks after create/update #123

noe.trevino · 2026-02-22T02:13:18-06:00

noe.trevino commented

2026-02-22 02:13:18 -06:00

Summary

Task creation and editing now respect active filter predicates (search, priority, type, assignee, labels) when reloading the task list, instead of bypassing them with an unfiltered query
Shows a warning notification ("Task created/updated but hidden by active filters") when the saved task won't be visible due to active filters
Changes formConfig.onComplete from func() to func() tea.Cmd to allow form completion handlers to return commands (e.g. notifications)

Root Cause

createNewTaskWithLabelsAndRelationships and updateExistingTaskWithLabelsAndRelationships called GetTaskSummariesByProject directly instead of fetchTasksForCurrentProject, which is the method that checks HasAnyFilter() and routes to the filtered query.

Testing

4 new tests in search_persistence_test.go covering create/update with and without active filters
All existing tests pass, golangci-lint clean

Closes #121

## Summary - Task creation and editing now respect active filter predicates (search, priority, type, assignee, labels) when reloading the task list, instead of bypassing them with an unfiltered query - Shows a warning notification ("Task created/updated but hidden by active filters") when the saved task won't be visible due to active filters - Changes `formConfig.onComplete` from `func()` to `func() tea.Cmd` to allow form completion handlers to return commands (e.g. notifications) ## Root Cause `createNewTaskWithLabelsAndRelationships` and `updateExistingTaskWithLabelsAndRelationships` called `GetTaskSummariesByProject` directly instead of `fetchTasksForCurrentProject`, which is the method that checks `HasAnyFilter()` and routes to the filtered query. ## Testing - 4 new tests in `search_persistence_test.go` covering create/update with and without active filters - All existing tests pass, `golangci-lint` clean Closes #121

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

2026-02-22 02:16:45 -06:00

internal/tui/update_forms.go

					
				@ -124,7 +125,7 @@ func (m *Model) updateExistingTaskWithLabelsAndRelationships(values taskFormValu

					if err != nil {

						slog.Error("failed to updating task", "error", err)

						m.UI.Notification.Add(state.LevelError, "Error updating task")

Correctness bug: notification fires even when the task is visible.

HasAnyFilter() checks whether any filter is active, not whether the newly created task is actually absent from the reloaded results. After m.AppState.SetTasks(tasksByColumn) is called on line 94, tasksByColumn already reflects what is visible. The new task's ID (task.ID, available from line 67) should be checked against that map instead.

Example false-positive: a user with an active search for "bug" creates a task titled "Bug fix". The filtered reload will include it (title matches), but the warning still fires.

// After SetTasks, check whether the created task is present
hiddenByFilters := m.UI.Filter.HasAnyFilter() && !taskExistsInMap(tasksByColumn, task.ID)
if hiddenByFilters {
    return m.addNotification(state.LevelWarning, "Task created but hidden by active filters")
}

The same issue exists in updateExistingTaskWithLabelsAndRelationships at line 184.

**Correctness bug: notification fires even when the task is visible.** `HasAnyFilter()` checks whether any filter is *active*, not whether the newly created task is actually *absent* from the reloaded results. After `m.AppState.SetTasks(tasksByColumn)` is called on line 94, `tasksByColumn` already reflects what is visible. The new task's ID (`task.ID`, available from line 67) should be checked against that map instead. Example false-positive: a user with an active search for "bug" creates a task titled "Bug fix". The filtered reload will include it (title matches), but the warning still fires. ```go // After SetTasks, check whether the created task is present hiddenByFilters := m.UI.Filter.HasAnyFilter() && !taskExistsInMap(tasksByColumn, task.ID) if hiddenByFilters { return m.addNotification(state.LevelWarning, "Task created but hidden by active filters") } ``` The same issue exists in `updateExistingTaskWithLabelsAndRelationships` at line 184.

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

2026-02-22 02:16:52 -06:00

internal/tui/search_persistence_test.go

					
				@ -341,0 +374,4 @@

						confirm:     true,

					})

					assert.Greater(t, svc.Task.CallCount("GetTaskSummariesWithFilters"), initialFilteredCalls,

Missing assertions for the notification feature.

The PR description calls out "Shows a warning notification when the saved task won't be visible due to active filters" as a key behaviour, but none of the four new tests assert it. This test has a filter active and GetTaskSummariesWithFiltersResult returns a task with ID 99, while the created task has ID 50 — so the task is hidden and the notification should fire. That return value from createNewTaskWithLabelsAndRelationships is currently discarded and never asserted.

Additionally, there is no test verifying the inverse: that when a newly created task matches the active filter (i.e. it appears in the filtered results), no warning notification is issued. Once the correctness bug in the production code is fixed, that case needs a regression test.

**Missing assertions for the notification feature.** The PR description calls out "Shows a warning notification when the saved task won't be visible due to active filters" as a key behaviour, but none of the four new tests assert it. This test has a filter active and `GetTaskSummariesWithFiltersResult` returns a task with ID 99, while the created task has ID 50 — so the task *is* hidden and the notification *should* fire. That return value from `createNewTaskWithLabelsAndRelationships` is currently discarded and never asserted. Additionally, there is no test verifying the inverse: that when a newly created task *matches* the active filter (i.e. it appears in the filtered results), no warning notification is issued. Once the correctness bug in the production code is fixed, that case needs a regression test.

claude[bot] commented

2026-02-22 02:17:14 -06:00

(Migrated from github.com)

Review Summary

Core fix is correct. Routing createNewTaskWithLabelsAndRelationships and updateExistingTaskWithLabelsAndRelationships through fetchTasksForCurrentProject instead of calling GetTaskSummariesByProject directly is the right fix, and the func() → func() tea.Cmd signature change propagates cleanly through all call sites.

Two issues found:

1. False-positive notification (correctness bug)
Both createNewTaskWithLabelsAndRelationships (line 99) and updateExistingTaskWithLabelsAndRelationships (line 184) fire the "hidden by active filters" warning whenever HasAnyFilter() is true, regardless of whether the task actually appears in the reloaded results. If a user searches for "bug" and creates a task titled "Bug fix", the filtered reload will include the new task, but the warning still fires incorrectly. The check should compare the task's ID against tasksByColumn after SetTasks is called.

2. Notification behaviour has no test coverage
The four new tests cover query routing (filtered vs. unfiltered) but none of them assert the notification return value or inspect whether a warning was issued. The PR description lists the notification as a primary feature. Missing cases:

Assert warning fires when the task is absent from filtered results.
Assert no warning fires when the task matches the filter and is present in results (this case also exposes the production bug above).
updateExistingTask has no without-filter counterpart test (asymmetric with the create tests).

Score: 6/10

## Review Summary **Core fix is correct.** Routing `createNewTaskWithLabelsAndRelationships` and `updateExistingTaskWithLabelsAndRelationships` through `fetchTasksForCurrentProject` instead of calling `GetTaskSummariesByProject` directly is the right fix, and the `func() → func() tea.Cmd` signature change propagates cleanly through all call sites. Two issues found: **1. False-positive notification (correctness bug)** Both `createNewTaskWithLabelsAndRelationships` (line 99) and `updateExistingTaskWithLabelsAndRelationships` (line 184) fire the "hidden by active filters" warning whenever `HasAnyFilter()` is true, regardless of whether the task actually appears in the reloaded results. If a user searches for "bug" and creates a task titled "Bug fix", the filtered reload will include the new task, but the warning still fires incorrectly. The check should compare the task's ID against `tasksByColumn` after `SetTasks` is called. **2. Notification behaviour has no test coverage** The four new tests cover query routing (filtered vs. unfiltered) but none of them assert the notification return value or inspect whether a warning was issued. The PR description lists the notification as a primary feature. Missing cases: - Assert warning fires when the task is absent from filtered results. - Assert no warning fires when the task matches the filter and is present in results (this case also exposes the production bug above). - `updateExistingTask` has no without-filter counterpart test (asymmetric with the create tests). **Score: 6/10**

👍 1

Sign in to join this conversation.

No reviewers