fix(workspace): notify about collections that fail to open in Workspa…#8719
fix(workspace): notify about collections that fail to open in Workspa…#8719ravindra-bruno wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughChangesFailed Collection Flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceConfig
participant WorkspaceActions
participant CollectionsList
participant Toast
WorkspaceConfig-->>WorkspaceActions: provide failedToOpen and failureReason
WorkspaceActions->>WorkspaceActions: skip failed collections during workspace switch
WorkspaceActions->>Toast: report unopened collection failures
CollectionsList->>Toast: show collection-specific open failure
CollectionsList->>WorkspaceActions: remove failed collection from workspace
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js (1)
374-380: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for switch-time failure aggregation.
Cover previously failed workspace entries,
openResult.failed,openResult.invalid, and both singular and plural toast messages inactions.spec.js. These branches can otherwise regress by reopening failed paths or silently omitting the notification.As per coding guidelines, add or update tests for meaningful code changes and prioritize high-value failure-path coverage.
Also applies to: 395-402, 404-409
🤖 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 `@packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js` around lines 374 - 380, Add regression tests in actions.spec.js for the workspace-switch flow covering previously failed entries, openResult.failed, and openResult.invalid. Verify failed paths are not reopened or included in collectionPaths, and assert the correct singular and plural toast messages for each failure count.Source: Coding guidelines
🤖 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
`@packages/bruno-app/src/components/WorkspaceHome/WorkspaceOverview/CollectionsList/index.js`:
- Around line 80-81: Update the collection-mapping logic in CollectionsList so
wc.failedToOpen takes precedence over any loadedCollection for the same path.
Only return or use loadedCollection when !wc.failedToOpen; otherwise preserve
the failure metadata and prevent stale collection data from being mounted.
In `@packages/bruno-electron/src/utils/workspace-config.js`:
- Around line 566-575: Update getCollectionFailureReason so filesystem
exceptions are classified as not-found only when the error code is ENOENT or
ENOTDIR; return invalid for other failures such as EACCES. Preserve the existing
not-found result for missing or non-directory paths.
---
Nitpick comments:
In `@packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js`:
- Around line 374-380: Add regression tests in actions.spec.js for the
workspace-switch flow covering previously failed entries, openResult.failed, and
openResult.invalid. Verify failed paths are not reopened or included in
collectionPaths, and assert the correct singular and plural toast messages for
each failure count.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 493eabd4-ed47-49e2-8815-de3c114fc436
📒 Files selected for processing (4)
packages/bruno-app/src/components/WorkspaceHome/WorkspaceOverview/CollectionsList/index.jspackages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.jspackages/bruno-electron/src/utils/workspace-config.jspackages/bruno-electron/tests/utils/workspace-config.spec.js
|
|
||
| if (unopened.length > 0) { | ||
| const message = unopened.length === 1 | ||
| ? `Collection could not be opened: ${unopened[0]}` | ||
| : `${unopened.length} collections could not be opened`; | ||
| toast.error(message); | ||
| } |
There was a problem hiding this comment.
This toast would fires on every workspace switch, indented behaviour ?
| {collection.failedToOpen && ( | ||
| <StatusBadge status="danger" size="xs"> | ||
| {collection.failureReason === 'not-found' ? 'Missing' : 'Failed to open'} | ||
| </StatusBadge> | ||
| )} | ||
| {!isDefaultWorkspace && collection.isLoaded === false && !collection.failedToOpen && ( |
There was a problem hiding this comment.
There is an assertion for the Git badge on workspace overview (git-backed-collections.spec.ts), we can do the same for the new badges and their click behaviour.
| if (collection.failedToOpen) { | ||
| if (collection.failureReason === 'not-found') { | ||
| toast.error(`Collection "${collection.name}" could not be opened — not found at ${collection.pathname}`); | ||
| } else { | ||
| toast.error(`Collection "${collection.name}" could not be opened — no valid collection found at ${collection.pathname}`); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Can we match the toast style used in actions.js? Would keep the two surfaces consistent.
[JIRA - BRU-2508]
Cause: When a workspace was opened, any local collection that couldn't be loaded, its folder had been moved or deleted, or the path held no valid collection config was silently discarded during resolution. Because it was dropped entirely, Workspace Home showed nothing, no notification fired, and the user had no way to see why it failed or to remove the stale entry.
Resolution: Instead of discarding the broken entry, it's now kept and flagged with a failure reason (either "not found" or "invalid"). On switching workspaces the user gets an error notification, the collection stays visible with a red "Missing" / "Failed to open" badge, clicking it explains the reason, and it can be removed from the workspace via the row menu. The flags are transient in-memory state, so nothing changes on disk.
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Bug Fixes