feat(app): add "Ignore Folder" sidebar context menu item (closes #1890)#8398
feat(app): add "Ignore Folder" sidebar context menu item (closes #1890)#8398justadityaraj wants to merge 1 commit into
Conversation
Folders can now be added to a collection's ignore list from the sidebar context menu instead of hand-editing bruno.json. The folder's collection-relative path (stored with forward slashes so it stays portable across operating systems) is written to brunoConfig.ignore via the existing updateBrunoConfig flow, and the folder is removed from the sidebar immediately. Adds getCollectionRelativePath and addPathToIgnoreList helpers with unit tests covering nested paths, Windows separators, trailing slashes, and ignore-list dedupe/immutability. Closes usebruno#1890
justadityaraj
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
WalkthroughAdds an "Ignore Folder" context menu action to the sidebar's ChangesIgnore Folder Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/Sidebar/Collections/Collection/CollectionItem/index.js`:
- Around line 347-352: The Ignore Folder flow in CollectionItem should not reuse
the Delete action path, because dispatch(deleteItem(...)) triggers
renderer:delete-item and can physically remove the folder instead of only hiding
it. Update the post-update cleanup in CollectionItem to use the same UI-removal
path as an ignore action, and make sure the call matches the deleteItem thunk
signature only if you keep that helper; otherwise switch to a dedicated
non-destructive tree-removal action after updateBrunoConfig succeeds.
- Around line 346-347: The update is rebuilding from a stale collection config
because CollectionItem reads collection?.brunoConfig directly before calling
updateBrunoConfig(). Fix it by using the latest config source in this flow, such
as deriving from the current ignore state or from the result of the previous
update, and ensure addPathToIgnoreList() always merges into the most recent
brunoConfig before dispatching updateBrunoConfig(). This should be adjusted in
the CollectionItem logic where the ignore action is triggered so consecutive
folder ignores cannot overwrite earlier changes.
In `@packages/bruno-app/src/utils/collections/ignore.js`:
- Around line 10-23: The fallback in getCollectionRelativePath is returning a
normalized absolute target path when itemPathname is not actually under the
collection root. Update the function so it only returns a relative suffix when
target equals root or starts with root/, and otherwise returns an empty string.
Keep the path normalization in toPosix, but avoid persisting any
non-collection-relative value into brunoConfig.ignore.
In `@packages/bruno-app/src/utils/collections/ignore.spec.js`:
- Around line 3-28: The getCollectionRelativePath test suite is missing
regression coverage for paths that cannot be derived relative to the collection
root. Add cases in ignore.spec.js for a non-descendant item path and for a
case-only mismatch between the collection root and item path, asserting that
getCollectionRelativePath returns an empty string in both scenarios. Use the
existing getCollectionRelativePath helper to keep the new tests aligned with the
current path-normalization behavior.
🪄 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: 120fcfd0-9eae-4694-816f-4f84269d529a
📒 Files selected for processing (3)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.jspackages/bruno-app/src/utils/collections/ignore.jspackages/bruno-app/src/utils/collections/ignore.spec.js
| const brunoConfig = addPathToIgnoreList(collection?.brunoConfig, relativePath); | ||
| dispatch(updateBrunoConfig(brunoConfig, collectionUid)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
This merge starts from stale brunoConfig.
Line 346 reads collection?.brunoConfig from Redux, but updateBrunoConfig() only persists over IPC and does not update that state slice. Ignoring folder B right after folder A in the same session will rebuild from the pre-change config and can drop A from bruno.json.
🤖 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/components/Sidebar/Collections/Collection/CollectionItem/index.js`
around lines 346 - 347, The update is rebuilding from a stale collection config
because CollectionItem reads collection?.brunoConfig directly before calling
updateBrunoConfig(). Fix it by using the latest config source in this flow, such
as deriving from the current ignore state or from the result of the previous
update, and ensure addPathToIgnoreList() always merges into the most recent
brunoConfig before dispatching updateBrunoConfig(). This should be adjusted in
the CollectionItem logic where the ignore action is triggered so consecutive
folder ignores cannot overwrite earlier changes.
| dispatch(updateBrunoConfig(brunoConfig, collectionUid)) | ||
| .then(() => { | ||
| // Remove the folder from the sidebar immediately; bruno.json keeps it | ||
| // ignored on subsequent loads. | ||
| dispatch(deleteItem({ itemUid: item.uid, collectionUid })); | ||
| toast.success(`Ignored folder: ${relativePath}`); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Don't use deleteItem to hide an ignored folder.
Line 351 dispatches the same thunk used by the Delete action, and that thunk invokes renderer:delete-item for the folder path. So once the call shape is corrected, Ignore Folder will physically delete the directory instead of just removing it from the tree. The current invocation is also passing { itemUid, collectionUid } even though the thunk signature is (itemUid, collectionUid), so the UI-removal step already rejects today.
🤖 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/components/Sidebar/Collections/Collection/CollectionItem/index.js`
around lines 347 - 352, The Ignore Folder flow in CollectionItem should not
reuse the Delete action path, because dispatch(deleteItem(...)) triggers
renderer:delete-item and can physically remove the folder instead of only hiding
it. Update the post-update cleanup in CollectionItem to use the same UI-removal
path as an ignore action, and make sure the call matches the deleteItem thunk
signature only if you keep that helper; otherwise switch to a dedicated
non-destructive tree-removal action after updateBrunoConfig succeeds.
| export const getCollectionRelativePath = (collectionPathname, itemPathname) => { | ||
| if (!collectionPathname || !itemPathname) return ''; | ||
|
|
||
| const toPosix = (p) => p.replace(/\\/g, '/').replace(/\/+$/, ''); | ||
| const root = toPosix(collectionPathname); | ||
| const target = toPosix(itemPathname); | ||
|
|
||
| if (target === root) return ''; | ||
| if (target.startsWith(`${root}/`)) { | ||
| return target.slice(root.length + 1); | ||
| } | ||
|
|
||
| return target; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return '' when the path is not actually collection-relative.
Line 22 currently falls back to the normalized absolute target path. If itemPathname is outside the collection root, or only differs by case on a case-insensitive filesystem, this writes a machine-specific absolute path into brunoConfig.ignore even though the helper contract says unresolved paths should return ''. The config reader/writer carries ignore entries through as-is, so this would persist a non-portable value into bruno.json. As per path instructions, "Ensure that all code is OS-agnostic" and "Never assume case-sensitive or case-insensitive filesystems."
🤖 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/utils/collections/ignore.js` around lines 10 - 23, The
fallback in getCollectionRelativePath is returning a normalized absolute target
path when itemPathname is not actually under the collection root. Update the
function so it only returns a relative suffix when target equals root or starts
with root/, and otherwise returns an empty string. Keep the path normalization
in toPosix, but avoid persisting any non-collection-relative value into
brunoConfig.ignore.
Source: Path instructions
| describe('getCollectionRelativePath', () => { | ||
| it('returns the folder path relative to the collection root', () => { | ||
| expect(getCollectionRelativePath('/home/adi/my-collection', '/home/adi/my-collection/auth')).toBe('auth'); | ||
| }); | ||
|
|
||
| it('returns nested folder paths with forward slashes', () => { | ||
| expect(getCollectionRelativePath('/home/adi/my-collection', '/home/adi/my-collection/v1/users')).toBe('v1/users'); | ||
| }); | ||
|
|
||
| it('normalizes windows-style separators to forward slashes', () => { | ||
| expect(getCollectionRelativePath('C:\\Users\\adi\\coll', 'C:\\Users\\adi\\coll\\auth\\login')).toBe('auth/login'); | ||
| }); | ||
|
|
||
| it('tolerates a trailing slash on the collection path', () => { | ||
| expect(getCollectionRelativePath('/home/adi/coll/', '/home/adi/coll/auth')).toBe('auth'); | ||
| }); | ||
|
|
||
| it('returns an empty string when the item is the collection root', () => { | ||
| expect(getCollectionRelativePath('/home/adi/coll', '/home/adi/coll')).toBe(''); | ||
| }); | ||
|
|
||
| it('returns an empty string when inputs are missing', () => { | ||
| expect(getCollectionRelativePath('', '/home/adi/coll/auth')).toBe(''); | ||
| expect(getCollectionRelativePath('/home/adi/coll', '')).toBe(''); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a regression for non-descendant and case-mismatch inputs.
The helper is documented to return '' when a relative path cannot be derived, but this suite never exercises an item outside the collection root or a case-only root/item mismatch. That leaves the portability edge case unprotected. As per coding guidelines, "Cover both the 'happy path' and the realistically problematic paths." As per path instructions, "Never assume case-sensitive or case-insensitive filesystems."
🤖 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/utils/collections/ignore.spec.js` around lines 3 - 28,
The getCollectionRelativePath test suite is missing regression coverage for
paths that cannot be derived relative to the collection root. Add cases in
ignore.spec.js for a non-descendant item path and for a case-only mismatch
between the collection root and item path, asserting that
getCollectionRelativePath returns an empty string in both scenarios. Use the
existing getCollectionRelativePath helper to keep the new tests aligned with the
current path-normalization behavior.
Sources: Coding guidelines, Path instructions
1 participant
Description
Adds an Ignore Folder item to the folder context menu in the sidebar, closing #1890.
Today the only way to add a folder to a collection's
ignorelist is to hand-editbruno.json. This adds a right-click action that does it from the UI:brunoConfig.ignoreand persisted tobruno.jsonbruno.jsonkeeps it ignored on subsequent loadsThe path is stored with forward slashes so the value stays portable when
bruno.jsonis committed and shared across operating systems.How it works
getCollectionRelativePath(collectionPathname, itemPathname)derives the portable relative pathaddPathToIgnoreList(brunoConfig, relativePath)returns a new config with the path added (immutable, dedupes)updateBrunoConfigthunk to persist, thendeleteItemto drop the folder from the treeTests
Added
utils/collections/ignore.spec.jscovering relative-path derivation (nested paths, Windows separators, trailing slashes, root/empty inputs) and ignore-list merging (append, create, dedupe, empty paths, immutability). 11 assertions, all passing.Notes
updateBrunoConfig) and tree-removal (deleteItem) machinery; no watcher changes.Summary by CodeRabbit
New Features
Bug Fixes
Tests