feat: surface optional course fields on score rows#44
Conversation
Pass through course_id, course_name, and facility_name from the GHIN getScores response. These were previously stripped by schemaScore, so consumers had no way to tell which course each score was played at. All three are optional and nullable to stay backward-compatible with responses that omit them. Adds tests covering present, numeric course_id, and absent cases.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesOptional Course Fields in schemaScore
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 #44 +/- ##
=======================================
Coverage ? 95.81%
=======================================
Files ? 54
Lines ? 2533
Branches ? 426
=======================================
Hits ? 2427
Misses ? 106
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 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.
🧹 Nitpick comments (1)
src/client/ghin/models/scores/score.test.ts (1)
53-78: ⚡ Quick winAdd test coverage for null values.
The schema marks
course_id,course_name, andfacility_nameas.nullable(), but the test suite doesn't verify that explicitnullvalues parse correctly. Consider adding a test case:it('accepts null for course fields', () => { const parsed = schemaScore.parse({ ...baseScore, course_id: null, course_name: null, facility_name: null, }) expect(parsed.course_id).toBeNull() expect(parsed.course_name).toBeNull() expect(parsed.facility_name).toBeNull() })This ensures the
.nullable()behavior is validated and distinguishesnull(explicitly absent) fromundefined(field omitted).🤖 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/models/scores/score.test.ts` around lines 53 - 78, Add a new test case to the schemaScore describe block that validates null values are accepted for the course fields. The test should parse an object containing baseScore with course_id, course_name, and facility_name explicitly set to null, then assert that each parsed field is null (not undefined). This distinguishes explicit null values from omitted fields and ensures the nullable schema behavior is properly tested. Add this test case after the 'treats the course fields as optional' test.
🤖 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.
Nitpick comments:
In `@src/client/ghin/models/scores/score.test.ts`:
- Around line 53-78: Add a new test case to the schemaScore describe block that
validates null values are accepted for the course fields. The test should parse
an object containing baseScore with course_id, course_name, and facility_name
explicitly set to null, then assert that each parsed field is null (not
undefined). This distinguishes explicit null values from omitted fields and
ensures the nullable schema behavior is properly tested. Add this test case
after the 'treats the course fields as optional' test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a3d9328e-5c7a-461a-bb4b-26033137da23
📒 Files selected for processing (3)
.changeset/manual-player-course-fields.mdsrc/client/ghin/models/scores/score.test.tssrc/client/ghin/models/scores/score.ts
Address CodeRabbit nitpick on #44 — verify course_id, course_name, and facility_name parse correctly when explicitly null, distinguishing null (present) from undefined (omitted).
|
Addressed CodeRabbit nitpick: added a test verifying |
1 participant
Summary
Passes through three optional course-identifier fields on score rows returned by the GHIN
getScoresendpoint.schemaScorepreviously stripped these, so consumers had no way to tell which course each score was played at.Changes
schemaScorenow surfacescourse_id(string or number),course_name, andfacility_name.nullable().optional()to stay backward-compatible with responses that omit themscore.test.tscovering present, numericcourse_id, and absent casesTesting
bun run test:run— newschemaScoretests pass./scripts/code-quality.sh— biome, lint+tsc, and build all passSummary by CodeRabbit