#198

fix(dataforseo): materialize defaults outside selector unions

Opensdivyanshu90wants to merge intofrom

Fixes #196.

I moved the common default-bearing schema outside the identifier alternatives for five DataForSEO endpoints. Omitting depth/limit now materializes the existing documented default before estimation and before sending the request, instead of failing with FN_CONTRACT. The original identifier union still enforces at least one selector.

This restores the connector's existing D5 contract: optional depth/limit, with the vendor default applied on the wire. Tests compare omitted and explicit defaults, capture outgoing defaulted and overridden values, and retain the missing-identifier checks. Schema-inspection helpers now traverse the resulting allOf as well as anyOf.

Validation (Deno 2.9.7):

  • New regression fails before the fix. DataForSEO suite: 22 passed, 0 failed, 2 gated live tests ignored.
  • All 251 test modules imported into one temporary shared-process entry point: 1,299 passed, 0 failed, 221 gated live tests ignored. This is not a standard isolated-module deno task test run.
  • deno task check, deno task lint, changed-file deno fmt --check, git diff --check, and deno task version:check --base upstream/main pass.

@ooctoo777, could you review and merge this fix?

Summary by CodeRabbit

  • Bug Fixes
    • Business review, domain technology, and shopping seller requests now consistently apply default values while still requiring the relevant search identifier.
    • When optional depth or limit values are omitted, charge estimates and submitted requests now include their defaults. Explicit values remain unchanged, and caller-provided input is preserved.

Activity

2
coderabbitai[bot]7d ago

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 09d54479142e36eff876383beacfbd37b33b5908 and fa0254982be84c996a1a9975da1265ca7d875ae4.

Files selected for processing: 6
  • connectors/dataforseo/endpoints/business/google-business-extended-reviews/endpoint.ts
  • connectors/dataforseo/endpoints/business/google-business-reviews/endpoint.ts
  • connectors/dataforseo/endpoints/business/tripadvisor-reviews/endpoint.ts
  • connectors/dataforseo/endpoints/domain/domain-domains-by-technology/endpoint.ts
  • connectors/dataforseo/endpoints/merchant/google-shopping-sellers/endpoint.ts
  • connectors/dataforseo/provider.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

Five DataForSEO selector schemas now apply shared defaults outside their identifier unions. The identifier requirements remain. Provider tests cover default values in estimates and submitted request bodies, explicit values, unchanged caller input, and a terminal vendor error after one request. Schema-property discovery and the identifier-gate test now account for nested allOf.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to fa025

No actionable issue remains from this review; the change is ready for normal merge checks.

Pre-merge checks: 4
Check nameStatusExplanation
Description CheckCheck skipped - CodeRabbit’s high-level summary is enabled.
Title checkThe title clearly and concisely describes the main change: moving default materialization outside selector unions for DataForSEO endpoints.
Linked Issues checkIssue #196 requires that omitting the documented default for five DataForSEO selector endpoints no longer fails estimation with FN_CONTRACT. The diff changes each of the five endpoint.ts body schemas …
Out of Scope Changes checkAll changes map to #196. The five endpoint.ts files change only the body schema wrapper and its comment. provider.test.ts changes are the regression test and two helper updates. propsOf and the 'at le…
Full details: Linked Issues check

Explanation

Issue #196 requires that omitting the documented default for five DataForSEO selector endpoints no longer fails estimation with FN_CONTRACT. The diff changes each of the five endpoint.ts body schemas from z.union([...]) to zBody.and(z.union([...])). zBody is the local schema with the vendor default (depth 10 for google-business/reviews and tripadvisor/reviews, depth 20 for google-business/extended-reviews, depth 10 for google-shopping/sellers, limit 100 for domain/domains-by-technology). Defaults now apply before the identifier union is checked. I confirmed zBody and its default in google-business/reviews/endpoint.ts and domain-domains-by-technology/endpoint.ts at the head. The identifier union remains in the intersection, so missing-identifier rejection is kept. provider.test.ts adds a regression test. It checks omitted versus explicit estimates, wire defaults for omitted values, explicit overrides, and that caller input is not mutated. Per the PR, the test failed before the fix and passes after it. I did not run the test suite. The zod intersection output merge behavior for defaults is supported by the reported test results, not by a run in this review.

Full details: Out of Scope Changes check

Explanation

All changes map to #196. The five endpoint.ts files change only the body schema wrapper and its comment. provider.test.ts changes are the regression test and two helper updates. propsOf and the 'at least one of' gate now traverse allOf as well as anyOf, which the new body shape requires. No unrelated endpoints, version files, or runtime modules appear in the whole-PR diff against the merge base.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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