fix: send ESG country/domain as query params (matches [FromQuery] binding)#7
Merged
Merged
Conversation
…ding) Cross-repo audit of all 47 SDK methods against the live QubitOn API surfaced one issue: lookupESGScore was sending country and domain in the JSON body, but the server binds them as [FromQuery] on ESGController -- they were silently dropped, causing scoring to default to global / no-domain results. Same drift surface as qubitonhq/qubiton-go#1, qubitonhq/qubiton-sap#2, qubitonhq/qubiton-oracle#2. Fix --- Strip country and domain off the request body and serialise them into the URL query string with URLSearchParams (handles percent-encoding). The body now contains only companyName, esgId, and BaseRequest fields. Existing call sites that don't set country/domain see no behaviour change. Call sites that DO set them (intending to filter by region or domain) now correctly produce ?country=…&domain=… on the URL. EsgScoresRequest's country and domain JSDoc was updated to flag they are sent as query parameters, not body. Audit findings outside this PR ------------------------------- 46 of 47 SDK methods are correctly aligned with the API. Spot-checked: - TaxValidateRequest: identityNumber/identityNumberType/country/entityName ✅ - TaxFormatValidateRequest: identityNumber/identityNumberType/countryIso2 ✅ - BusinessRegistrationRequest: entityName ✅ - EntityRiskRequest.CountryOfIncorporation: PascalCase, matches the pinned [JsonPropertyName("CountryOfIncorporation")] override on the server DTO that escapes the camelCase naming policy. ✅ The Node SDK maintainer had already documented this trap in the model comment ("the lowercase countryOfIncorporation alias is NOT accepted by the server"). - BeneficialOwnershipRequest: countryIso2 ✅ Tests ----- tsc --noEmit clean. Full test suite passes (22/22 tests).
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
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.
Summary
Cross-repo audit of all 47 SDK methods against the live QubitOn API surfaced 1 issue: `lookupESGScore` was sending `country` and `domain` in the JSON body, but the server binds them as `[FromQuery]` on `ESGController` — they were silently dropped, causing scoring to default to global / no-domain results.
Same drift surface as the SAP, Oracle, and Go connector PRs:
Fix
Strip `country` and `domain` off the request body and serialise them into the URL query string with `URLSearchParams` (handles percent-encoding). The body now contains only `companyName`, `esgId`, and `BaseRequest` fields.
```ts
async lookupESGScore(req: EsgScoresRequest, options?: RequestOptions): Promise {
const { country, domain, ...body } = req;
const params = new URLSearchParams();
if (country) params.set('country', country);
if (domain) params.set('domain', domain);
const qs = params.toString();
const path = qs ? `/api/esg/Scores?${qs}` : '/api/esg/Scores';
return this.post(path, body, options);
}
```
Existing call sites that don't set `country`/`domain` see no behaviour change. Call sites that DO set them (intending to filter by region or domain) now correctly produce `?country=…&domain=…` on the URL.
`EsgScoresRequest.country` and `.domain` JSDoc updated to flag they are sent as query parameters, not body.
Audit findings outside this PR
46 of 47 SDK methods are correctly aligned with the API. Spot-checked:
The Node SDK is the cleanest of the four connector audits — the maintainer had already preempted three of the four common drift traps with pinned wire field names and documented JSDoc comments. Only the ESG query-string binding was missed.
Test plan
Diff
`+19 / -1` across `src/client.ts` and `src/models.ts`.