fix#6247: Interpolate dynamic variables in path param

[NikHillAgar]

Description

This PR fix #6247. When path param are replaced in the url during code snippet generation it does not take into account that value could refer to a dynamic variable.

Contribution Checklist:

• The pull request only addresses one issue or adds one feature.
• The pull request does not introduce any breaking changes
• I have added screenshots or gifs to help explain the change if applicable.
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

Summary by CodeRabbit

• Bug Fixes
  • Improved URL construction so path parameters now correctly support variable substitution, preventing incorrect or broken links when variables are used in paths.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[helloanoop]

Hey @NikHillAgar

Could you add a test for this please ?

[coderabbitai]

Walkthrough

Updated URL interpolation so path-parameter replacement is performed and then variable interpolation is applied using a provided variables object when computing the final URL.

Changes

Cohort / File(s)	Summary
URL interpolation util packages/bruno-app/src/utils/url/index.js, packages/bruno-app/src/utils/url/index.spec.js	interpolateUrlPathParams now accepts a third variables argument; internal logic applies variable interpolation after replacing path params. Tests updated to cover variable-based path values.
Code generation usage packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js	Final URL construction updated to pass variables through to interpolateUrlPathParams so runtime variables (e.g., {{varXXX}}) are resolved in generated code.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Inspect interpolateUrlPathParams changes for edge cases (missing variables, falsy values).
• Check all call sites of interpolateUrlPathParams for correct argument passing.
• Verify tests in index.spec.js cover both static and variable-driven path params.
• Ensure no accidental URL-encoding or injection changes when interpolating variables.

Poem

🔁 Paths once froze when variables hid inside,
Now replacements run, then variables glide.
Codegen hums smoothly, no more broken view—
A tiny fix, a clearer URL crew 🌟

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the issue being fixed (#6247) and accurately describes the main change: interpolating dynamic variables in path parameters during code generation.
Linked Issues check	✅ Passed	The PR successfully implements the objective from #6247 by extending interpolateUrlPathParams to accept variables and applying interpolation to path parameters, resolving the issue where dynamic runtime variables were left unresolved in generated code.
Out of Scope Changes check	✅ Passed	All changes are scoped to the URL interpolation utility and its tests, directly addressing the path parameter interpolation requirement from #6247 with no extraneous modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/bruno-app/src/utils/url/index.js (1)

101-142: Minor robustness: default params inside helper to avoid potential .find on undefined

To make interpolateUrlPathParams safer against unexpected params being undefined (e.g. when a caller passes nothing or null), you can default the helper argument:

-export const interpolateUrlPathParams = (url, params, variables) => {
-  const getInterpolatedBasePath = (pathname, params) => {
+export const interpolateUrlPathParams = (url, params, variables) => {
+  const getInterpolatedBasePath = (pathname, params = []) => {

This keeps behavior the same for normal callers but avoids a TypeError from params.find(...) if something upstream forgets to supply a params array.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd72ee5 and d2859b1.

📒 Files selected for processing (2)

• packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js (1 hunks)
• packages/bruno-app/src/utils/url/index.js (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js (4)

packages/bruno-electron/src/utils/collection.js (1)

• variables (535-535)

packages/bruno-app/src/utils/collections/index.js (3)

• variables (1072-1072)
• variables (1098-1098)
• variables (1313-1313)

packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/utils/snippet-generator.js (1)

• variables (12-12)

packages/bruno-cli/src/utils/bru.js (1)

• variables (131-131)

⏰ 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: SSL Tests - macOS
• GitHub Check: Unit Tests
• GitHub Check: SSL Tests - Windows
• GitHub Check: SSL Tests - Linux
• GitHub Check: CLI Tests

🔇 Additional comments (2)

packages/bruno-app/src/utils/url/index.js (1)

101-142: Path-param interpolation fix looks correct and is backward compatible

Using variables to run interpolate on the replacedPathname after substituting :param/OData segments is exactly what’s needed to resolve {{var}} in path param values (e.g. {{varXXX}} → 1000000). Existing callers that don’t pass variables will still work, since interpolate is already used elsewhere with possibly undefined variables.

Please ensure there’s at least one test that covers:

• URL like https://www.url.com/:path-param/something
• Path param path-param with value {{varXXX}}
• variables = { varXXX: '1000000' }
  and verifies the final interpolated path is /1000000/something.

If you don’t already have tests around interpolateUrlPathParams, consider adding one in the existing URL utils test suite.

packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js (1)

86-100: Correct wiring of variables into path-param interpolation

Passing variables into interpolateUrlPathParams after building interpolatedUrl ensures:

1. Base URL (scheme/host/query) is interpolated once via interpolateUrl.
2. Path params are substituted using requestData.params.
3. Any {{var}} that came from path param values is then resolved using the same variables map.

This matches the issue’s scenario and shouldn’t introduce double-interpolation for already-resolved parts of the URL.

Once tests are in place for the utils, consider also adding a higher-level test (or e2e) that exercises GenerateCodeItem end-to-end for a request with:

• url = "https://www.url.com/:path-param/something"
• path-param = {{varXXX}}
• variables.varXXX = "1000000"
  to confirm finalUrl and the code snippet both use https://www.url.com/1000000/something.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/bruno-app/src/utils/url/index.spec.js (1)

329-329: Good backward compatibility updates.

Existing tests properly updated to pass the new variables parameter. Consider adding edge-case tests for robustness:

• When variables parameter is null or undefined
• When a path param references a variable that doesn't exist in the variables object
• When path param values contain multiple variable references (e.g., {{var1}}-{{var2}})

Also applies to: 339-339, 349-349

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2859b1 and 8e96279.

📒 Files selected for processing (1)

• packages/bruno-app/src/utils/url/index.spec.js (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

packages/bruno-app/src/utils/url/index.spec.js (1)

packages/bruno-app/src/utils/url/index.js (4)

• interpolateUrlPathParams (101-160)
• interpolateUrlPathParams (101-160)
• interpolateUrl (93-99)
• interpolateUrl (93-99)

⏰ 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 - Linux
• GitHub Check: SSL Tests - macOS
• GitHub Check: SSL Tests - Windows
• GitHub Check: Unit Tests
• GitHub Check: Playwright E2E Tests
• GitHub Check: CLI Tests

🔇 Additional comments (2)

packages/bruno-app/src/utils/url/index.spec.js (2)

301-310: LGTM! Test properly verifies the fix.

This test case directly addresses the reported issue where path parameter values containing dynamic variable references (e.g., {{user}}) are now correctly interpolated to their actual values.

---

312-322: Integration test correctly validates the full interpolation flow.

The test properly exercises the two-step process (URL variable interpolation → path parameter interpolation) and confirms that variable references in path parameter values are resolved in the final URL.

[NikHillAgar]

@helloanoop I have added the tests considering dynamic variable in path params

[helloanoop]

Thank you @NikHillAgar
@sid-bruno and @anusree-bruno on our team will track this to get it reviewed and merged in second half of Dec