Changes in KYC Validation #419
Conversation
WalkthroughThe changes involve the removal of the Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #419 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 50 49 -1
Lines 1124 1103 -21
=========================================
- Hits 1124 1103 -21
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
🧹 Nitpick comments (1)
cuenca/resources/kyc_validations.py (1)
17-17: Typo in class attribute name.There's a typo in the class attribute:
attempsshould beattempts. While this is pre-existing code and not part of the current PR, it would be good to fix it in a future update.- attemps: Optional[int] = None + attempts: Optional[int] = None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cuenca/__init__.py(0 hunks)cuenca/resources/__init__.py(0 hunks)cuenca/resources/kyc_validations.py(3 hunks)cuenca/resources/kyc_verifications.py(0 hunks)cuenca/version.py(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/resources/cassettes/test_kyc_verification_create.yaml(0 hunks)tests/resources/cassettes/test_kyc_verification_retrieve.yaml(0 hunks)tests/resources/cassettes/test_kyc_verification_update.yaml(0 hunks)tests/resources/test_kyc_validations.py(1 hunks)tests/resources/test_kyc_verifications.py(0 hunks)
💤 Files with no reviewable changes (7)
- cuenca/init.py
- tests/resources/test_kyc_verifications.py
- tests/resources/cassettes/test_kyc_verification_create.yaml
- tests/resources/cassettes/test_kyc_verification_retrieve.yaml
- cuenca/resources/init.py
- tests/resources/cassettes/test_kyc_verification_update.yaml
- cuenca/resources/kyc_verifications.py
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Enforce Relative Imports for Internal Modules
Ensure that any imports referencing internal modules use relative paths. However, if modules reside in the main module dir...
**/*.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).
setup.pytests/resources/test_kyc_validations.pycuenca/version.pycuenca/resources/kyc_validations.py
`**/*.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...
**/*.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.
setup.pytests/resources/test_kyc_validations.pycuenca/version.pycuenca/resources/kyc_validations.py
🔇 Additional comments (7)
cuenca/version.py (1)
1-1: Version update looks appropriate.The version bump from '2.1.2' to '2.1.3' aligns with the changes described in the PR objectives, which involve removing the
KYCVerificationcomponent, unifying it withKYCValidation, and introducing theKYCValidationTypeenumeration.requirements.txt (1)
2-2: Consider using a stable version instead of a development release.The dependency on
cuenca-validations==2.1.4.dev1uses a development version which might contain unstable features. Development releases are typically not recommended for production environments.Is there a specific reason for using a development version? Is there a timeline for when a stable version (without the
.dev1suffix) will be available?tests/resources/test_kyc_validations.py (2)
2-2: Import ofKYCValidationTypecorrectly added.The import of
KYCValidationTypefromcuenca_validations.typesaligns with the PR objective of introducing this enumeration to specify the validation type.
9-12: Test correctly updated to includevalidation_typeparameter.The
KYCValidation.createmethod now includes the newvalidation_typeparameter set toKYCValidationType.background, which aligns with the PR objectives. This test now validates that KYC validations can be classified by type.Note that the test still expects a
verification_idon theKYCValidationobject, which suggests that although theKYCVerificationcomponent is removed as mentioned in the PR objectives, the concept of a verification ID is still maintained within theKYCValidationclass.cuenca/resources/kyc_validations.py (3)
3-7: Import change looks good.The addition of
KYCValidationTypefromcuenca_validations.typesis appropriate for the new parameter in thecreatemethod.
47-47: Correct implementation of validation_type parameter.The parameter is correctly passed to the
KYCValidationRequestconstructor, maintaining consistency with the method signature change.
38-38:Details
✅ Verification successful
New parameter addition is appropriate.
Adding the
validation_typeparameter aligns with the PR objective of unifying KYC validation and verification. This parameter will help specify whether validation occurs on the client side or server side.
🏁 Script executed:
#!/bin/bash # Search for all existing calls to KYCValidation.create to ensure they're updated with the new required parameter rg "KYCValidation.create\(" --type py rg "KYCValidation\.create\(" --type pyLength of output: 276
Approval: New parameter addition verified and aligned with PR objectives.
The new
validation_type: KYCValidationTypeparameter in cuenca/resources/kyc_validations.py has been correctly added. I ran a search for calls toKYCValidation.create(and found one occurrence in tests/resources/test_kyc_validations.py. It appears that the test call is the only call in the codebase and has been updated accordingly. No issues were detected regarding outdated call sites or missing parameter updates.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cuenca/resources/kyc_validations.py (1)
13-13: Minor typo in existing field name.There's a typo in the field name
attemps(should beattempts). While not directly related to your current changes, it would be good to fix this in a future PR.- attemps: Optional[int] = None + attempts: Optional[int] = None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (5)
cuenca/resources/kyc_validations.py(2 hunks)cuenca/version.py(1 hunks)requirements.txt(1 hunks)setup.py(1 hunks)tests/resources/test_kyc_validations.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- setup.py
- cuenca/version.py
- requirements.txt
- tests/resources/test_kyc_validations.py
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Enforce Relative Imports for Internal Modules
Ensure that any imports referencing internal modules use relative paths. However, if modules reside in the main module dir...
**/*.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).
cuenca/resources/kyc_validations.py
`**/*.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...
**/*.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.
cuenca/resources/kyc_validations.py
🔇 Additional comments (3)
cuenca/resources/kyc_validations.py (3)
3-3: Import statement correctly updated to includeKYCValidationFlow.The import statement has been updated to include
KYCValidationFlowfromcuenca_validations.types, which aligns with the PR objectives of introducing an enumeration to specify validation flow type. The use of absolute imports for external modules follows the coding guidelines.
34-35: Method signature properly updated withvalidation_flowparameter.The addition of the
validation_flowparameter of typeKYCValidationFlowaligns with the PR objectives. The parameter follows snake_case naming convention as required by the coding guidelines. The parameter placement before the optional parameters is appropriate.
41-41: Request instantiation correctly updated to usevalidation_flow.The
KYCValidationRequestinstantiation has been properly updated to include thevalidation_flowparameter, replacing the previously existingdocumentsparameter as intended in the PR objectives.
KYCVerificationy se unifica conKYCValidationKYCValidationFlowpara indicar si es validación del lado en el cliente o del server.Summary by CodeRabbit
cuenca-validations==2.1.5.