Skip to content

fix(sdk): handle query params separately to avoid unexpected encoding or missing#8531

Merged
ihexxa merged 3 commits intodevelopfrom
fix/sdk/url-query-transform
Apr 1, 2025
Merged

fix(sdk): handle query params separately to avoid unexpected encoding or missing#8531
ihexxa merged 3 commits intodevelopfrom
fix/sdk/url-query-transform

Conversation

@ihexxa
Copy link
Copy Markdown
Contributor

@ihexxa ihexxa commented Mar 27, 2025

Changes

  • Handle query params separately to avoid unexpected encoding or missing
  • Added UTs
  • Added smoke test

How to test

Add some scripts and do following things and see if it works well:

  1. Specify query params through Editor:
  2. Specify query params with an invalid url

#8491 INS-5160

@ihexxa ihexxa self-assigned this Mar 27, 2025
@ihexxa ihexxa force-pushed the fix/sdk/url-query-transform branch 2 times, most recently from afb8e20 to ce4dfaf Compare March 31, 2025 02:33
@notjaywu notjaywu requested a review from Copilot March 31, 2025 09:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the handling of query parameters by separating them from the URL object to prevent unwanted encoding and missing values. Key changes include updating the SDK URL object logic to maintain query parameters separately, modifying request merging to use the new URL string conversion method, and enhancing test coverage with new smoke tests and unit tests.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/insomnia-smoke-test/tests/smoke/pre-request-script-features.test.ts Added tests to validate the proper transformation of query params in pre-request scripts.
packages/insomnia-smoke-test/fixtures/pre-request-collection.yaml Introduced a new test collection for query params and updated environment values.
packages/insomnia-sdk/src/objects/urls.ts Refactored URL handling to separate query params from URL encoding and added utility methods.
packages/insomnia-sdk/src/objects/response.ts Updated the error message in the Response constructor to include more context.
packages/insomnia-sdk/src/objects/request.ts Modified URL string conversion in request merge to exclude query params from the URL field.
packages/insomnia-sdk/src/objects/insomnia.ts Adjusted query params mapping when initializing the Insomnia object.
packages/insomnia-sdk/src/objects/tests/urls.test.ts Updated tests to verify URL string output and to cover additional edge cases.
Comments suppressed due to low confidence (2)

packages/insomnia-sdk/src/objects/urls.ts:271

  • When splitting a query parameter string, ensure that the result contains both a key and a value to avoid potential undefined values. Consider adding a check or providing a default value if parts[1] is missing.
const parts = pair.split('=');

packages/insomnia-sdk/src/objects/urls.ts:272

  • [nitpick] Instead of using the spread operator to append to 'this.queryParams' repeatedly, consider using the Array.push() method (e.g., this.queryParams.push(...)) for better performance and clarity.
this.queryParams = [...this.queryParams, new QueryParam({ key: parts[0], value: parts[1] })];

@ihexxa
Copy link
Copy Markdown
Contributor Author

ihexxa commented Mar 31, 2025

Pull Request Overview

This PR fixes the handling of query parameters by separating them from the URL object to prevent unwanted encoding and missing values. Key changes include updating the SDK URL object logic to maintain query parameters separately, modifying request merging to use the new URL string conversion method, and enhancing test coverage with new smoke tests and unit tests.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
Comments suppressed due to low confidence (2)

Cool so can you approve it? :)

@ihexxa ihexxa force-pushed the fix/sdk/url-query-transform branch from ce4dfaf to f83ce32 Compare April 1, 2025 02:00
@ihexxa ihexxa enabled auto-merge (squash) April 1, 2025 10:01
@ihexxa ihexxa force-pushed the fix/sdk/url-query-transform branch from f83ce32 to 7dd6354 Compare April 1, 2025 10:01
@ihexxa ihexxa merged commit 4bb6c92 into develop Apr 1, 2025
9 checks passed
@ihexxa ihexxa deleted the fix/sdk/url-query-transform branch April 1, 2025 10:16
@arthurvanl
Copy link
Copy Markdown

Awesome thanks ihexxa!

cwangsmv pushed a commit that referenced this pull request Apr 21, 2025
… or missing (#8531)

* fix(sdk): handle query params separately to avoid unexpected encoding or missing

* fix: lint issue

* fix: smoke test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants