Repository navigation
Codex/unify project sidebar - #1139
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFilters workspace-mode device options in ProjectCreateDialog to exclude devices with bind_shell === 'openclaw' and adds a Jest test verifying ClaudeCode devices appear while OpenClaw devices do not. ChangesWorkspace device filtering and test
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9df23e880a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| open={createDialogOpen} | ||
| onOpenChange={setCreateDialogOpen} | ||
| mode={isWorkspaceSection ? 'workspace' : 'group'} | ||
| mode={isUnifiedSection || isWorkspaceSection ? 'workspace' : 'group'} |
There was a problem hiding this comment.
When NEXT_PUBLIC_ENABLE_PROJECT_WORKSPACE is false or the user is outside PROJECT_WORKSPACE_WHITELIST, ProjectContext.isWorkspaceEnabled is false, but the sidebar now renders the default unified section and this line always opens ProjectCreateDialog in workspace mode for that section. This bypasses the existing feature gate that previously guarded the workspace section in TaskSidebar, so users with the workspace feature disabled can still click the sidebar + and create workspace projects; gate the create action/dialog mode on isWorkspaceEnabled or fall back to group creation when disabled.
Useful? React with 👍 / 👎.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/__tests__/utils/version.test.ts`:
- Around line 8-12: Add assertions to the isVersionAtLeast tests to cover
uppercase 'V' and whitespace normalization: update the test case in
frontend/src/__tests__/utils/version.test.ts (the test block using
isVersionAtLeast) to include checks like isVersionAtLeast('V1.7.11', '1.7.11')
and isVersionAtLeast(' v1.7.11 ', '1.7.11') (and their symmetric variants) to
ensure the parser trims input and accepts both 'v' and 'V' prefixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5245237-bc39-4148-8f7a-78720285608f
📒 Files selected for processing (14)
frontend/src/__tests__/features/projects/components/ProjectCreateDialog.test.tsxfrontend/src/__tests__/features/projects/components/project-sidebar-behavior.test.tsxfrontend/src/__tests__/utils/version.test.tsfrontend/src/features/projects/components/DroppableProject.tsxfrontend/src/features/projects/components/ProjectCreateDialog.tsxfrontend/src/features/projects/components/ProjectSection.tsxfrontend/src/features/projects/components/ProjectTaskMenu.tsxfrontend/src/features/projects/contexts/dndContext.tsxfrontend/src/features/projects/utils/projectClassification.tsfrontend/src/features/tasks/components/sidebar/TaskMenu.tsxfrontend/src/features/tasks/components/sidebar/TaskSidebar.tsxfrontend/src/i18n/locales/en/projects.jsonfrontend/src/i18n/locales/zh-CN/projects.jsonfrontend/src/lib/utils.ts
| test('accepts versions with a v prefix', () => { | ||
| expect(isVersionAtLeast('v1.7.11', '1.7.11')).toBe(true) | ||
| expect(isVersionAtLeast('1.7.11', 'v1.7.11')).toBe(true) | ||
| expect(isVersionAtLeast('v1.7.10', 'v1.7.11')).toBe(false) | ||
| }) |
There was a problem hiding this comment.
Add coverage for uppercase V and whitespace normalization.
Line 8 currently validates only lowercase v. Since parser logic now trims and accepts V, add explicit assertions to prevent regressions.
Suggested test additions
describe('version utils', () => {
test('accepts versions with a v prefix', () => {
expect(isVersionAtLeast('v1.7.11', '1.7.11')).toBe(true)
expect(isVersionAtLeast('1.7.11', 'v1.7.11')).toBe(true)
expect(isVersionAtLeast('v1.7.10', 'v1.7.11')).toBe(false)
+ expect(isVersionAtLeast('V1.7.11', '1.7.11')).toBe(true)
+ expect(isVersionAtLeast(' v1.7.11 ', '1.7.11')).toBe(true)
})
})📝 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.
| test('accepts versions with a v prefix', () => { | |
| expect(isVersionAtLeast('v1.7.11', '1.7.11')).toBe(true) | |
| expect(isVersionAtLeast('1.7.11', 'v1.7.11')).toBe(true) | |
| expect(isVersionAtLeast('v1.7.10', 'v1.7.11')).toBe(false) | |
| }) | |
| test('accepts versions with a v prefix', () => { | |
| expect(isVersionAtLeast('v1.7.11', '1.7.11')).toBe(true) | |
| expect(isVersionAtLeast('1.7.11', 'v1.7.11')).toBe(true) | |
| expect(isVersionAtLeast('v1.7.10', 'v1.7.11')).toBe(false) | |
| expect(isVersionAtLeast('V1.7.11', '1.7.11')).toBe(true) | |
| expect(isVersionAtLeast(' v1.7.11 ', '1.7.11')).toBe(true) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/__tests__/utils/version.test.ts` around lines 8 - 12, Add
assertions to the isVersionAtLeast tests to cover uppercase 'V' and whitespace
normalization: update the test case in
frontend/src/__tests__/utils/version.test.ts (the test block using
isVersionAtLeast) to include checks like isVersionAtLeast('V1.7.11', '1.7.11')
and isVersionAtLeast(' v1.7.11 ', '1.7.11') (and their symmetric variants) to
ensure the parser trims input and accepts both 'v' and 'V' prefixes.
8296d6e to
0065b01
Compare
0065b01 to
e99f7b7
Compare
2 participants
Summary by CodeRabbit
New Features
Tests