feat(collections): open multiple collections at once#8583
Open
gopu-bruno wants to merge 8 commits into
Open
Conversation
gopu-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
Contributor
|
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:
WalkthroughOpen Collection now uses renderer-driven directory browsing, Redux-controlled opening state, and a collection selection modal. Electron forwards menu and renderer requests, while tests cover direct opening, nested collections, filtering, selection, cancellation, and invalid paths. ChangesOpen Collection Flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ElectronIPC
participant ReduxAppState
participant OpenCollection
participant FilesystemIPC
User->>ElectronIPC: Choose Open Collection
ElectronIPC->>ReduxAppState: send main:open-collection
ReduxAppState->>OpenCollection: setIsOpeningCollection(true)
OpenCollection->>FilesystemIPC: invoke renderer:browse-directories
FilesystemIPC-->>OpenCollection: return selected directories
OpenCollection->>OpenCollection: scan and select collections
OpenCollection->>ReduxAppState: dispatch openMultipleCollections
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
✨ 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 |
gopu-bruno
force-pushed
the
feat/open-multiple-collections
branch
from
a3d0502 to
871c259
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/collection/open/open-collection-selection.spec.ts (2)
319-328: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a stable selector for the "Open" link.
getByText('Open', { exact: true })could match unintended elements. Add adata-testidto the "Open" link inCreateOrOpenCollectionand usegetByTestIdhere.As per path instructions, replace brittle text/index selectors with role, label, test id, or stable user-facing selectors.
🔧 Proposed fix
In
packages/bruno-app/src/components/Sidebar/Collections/CreateOrOpenCollection/index.js:const OpenLink = () => ( <LinkStyle className="underline text-link cursor-pointer" theme={theme} + data-testid="open-collection-link" onClick={() => handleOpenCollection(true)} >In the test:
- await page.getByText('Open', { exact: true }).click(); + await page.getByTestId('open-collection-link').click();🤖 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 `@tests/collection/open/open-collection-selection.spec.ts` around lines 319 - 328, Replace the brittle exact-text lookup in the test with a dedicated test ID, and add that data-testid to the “Open” link rendered by CreateOrOpenCollection. Use the new stable selector via getByTestId while preserving the existing click and picker assertions.Source: Path instructions
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize raw selectors in page modules. Three raw CSS selectors are inlined in the spec instead of being centralized in page modules under
tests/utils/page/*.
tests/collection/open/open-collection-selection.spec.ts#L51-L51: MovelistTitlesselector ([data-testid="selection-list"] .selection-item-title) to a page module method.tests/collection/open/open-collection-selection.spec.ts#L172-L173: Move the[data-testid="selection-list"] liselector to a page module method (e.g.,selectionList.item()).tests/collection/open/open-collection-selection.spec.ts#L334-L334: Replace.titlebar-left .home-buttonwith agetByTestIdorgetByRolelocator centralized inbuildCommonLocators.As per path instructions, never inline raw selectors in a spec; centralize locators and actions in page modules under
tests/utils/page/*.🤖 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 `@tests/collection/open/open-collection-selection.spec.ts` at line 51, The spec contains three raw selectors that must be centralized: in tests/collection/open/open-collection-selection.spec.ts lines 51-51, add a page-module method for listTitles; at lines 172-173, add or reuse selectionList.item() for the selection-list li locator; and at lines 334-334, replace .titlebar-left .home-button with a getByTestId or getByRole locator in buildCommonLocators. Update the spec to use these page-module locators and leave no raw CSS selectors inline.Source: Path instructions
🤖 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/OpenCollection/index.js`:
- Around line 80-83: Update the openMultipleCollections flow in OpenCollection
to await its resolved { opened, failed, invalid } result and report any failed
or invalid entries instead of handling only IPC rejection. Close the modal only
when at least one collection opens; keep it open when none open, while
preserving error handling for rejected dispatches.
- Around line 40-84: Update the scan-result handling around failedScans and
noNestedCollections so failed or empty selected folders are not silently
discarded. Show a warning when any selected folder fails to scan or produces no
collection, and require every picked folder to have a successful result
resolving to that exact folder before directly dispatching
openMultipleCollections; otherwise preserve the selection flow.
In `@tests/collection/open/open-collection-selection.spec.ts`:
- Around line 211-221: Update the test “cancelling the picker shows no modal and
no error toast” to remove the negative toast assertions for “An error occurred
while scanning for collections” and “No Bruno collections found”. Keep the
positive non-toast assertion that openCollectionModal(page) has count zero, and
retain the cancellation setup and openViaSidebar flow.
---
Nitpick comments:
In `@tests/collection/open/open-collection-selection.spec.ts`:
- Around line 319-328: Replace the brittle exact-text lookup in the test with a
dedicated test ID, and add that data-testid to the “Open” link rendered by
CreateOrOpenCollection. Use the new stable selector via getByTestId while
preserving the existing click and picker assertions.
- Line 51: The spec contains three raw selectors that must be centralized: in
tests/collection/open/open-collection-selection.spec.ts lines 51-51, add a
page-module method for listTitles; at lines 172-173, add or reuse
selectionList.item() for the selection-list li locator; and at lines 334-334,
replace .titlebar-left .home-button with a getByTestId or getByRole locator in
buildCommonLocators. Update the spec to use these page-module locators and leave
no raw CSS selectors inline.
🪄 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: 97f1c3c4-0916-4222-afa7-c2e18b7f3573
📒 Files selected for processing (19)
packages/bruno-app/src/components/AppTitleBar/AppMenu/index.jspackages/bruno-app/src/components/Sidebar/Collections/CreateOrOpenCollection/index.jspackages/bruno-app/src/components/Sidebar/OpenCollection/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/OpenCollection/index.jspackages/bruno-app/src/components/Sidebar/Sections/CollectionsSection/index.jspackages/bruno-app/src/components/WorkspaceHome/WorkspaceOverview/index.jspackages/bruno-app/src/pages/Bruno/index.jspackages/bruno-app/src/providers/App/useIpcEvents.jspackages/bruno-app/src/providers/ReduxStore/slices/app.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.jspackages/bruno-electron/src/app/collections.jspackages/bruno-electron/src/app/menu-template.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/filesystem.jspackages/bruno-electron/src/utils/filesystem.jspackages/bruno-electron/src/utils/tests/filesystem/browse-directories.spec.jstests/collection/open/open-collection-selection.spec.tstests/collection/open/open-multiple-collections.spec.ts
Contributor
|
@gopu-bruno Lets also add tests to scan for deeply nested collections, like collection nested under folders. |
gopu-bruno
force-pushed
the
feat/open-multiple-collections
branch
from
a5c8e35 to
e9f8ea5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/collection/open/open-collection-selection.spec.ts (1)
198-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider wrapping test phases in
test.stepfor clearer reports.The path instructions promote
test.stepusage for readable reports. This test has distinct arrange/act/assert phases that would benefit from step boundaries. Applies to all tests in this file.As per path instructions, promote the use of `test.step` as much as possible so the generated reports are easier to read.♻️ Example test.step wrapping for the deeply nested test
test('scans for deeply nested collections', async ({ page, electronApp, createTmpDir }) => { const locators = buildCommonLocators(page); - const base = await createTmpDir('nested-collection'); - writeCollection(path.join(base, 'api', 'v2', 'users'), 'Users API'); - - await mockPickerPaths(electronApp, [base]); - await openViaSidebar(page); - - await expect(openCollectionModal(page)).toBeVisible(); - await expect(listTitles(page)).toHaveText('Users API'); - await expect(listDescriptions(page)).toContainText('api/v2/users'); - - await toggleCollection(page, 'Users API'); - await locators.modal.button('Open').click(); - - await expect(locators.sidebar.collection('Users API')).toBeVisible(); + await test.step('arrange nested collection fixture', async () => { + const base = await createTmpDir('nested-collection'); + writeCollection(path.join(base, 'api', 'v2', 'users'), 'Users API'); + await mockPickerPaths(electronApp, [base]); + }); + + await test.step('open via sidebar and verify modal', async () => { + await openViaSidebar(page); + await expect(openCollectionModal(page)).toBeVisible(); + await expect(listTitles(page)).toHaveText('Users API'); + await expect(listDescriptions(page)).toContainText('api/v2/users'); + }); + + await test.step('toggle and open collection', async () => { + await toggleCollection(page, 'Users API'); + await locators.modal.button('Open').click(); + await expect(locators.sidebar.collection('Users API')).toBeVisible(); + }); await closeAllCollections(page); });🤖 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 `@tests/collection/open/open-collection-selection.spec.ts` around lines 198 - 215, Wrap the distinct arrange, act, and assert phases of the deeply nested collection test around test('scans for deeply nested collections', ...) in descriptive test.step blocks, covering setup and path mocking, opening and selecting the collection, and verifying the sidebar result and cleanup. Apply the same step-boundary pattern consistently to the other tests in this file.Source: Path instructions
🤖 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/OpenCollection/index.js`:
- Around line 137-147: Update handleConfirm so onClose is called only when at
least one collection opens successfully: preserve the selected collections and
keep the modal open when result.opened is empty or dispatch rejects, while
retaining existing notifications and error handling.
---
Nitpick comments:
In `@tests/collection/open/open-collection-selection.spec.ts`:
- Around line 198-215: Wrap the distinct arrange, act, and assert phases of the
deeply nested collection test around test('scans for deeply nested collections',
...) in descriptive test.step blocks, covering setup and path mocking, opening
and selecting the collection, and verifying the sidebar result and cleanup.
Apply the same step-boundary pattern consistently to the other tests in this
file.
🪄 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: c10f69ef-3212-4606-9892-daece163b4d5
📒 Files selected for processing (5)
packages/bruno-app/src/components/Sidebar/OpenCollection/index.jspackages/bruno-app/src/providers/App/useIpcEvents.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-electron/src/app/collections.jstests/collection/open/open-collection-selection.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-electron/src/app/collections.js
- packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/collection/open/open-collection-selection.spec.ts (1)
51-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMove raw selectors into a page module.
listTitles,listDescriptions, andtoggleCollectioninline[data-testid="selection-list"] ...locators directly in the spec. The same applies to inlinepage.getByTestId('selection-count'/'selection-select-all-toggle'/'selection-search-input')and the.titlebar-left .home-buttonCSS selector further down. Centralize these in atests/utils/page/*module (e.g. extendbuildCommonLocators) so specs only consume named actions/locators.As per coding guidelines: "Centralise locators and actions in page modules under
tests/utils/page/*— never inline raw selectors in a spec." As per path instructions: "Centralize selectors/actions intests/utils/page/*page modules; don't inline rawpage.locator(...)/page.getByTestId(...)in specs."🤖 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 `@tests/collection/open/open-collection-selection.spec.ts` around lines 51 - 60, Move the raw selection-list selectors and related actions out of the spec into the appropriate page module under tests/utils/page, extending buildCommonLocators or an equivalent module. Centralize listTitles, listDescriptions, toggleCollection, the selection-count/select-all/search locators, and the home-button selector there, then update the spec to consume only the named page-module locators and actions.Sources: Coding guidelines, Path instructions
🤖 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 `@tests/collection/open/open-collection-selection.spec.ts`:
- Around line 51-60: Move the raw selection-list selectors and related actions
out of the spec into the appropriate page module under tests/utils/page,
extending buildCommonLocators or an equivalent module. Centralize listTitles,
listDescriptions, toggleCollection, the selection-count/select-all/search
locators, and the home-button selector there, then update the spec to consume
only the named page-module locators and actions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1915607e-0681-4491-80fb-03894643c723
📒 Files selected for processing (19)
packages/bruno-app/src/components/AppTitleBar/AppMenu/index.jspackages/bruno-app/src/components/Sidebar/Collections/CreateOrOpenCollection/index.jspackages/bruno-app/src/components/Sidebar/OpenCollection/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/OpenCollection/index.jspackages/bruno-app/src/components/Sidebar/Sections/CollectionsSection/index.jspackages/bruno-app/src/components/WorkspaceHome/WorkspaceOverview/index.jspackages/bruno-app/src/pages/Bruno/index.jspackages/bruno-app/src/providers/App/useIpcEvents.jspackages/bruno-app/src/providers/ReduxStore/slices/app.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.jspackages/bruno-electron/src/app/collections.jspackages/bruno-electron/src/app/menu-template.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/filesystem.jspackages/bruno-electron/src/utils/filesystem.jspackages/bruno-electron/src/utils/tests/filesystem/browse-directories.spec.jstests/collection/open/open-collection-selection.spec.tstests/collection/open/open-multiple-collections.spec.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- tests/collection/open/open-multiple-collections.spec.ts
- packages/bruno-app/src/components/AppTitleBar/AppMenu/index.js
- packages/bruno-electron/src/app/menu-template.js
- packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js
- packages/bruno-electron/src/utils/filesystem.js
- packages/bruno-app/src/providers/ReduxStore/slices/app.js
- packages/bruno-electron/src/ipc/filesystem.js
- packages/bruno-app/src/components/WorkspaceHome/WorkspaceOverview/index.js
- packages/bruno-app/src/components/Sidebar/Collections/CreateOrOpenCollection/index.js
- packages/bruno-app/src/providers/App/useIpcEvents.js
- packages/bruno-electron/src/ipc/collection.js
- packages/bruno-electron/src/app/collections.js
- packages/bruno-app/src/components/Sidebar/Sections/CollectionsSection/index.js
- packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds support for discovering nested Bruno collections when opening a folder. If multiple collections are found, Bruno presents them in a selection modal so users can choose which collections to open. The workflow mirrors the existing Git clone experience.
Ref: BRU-141
Changes
This gives users visibility and control when opening folders that contain multiple collections or nested collections.
Screen Recording
Screen.Recording.2026-07-13.at.6.14.26.PM.mov
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
Summary
New Features
Bug Fixes
Tests