feat(studio): when model fileset created, land on detail page#136
Conversation
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFilesetCreateModal now navigates to the model detail Files or External tab after creating a model fileset; ModelCardTab renders a fileset description above the README; tests updated to expect the model detail Files tab route. ChangesModel fileset creation navigation and display
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/studio/src/components/FilesetCreateModal/index.spec.tsx (1)
263-290: ⚡ Quick winMissing coverage for Model + External → Card tab.
The new
isExternal ? ModelDetailTab.Card : Filesbranch only tests the Local path. The External (?tab=card) branch is untested. Consider adding a case mirroring the dataset external flow.🤖 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 `@web/packages/studio/src/components/FilesetCreateModal/index.spec.tsx` around lines 263 - 290, Add a new test case mirroring the Local model test but covering the External branch: render FilesetCreateModal with purpose={FilesetPurpose.model}, set mockMutate.mockResolvedValue to return the fileset with name 'mymodel' and isExternal/is_external true (similar to how dataset external flow is mocked), simulate typing the name and clicking the create button, then assert navigate was called with a URL containing '/workspaces/default/models/mymodel?tab=card' to verify the Model+External → Card tab branch is covered (use the same helpers: mockUseNavigate, mockMutate, userEvent, and expect.stringContaining to locate the URL).
🤖 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.
Nitpick comments:
In `@web/packages/studio/src/components/FilesetCreateModal/index.spec.tsx`:
- Around line 263-290: Add a new test case mirroring the Local model test but
covering the External branch: render FilesetCreateModal with
purpose={FilesetPurpose.model}, set mockMutate.mockResolvedValue to return the
fileset with name 'mymodel' and isExternal/is_external true (similar to how
dataset external flow is mocked), simulate typing the name and clicking the
create button, then assert navigate was called with a URL containing
'/workspaces/default/models/mymodel?tab=card' to verify the Model+External →
Card tab branch is covered (use the same helpers: mockUseNavigate, mockMutate,
userEvent, and expect.stringContaining to locate the URL).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6ee71d01-23cc-46f8-bc48-70dbbcb00006
📒 Files selected for processing (2)
web/packages/studio/src/components/FilesetCreateModal/index.spec.tsxweb/packages/studio/src/components/FilesetCreateModal/index.tsx
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
|
Summary by CodeRabbit
Bug Fixes
New Features
Tests