Conversation
WalkthroughThe changes introduce a new enumeration class, 🪧 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 #371 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1296 1297 +1
=========================================
+ Hits 1296 1297 +1
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
🔭 Outside diff range comments (1)
cuenca_validations/types/requests.py (1)
602-602:⚠️ Potential issueRemove excess blank lines
There are too many blank lines here, causing a linting error.
Fix the spacing issue:
class LimitedWalletRequest(BaseRequest): allowed_curp: Curp allowed_rfc: Optional[Rfc] = None - class PlatformRequest(BaseModel):🧰 Tools
🪛 GitHub Actions: test
[error] 602-602: Flake8: E303 too many blank lines (3)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cuenca_validations/types/enums.py(1 hunks)cuenca_validations/types/requests.py(3 hunks)cuenca_validations/version.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Enforce Relative Imports for Internal Modules
En...
**/*.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_validations/version.pycuenca_validations/types/enums.pycuenca_validations/types/requests.py
`**/*.py`:
Rule: Enforce Snake Case in Python Backend
- N...
**/*.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_validations/version.pycuenca_validations/types/enums.pycuenca_validations/types/requests.py
🪛 GitHub Actions: test
cuenca_validations/types/enums.py
[error] 252-252: Flake8: E302 expected 2 blank lines, found 1
[error] 256-256: Flake8: E302 expected 2 blank lines, found 1
cuenca_validations/types/requests.py
[error] 602-602: Flake8: E303 too many blank lines (3)
🔇 Additional comments (4)
cuenca_validations/version.py (1)
1-1: Version updated to development buildThe version has been incremented from 2.1.3 to 2.1.4.dev0, correctly indicating a development version with the new KYC validation features.
cuenca_validations/types/requests.py (3)
31-31: Import added for new KYCValidationType enumThe import has been correctly added to the existing import block.
426-426:Details
❓ Verification inconclusive
User level maximum constraint reduced from 4 to 3
The maximum value for required_level has been reduced from 4 to 3, which affects how high a user can progress in the system.
This might impact existing users who already have a level of 4. Before deploying, please verify that there are no users with level 4 in the production database:
🏁 Script executed:
#!/bin/bash # Check if there are any users with level 4 that might be affected by this change echo "You should run a query in your production database to check for:" echo "SELECT COUNT(*) FROM users WHERE required_level = 4;" echo "If this count is greater than 0, you'll need a migration plan for these users."Length of output: 423
Action Required: Verify Production Database for Users with Level 4
The change in
cuenca_validations/types/requests.py(line 426) reduces the maximum allowedrequired_levelfrom 4 to 3. This update may affect existing users if any currently have a level of 4. Before deploying, please verify that there are no such users in the production database by running the following query:
SELECT COUNT(*) FROM users WHERE required_level = 4;If the result is greater than 0, you'll need to prepare a migration plan for these users.
623-623:Details
✅ Verification successful
Added validation_type field to KYCValidationRequest
The new field correctly uses the KYCValidationType enum and follows the snake_case naming convention.
This change requires that all API consumers now provide a validation_type. Check if there are existing API clients that need to be updated:
🏁 Script executed:
#!/bin/bash # Search for existing code that calls KYCValidationRequest without validation_type echo "Searching for existing usage of KYCValidationRequest..." rg "KYCValidationRequest\(" -A 5 -B 5Length of output: 805
Action Required: Confirm External API Consumers Update for New Field
I verified that the new
validation_typefield is correctly added to theKYCValidationRequestdeclaration, and a repository-wide search did not reveal any invocations ofKYCValidationRequestomitting this required parameter. This indicates that internal uses are consistent with the change. However, please ensure that any external API consumers (if any) that instantiateKYCValidationRequestare updated to provide a validKYCValidationType.
- File:
cuenca_validations/types/requests.py- Location: Definition of
KYCValidationRequestincludes the new field.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
cuenca_validations/types/requests.py(3 hunks)cuenca_validations/version.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cuenca_validations/version.py
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Enforce Relative Imports for Internal Modules
En...
**/*.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_validations/types/requests.py
`**/*.py`:
Rule: Enforce Snake Case in Python Backend
- N...
**/*.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_validations/types/requests.py
🔇 Additional comments (2)
cuenca_validations/types/requests.py (2)
31-31: Looks good.The new import statement adheres to the relative import guideline and leverages standard PascalCase for the enum class name. No changes needed.
426-426:Details
✅ Verification successful
Verify downstream references to updated max level.
Reducing the maximum allowed
required_levelfrom 4 to 3 may break or alter existing logic or validations that expect a max of 4. Please confirm there are no other references assuming 4.
🏁 Script executed:
#!/bin/bash # Search the repository for references to `required_level` to ensure # no leftover code or tests still assume a maximum of 4 instead of 3. rg -A 5 'required_level'Length of output: 751
Downstream references verified for
required_levelupdate.
The repository search confirms that all tests and relevant code now use a maximum value of 3 forrequired_level, with tests intests/test_types.pyexplicitly passingrequired_level=3. There are no references assuming a maximum of 4.
|
@cuenca-dependabot cierra los comments y vuelve a revisar |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cuenca_validations/types/requests.py (1)
622-622:Details
❓ Verification inconclusive
Consider renaming the validation_flow field for clarity.
The new
validation_flowfield of typeKYCValidationFlowindicates whether validation happens on client or server side, which matches the PR objective. However, previous review comments suggested naming this fieldvalidation_source_typeorsource_typefor better clarity.- validation_flow: KYCValidationFlow + validation_source_type: KYCValidationFlowAlso, remember to add test coverage for this new field, as mentioned in a previous review comment.
🏁 Script executed:
#!/bin/bash # Check for existing tests of KYCValidationRequest with the validation_flow field rg -A 5 -B 5 "KYCValidationRequest" tests/Length of output: 42
Rename Field & Verify Test Coverage
In
cuenca_validations/types/requests.py(line 622), please rename the
validation_flowfield tovalidation_source_typeso that its purpose—indicating whether
validation is performed on the client or server side—is clearer:- validation_flow: KYCValidationFlow + validation_source_type: KYCValidationFlowAdditionally, as noted previously, please ensure that proper tests are added to cover
this new field. Since our initial automated search for tests referencingKYCValidationRequest
returned no hits, kindly verify manually that test coverage for this field is in place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
cuenca_validations/types/__init__.py(2 hunks)cuenca_validations/types/enums.py(1 hunks)cuenca_validations/types/requests.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cuenca_validations/types/init.py
- cuenca_validations/types/enums.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_validations/types/requests.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_validations/types/requests.py
🧬 Code Definitions (1)
cuenca_validations/types/requests.py (1)
cuenca_validations/types/enums.py (1)
KYCValidationFlow(253-255)
🔇 Additional comments (3)
cuenca_validations/types/requests.py (3)
31-31: Imported KYCValidationFlow enum correctly.The addition of
KYCValidationFlowto the import list is necessary to support the newvalidation_flowfield in theKYCValidationRequestclass. This follows the established pattern for importing enums in this file.
426-429: Adjusted validation range for required_level field.The validation constraints for
required_levelhave been changed fromge=-1, le=4toge=1, le=3, which:
- Aligns with the PR objective of setting the maximum allowed level to 3
- Removes the ability to set negative or zero values that weren't being used (as noted in past review comments)
This change appears to address the comment from reviewer felipao-mx questioning the purpose of allowing -1 or 0 values.
426-429: Fix redundant Field annotation pattern.There's a redundancy in using
Fieldboth inside theAnnotatedtype and as the default value. This was mentioned in a previous review comment.Consider simplifying to this pattern for consistency with other fields:
- required_level: Optional[Annotated[int, Field(ge=1, le=3)]] = Field( - None, - description='Maximum level a User can reach. ' 'Defined by platform', - ) + required_level: Optional[int] = Field( + None, + ge=1, + le=3, + description='Maximum level a User can reach. Defined by platform' + )
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
cuenca_validations/types/requests.py(3 hunks)
🧰 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_validations/types/requests.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_validations/types/requests.py
🧬 Code Definitions (1)
cuenca_validations/types/requests.py (1)
cuenca_validations/types/enums.py (1)
KYCValidationFlow(253-255)
🔇 Additional comments (2)
cuenca_validations/types/requests.py (2)
31-31: LGTM: Import of KYCValidationFlow enum.The addition of the
KYCValidationFlowimport is correctly placed and alphabetically ordered within the enums import block.
426-431:Details
❓ Verification inconclusive
Confirm breaking change in required_level range.
The
required_levelfield's constraints have been modified:
- Minimum value changed from
-1to1- Maximum value reduced from
4to3This is a breaking change that could affect existing code expecting values of -1 or 0 to be valid.
🏁 Script executed:
#!/bin/bash # Check for existing usage of required_level with values less than 1 # First, search for direct assignments in the codebase echo "Searching for direct assignments of required_level < 1..." rg -A 2 -B 2 "required_level\s*=\s*(0|-1)" --type py # Next, check for any tests that might verify behavior with these values echo "Searching for tests using required_level < 1..." rg -A 2 -B 2 "required_level.*(0|-1)" tests/ --type pyLength of output: 341
Breaking Change – Confirm Impact on
required_levelConstraintsThe
required_levelfield now enforces a range of 1–3 (changed from a minimum of -1 and a maximum of 4), which is a breaking change. Automated searches did not reveal any direct assignments or tests using values less than 1. However, since this output isn’t fully conclusive, please manually verify that there are no indirect usages or runtime dependencies expecting the legacy range (e.g., dynamic assignments or validations that might allow -1 or 0).
- Confirm no indirect assignments or validations are using legacy values.
- Double-check tests to ensure they do not expect the old range.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
cuenca_validations/types/__init__.py(2 hunks)cuenca_validations/types/enums.py(1 hunks)cuenca_validations/types/requests.py(3 hunks)cuenca_validations/version.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- cuenca_validations/version.py
- cuenca_validations/types/enums.py
- cuenca_validations/types/init.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_validations/types/requests.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_validations/types/requests.py
🧬 Code Definitions (1)
cuenca_validations/types/requests.py (1)
cuenca_validations/types/enums.py (1)
KYCValidationSource(253-255)
🔇 Additional comments (2)
cuenca_validations/types/requests.py (2)
31-31: Relative import ofKYCValidationSourceis correct.Using
..types.enumsensures compliance with the guideline to prefer relative imports for internal modules.
426-430:Details
✅ Verification successful
Range restriction for
required_levelis consistent with PR objectives.The new constraints
ge=1andle=3align with the updated validation rules. However, be sure no legacy logic relies on negative or zero values.Run this script to confirm no references to negative or zero
required_levelremain in the codebase:
🏁 Script executed:
#!/bin/bash rg -A 3 -B 3 "required_level" --type pyLength of output: 819
Validation Range Confirmed for
required_levelThe new constraints (
ge=1andle=3) incuenca_validations/types/requests.py(lines 426-430) align with the updated PR objectives. The output of the verification script confirms that all references (e.g., intests/test_types.py) use valid values (such as3), with no legacy reliance on negative or zero values.
|
|
||
| class KYCValidationRequest(BaseRequest): | ||
| user_id: str | ||
| source_type: KYCValidationSource |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Add coverage for the new source_type field in tests.
The field source_type was added to KYCValidationRequest; ensure it’s validated or tested to confirm correctness.
Execute this script to verify test coverage specifically targeting source_type:
🏁 Script executed:
#!/bin/bash
rg -A 5 "source_type" tests/ --type pyLength of output: 38
Missing Test Coverage for source_type Field
The current grep command revealed no test cases addressing the new source_type field in the KYCValidationRequest type, which means its validation isn’t being explicitly verified in the tests. Please add appropriate tests to confirm that the field functions as intended. For example, include scenarios validating the schema or data that stipulate the proper handling of source_type.
3 participants
KYCVerificationy se unifica conKYCValidationKYCValidationModepara indicar si es validación del lado en el cliente o del server.Summary by CodeRabbit
New Features
Bug Fixes
Chores