Add ListAnswersByPerson to CheckinsService#276
Conversation
Spec Change Impact
SDKs requiring updates:
All SDKs need regeneration to incorporate the added operation. |
Wraps the existing GetAnswersByPersonWithResponse generated endpoint with
a proper service method, following the same pagination pattern as ListAnswers.
The BC3 API has supported GET /questions/{id}/answers/by/{personId}.json
but the Go SDK had no service-layer wrapper for it.
15fed52 to
2fe5be2
Compare
Match the Basecamp domain term (Person, not User), the personID
parameter, and the URL path /by/{personId}. Other SDKs (TS, Ruby,
Python, Kotlin, Swift) all use byPerson/by_person.
Also drop the now-stale GetAnswersByPerson entry from EXCLUDED_OPS
in the drift check (this PR wraps it), tighten the test to assert
exact HTTP method and path including the account prefix, and align
the ListAnswers / ListAnswersByPerson godoc.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="go/pkg/basecamp/checkins.go">
<violation number="1" location="go/pkg/basecamp/checkins.go:521">
P1: Export the wrapper as `ListAnswersByUser` to match the requested SDK API; `ListAnswersByPerson` is the wrong public method name.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
This PR adds a Go CheckinsService wrapper for the “answers by person” check-in endpoint so callers can list a specific person’s answers using the same pagination/limit pattern as existing answer listing helpers.
Changes:
- Add
CheckinsService.ListAnswersByPerson(...)wrapper around the generatedGetAnswersByPersonWithResponseoperation, including limit + Link-header pagination handling. - Add a service-level test and a new fixture for the single-person answers response.
- Update the service-drift script exclusions so
GetAnswersByPersonis no longer treated as intentionally unwrapped.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| go/pkg/basecamp/checkins.go | Adds ListAnswersByPerson service wrapper with pagination/limit behavior and updates pagination docs. |
| go/pkg/basecamp/checkins_test.go | Adds a test verifying the correct endpoint path is called and the response is parsed. |
| spec/fixtures/checkins/answers_by_person.json | Adds fixture data for the “answers by person” list response. |
| scripts/check-service-drift.sh | Removes GetAnswersByPerson from the intentionally-unwrapped operation exclusions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds a table-driven multi-page test exercising the four distinct branches of the answer-listing pagination flow: - multi-page collect across Link headers (no limit) - Page option returns first page only and skips Link follow - Limit smaller than first page triggers service-level truncation - Limit straddling the page boundary trims inside followPagination ListAnswers shares the same followPagination helper, so this also retroactively covers its (previously untested) pagination path.
Summary
The BC3 API supports
GET /questions/{id}/answers/by/{personId}.jsonto fetch a specific person's answers to a check-in question, and the generated Go client already exposesGetAnswersByPersonWithResponse. However, theCheckinsServicehad no service-layer wrapper for it.This PR adds
ListAnswersByPerson(ctx, questionID, personID int64, opts *AnswerListOptions) (*AnswerListResult, error)toCheckinsService, following the same pagination pattern as the existingListAnswersmethod.Changes
go/pkg/basecamp/checkins.go— newListAnswersByPersonmethod onCheckinsServicego/pkg/basecamp/checkins_test.go—TestCheckinsService_ListAnswersByPersonverifies path/method, plus a table-driven_Paginationtest exercising multi-page collect,Pageshort-circuit, and limit truncation on both first page and mid-second-page (also retroactively coversListAnswers's shared pagination path)spec/fixtures/checkins/answers_by_person.json— fixture for the answers-by-person list responsescripts/check-service-drift.sh— drop the now-staleGetAnswersByPersonentry fromEXCLUDED_OPS(this PR wraps it)Naming
The method is
ListAnswersByPersonrather thanListAnswersByUserto match upstream conventions: the Smithy spec definesGetAnswersByPerson, the Go generated client exposesGetAnswersByPersonWithResponse, the URL path is/by/{personId}, the parameter type isPersonId, and Ruby, Python, TypeScript, Kotlin, and Swift all useby_person/byPerson. "Person" is also the correct Basecamp domain term — it covers users, clients, employees, and vendors, all of whom can submit check-in answers.Other languages
Ruby, Python, TypeScript, Kotlin, and Swift all already expose this endpoint — their SDKs are fully generated from the OpenAPI spec so
byPerson/by_personwas included from the start. The Go SDK is unique in having a hand-written service layer on top of the generated client, which is why only Go needed this change.Context
This unblocks basecamp/basecamp-cli#443, which requests a
--byfilter onbasecamp checkins answers. The CLI PR depends on this SDK method being available.