fix(gpa): align all four wrappers with staging UAT shapes#41
Conversation
Every `client.gpa.*` method against api-uat.ghin.com either rejected the
real response at the Zod layer or sent a malformed request. Fixes all
five issues in one pass.
- `getAccesses()` now models `/users/accesses.json` correctly (USGA's
"UserAccesses" envelope: federations / associations / clubs / golfers /
super_user / subtype) and flattens the `golfers` branch — the only one
carrying GPA state — into `Array<{ golferId, userAccessId, golferName,
gpaStatus }>`. IDs arrive as numeric strings and are coerced to number.
- `requestAccess()` gains a required `{ email }` body parameter (breaking
signature change — USGA returns 400 without it) and its response shape
is corrected to `{ success: string }`.
- `updateStatus()` and `revokeAccess()` response schemas corrected to
`{ success: string }`. Documented inline that `updateStatus`'s
`user_id` is the credentialed admin user, not the golfer.
Fixtures captured from staging back the new schemas + flattening.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates the GHIN client's GPA consent flow to match USGA staging UAT API behavior. It introduces new Zod schemas for the UserAccesses endpoint, refactors method implementations to flatten nested responses and enforce stricter request validation, and updates all corresponding tests and documentation to reflect breaking signature changes to ChangesGPA API Alignment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #41 +/- ##
=======================================
Coverage ? 95.81%
=======================================
Files ? 54
Lines ? 2530
Branches ? 424
=======================================
Hits ? 2424
Misses ? 106
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/client/ghin/index.test.ts (1)
245-255: ⚡ Quick winKeep the empty-accesses case on the same schema-validation path as production.
This test returns a raw envelope from
mockFetch, while the happy-path case goes throughschemaUserAccessesResponse.parse(...). That means this case can still pass with shapes the realRequestClientwould reject, so it should parse the empty fixture through the schema too.♻️ Proposed fix
mockFetch.mockResolvedValue( - ok({ - federations: [], - associations: [], - clubs: [], - golfers: [], - super_user: 'false', - subtype: null, - }), + ok( + schemaUserAccessesResponse.parse({ + federations: [], + associations: [], + clubs: [], + golfers: [], + super_user: 'false', + subtype: null, + }), + ), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/ghin/index.test.ts` around lines 245 - 255, The test is returning a raw envelope from mockFetch instead of validating it the same way production does; update the test so the empty fixture is fed through the same validator by calling schemaUserAccessesResponse.parse(...) (or otherwise passing the parsed result) before resolving mockFetch, so the empty-accesses case follows the same schema-validation path as the happy-path that uses schemaUserAccessesResponse.parse; locate usages of mockFetch.mockResolvedValue in this test and replace the raw object with the parsed/validated value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/ghin/index.ts`:
- Around line 123-127: The GPA API methods (gpa.getAccesses, gpa.requestAccess,
gpa.updateStatus, gpa.revokeAccess) and their GhinClient implementations
currently return Promise<T> and throw on error; change their signatures to
return Promise<Result<T, E>> (use neverthrow Result) with appropriate error type
(e.g., GpaError) — update the GhinClient methods to catch internal exceptions
and return Result.ok(value) on success or Result.err(error) on failure instead
of throwing, and propagate these Result types through any callers and exported
types so the public surface no longer throws but returns Result values.
---
Nitpick comments:
In `@src/client/ghin/index.test.ts`:
- Around line 245-255: The test is returning a raw envelope from mockFetch
instead of validating it the same way production does; update the test so the
empty fixture is fed through the same validator by calling
schemaUserAccessesResponse.parse(...) (or otherwise passing the parsed result)
before resolving mockFetch, so the empty-accesses case follows the same
schema-validation path as the happy-path that uses
schemaUserAccessesResponse.parse; locate usages of mockFetch.mockResolvedValue
in this test and replace the raw object with the parsed/validated value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40818890-c3f9-4376-9159-19a764d3ad3c
📒 Files selected for processing (7)
.changeset/gpa-staging-shapes.mdsrc/client/ghin/index.test.tssrc/client/ghin/index.tssrc/client/ghin/models/gpa/__fixtures__/index.tssrc/client/ghin/models/gpa/access.test.tssrc/client/ghin/models/gpa/access.tssrc/playground/gpa-consent.ts
|
Responding to CodeRabbit nitpick on Fixed — the empty fixture is now routed through |
Routes the empty-fixture case through schemaUserAccessesResponse.parse(...) to mirror the production path, so the test fails on shapes the real RequestClient would reject. Addresses CodeRabbit nitpick on #41.
1 participant
Summary
client.gpa.*method againstapi-uat.ghin.com: prior to this PR,getAccesses()/requestAccess()/updateStatus()/revokeAccess()would all either reject the real response at the Zod layer or send a malformed request. Discovered while building the GPA opt-in flow in the spicy monorepo, which had to bypass the lib viahttpClient.fetchCustomPath+z.unknown().requestAccess()gains a required{ email }second argument (breaking signature change — USGA rejects with400 { errors: { email: ["can't be blank"] } }without it). All other inputs unchanged. Bumped to minor via changeset.getAccesses()flattens the/users/accesses.json"UserAccesses" envelope (which also returns unrelatedfederations/associations/clubs/super_user/subtypesiblings) to a cleanArray<{ golferId, userAccessId, golferName, gpaStatus }>. IDs arrive as numeric strings on the wire and are Zod-coerced tonumber.The five bugs, mapped to the fix
getAccessesschema expected{ gpa_accesses: [] }; real shape is the UserAccesses envelope with string IDs and agpa_statusfieldschemaUserAccessesResponsematches the real wire shape; wrapper flattensgolfersand coerces IDsrequestAccesssent empty body; USGA requiresemailrequestAccess(golferId, { email })requestAccessresponse expected{ golfer_id, status }; real is{ success: string }schemaGpaSuccessResponseupdateStatusresponse expected{ golfer_id, status }; real is{ success: string }schemaGpaSuccessResponse; also documented inline thatuser_idis the credentialed admin user's id from/users/login.json, not the golfer's user and notuserAccessIdrevokeAccessresponse expected{ golfer_id }; real is{ success: string }(with USGA's trailing space)schemaGpaSuccessResponseFixtures
Real staging responses captured under
src/client/ghin/models/gpa/__fixtures__/. The newaccess.test.tsparses each fixture through its schema (catches drift loudly); the existingindex.test.tsre-uses them through the mocked request-client.Test plan
bun run test:run— 298 tests pass (10 new GPA model tests + 4 added wrapper tests covering email body, validation errors, flattening, and the empty-golfers case)bun run lint— biome + tsc cleanbun run build— tsup produces dist with no errors./scripts/code-quality.sh— full pre-commit gate clean0.12.0is published — drop thefetchCustomPathbypasses frompackages/api/scripts/gpa-*.tsand confirm cleanSummary by CodeRabbit
Breaking Changes
requestAccess()now requires email parameter (USGA requirement)getAccesses()returns flattened array of golfer/access recordsupdateStatus(),requestAccess(), andrevokeAccess()return standardized success response format