fix(desktop): hide recent history entries without matching data#1444
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds filtering of recently viewed entries in HistoryDropdown, replacing recentEntries with filteredEntries for empty-state and list rendering. Adds test-time mocks for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Filter out workspace and task entries from the "Recently Viewed" dropdown when their data can no longer be resolved, instead of showing "Unknown" fallback rows.
saddlepaddle
force-pushed
the
clean-up-recent-history
branch
4 times, most recently
from
5fe35a0 to
f015413
Compare
![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
🤖 Fix all issues with AI agents
In `@apps/desktop/test-setup.ts`:
- Around line 148-168: The global mock defines EXTERNAL_APPS as an empty array
which causes z.enum(EXTERNAL_APPS) to throw at runtime; update the mock.module
call so EXTERNAL_APPS is a non-empty tuple of strings (e.g., include a single
representative value like "none" or "internal") to satisfy z.enum(EXTERNAL_APPS)
and any consumers that expect at least one option; ensure the value shape
matches how EXTERNAL_APPS is used elsewhere (string tuple) and adjust other
mocks if they rely on specific entries.
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/terminal/terminal.stream.test.ts (1)
88-96: Local mock diverges from the globaltest-setup.tsmock surfaceThis local mock omits tables (
users,organizations,organizationMembers,tasks) and constants (BRANCH_PREFIX_MODES,TERMINAL_LINK_BEHAVIORS,FILE_OPEN_MODES) that the global mock provides. It works today because terminal tests don't touch those, but if the terminal module's dependency graph expands, you'll get silentundefinedvalues instead of a clear failure. Consider keeping the two in sync or extracting a shared helper.
| // ============================================================================= | ||
| // @superset/local-db Schema Mock (drizzle-orm/sqlite-core not available in Bun tests) | ||
| // ============================================================================= | ||
|
|
||
| const mockTable = (name: string) => ({ id: `${name}_id` }); | ||
|
|
||
| mock.module("@superset/local-db", () => ({ | ||
| projects: mockTable("projects"), | ||
| workspaces: mockTable("workspaces"), | ||
| worktrees: mockTable("worktrees"), | ||
| settings: mockTable("settings"), | ||
| users: mockTable("users"), | ||
| organizations: mockTable("organizations"), | ||
| organizationMembers: mockTable("organization_members"), | ||
| tasks: mockTable("tasks"), | ||
| EXTERNAL_APPS: [], | ||
| EXECUTION_MODES: ["sequential", "parallel"], | ||
| BRANCH_PREFIX_MODES: ["none", "github", "author", "custom"], | ||
| TERMINAL_LINK_BEHAVIORS: ["external-editor", "file-viewer"], | ||
| FILE_OPEN_MODES: ["split-pane", "new-tab"], | ||
| })); |
There was a problem hiding this comment.
EXTERNAL_APPS: [] will crash any test path that reaches z.enum(EXTERNAL_APPS)
In external/index.ts (line 14), z.enum(EXTERNAL_APPS) requires a non-empty tuple. An empty array causes a Zod runtime error. If any test transitively imports the external router while relying on this global mock, it will fail. Consider providing at least one representative value.
Proposed fix
mock.module("@superset/local-db", () => ({
projects: mockTable("projects"),
workspaces: mockTable("workspaces"),
worktrees: mockTable("worktrees"),
settings: mockTable("settings"),
users: mockTable("users"),
organizations: mockTable("organizations"),
organizationMembers: mockTable("organization_members"),
tasks: mockTable("tasks"),
- EXTERNAL_APPS: [],
+ EXTERNAL_APPS: ["cursor"],
EXECUTION_MODES: ["sequential", "parallel"],
BRANCH_PREFIX_MODES: ["none", "github", "author", "custom"],
TERMINAL_LINK_BEHAVIORS: ["external-editor", "file-viewer"],
FILE_OPEN_MODES: ["split-pane", "new-tab"],
}));📝 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.
| // ============================================================================= | |
| // @superset/local-db Schema Mock (drizzle-orm/sqlite-core not available in Bun tests) | |
| // ============================================================================= | |
| const mockTable = (name: string) => ({ id: `${name}_id` }); | |
| mock.module("@superset/local-db", () => ({ | |
| projects: mockTable("projects"), | |
| workspaces: mockTable("workspaces"), | |
| worktrees: mockTable("worktrees"), | |
| settings: mockTable("settings"), | |
| users: mockTable("users"), | |
| organizations: mockTable("organizations"), | |
| organizationMembers: mockTable("organization_members"), | |
| tasks: mockTable("tasks"), | |
| EXTERNAL_APPS: [], | |
| EXECUTION_MODES: ["sequential", "parallel"], | |
| BRANCH_PREFIX_MODES: ["none", "github", "author", "custom"], | |
| TERMINAL_LINK_BEHAVIORS: ["external-editor", "file-viewer"], | |
| FILE_OPEN_MODES: ["split-pane", "new-tab"], | |
| })); | |
| // ============================================================================= | |
| // `@superset/local-db` Schema Mock (drizzle-orm/sqlite-core not available in Bun tests) | |
| // ============================================================================= | |
| const mockTable = (name: string) => ({ id: `${name}_id` }); | |
| mock.module("@superset/local-db", () => ({ | |
| projects: mockTable("projects"), | |
| workspaces: mockTable("workspaces"), | |
| worktrees: mockTable("worktrees"), | |
| settings: mockTable("settings"), | |
| users: mockTable("users"), | |
| organizations: mockTable("organizations"), | |
| organizationMembers: mockTable("organization_members"), | |
| tasks: mockTable("tasks"), | |
| EXTERNAL_APPS: ["cursor"], | |
| EXECUTION_MODES: ["sequential", "parallel"], | |
| BRANCH_PREFIX_MODES: ["none", "github", "author", "custom"], | |
| TERMINAL_LINK_BEHAVIORS: ["external-editor", "file-viewer"], | |
| FILE_OPEN_MODES: ["split-pane", "new-tab"], | |
| })); |
🤖 Prompt for AI Agents
In `@apps/desktop/test-setup.ts` around lines 148 - 168, The global mock defines
EXTERNAL_APPS as an empty array which causes z.enum(EXTERNAL_APPS) to throw at
runtime; update the mock.module call so EXTERNAL_APPS is a non-empty tuple of
strings (e.g., include a single representative value like "none" or "internal")
to satisfy z.enum(EXTERNAL_APPS) and any consumers that expect at least one
option; ensure the value shape matches how EXTERNAL_APPS is used elsewhere
(string tuple) and adjust other mocks if they rely on specific entries.
Add mock.module for @superset/local-db in terminal.stream and external/helpers tests. Bun's mock.module leaks across test files, so the terminal test's mock must include EXTERNAL_APPS to avoid breaking the helpers test when both run in the same process.
saddlepaddle
force-pushed
the
clean-up-recent-history
branch
from
f015413 to
4047609
Compare
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
Test plan
Summary by CodeRabbit
Bug Fixes
Tests
Refactor
Chores