fix: openapi body import #6288
fix: openapi body import #6288
Conversation
pooja-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughThe OpenAPI-to-Bruno converter now treats schemas that declare Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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: 0
🧹 Nitpick comments (2)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
1-66: Test coverage validates the JSON case well.The test confirms that schemas with
propertiesbut no explicittype: objectare correctly imported. However, the implementation changes also affectformUrlEncodedandmultipartFormcontent types (lines 501 and 514 in openapi-to-bruno.js).Consider adding test cases for these content types to ensure comprehensive coverage of the fix.
Example additional test cases:
it('should import body fields for form-urlencoded when schema has properties without explicit type: object', () => { const openApiSpec = ` openapi: "3.0.0" info: version: "1.0.0" title: "FormUrlEncoded Test" servers: - url: "https://api.example.com" paths: /submit: post: operationId: "submitForm" requestBody: content: application/x-www-form-urlencoded: schema: properties: name: type: string email: type: string responses: '200': description: "OK" `; const result = openApiToBruno(openApiSpec); expect(result.items[0].request.body.mode).toBe('formUrlEncoded'); expect(result.items[0].request.body.formUrlEncoded.length).toBe(2); });packages/bruno-converters/src/openapi/openapi-to-bruno.js (1)
163-165: Minor inconsistency in object-like schema detection.The
getExampleFromSchemafunction uses(schema.properties && !schema.type)while the new changes at lines 492, 501, 514 use justbodySchema.properties. The new approach is more permissive.Consider aligning the conditions for consistency:
- if (schema.type === 'object' || (schema.properties && !schema.type)) { + if (schema.type === 'object' || schema.properties) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-converters/src/openapi/openapi-to-bruno.js(3 hunks)packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.jspackages/bruno-converters/src/openapi/openapi-to-bruno.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
🧬 Code graph analysis (1)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (2)
openApiToBruno(1177-1195)openApiToBruno(1177-1195)
⏰ 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: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (3)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (3)
492-498: Logic correctly handles schemas with implicit object type.The condition now properly detects object-like schemas that define
propertieswithout an explicittype: object. The subsequent array check on line 496 ensures no conflict with array-typed bodies.
499-511: Consistent extension for formUrlEncoded handling.The change aligns with the JSON body handling pattern. The fallback to empty object in
each(bodySchema.properties || {}, ...)ensures safe iteration.
512-525: Consistent extension for multipartForm handling.Mirrors the formUrlEncoded pattern correctly.
tthornton3-chwy
left a comment
tthornton3-chwy
left a comment
There was a problem hiding this comment.
My approval means nothing haha but I just got hit by this, and am excited to see it in! Is there a mechanism set up here to spit out a "dev" package of this up on npmjs or packaged as a zip, etc? I can build it myself and push it to our artifactory for the time being, but that'd be awesome if the former was an option!
|
@pooja-bruno You can ignore that comment, I built it myself while we wait for this to come in, and keeping it packaged in the devtools repo we got :) |
|
Hey @tthornton3-chwy, Can you re-open the PR, I'm not used to PRs being raised on my forks. Apologies if you think I stole your credit, I'd stay away from raising forks on the internal dev forks since the commits are squashed and you might loose your credit if we aren't careful while merging. For this specific case, I'll be cherry picking your commit into the repo to make sure it goes in with your name. |
This reverts commit 3219e8a.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (2)
492-495: Consider alignment withgetExampleFromSchemadefensive check.Line 163 uses
(schema.properties && !schema.type)to explicitly verify thattypeis not set before treating a schema as object-like. Your change at line 492 usesbodySchema.propertieswithout the!schema.typeguard. This means if a malformed schema has bothtype: 'string'andproperties: {...}, line 163 would not treat it as an object, but line 492 would.For well-formed OpenAPI specs this shouldn't matter, but consider whether you want to mirror the defensive pattern from line 163 for consistency.
Apply this diff if you want to align with line 163's pattern:
- if (bodySchema && (bodySchema.type === 'object' || bodySchema.properties)) { + if (bodySchema && (bodySchema.type === 'object' || (bodySchema.properties && !bodySchema.type))) {
492-495: Consider extracting repeated object-like detection logic.The condition
(bodySchema.type === 'object' || bodySchema.properties)is duplicated across lines 492, 501, and 514. Extracting this to a helper function likeisObjectLikeSchema(schema)would improve maintainability and make future updates easier.Example helper:
const isObjectLikeSchema = (schema) => { return schema && (schema.type === 'object' || schema.properties); };Then use it at lines 492, 501, 514:
- if (bodySchema && (bodySchema.type === 'object' || bodySchema.properties)) { + if (isObjectLikeSchema(bodySchema)) {Also applies to: 501-511, 514-525
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
1-158: Expand test coverage for edge cases.The current tests cover basic happy paths well, but consider adding tests for:
- Nested objects: Schemas with properties containing object-type properties
- Array fields: Properties of type array with object items
- Empty properties: Schema with
properties: {}to ensure no errors- Mixed explicit type: Schema with both
type: 'object'andpropertiesto verify existing behavior isn't broken- Number type fields: While integer is tested implicitly, explicit tests with
type: 'number'fields would validate numeric handlingBased on learnings, comprehensive test coverage for new functionality helps catch regressions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-converters/src/openapi/openapi-to-bruno.js(3 hunks)packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.jspackages/bruno-converters/src/openapi/openapi-to-bruno.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
🧬 Code graph analysis (1)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (2)
openApiToBruno(1177-1195)openApiToBruno(1177-1195)
⏰ 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: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
🔇 Additional comments (1)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
5-66: Test implementation looks solid.All three test cases properly validate the fix for $ref resolution with inline schemas. They:
- Use realistic OpenAPI specs with $ref pointing to components/requestBodies
- Verify correct body mode assignment
- Validate that properties are properly extracted and populated
Good work covering the JSON, formUrlEncoded, and multipartForm scenarios.
Also applies to: 68-111, 113-157
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
4-85: Solid coverage for JSON requestBody$refwith inlineproperties; consider asserting nested numeric fieldsThis test nicely nails the core regression:
requestBodyresolving via$reftocomponents.requestBodieswhere the schema only definesproperties, plus the nestedfeesarray whose items are a$ref. That’s exactly aligned with the PR objective and the testing guideline of covering new behavior. Based on learnings, this is the right place for the regression test.If you want slightly stronger coverage with minimal noise, you could also assert that the
feesobject includes all defined properties (e.g.amountas anumber), e.g.:- expect(bodyJson['fees'][0]).toHaveProperty('id'); + expect(bodyJson['fees'][0]).toHaveProperty('id'); + expect(bodyJson['fees'][0]).toHaveProperty('amount');This would guard against regressions where only a subset of properties from the referenced schema are picked up.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-converters/src/openapi/openapi-to-bruno.js(4 hunks)packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-converters/src/openapi/openapi-to-bruno.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js
🧬 Code graph analysis (1)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (2)
openApiToBruno(1177-1195)openApiToBruno(1177-1195)
⏰ 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: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (3)
packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js (3)
87-130: formUrlEncoded$refcoverage looks correct and focusedThe test clearly verifies that a
requestBody$refwith an inline schema (no explicittype: object) is imported asformUrlEncodedwith the expected field names. Assertions are tight and aligned with the converter behavior.
132-176: multipart/form-data$refhandling is well coveredGood targeted check that a
$ref-basedrequestBodywith inlinepropertiesformultipart/form-dataends up inmultipartFormmode with the expected fields (file,description). This complements the JSON and formUrlEncoded cases nicely.
1-2: ConfirmopenApiToBrunoexport style matches this default importIf
openApiToBrunois only a named export (export const openApiToBruno = ...), this default import will beundefinedat runtime. Either ensure there is anexport default openApiToBrunoinsrc/openapi/openapi-to-bruno.js, or switch this line to a named import:-import openApiToBruno from '../../../src/openapi/openapi-to-bruno'; +import { openApiToBruno } from '../../../src/openapi/openapi-to-bruno';
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (1)
926-935: Minor bug: emptysecuritySchemescheck uses array equality instead of length
if (Object.keys(securitySchemes) === 0)will never be true because it compares an array to0. This means the early-return path for “no security schemes” is effectively dead code, even whencomponents.securitySchemesis an empty object.Consider changing it to:
- if (Object.keys(securitySchemes) === 0) { + if (Object.keys(securitySchemes).length === 0) {Functionally this only affects specs that define
components.securitySchemes: {}with no entries, so it’s safe but worth correcting.
🧹 Nitpick comments (2)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (2)
129-146: Improved empty JSON body construction for object-like and numeric schemas looks goodBroadening object detection to
prop.propertiesand treating bothintegerandnumberas0is a sensible enhancement and should better support schemas wheretype: 'object'is omitted or where array items are$ref-resolved objects. This should help avoid missing nested structures in generated bodies without impacting existing primitive handling.If you find this pattern repeating further, consider a small helper like
isObjectSchema(schema)(checkingtype === 'object' || schema.properties) to centralize object-likeness checks across the file.
490-525: Expanded object-like detection for JSON, formUrlEncoded, and multipart bodies aligns with requestBody $ref needsUpdating the checks to
bodySchema && (bodySchema.type === 'object' || bodySchema.properties)for JSON,application/x-www-form-urlencoded, andmultipart/form-datacorrectly covers request bodies whose schemas only defineproperties(common when coming from$refintocomponents/requestBodies). This should fix the missing field imports called out in BRU-2128 and still preserves previous behavior for explicittype: 'object'schemas and arrays.You might later want to hoist
const isObjectSchema = bodySchema && (bodySchema.type === 'object' || bodySchema.properties);to avoid repeating the same condition three times.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-converters/src/openapi/openapi-to-bruno.js(4 hunks)packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-converters/tests/openapi/openapi-to-bruno/openapi-body.spec.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, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-converters/src/openapi/openapi-to-bruno.js
⏰ 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: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: Unit Tests
2 participants
Description
Fix OpenAPI 3 import to handle requestBody
$refpointing to#/components/requestBodies/...where the referenced schema has properties.jira
issue: #4808
Contribution Checklist:
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.