feat(app): introduce standalone app functionality#8338
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughIntroduces a first-class ChangesCollection Apps Feature
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
note over User,Redux: Creating an App Item
User->>Sidebar: Click "New App" menu
Sidebar->>NewApp: Open modal
NewApp->>Redux: dispatch newApp({ appName, filename, collectionUid })
Redux->>Electron IPC: renderer:new-request (type: 'app')
Electron IPC-->>Redux: item created on disk
Redux->>Redux: enqueue OPEN_REQUEST task
end
rect rgba(60, 179, 113, 0.5)
note over CollectionApp,Guest: Host/Guest RPC in Preview
CollectionApp->>useAppWebview: mount with handleGuestMessage
useAppWebview->>webview: dom-ready → flush outbox via executeJavaScript
webview-->>Guest: __brunoReceive({ type: 'state', ... })
Guest->>webview: console.log(SENTINEL + JSON { type: 'runRequest' })
webview-->>useAppWebview: console-message event
useAppWebview->>CollectionApp: handleGuestMessage({ type: 'runRequest' })
CollectionApp->>Redux: dispatch sendNetworkRequest
Redux-->>CollectionApp: response
CollectionApp->>useAppWebview: pushToGuest({ type: 'reply', result })
useAppWebview->>webview: executeJavaScript(__brunoReceive(...))
webview-->>Guest: pending[replyId] resolves
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 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 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: 8
🧹 Nitpick comments (1)
packages/bruno-filestore/src/formats/yml/items/parseApp.ts (1)
14-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider validating
ocApp.infopresence before casting.Line 15 falls back to an empty object when
ocApp.infois missing, which means all nested fields will beundefined. While the code handles this with defaults (line 21 defaultsnameto'App', line 22 defaultstagsto[]), this silently accepts malformed input.If the AppFile contract requires
infoto be present, consider throwing an error early rather than silently recovering.🛡️ Proposed validation guard
const parseApp = (ocApp: AppFile): BrunoItem => { - const info = ocApp.info || ({} as AppFile['info']); + if (!ocApp.info) { + throw new Error('Invalid app item: missing info field'); + } + const info = ocApp.info;🤖 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-filestore/src/formats/yml/items/parseApp.ts` around lines 14 - 35, The parseApp function silently accepts a missing ocApp.info field by defaulting to an empty object, which can hide malformed input. Add a validation check at the beginning of the parseApp function to ensure that ocApp.info is present and defined; if it's missing and required by the AppFile contract, throw a descriptive error early before proceeding with the rest of the function logic and object creation.
🤖 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/AppView/EmptyAppState.js`:
- Line 29: The component inconsistently accesses theme properties by mixing two
different patterns: the Wrapper uses theme.colors.text.muted while the title
styling uses top-level theme.text. To resolve this inconsistency, update the
title color property in the styled component to access the text color through
the same theme.colors.text.* pattern used by the Wrapper, ensuring uniform theme
property access throughout the EmptyAppState component.
In `@packages/bruno-app/src/components/Sidebar/NewApp/index.js`:
- Around line 63-78: The form and input elements in the NewApp component lack
data-testid attributes required for stable Playwright E2E test selectors. Add a
data-testid attribute to the form element with className "bruno-form" (for
example, something like "newAppForm" or "newApp-modal") and add a data-testid
attribute to the input element with id "appName" (for example, "appName-input"
or "newApp-nameInput") to provide explicit, stable selectors for E2E testing.
- Around line 47-49: Replace the useEffect hook containing setTimeout with a
native autoFocus attribute on the input element. Remove the entire useEffect
block that calls inputRef.current?.focus() with a 50ms delay, and instead add
the autoFocus attribute directly to the input element that inputRef was
referencing. This eliminates unnecessary useEffect usage per the coding
guidelines. Note that the same pattern appears in another location (lines 67-72)
and should be fixed there as well.
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`:
- Around line 1820-1823: The code at line 1820-1822 retrieves a parent item
using findItemInCollection based on itemUid, but does not validate that the
resolved parent is a folder-type item before accessing its items property. If
itemUid resolves to a non-folder item (like a file), then parent.items will be
undefined and parent.pathname will point to a file rather than a directory,
causing path construction failures at line 1838-1842 when creating child items.
After finding the parent item, validate that it is a folder type or directory;
if itemUid resolves to a non-folder item, traverse up to its containing folder
instead of using the non-folder item directly for computing parentPath and
siblings.
In `@packages/bruno-filestore/src/formats/bru/index.ts`:
- Around line 139-153: In the code block that handles standalone app items where
the type === 'app' check is made, the bruJson object is missing the settings
property which causes app settings to be lost during save/reload cycles. Add a
settings property to the bruJson object by extracting it from the incoming json
using _.get(json, 'settings') and include it alongside the meta and app
properties in the bruJson structure that gets passed to jsonToBruV2.
In `@packages/bruno-filestore/src/formats/yml/parseItem.ts`:
- Around line 91-93: Replace the unsafe `as any` type cast on ocItem in the
'app' case with a proper type annotation that matches the shape expected by
parseApp, similar to how other cases in the switch statement use specific type
casts like `as HttpRequest` or `as GraphQLRequest`. Either export an AppFile
type from parseApp.ts and import it here to use as the cast, or define a type
that represents the expected shape of the YML object for the app case and cast
ocItem to that type instead of using any.
In `@tests/apps/collection-apps.spec.ts`:
- Around line 15-26: The guestEval function currently selects the first webview
matching the data:text/html pattern globally, which causes non-determinism when
multiple preview webviews exist across different CollectionApp instances. Modify
the guestEval function to anchor the guest webview resolution to a specific
CollectionApp instance rather than searching globally. The webview selection
logic in the find method needs to be updated to filter based on the parent
window or app context so that it consistently targets the correct webview
belonging to the intended CollectionApp instance, making the test deterministic
and parallel-safe.
In `@tests/utils/page/actions.ts`:
- Around line 2046-2057: The folder branch in the conditional block uses global
name-only locators (locators.sidebar.folder(parent.folderName)) which can match
duplicate folder names across different collections, causing test flakiness in
parallel execution. Refactor the if block that handles parent.folderName to
scope the folder locators to the parent collection, similar to how the else
block scopes collection actions using parent.collectionName. This ensures the
folder hover and action click target the correct folder within the specified
collection context rather than relying on global folder name matching.
---
Nitpick comments:
In `@packages/bruno-filestore/src/formats/yml/items/parseApp.ts`:
- Around line 14-35: The parseApp function silently accepts a missing ocApp.info
field by defaulting to an empty object, which can hide malformed input. Add a
validation check at the beginning of the parseApp function to ensure that
ocApp.info is present and defined; if it's missing and required by the AppFile
contract, throw a descriptive error early before proceeding with the rest of the
function logic and object creation.
🪄 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: 51c5ddfc-10d8-4e7e-b660-6b4f27e16ef1
📒 Files selected for processing (25)
packages/bruno-app/src/components/AppView/EmptyAppState.jspackages/bruno-app/src/components/AppView/index.jspackages/bruno-app/src/components/AppView/webview-bridge.jspackages/bruno-app/src/components/CollectionApp/StyledWrapper.jspackages/bruno-app/src/components/CollectionApp/index.jspackages/bruno-app/src/components/RequestTabPanel/index.jspackages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/CollectionItemIcon/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/index.jspackages/bruno-app/src/components/Sidebar/NewApp/index.jspackages/bruno-app/src/providers/ReduxStore/middlewares/draft/middleware.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-filestore/src/formats/bru/index.tspackages/bruno-filestore/src/formats/yml/items/parseApp.tspackages/bruno-filestore/src/formats/yml/items/stringifyApp.tspackages/bruno-filestore/src/formats/yml/parseItem.tspackages/bruno-filestore/src/formats/yml/stringifyItem.tspackages/bruno-schema-types/src/collection/item.tspackages/bruno-schema/src/collections/index.jstests/apps/collection-apps.spec.tstests/utils/page/actions.ts
utkarsh-bruno
left a comment
There was a problem hiding this comment.
Looks good. Since the app at collection level gets listed under collection, should the search bar on the side also return the apps ?
If yes, then doesCollectionHaveItemsMatchingSearchText() method might need some change.
Rest looks fine to me.
Description
JIRA
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