Align endpoint error JSON to Weblate/DRF conventions#165
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughError serialization, serializer validation, and view handling now emit DRF-style ChangesDRF-standard error payload refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/endpoint/test_errors.py (1)
145-165: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest locks in the
attr_for_fieldtrailing-dot bug.
errors[0]["attr"] == "add_or_update."asserts the malformed trailing-dot output produced byattr_for_fieldwhenlanguage=""(see comment onerrors.pylines 62-73). Once that helper is fixed to treat empty-string language as absent, this assertion should be updated to"add_or_update".🤖 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 `@tests/endpoint/test_errors.py` around lines 145 - 165, The test in boost_validation_errors_builds_list is asserting the malformed trailing-dot attr produced by attr_for_field when language is empty. Update the expectation for the INVALID_LANGUAGE_CODE case so it matches the fixed behavior of attr_for_field in errors.py, using "add_or_update" instead of "add_or_update.", while keeping the INVALID_SUBMODULE_LIST assertion unchanged.
🧹 Nitpick comments (2)
tests/endpoint/test_errors.py (1)
167-190: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing coverage for raw DRF error codes not present in
BoostEndpointErrorCode.This test only uses codes already defined on the enum (
required_field,invalid_submodule_list). It doesn't cover the case where anErrorDetail.codeis a native DRF code (e.g.required,invalid) not in the enum, which would raiseValueErrorinsideflatten_validation_detail(see the corresponding comment inerrors.py). Consider adding a case with anErrorDetail(..., code="required")to catch this regression once addressed.🤖 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 `@tests/endpoint/test_errors.py` around lines 167 - 190, Add coverage in test_flatten_validation_detail_matches_drf_shape for a raw DRF ErrorDetail.code such as "required" or "invalid" that is not present in BoostEndpointErrorCode. Update the existing flatten_validation_detail test in test_errors.py to include one such ErrorDetail so the behavior in errors.py is exercised and the regression where enum casting raises ValueError is caught.tests/endpoint/test_serializers.py (1)
340-350: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for non-string
extensionselements.Only the "extensions is a dict" case is tested here. A case like
"extensions": ["ok", 1]would exercisevalidate_extensions's non-string-element branch (serializers.py Line 234-237), which currently maps to the wrong Boost code (INVALID_SUBMODULE_LISTinstead of an extensions-specific code) — see the related comment on serializers.py. Adding this test would have caught that.🤖 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 `@tests/endpoint/test_serializers.py` around lines 340 - 350, Add a test in test_add_or_update_serializer_rejects_extensions_dict or a new test in test_serializers for AddOrUpdateRequestSerializer that passes a non-string element inside extensions, such as a mixed list, and asserts validation fails with an extensions error. Use this to cover the validate_extensions branch that checks element types, and verify the standardized error is attributed to extensions rather than only testing the dict-shape rejection.
🤖 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/boost_weblate/endpoint/errors.py`:
- Around line 62-73: Fix attr_for_field so an empty language string is treated
like no language and does not append a trailing dot; update the language check
in attr_for_field accordingly and keep the existing behavior only when language
has a real value. Also update the related boost_validation_errors expectation in
test_errors.py so the empty-language case asserts the base field path rather
than the malformed dotted attr.
In `@src/boost_weblate/endpoint/serializers.py`:
- Around line 234-237: The `extensions` validation branch is reusing the wrong
DRF error code and enum form, causing non-string extension items to map to the
same Boost code as a non-list language value. Update the serializer logic around
`extensions` so it uses the string-valued DRF code consistently and ensure
`_code_for_drf_error` differentiates by field instead of mapping `NOT_A_LIST`
unconditionally. Check the `extensions` validation path in `serializers.py` and
add/adjust a test to assert the returned Boost error code for non-string
extension elements is field-appropriate.
---
Outside diff comments:
In `@tests/endpoint/test_errors.py`:
- Around line 145-165: The test in boost_validation_errors_builds_list is
asserting the malformed trailing-dot attr produced by attr_for_field when
language is empty. Update the expectation for the INVALID_LANGUAGE_CODE case so
it matches the fixed behavior of attr_for_field in errors.py, using
"add_or_update" instead of "add_or_update.", while keeping the
INVALID_SUBMODULE_LIST assertion unchanged.
---
Nitpick comments:
In `@tests/endpoint/test_errors.py`:
- Around line 167-190: Add coverage in
test_flatten_validation_detail_matches_drf_shape for a raw DRF ErrorDetail.code
such as "required" or "invalid" that is not present in BoostEndpointErrorCode.
Update the existing flatten_validation_detail test in test_errors.py to include
one such ErrorDetail so the behavior in errors.py is exercised and the
regression where enum casting raises ValueError is caught.
In `@tests/endpoint/test_serializers.py`:
- Around line 340-350: Add a test in
test_add_or_update_serializer_rejects_extensions_dict or a new test in
test_serializers for AddOrUpdateRequestSerializer that passes a non-string
element inside extensions, such as a mixed list, and asserts validation fails
with an extensions error. Use this to cover the validate_extensions branch that
checks element types, and verify the standardized error is attributed to
extensions rather than only testing the dict-shape rejection.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12e240e6-fbcf-47c3-9874-627085bf42dc
📒 Files selected for processing (7)
src/boost_weblate/endpoint/errors.pysrc/boost_weblate/endpoint/serializers.pysrc/boost_weblate/endpoint/tasks.pysrc/boost_weblate/endpoint/views.pytests/endpoint/test_errors.pytests/endpoint/test_serializers.pytests/endpoint/test_views.py
|
Location: Issue: The doc still shows HTTP 400 as Recommendation: Update the 400 JSON example, the 400 table, and the Error handling section to distinguish HTTP validation ( |
|
Location: Issue: The integration test only checks Recommendation: Mirror the unit test assertions on |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/postgres/test_endpoint_trust_boundary.py`:
- Around line 46-52: The test only verifies that INVALID_CLONE_URL appears, but
it does not check the matching error’s attr path. Update the assertions in the
response validation test to locate the error entry for
BoostEndpointErrorCode.INVALID_CLONE_URL.value and assert its attr is the
expected organization path, using the existing response.data["errors"] structure
and the code/attr fields.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f7138bb-f212-4911-8588-67a68e3b7c6a
📒 Files selected for processing (2)
docs/boost-endpoint-api.mdtests/postgres/test_endpoint_trust_boundary.py
3 participants
Close #161.
Summary by CodeRabbit
code,detail, and optionalattr, including language-scoped, flattened field paths.standardized_errors.detail/attrshape.