-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: autosave for environment tabs and folder-level auth #6510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: autosave for environment tabs and folder-level auth #6510
Conversation
WalkthroughCentralized autosave routing via a new determineSaveHandler, expanded intercepted actions to include environment/folder/collection drafts, and updated scheduling to use handler results. Folder auth components now receive a wrapped updateFolderAuth that injects the folder UID into auth updates. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-17T21:41:24.730ZApplied to files:
📚 Learning: 2026-01-09T18:25:14.640ZApplied to files:
🧬 Code graph analysis (1)packages/bruno-app/src/components/FolderSettings/Auth/index.js (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (3)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/providers/ReduxStore/middlewares/autosave/middleware.js (1)
139-141: Add defensive check for undefined payload.If an action in
actionsToIntercepthas an undefined payload, the destructuring at line 140 will throw a TypeError.🔎 Suggested fix
const determineSaveHandler = (actionType, payload, dispatch, getState) => { + if (!payload) return null; const { itemUid, folderUid, collectionUid, environmentUid } = payload;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/providers/ReduxStore/middlewares/autosave/middleware.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/providers/ReduxStore/middlewares/autosave/middleware.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/providers/ReduxStore/middlewares/autosave/middleware.js
🧬 Code graph analysis (1)
packages/bruno-app/src/providers/ReduxStore/middlewares/autosave/middleware.js (2)
packages/bruno-app/src/providers/ReduxStore/middlewares/draft/middleware.js (1)
actionsToIntercept(3-82)packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js (33)
action(147-147)action(174-174)action(207-207)action(213-213)action(221-221)action(230-230)action(251-251)action(263-263)action(329-329)action(378-378)action(386-386)action(444-444)action(481-481)action(625-625)action(698-698)action(939-939)collectionUid(314-314)collection(113-113)collection(148-148)collection(154-154)collection(162-162)collection(168-168)collection(175-175)collection(182-182)collection(214-214)collection(223-223)collection(232-232)collection(244-244)collection(252-252)collection(264-264)collection(279-279)collection(296-296)collection(303-303)
🔇 Additional comments (5)
packages/bruno-app/src/providers/ReduxStore/middlewares/autosave/middleware.js (5)
1-2: LGTM!Imports are correctly added for the new environment save actions.
91-94: Good use of Set for O(1) lookup.The
includes('Folder')heuristic works for the current action set. Be mindful that future actions containing "Folder" in their name will automatically be categorized as folder actions.
143-170: LGTM!Environment draft handling correctly re-reads state at execution time and validates the draft before dispatching. This ensures the latest changes are saved even if multiple updates occur before the timer fires.
172-204: LGTM!The handler priority logic correctly addresses the PR objective: folder actions using
itemUid(likeupdateFolderAuth) are now properly recognized via thefolderActions.has()check, ensuring they save as folder drafts rather than request drafts.
231-234: LGTM!Clean integration with the new
determineSaveHandlerhelper. The null check before scheduling is appropriate.
|
Everything else looks good just one thing if a user enables autosave while having pending environment changes, those drafts won't be picked up and saved. |
aec930c to
d6dcae3
Compare
d6dcae3 to
75f4e57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-app/src/components/FolderSettings/Auth/index.js (2)
93-99: Consider memoizingwrappedUpdateAuthwithuseCallback.This function is recreated on every render. If any child components use it in their dependency arrays or memoization, it could trigger unnecessary re-renders.
♻️ Suggested refactor
+import { useCallback } from 'react'; +// ... existing imports - const wrappedUpdateAuth = (payload) => { - const { itemUid, ...rest } = payload; - return updateFolderAuth({ - ...rest, - folderUid: folder.uid - }); - }; + const wrappedUpdateAuth = useCallback((payload) => { + const { itemUid, ...rest } = payload; + return updateFolderAuth({ + ...rest, + folderUid: folder.uid + }); + }, [folder.uid]);
104-112: Minor: Simplify save prop.The arrow function wrapper
save={() => handleSave()}is redundant sincehandleSavealready takes no arguments. This pattern is repeated across all auth views in this file.♻️ Suggested simplification
- save={() => handleSave()} + save={handleSave}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/components/FolderSettings/Auth/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/FolderSettings/Auth/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-02T07:24:50.311Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-app/src/components/FolderSettings/Auth/index.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/FolderSettings/Auth/index.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/components/FolderSettings/Auth/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/FolderSettings/Auth/index.js (2)
packages/bruno-app/src/components/CollectionSettings/Auth/OAuth2/index.js (4)
GrantTypeComponentMap(13-40)save(16-18)request(20-20)request(43-43)packages/bruno-app/src/components/CollectionSettings/Auth/index.js (2)
getAuthView(23-50)authMode(18-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (2)
packages/bruno-app/src/components/FolderSettings/Auth/index.js (2)
23-46: LGTM on the GrantTypeComponentMap update.The signature change to accept
updateAuthas a prop aligns well with the pattern used inCollectionSettings/Auth/OAuth2/index.js. Passing the auth updater as a prop allows the parent component to control how folder UID is injected.
180-191: LGTM on OAuth2 section wiring.The
GrantTypeSelectorandGrantTypeComponentMapboth receive the samewrappedUpdateAuth, ensuring consistent folder UID injection across all OAuth2 grant type flows. This aligns with the autosave middleware changes mentioned in the PR objectives.
…thentication components
BRU-2400
Issue:
Autosave wasn't working for environment tabs and folder-level auth.
Fix:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.