feat(openapi-sync): preserve user-configured request values on sync#8204
Open
sundram-bruno wants to merge 1 commit into
Open
feat(openapi-sync): preserve user-configured request values on sync#8204sundram-bruno wants to merge 1 commit into
sundram-bruno wants to merge 1 commit into
Conversation
sundram-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
Contributor
|
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 (6)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughAdds a preserveValues toggle in the SyncReviewPage UI and threads that choice into ExpandableEndpointRow, useSyncFlow, and Electron IPC handlers; implements preserve-aware masking and merge helpers in main-process code, exports helpers for tests, and adds extensive Jest coverage. ExpandableEndpointRow also suppresses stale diff responses with a requestIdRef. ChangesOpenAPI Preserve Values Synchronization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js (1)
243-251: ⚡ Quick winAdd
data-testidfor Playwright testing.The toggle button is a key user interaction point but lacks a test identifier.
🧪 Suggested fix
<button className={`bulk-btn ${preserveValues ? 'active' : ''}`} onClick={() => setPreserveValues((v) => !v)} aria-pressed={preserveValues} title="When on, your edited values (body, params, headers, auth, {{vars}}) are kept; only fields the spec adds or removes change. When off, spec values overwrite yours." + data-testid="preserve-values-toggle" >As per coding guidelines: "Add
data-testidto testable elements for Playwright".🤖 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/OpenAPISyncTab/SyncReviewPage/index.js` around lines 243 - 251, The toggle button in SyncReviewPage lacks a Playwright test id; add a data-testid attribute to the button element that contains the preserveValues state and setPreserveValues handler (the button rendering the IconCheck/IconInfoCircle) — e.g., data-testid="preserve-values-toggle" — so tests can reliably select it; ensure the attribute is added on the same button that uses preserveValues, setPreserveValues, IconCheck and IconInfoCircle.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.
Nitpick comments:
In `@packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js`:
- Around line 243-251: The toggle button in SyncReviewPage lacks a Playwright
test id; add a data-testid attribute to the button element that contains the
preserveValues state and setPreserveValues handler (the button rendering the
IconCheck/IconInfoCircle) — e.g., data-testid="preserve-values-toggle" — so
tests can reliably select it; ensure the attribute is added on the same button
that uses preserveValues, setPreserveValues, IconCheck and IconInfoCircle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ddce66d-4c18-4b24-8452-a9b0a7f89b0f
📒 Files selected for processing (5)
packages/bruno-app/src/components/OpenAPISyncTab/EndpointChangeSection/ExpandableEndpointRow.jspackages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.jspackages/bruno-app/src/components/OpenAPISyncTab/hooks/useSyncFlow.jspackages/bruno-electron/src/ipc/openapi-sync.jspackages/bruno-electron/tests/ipc/openapi-sync-merge.spec.js
sundram-bruno
force-pushed
the
feature/openapi-sync-preserve-values
branch
2 times, most recently
from
f3a2f51 to
1caa8b3
Compare
Reconcile request structure against the spec on sync while preserving the
user's values (JSON body, params, headers, auth) and {{var}} references for
fields that still exist. A "Preserve values" toggle (default on) on the Spec
Updates review controls it; turning it off lets spec values overwrite. The diff
preview's EXPECTED column shows the post-merge result so unchanged values do not
render as changes.
- field-level merge for JSON body (by key path), form fields and params/headers
(by name, duplicate names paired positionally), preserving value + enabled
- {{var}} masking so interpolated bodies parse, merge and restore safely,
using a private-use sentinel that never collides with body text
- auth merged by mode: same mode keeps the user's values and adds spec-introduced
fields; a mode change takes the spec. compareRequestFields compares auth by
mode only, so preserved auth values no longer mark the collection out of sync
- preserveValues threaded through apply and diff-preview IPC handlers
- reset path left unchanged; scripts/tests/assertions preserved in sync and reset
- 67 unit tests covering the merge helpers and masking edge cases
sundram-bruno
force-pushed
the
feature/openapi-sync-preserve-values
branch
from
1caa8b3 to
cce8319
Compare
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
1 participant
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
Discussion: #7401
On the Spec Updates sync, Bruno currently overwrites a request's body, params, headers and auth wholesale, destroying user-entered values and
{{envVar}}references whenever the spec changes shape. This PR makes sync reconcile the request structure against the spec while preserving the user's values for fields that still exist.JIRA : https://usebruno.atlassian.net/browse/BRU-3205
A "Preserve values" toggle (default ON) on the Spec Updates review toolbar controls this. With it on, your edits are kept and only the spec's structural additions/removals are applied; with it off, spec values overwrite (the seam for an explicit "take the spec's version"). The diff preview's EXPECTED column shows the post-merge result, so unchanged values no longer render as changes.
What changed
Backend —
packages/bruno-electron/src/ipc/openapi-sync.jsvalue+enabled.{{var}}masking so an interpolated body can be parsed, merged and restored safely — using a private-use sentinel that never collides with body text and never reaches disk.compareRequestFieldsnow compares auth by mode only, so preserved auth values no longer keep the collection permanently "out of sync".preserveValues(defaulttrue) threaded throughrenderer:apply-openapi-syncandrenderer:get-endpoint-diff-data. Reset path left unchanged; scripts/tests/assertions preserved in both sync and reset.Frontend —
OpenAPISyncTabNote for reviewers — one intended behavior change:
compareRequestFieldsno longer flags auth config differences (e.g. apikey placement, oauth2 url/scope) when the auth mode is unchanged; only a mode change is drift. This is required so preserved auth values don't perpetually re-flag the endpoint.Tests: 67 unit tests in
packages/bruno-electron/tests/ipc/openapi-sync-merge.spec.jscovering the merge helpers and{{var}}masking edge cases (string boundaries, object keys, nested/array bodies, many vars, dual-side). Fullbruno-electronandbruno-appsuites pass.Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge.
Summary by CodeRabbit