add UpdateTransferRequest class with validation tests#405
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds UpdateTransferRequest, a new Pydantic request model with a status field typed as TransactionStatus and validated to accept only TransactionStatus.succeeded or TransactionStatus.failed. Exposes the new model from cuenca_validations.types via import and all, adds pytest coverage for valid/invalid/missing/extra-key scenarios, and updates package version to 2.1.35. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #405 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1466 1474 +8
=========================================
+ Hits 1466 1474 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review 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
🤖 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/test_requests.py`:
- Around line 107-124: Remove the three redundant tests that assert Pydantic's
built-in validation: delete the functions
test_update_transfer_request_invalid_action,
test_update_transfer_request_missing_action, and
test_update_transfer_request_forbids_extra from tests/test_requests.py; keep the
existing test_update_transfer_request_valid_action (which documents the allowed
Literal values) and ensure no other tests assert Pydantic's internal behavior
for UpdateTransferRequest or BaseRequest.
🪄 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: cf045293-de48-4560-b32c-e032d750daf8
📒 Files selected for processing (4)
cuenca_validations/types/__init__.pycuenca_validations/types/requests.pycuenca_validations/version.pytests/test_requests.py
| def test_update_transfer_request_invalid_action() -> None: | ||
| with pytest.raises(ValidationError) as ex: | ||
| UpdateTransferRequest.model_validate({'action': 'cancel'}) | ||
| assert 'action' in str(ex.value) | ||
|
|
||
|
|
||
| def test_update_transfer_request_missing_action() -> None: | ||
| with pytest.raises(ValidationError) as ex: | ||
| UpdateTransferRequest.model_validate({}) | ||
| assert 'action' in str(ex.value) | ||
|
|
||
|
|
||
| def test_update_transfer_request_forbids_extra() -> None: | ||
| with pytest.raises(ValidationError) as ex: | ||
| UpdateTransferRequest.model_validate( | ||
| {'action': 'approve', 'foo': 'bar'} | ||
| ) | ||
| assert 'Extra inputs are not permitted' in str(ex.value) |
There was a problem hiding this comment.
Remove tests that validate Pydantic's built-in validators.
These three test functions validate Pydantic's built-in behavior rather than application-specific business logic:
test_update_transfer_request_invalid_action- Tests thatLiteral['approve', 'reject']rejects invalid valuestest_update_transfer_request_missing_action- Tests that required fields raise ValidationError when missingtest_update_transfer_request_forbids_extra- Tests thatextra='forbid'(inherited from BaseRequest) rejects extra keys
As per coding guidelines: "Do not write unit tests specifically for validating the behavior of Pydantic's built-in validators. These are already well-tested by the Pydantic library itself." This is a mandatory rule requiring removal of such tests.
The test_update_transfer_request_valid_action test on lines 100-104 is acceptable as it documents the API contract for valid input values.
♻️ Proposed change to remove redundant tests
-def test_update_transfer_request_invalid_action() -> None:
- with pytest.raises(ValidationError) as ex:
- UpdateTransferRequest.model_validate({'action': 'cancel'})
- assert 'action' in str(ex.value)
-
-
-def test_update_transfer_request_missing_action() -> None:
- with pytest.raises(ValidationError) as ex:
- UpdateTransferRequest.model_validate({})
- assert 'action' in str(ex.value)
-
-
-def test_update_transfer_request_forbids_extra() -> None:
- with pytest.raises(ValidationError) as ex:
- UpdateTransferRequest.model_validate(
- {'action': 'approve', 'foo': 'bar'}
- )
- assert 'Extra inputs are not permitted' in str(ex.value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_update_transfer_request_invalid_action() -> None: | |
| with pytest.raises(ValidationError) as ex: | |
| UpdateTransferRequest.model_validate({'action': 'cancel'}) | |
| assert 'action' in str(ex.value) | |
| def test_update_transfer_request_missing_action() -> None: | |
| with pytest.raises(ValidationError) as ex: | |
| UpdateTransferRequest.model_validate({}) | |
| assert 'action' in str(ex.value) | |
| def test_update_transfer_request_forbids_extra() -> None: | |
| with pytest.raises(ValidationError) as ex: | |
| UpdateTransferRequest.model_validate( | |
| {'action': 'approve', 'foo': 'bar'} | |
| ) | |
| assert 'Extra inputs are not permitted' in str(ex.value) |
🤖 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/test_requests.py` around lines 107 - 124, Remove the three redundant
tests that assert Pydantic's built-in validation: delete the functions
test_update_transfer_request_invalid_action,
test_update_transfer_request_missing_action, and
test_update_transfer_request_forbids_extra from tests/test_requests.py; keep the
existing test_update_transfer_request_valid_action (which documents the allowed
Literal values) and ensure no other tests assert Pydantic's internal behavior
for UpdateTransferRequest or BaseRequest.
…us instead of action with corresponding tests
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e21429. Configure here.
…te related tests for valid and invalid statuses
2 participants
Note
Low Risk
New optional validation schema and tests only; no runtime auth, payment, or data-path changes in this package.
Overview
Introduces
UpdateTransferRequest, a Pydantic request model for patching a transfer’sstatusto a terminal value only:succeededorfailed. OtherTransactionStatusvalues are rejected at validation time. The type is exported from the publiccuenca_validations.typespackage, and the library version is bumped to 2.1.35.Tests cover accepted statuses, rejection of intermediate statuses, required
status, andBaseRequest’s forbid-extra behavior.Reviewed by Cursor Bugbot for commit f554ff4. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit