Skip to content

fix(angular): include query params in multi-content-type HTTP calls (#3102) - #3196

Merged
melloware merged 8 commits into
orval-labs:masterfrom
zeriong:fix/angular-multi-content-query-params-3102
Apr 6, 2026
Merged

fix(angular): include query params in multi-content-type HTTP calls (#3102)#3196
melloware merged 8 commits into
orval-labs:masterfrom
zeriong:fix/angular-multi-content-query-params-3102

Conversation

@zeriong

@zeriong zeriong commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #3102

  • Bug: When an OpenAPI endpoint defines multiple response content types (e.g. application/json + text/plain), the Angular HttpClient generator included query parameters in the function signature but never passed them to http.get() / http.post() calls.
  • Root cause: angularParamsRef was gated on isRequestOptions, so when requestOptions=false or when the observe-branching condition excluded multi-content-type paths, filteredParams was never generated.
  • Fix: Broadened angularParamsRef to always be set when queryParams exists. Split paramsDeclaration into two branches: shared filterParams helper (when isRequestOptions=true) and inline IIFE filtering (when isRequestOptions=false).

Changes

File Change
packages/angular/src/http-client.ts Import getAngularFilteredParamsExpression, broaden angularParamsRef guard, add isRequestOptions branching for paramsDeclaration
packages/angular/src/http-client.test.ts Add unit test for requestOptions=false + multi-content-type + query params
tests/specifications/petstore.yaml Add text/plain content type to listPets 200 response for snapshot coverage
148 snapshot files Auto-updated to reflect spec change

Test plan

  • Unit tests pass: bun vitest run packages/angular/src/http-client.test.ts (37/37)
  • Full unit tests pass: bun vitest run (2179/2182, 3 pre-existing failures in resolve-version.test.ts)
  • Snapshot tests pass: bun run test:snapshots (67/67 tasks)
  • Verified generated listPets snapshot includes params: filteredParams in all content-type branches

Summary by CodeRabbit

  • Bug Fixes

    • Query parameters are now properly preserved and filtered for multi-content API responses, including scenarios with disabled request-options overrides.
  • Tests

    • Added test coverage for multi-content response handling to validate query parameter preservation and filtering behavior.

…rval-labs#3102)

When an endpoint defines multiple response content types, the Angular
HttpClient generator dropped query parameters from generated http.get()
calls. The params appeared in the function signature but were never
passed to the HTTP request.

Broaden the angularParamsRef guard so filteredParams is always generated
when queryParams exists, and add inline IIFE filtering for the
requestOptions=false edge case. Add unit and snapshot test coverage.

Signed-off-by: jaery <jaeryong95@gmail.com>
@coderabbitai

coderabbitai Bot commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4768e7b5-fe25-49fb-924b-87d1891ebe8f

📥 Commits

Reviewing files that changed from the base of the PR and between b52e2cb and 0b45bb7.

📒 Files selected for processing (10)
  • packages/angular/src/http-client.test.ts
  • packages/angular/src/http-client.ts
  • tests/__snapshots__/angular/multi-content-query-params/endpoints.ts
  • tests/__snapshots__/angular/multi-content-query-params/model/error.ts
  • tests/__snapshots__/angular/multi-content-query-params/model/index.ts
  • tests/__snapshots__/angular/multi-content-query-params/model/item.ts
  • tests/__snapshots__/angular/multi-content-query-params/model/items.ts
  • tests/__snapshots__/angular/multi-content-query-params/model/listItemsParams.ts
  • tests/configs/angular.config.ts
  • tests/specifications/angular-multi-content-query-params.yaml

📝 Walkthrough

Walkthrough

This PR fixes query parameter handling in the Angular HTTP client code generator for endpoints with multiple response content types. The parameter filtering logic is updated to consistently apply query parameters regardless of the requestOptions configuration, with a new test case validating this behavior.

Changes

Cohort / File(s) Summary
Core Implementation
packages/angular/src/http-client.ts, packages/angular/src/http-client.test.ts
Updated parameter filtering logic to set angularParamsRef whenever queryParams exists (not gated by isRequestOptions). Split filtering expression generation into two paths: getAngularFilteredParamsCallExpression() when isRequestOptions is true, and getAngularFilteredParamsExpression() with inline fallback when false. Added test case asserting query param preservation in multi-content responses with inline filtering.
Test Specification & Configuration
tests/specifications/angular-multi-content-query-params.yaml, tests/configs/angular.config.ts
Added OpenAPI 3.0 specification for Angular multi-content query params test defining GET /items with query parameters (limit, filter) and multiple response types (application/json, text/plain). Added corresponding Orval generator configuration entry pointing to the new specification.
Generated Test Snapshots
tests/__snapshots__/angular/multi-content-query-params/endpoints.ts, tests/__snapshots__/angular/multi-content-query-params/model/*
Generated Angular service implementing AngularMultiContentQueryParamsTestService with overloaded listItems() methods handling multi-content responses. Includes model types (Item, Items, Error, ListItemsParams) and a filterParams helper for query parameter sanitization.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Caller
    participant Service as AngularMultiContentQueryParamsTestService
    participant HttpClient as Angular HttpClient
    participant Server as Backend

    Client->>Service: listItems(params, accept, options?)
    activate Service
    
    Service->>Service: Merge params with options.params
    Service->>Service: filterParams() - sanitize query params
    Service->>Service: Build/override Accept header
    
    alt accept = 'application/json'
        Service->>Service: Set responseType: 'json'
    else accept = 'text/plain'
        Service->>Service: Set responseType: 'text'
    end
    
    Service->>HttpClient: get(url, { params, headers, responseType })
    deactivate Service
    activate HttpClient
    
    HttpClient->>Server: GET /items?filter=...&limit=... + Accept header
    deactivate HttpClient
    activate Server
    
    Server-->>HttpClient: 200 OK + matching Content-Type
    deactivate Server
    activate HttpClient
    
    HttpClient-->>Client: Observable<Items | string>
    deactivate HttpClient
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly Related PRs

Suggested Labels

angular

Suggested Reviewers

  • melloware
  • snebjorn

Poem

🐰 Hops with joy, the query params flow,
Multi-content responses, now they'll go!
No more lost filters in the Accept header's dance,
Angular endpoints get their proper chance! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: preventing query parameters from being omitted in Angular HTTP calls when multiple response content types are defined.
Linked Issues check ✅ Passed The PR directly addresses issue #3102 by ensuring query parameters are passed to HTTP requests regardless of multiple response content types, matching the expected behavior stated in the issue.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the query parameter omission bug: core logic updates in http-client.ts, unit test coverage, snapshot updates, and a dedicated multi-content test specification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

zeriong added 5 commits April 4, 2026 18:44
…rval-labs#3102)

When an endpoint defines multiple response content types, the Angular
HttpClient generator dropped query parameters from generated http.get()
calls. The params appeared in the function signature but were never
passed to the HTTP request.

Broaden the angularParamsRef guard so filteredParams is always generated
when queryParams exists, and add inline IIFE filtering for the
requestOptions=false edge case. Add unit and snapshot test coverage with
a dedicated multi-content-type spec to avoid affecting other generators.

Signed-off-by: jaery <jaeryong95@gmail.com>
…rval-labs#3102)

When an endpoint defines multiple response content types, the Angular
HttpClient generator dropped query parameters from generated http.get()
calls. The params appeared in the function signature but were never
passed to the HTTP request.

Broaden the angularParamsRef guard so filteredParams is always generated
when queryParams exists, and add inline IIFE filtering for the
requestOptions=false edge case. Add unit and snapshot test coverage with
a dedicated multi-content-type spec to avoid affecting other generators.

Signed-off-by: jaery <jaeryong95@gmail.com>
The previous commit inadvertently modified the shared petstore.yaml,
adding a text/plain content type to listPets. This exposed pre-existing
bugs in httpResource (TS1016: required param after optional) and mock
generator (TS2451: duplicate variable). Revert petstore.yaml and all
affected snapshots to master state. The multi-content-type + query
params scenario is covered by a dedicated spec instead.

Signed-off-by: jaery <jaeryong95@gmail.com>
@melloware melloware added the angular Related to Angular generation issues label Apr 4, 2026
@melloware
melloware requested a review from snebjorn April 4, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

angular Related to Angular generation issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

not using query params when multiple responses defined

3 participants