docs: emergency requirement meta and area levels as the API sends them - #60
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change updates emergency requirement documentation, fixtures, and tests. It documents decimal-string pricing, numeric ChangesEmergency requirement metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR updates emergency-requirement documentation and test fixtures to match the API’s price and nullable area-level representations; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@Fivell could you take this one? I cannot add you as a reviewer myself — the branch comes from a fork and I have no write access on the org repos, so a review request has to be set by someone who does. This is one of seven SDK pull requests from the same API change (
Worth knowing before merging: the |
There was a problem hiding this comment.
🟡 Changes recommended
The Unreleased changelog contains contradictory statements about the current setup_price wire format.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates emergency requirement documentation and fixtures to match current API wire formats.
Changes:
- Documents nullable identity area levels and decimal-string prices.
- Updates fixtures while retaining numeric-price coercion coverage.
- Expands deserialization assertions.
File summaries
| File | Description |
|---|---|
EmergencyRequirement.java |
Clarifies API response formats. |
EmergencyRequirementTest.java |
Tests updated formats and nullability. |
index.json |
Updates list fixture. |
show.json |
Updates detail fixture. |
CHANGELOG.md |
Records response-format changes. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
07fbd2e to
0e704f6
Compare
0e704f6 to
1cd34fa
Compare
…sends them
The class documented setup_price as the JSON number 0 read back as "0". The
server side of that mismatch is fixed: a priced requirement now returns the
decimal string "0.0", like every other price in the API. MetaMap keeps
coercing the numeric form, so both wire shapes are read back as strings, and
one fixture holds the numeric form to guard that.
The area level attributes are documented as null when the country does not
accept that kind of identity, which the fixtures now exercise. No code change
there: the fields are String and have always read a null back as null. The
fixtures also drop area level values the API cannot return ("region").
1cd34fa to
442f39d
Compare
|
What
EmergencyRequirement's Javadoc saidsetup_price"is sent as the JSON number 0 and is readback as the string "0"". The server side of that mismatch is fixed, so a priced requirement
now returns
"0.0". The class documents that, and documents the area level attributes as nullwhen the country does not accept that kind of identity.
No code change:
MetaMap(added in #58) already coerces both wire forms, and the area levelfields are
String. The fixtures carry the new wire form, with one entry keeping the numericsetup_priceas a coercion guard, and drop area level values the API cannot return (region).Background
Two things prompted this, both on the server side:
GET /v3/emergency_requirementsreturnedmeta.setup_priceas the JSON number0whilemeta.monthly_priceon the same object was a decimal string — the mismatch behind the JavaSDK's
ClassCastException. The API now returns"0.0", so both meta prices are decimalstrings.
personal_area_levelandbusiness_area_levelare nullable on bothemergency_requirementsandaddress_requirements. A country that accepts only one kind of identity leaves the otherlevel unset.
The
setup_pricechange (1) is merged on the API side but has not been released toproduction; the release is expected shortly. Until then the endpoint still sends the JSON
number
0, which is why nothing here rejects the numeric form.The area level nullability (2) is not a new behaviour — the API has always returned null
there and does so on production today (
AL/Localis business-only and returns"personal_area_level": null). Only the published document was wrong about it.Verification
./gradlew testand./gradlew javadocare green locally (the 4 javadoc warnings are pre-existing, in other files).Summary by CodeRabbit
Documentation
Tests