-
Notifications
You must be signed in to change notification settings - Fork 14
Add ExistPhone resource #424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #424 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 54 +2
Lines 1156 1179 +23
=========================================
+ Hits 1156 1179 +23
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:
|
WalkthroughThis change introduces two new resource classes, Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Note ⚡️ Unit Test Generation - BetaCodeRabbit's unit test generation is now available in Beta! Automatically generate comprehensive unit tests for your code changes, ensuring better test coverage and catching edge cases you might miss. Our AI analyzes your code structure and creates tests that follow best practices and your project's testing patterns. Learn more here, or just try it under ✨ Finishing Touches. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
cuenca/resources/__init__.py (1)
94-137: Critical: ExistPhone missing from resource_classes list.The ExistPhone class is not included in the resource_classes list, which means it won't be registered in the RESOURCES dictionary at line 139. This will cause runtime errors when trying to use the ExistPhone resource.
Add ExistPhone to the resource_classes list:
resource_classes = [ ApiKey, Account, Arpc, BalanceEntry, BillPayment, Card, CardActivation, CardTransaction, CardValidation, CashReference, Clabe, CurpValidation, Commission, Deposit, Endpoint, + ExistPhone, File, FileBatch, Identity, IdentityEvent, KYCValidation, LimitedWallet, LoginToken, Questionnaires, Saving, Session, ServiceProvider, Statement, Transfer, User, UserCredential, UserEvent, UserListsValidation, UserLogin, Verification, WalletTransaction, WhatsappTransfer, Webhook, Platform, PostalCodes, JwtToken, TermsOfService, UserTOSAgreement, ]
🧹 Nitpick comments (1)
cuenca/resources/exist_phone.py (1)
11-12: Consider clearer boolean field name
existis understandable, butexistsoris_registeredconvey the boolean semantics more explicitly and align with typical English usage. If the API contract allows, consider renaming.No action required if the field name is dictated by the upstream JSON.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cuenca/__init__.py(2 hunks)cuenca/resources/__init__.py(2 hunks)cuenca/resources/exist_phone.py(1 hunks)cuenca/version.py(1 hunks)tests/resources/cassettes/test_exist_phone_retrieve.yaml(1 hunks)tests/resources/test_exist_phone.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (3)
cuenca/__init__.py (1)
cuenca/resources/exist_phone.py (1)
ExistPhone(8-12)
cuenca/resources/__init__.py (1)
cuenca/resources/exist_phone.py (1)
ExistPhone(8-12)
cuenca/resources/exist_phone.py (1)
cuenca/resources/base.py (1)
Retrievable(36-50)
🔇 Additional comments (8)
cuenca/version.py (1)
1-1: Version bump looks good for the new ExistPhone resource addition.The version increment from 2.1.6 to 2.1.7 is appropriate for adding a new resource to the public API.
cuenca/__init__.py (2)
48-48: ExistPhone correctly added to public API exports.The addition maintains alphabetical ordering and follows the established pattern for exposing new resources.
68-68: Import correctly positioned and follows existing patterns.The ExistPhone import is properly placed within the resources import block and maintains alphabetical ordering.
cuenca/resources/__init__.py (2)
19-19: ExistPhone correctly added to all exports.The addition maintains alphabetical ordering and follows the established pattern.
62-62: Import correctly uses relative path per coding guidelines.The relative import
from .exist_phone import ExistPhonefollows the coding guidelines for internal modules.tests/resources/test_exist_phone.py (1)
1-12: Test implementation looks good.The test correctly:
- Uses absolute import (acceptable for /tests directory per guidelines)
- Follows the VCR pattern for HTTP interaction recording
- Tests the core functionality of ExistPhone.retrieve()
- Uses appropriate assertions for the resource properties
tests/resources/cassettes/test_exist_phone_retrieve.yaml (1)
1-41: VCR cassette correctly records ExistPhone retrieval interaction.The cassette appropriately captures:
- GET request to the correct endpoint
/exist_phone/+527331256958- Response with the expected JSON structure
{"id":"+527331256958","exist":true}- Proper headers including API version and user agent
- 200 OK status indicating successful retrieval
cuenca/resources/exist_phone.py (1)
8-10: SpecifyClassVartype parameterAnnotate
_resourceasClassVar[str]for full typing information and to keep consistency with the rest of the codebase.- _resource: ClassVar = 'exist_phone' + _resource: ClassVar[str] = 'exist_phone'[ suggest_nitpick ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/resources/test_phone_verification_association.py (1)
6-13: Test implementation is basic but functional.The test correctly uses VCR for HTTP recording and validates the core functionality. However, consider enhancing the test coverage:
- Validate actual values, not just truthiness
- Add error handling test cases
- Test edge cases
Consider expanding the test:
@pytest.mark.vcr def test_phone_verification_association_create(): phone_verification_association = PhoneVerificationAssociation.create( verification_id='VEeCjasQQQM-Hr-odTGoKoQ', ) - assert phone_verification_association.verification_id - assert phone_verification_association.user_id + assert phone_verification_association.verification_id == 'VEeCjasQQWQM-Hr-odTGoKoQ' + assert phone_verification_association.user_id + assert phone_verification_association.created_at + assert isinstance(phone_verification_association.created_at, dt.datetime)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cuenca/__init__.py(3 hunks)cuenca/resources/__init__.py(4 hunks)cuenca/resources/phone_verification_association.py(1 hunks)cuenca/version.py(1 hunks)requirements.txt(1 hunks)tests/resources/cassettes/test_phone_verification_association_create.yaml(1 hunks)tests/resources/test_phone_verification_association.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- requirements.txt
- tests/resources/cassettes/test_phone_verification_association_create.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- cuenca/version.py
- cuenca/init.py
- cuenca/resources/init.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
tests/resources/test_phone_verification_association.pycuenca/resources/phone_verification_association.py
🧬 Code Graph Analysis (2)
tests/resources/test_phone_verification_association.py (1)
cuenca/resources/phone_verification_association.py (2)
PhoneVerificationAssociation(12-26)create(20-26)
cuenca/resources/phone_verification_association.py (1)
cuenca/resources/base.py (2)
Creatable(53-62)_create(55-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: coverage
- GitHub Check: pytest (3.13)
- GitHub Check: pytest (3.9)
- GitHub Check: lint
- GitHub Check: publish-pypi
🔇 Additional comments (4)
tests/resources/test_phone_verification_association.py (1)
3-3: Import follows acceptable pattern.The absolute import from the main
cuencapackage is acceptable per the coding guidelines since this is a test file importing from the main library.cuenca/resources/phone_verification_association.py (3)
1-9: Import structure follows coding guidelines correctly.All imports properly use relative paths for internal modules (
..http,.base) and absolute imports for external libraries (cuenca_validations). The structure adheres to the coding guidelines.
12-17: Class definition follows established patterns.The class properly inherits from
Creatable, uses appropriate snake_case naming for attributes, includes proper type hints, and follows the resource naming convention with the_resourceClassVar.
19-26: Create method implementation is well-structured.The method follows established patterns:
- Proper type hints including return type annotation
- Dependency injection for session parameter
- Uses Pydantic model for request validation
- Leverages inherited
_createmethod fromCreatablebase class- Modern Pydantic
model_dump()usage
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
cuenca-validationsdependency to version 2.1.12.