docs: note None area levels on emergency requirements - #44
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 (1)
🚧 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; 4 remain after this review. 📝 WalkthroughWalkthroughThe emergency requirement documentation, fixtures, and tests now cover nullable business area levels and decimal-string pricing metadata. Related fixtures normalize enum values and include an activation timestamp. ChangesEmergency requirement metadata
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized documentation, model metadata, and fixture update introduces no actionable merge-blocking risk and is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
6c7fa08 to
d60b616
Compare
|
@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.
🟢 Approval recommended
Changes are limited to documentation, tests, and fixtures/cassettes and appear consistent with existing SDK parsing behavior.
Pull request overview
Updates SDK documentation and test fixtures to reflect current/expected API payload semantics for emergency requirements (nullable area levels and per-resource pricing meta), ensuring recorded VCR cassettes exercise those cases.
Changes:
- Extend
EmergencyRequirementdocstring to document nullablepersonal_area_level/business_area_leveland decimal-string pricing meta. - Update emergency requirement resource test to assert
business_area_level is Noneand thatmeta.setup_price/meta.monthly_priceare strings. - Refresh VCR cassettes to include resource-level
metaand align enum-like fields with API values (e.g.,any,world_wide,area).
File summaries
| File | Description |
|---|---|
| tests/resources/test_emergency_requirement.py | Adds assertions for nullable business area level and string pricing meta. |
| tests/fixtures/emergency_requirements/show.yaml | Updates show cassette to include business_area_level: null, empty business fields, and resource meta prices. |
| tests/fixtures/emergency_requirements/list.yaml | Updates list cassette similarly, including per-resource meta prices. |
| tests/fixtures/emergency_calling_services/show_with_includes.yaml | Normalizes included emergency requirement area-level values to world_wide. |
| tests/fixtures/address_requirements/list.yaml | Normalizes enum-like fields to lowercase/snake_case values consistent with SDK enums. |
| src/didww/resources/emergency_requirement.py | Docstring clarifies nullable area levels and decimal-string pricing meta. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d60b616 to
2a38c5d
Compare
There was a problem hiding this comment.
🟢 Approval recommended
Changes are limited to fixtures/tests and docstring clarification, and the updated fixtures align with existing enum values and per-resource meta handling in the SDK.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
A country that accepts only one kind of identity leaves the other area level unset and the API returns null. The cassettes now hold such a requirement and carry the resource meta, so both the None level and the decimal-string prices are exercised. The requirement records in the cassettes also drop values the API cannot return: it answers `any`, not `Any`, and a personal or business area level is only ever `world_wide` or `country`.
2a38c5d to
4b858e0
Compare
|
What
EmergencyRequirement's docstring sayspersonal_area_level/business_area_levelareNonewhen the country does not accept that kind of identity, and that the meta prices aredecimal strings. The cassettes now hold such a requirement and carry the resource meta, so both
the
Nonelevel and the string prices are exercised.The requirement records in the cassettes also drop values the API cannot return: it answers
any, notAny, and a personal or business area level is only everworld_wideorcountry.No code change:
SafeAttributeFieldandto_enumalready passNonethrough.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
uvis not installed locally, so pytest runs here in CI.Summary by CodeRabbit
Documentation
Bug Fixes