Fix TodolistOrGroup union deserialization for Get/Update#179
Merged
Conversation
The API returns flat JSON ({"id": 123, "type": "Todolist", ...}) but the
generated AsTodolistOrGroup0/1() methods expect envelope-wrapped variants
({"todolist": {...}}), yielding zero-valued structs. Bypass the broken
union helpers by decoding resp.Body directly into the target generated
type (Todolist or TodolistGroup). Affects Get() and Update() in both
todolists and todolist_groups services.
Tests Get and Update for both TodolistsService and TodolistGroupsService using httptest.NewServer. Each test asserts request method, path, response field population, and (for Update) the JSON request body payload.
There was a problem hiding this comment.
Pull request overview
Updates the Go Basecamp SDK’s todolist and todolist group services to correctly decode responses from the polymorphic /todolists/:id endpoint (which returns flat JSON), and adds httptest-based contract tests to lock in the expected request/response behavior.
Changes:
- Decode
Get/Updateresponses by unmarshallingresp.Bodyintogenerated.Todolist/generated.TodolistGroupinstead of using the union-envelope helpers. - Add httptest contract tests for
TodolistsService.Get/UpdateandTodolistGroupsService.Get/Update.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| go/pkg/basecamp/todolists.go | Switches Get/Update response decoding to unmarshal flat JSON from resp.Body. |
| go/pkg/basecamp/todolists_test.go | Adds httptest contract tests validating paths, request bodies, and basic response mapping for todolists. |
| go/pkg/basecamp/todolist_groups.go | Switches Get/Update response decoding to unmarshal flat JSON from resp.Body. |
| go/pkg/basecamp/todolist_groups_test.go | Adds httptest contract tests validating paths, request bodies, and basic response mapping for todolist groups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Get()andUpdate()methods. The API returns flat JSON ({"id": 123, "type": "Todolist", ...}) but the generatedAsTodolistOrGroup0/1()methods expect envelope-wrapped variants ({"todolist": {...}}), yielding zero-valued structs. Bypasses the union helpers by decodingresp.Bodydirectly into the target generated type.Go-only containment fix — no changes to the shared
openapi.json.Test plan
go test ./pkg/basecamp/...— 4 new service-level tests passgo test ./...— full Go test suite passesmake check— all cross-SDK checks pass (Smithy, Go, TS, Ruby, Kotlin, Swift, conformance)Summary by cubic
Fix union deserialization in todolists and todolist groups
Get()andUpdate()by correctly parsing the flat JSON the API returns. Go-only fix; no changes toopenapi.json.Bug Fixes
resp.Bodyintogenerated.Todolistandgenerated.TodolistGroupinstead of usingAsTodolistOrGroup0/1.Get()andUpdate()inTodolistsServiceandTodolistGroupsService.Tests
httptestcontract tests forGet()andUpdate()in both services.Written for commit d1a620b. Summary will update on new commits.