Skip to content

fix: serialize nested query dictionaries with bracket notation - #59

Merged
javorosas merged 9 commits into
mainfrom
fix/nested-query-params
Sep 10, 2026
Merged

javorosas merged 9 commits into
mainfrom
fix/nested-query-params

Conversation

@javorosas

@javorosas javorosas commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Cambios (7.0.0)

Breaking

  • SearchResult.Page/TotalPages/TotalResults ahora son int? (las páginas de cursor posteriores omiten totales; la ausencia ya no se deserializa como 0).

Added

  • SearchResult expone TotalsAreCapped, NextCursor, PreviousCursor.

Fixed

  • Serialización de params anidados con corchetes (date[gte]=...&date[lt]=...) y arrays a status[]=....

El cambio int → int? altera firmas públicas, por lo que se libera como 7.0.0 (major); CHANGELOG [7.0.0] incluido.

Query values that were dictionaries or lists were sent through ToString(),
producing garbage for a date range (Dictionary.ToString()) and for arrays
(List.ToString()). Recurse into IDictionary values expanding them to bracket
keys (date[gte]=...&date[lt]=...), expand non-string enumerables to repeated
empty-bracket keys (status[]=...), and keep null values serialized as empty
(foo=), matching the v2 API contract.

Bump to 6.8.1 and add a regression test.
SearchResult gains TotalsAreCapped, NextCursor, and PreviousCursor so callers
can follow cursor pagination and detect capped totals once the capping wave is
live (page totals capped; cursor mode returns totals only on the first page).
The nested query serialization fix is a patch, but the new cursor and
capped-total properties on SearchResult are additive public API, so the
release is a minor per semver, consistent with the node SDK bump.
@javorosas
javorosas requested review from raul-facturapi and a balanced review from Copilot September 9, 2026 13:43
@javorosas javorosas self-assigned this Sep 9, 2026

This comment was marked as resolved.

SearchResult cursor/capped metadata under Added; nested query param
serialization fix under Fixed.
- SearchResult.Page/TotalPages/TotalResults are now nullable: later cursor
  pages omit them and deserializing absence as 0 misreported metadata.
- Regression tests: a later-cursor response (only cursors + data) maps to
  null totals, and array params serialize to repeated bracket keys
  (status[]=valid&status[]=canceled), covering the IEnumerable branch.

This comment was marked as resolved.

Changing Page/TotalPages/TotalResults from int to int? alters public member
signatures, so the release is a major (7.0.0), with the additive cursor/capped
metadata under Added and the serialization fix under Fixed.
@javorosas

Copy link
Copy Markdown
Member Author

Semver atendido en 38be2c5: al cambiar int → int? (firmas públicas) el release pasa a 7.0.0 (major), con CHANGELOG [7.0.0] (Breaking + Added + Fixed) y el body actualizado.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The changelog incorrectly duplicates the release changes under version 6.9.0.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread CHANGELOG.md Outdated
@javorosas

Copy link
Copy Markdown
Member Author

Corregido en d684018: se eliminó la sección [6.9.0] duplicada del CHANGELOG; todo el trabajo se libera una sola vez como 7.0.0.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation matches the stated release scope and includes focused behavioral coverage.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@javorosas

Copy link
Copy Markdown
Member Author

One more commit: lists now serialize as repeated keys (status=valid&status=canceled) instead of status[]=valid&status[]=canceled, which is the OpenAPI default form for arrays (form with explode) and what the other official SDKs send. Nested dictionaries keep bracket notation (date[gte]=...). The existing null → key= behavior is intentionally unchanged (covered by Router_ListCustomers_AllowsNullQueryValues). dotnet test: 38/38 passing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@javorosas
javorosas merged commit ea1d7d2 into main Sep 10, 2026
3 of 4 checks passed
@javorosas
javorosas deleted the fix/nested-query-params branch September 10, 2026 17:21
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.

3 participants