Conversation
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1391 1394 +3
=========================================
+ Hits 1391 1394 +3
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: 0
🧹 Nitpick comments (1)
cuenca_validations/types/identities.py (1)
111-127: Avoid Union[PhoneNumber, str] in responses; pick a single, well-defined type.
Union here yields redundant oneOf schema (both serialize to strings) and weakens validation. Prefer a single type for clearer OpenAPI and consistency.Apply either:
- Option A (preferred): keep strong typing with PhoneNumber.
-class BeneficiaryResponse(BaseModel): - name: str - birth_date: dt.date - phone_number: Union[PhoneNumber, str] +class BeneficiaryResponse(BaseModel): + name: str + birth_date: dt.date + phone_number: PhoneNumber
- Option B: if legacy data may be non‑E.164, switch to plain str and drop Union.
-from typing import Annotated, Optional, Union +from typing import Annotated, Optional @@ - phone_number: Union[PhoneNumber, str] + phone_number: strI can add a field serializer if you want to guarantee string output while validating with PhoneNumber.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cuenca_validations/types/__init__.py(2 hunks)cuenca_validations/types/identities.py(2 hunks)cuenca_validations/types/requests.py(2 hunks)cuenca_validations/version.py(1 hunks)tests/test_types.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_types.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:
cuenca_validations/types/__init__.pycuenca_validations/version.pycuenca_validations/types/requests.pycuenca_validations/types/identities.py
🧬 Code graph analysis (1)
cuenca_validations/types/__init__.py (1)
cuenca_validations/types/identities.py (1)
BeneficiaryResponse(111-127)
🔇 Additional comments (6)
cuenca_validations/version.py (1)
1-1: Version bump to 2.1.17 — LGTM.cuenca_validations/types/requests.py (2)
498-533: UserUpdateRequest now inherits BaseRequest — confirm no reliance on extras or None emission.
Same concerns as above; updates frequently include optional fields and sometimes stray keys.If needed, we can tailor a per-model model_dump override or relax extras; say the word and I’ll draft it.
448-496: No downstream break — BaseRequest defaults are intentional. BaseRequest already sets extra="forbid" and its model_dump defaults to exclude_none=True and exclude_unset=True (cuenca_validations/types/requests.py:99-105); tests exercise UserRequest serialization (tests/test_types.py:326-328) and a password-specific override preserves None where needed (cuenca_validations/types/requests.py:176-181). No callers found that rely on permissive extras or on None being emitted.cuenca_validations/types/identities.py (1)
2-2: Importing Union for BeneficiaryResponse — OK.cuenca_validations/types/__init__.py (2)
17-18: Expose BeneficiaryResponse in all — LGTM.
189-192: Re-export BeneficiaryResponse — LGTM.
| ) | ||
|
|
||
|
|
||
| class BeneficiaryResponse(BaseModel): |
There was a problem hiding this comment.
no convendrá que este se llame Beneficiary y el de arriba se renombre a BeneficiaryRequest? este validator sería el que usa en el resource de cuenca-python y el otro solo para los requests en el endpoint de identify y en el User.create or User.update de cuenca-python
There was a problem hiding this comment.
va, suena más natural
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cuenca_validations/types/requests.py (2)
498-506: Validate beneficiaries with the request schema (avoid lenient phone parsing on writes).
UserUpdateRequestshould consumeBeneficiaryRequest, notBeneficiary(which allows rawstrphone numbers).- beneficiaries: Optional[list[Beneficiary]] = None + beneficiaries: Optional[list[BeneficiaryRequest]] = NoneAdd missing import:
# at the existing imports from .identities from .identities import (..., BeneficiaryRequest, ...)
593-607: Broken recipient validator uses builtintypeand wrong signature.The validator references Python’s builtin
typeinstead of the model field and ignores Pydantic v2’sValidationInfo, causing incorrect coercion (emails treated as phone numbers).Apply this fix:
- @field_validator('recipient') - def validate_sender(cls, recipient: str, values): - return ( - EmailStr(recipient) - if type == VerificationType.email - else PhoneNumber(recipient) - ) + @field_validator('recipient') + @classmethod + def validate_recipient(cls, recipient: str, info: 'ValidationInfo'): + # Ensure recipient matches the declared type + t = (getattr(info, 'data', None) or {}).get('type') + if t == VerificationType.email: + return EmailStr(recipient) + if t == VerificationType.phone: + return PhoneNumber(recipient) + return recipientAnd add the missing import:
# augment existing pydantic imports from pydantic import (..., ValidationInfo, ...)Optional (stronger guarantee after both fields parsed):
@model_validator(mode='after') def _match_recipient_type(self): if self.type == VerificationType.email and not isinstance(self.recipient, EmailStr): raise ValueError('recipient must be an email when type=email') if self.type == VerificationType.phone and not isinstance(self.recipient, PhoneNumber): raise ValueError('recipient must be a phone number when type=phone') return self
♻️ Duplicate comments (1)
cuenca_validations/types/identities.py (1)
111-128: Use strict int and confine lenient phone parsing to read-only contexts.
- Make
percentagestrict (same rationale as above).- If
Beneficiaryis intended for legacy/read models, ensure request payloads don’t use it; otherwiseUnion[PhoneNumber, str]will accept invalid phone numbers in write paths.The rename aligns with prior feedback. If this model is read-only, confirm all request models use
BeneficiaryRequest.- phone_number: Union[PhoneNumber, str] - percentage: Annotated[int, Field(ge=1, le=100)] + phone_number: Union[PhoneNumber, str] + percentage: Annotated[int, Field(strict=True, ge=1, le=100)]
🧹 Nitpick comments (2)
cuenca_validations/types/identities.py (1)
92-109: Add strict int validation for percentage.Prevent float-to-int coercion (e.g., 33.3 -> 33) by making the field strict.
- percentage: Annotated[int, Field(ge=1, le=100)] + percentage: Annotated[int, Field(strict=True, ge=1, le=100)]cuenca_validations/types/requests.py (1)
518-526: Percentage sum validator: consider strict int to avoid float coercion.If upstream sends floats, Pydantic may coerce to int before summing. If that’s undesirable, enforce strict ints on the model type (see identities comment).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cuenca_validations/types/__init__.py(2 hunks)cuenca_validations/types/identities.py(3 hunks)cuenca_validations/types/requests.py(7 hunks)tests/test_types.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cuenca_validations/types/init.py
- tests/test_types.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:
cuenca_validations/types/requests.pycuenca_validations/types/identities.py
🔇 Additional comments (8)
cuenca_validations/types/identities.py (1)
2-2: No issues with added import.
Unionis necessary for the newBeneficiarytype.cuenca_validations/types/requests.py (7)
333-366: Switch to BaseRequest looks good.Serialization defaults via
BaseRequest.model_dump()are appropriate; tests were updated accordingly.
442-446: LGTM on BaseRequest adoption.No functional concerns.
448-496: LGTM on BaseRequest and validators.No issues spotted.
617-626: LGTM.Constraints and example are correct.
658-660: LGTM.Moving to
BaseRequestis consistent with the rest of the API.
662-717: LGTM.Validator logic remains sound; BaseRequest adoption OK.
761-763: LGTM.No issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cuenca_validations/types/requests.py (1)
593-607: Broken validator: wrong signature and use of built-intype.
- The field validator uses a v1-style signature and an undefined
valuesparam.- It compares against the built-in
typeinstead of the model field, so the branch is always the else-case.Apply both diffs:
- Import ValidationInfo:
from pydantic import ( BaseModel, ConfigDict, EmailStr, Field, GetCoreSchemaHandler, StrictStr, StringConstraints, field_validator, model_validator, + ValidationInfo, )
- Fix the validator:
- @field_validator('recipient') - def validate_sender(cls, recipient: str, values): - return ( - EmailStr(recipient) - if type == VerificationType.email - else PhoneNumber(recipient) - ) + @field_validator('recipient') + @classmethod + def validate_recipient(cls, recipient: str, info: ValidationInfo): + vtype = (info.data or {}).get('type') + if vtype == VerificationType.email: + return EmailStr(recipient) + return PhoneNumber(recipient)Also applies to: 608-615
🧹 Nitpick comments (3)
cuenca_validations/types/requests.py (3)
448-481: Fix example value for profession.The example uses 'engineer', which isn't in the Profession enum. Suggest using 'profesionista' (or any valid enum value).
- 'profession': 'engineer', + 'profession': 'profesionista',
503-506: Beneficiaries switched to BeneficiaryRequest — verify nested extra handling.Top-level BaseRequest forbids extras, but BeneficiaryRequest (in identities.py) allows extras by default in Pydantic v2. If you want nested strictness, set extra='forbid' in BeneficiaryRequest.
521-526: Tighten validator signature and return type.Defaulting the validator arg to None is unnecessary and obscures intent. Add an explicit return type.
- def beneficiary_percentage( - cls, beneficiaries: Optional[list[BeneficiaryRequest]] = None - ): + def beneficiary_percentage( + cls, beneficiaries: Optional[list[BeneficiaryRequest]] + ) -> Optional[list[BeneficiaryRequest]]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cuenca_validations/types/requests.py(9 hunks)
🧰 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:
cuenca_validations/types/requests.py
🧬 Code graph analysis (1)
cuenca_validations/types/requests.py (2)
cuenca_validations/types/identities.py (2)
BeneficiaryRequest(92-108)AddressRequest(73-89)cuenca_validations/types/enums.py (1)
Profession(693-707)
🔇 Additional comments (7)
cuenca_validations/types/requests.py (7)
76-82: Relative import for BeneficiaryRequest is correct.Good adherence to the relative import guideline. No action needed.
442-446: UserTOSAgreementRequest now uses BaseRequest — OK.Consistent with the new request-base.
617-625: VerificationAttemptRequest moved to BaseRequest — OK.Consistent with the request-base pattern.
658-660: BankAccountValidationRequest moved to BaseRequest — OK.No issues spotted.
662-717: UserListsRequest moved to BaseRequest — OK.Validator logic remains clear and aligns with guidelines; shallow dict access via .get is fine here.
761-763: PhoneVerificationAssociationRequest moved to BaseRequest — OK.Looks good.
333-366: Action: Confirm BaseRequest switch (extra='forbid' and changed model_dump defaults); add release note or revert
- Evidence: CurpValidationRequest defined at cuenca_validations/types/requests.py:333; instantiations only found in tests at tests/test_types.py (lines 383, 396, 404, 412, 418).
- Action: Verify external API consumers for breaking-change risk. If intentional: publish a release note and update tests/clients that rely on accepting unknown fields or previous model_dump exclude_none behavior. If unintentional: revert the base class or set Config to allow extra and restore prior model_dump semantics.
…ved structure and clarity
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cuenca_validations/types/identities.py (2)
92-97: Tighten string fields to prevent empty values.Use our NonEmptyStr for name and user_relationship to avoid accepting empty strings.
Apply:
class BaseBeneficiary(BaseModel): - name: str + name: NonEmptyStr birth_date: dt.date - user_relationship: str + user_relationship: NonEmptyStr percentage: Annotated[int, Field(ge=1, le=100)]
98-106: Fix example: phone_number no longer belongs to BaseBeneficiary.Remove phone_number from the BaseBeneficiary example (it now lives in child classes).
json_schema_extra={ "example": { "name": "Juan Perez", "birth_date": "1907-07-06", - "phone_number": "+525500998877", "user_relationship": "friend", "percentage": 100, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cuenca_validations/types/identities.py(3 hunks)
🧰 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:
cuenca_validations/types/identities.py
🔇 Additional comments (3)
cuenca_validations/types/identities.py (3)
2-2: Imports look good.Typing additions are minimal and consistent with existing style; relative imports remain compliant with our guidelines.
111-113: Good: strict phone validation for requests.BeneficiaryRequest using PhoneNumber leverages built-in validators as required. 👍
115-117: Document intended leniency for Beneficiary.phone_numberUnion[PhoneNumber, str] allows non‑E.164 strings — if this is intentional for legacy persisted data, keep it and add a model example; otherwise restrict to PhoneNumber.
Location: cuenca_validations/types/identities.py (class Beneficiary, lines ~115-116)
class Beneficiary(BaseBeneficiary): phone_number: Union[PhoneNumber, str] + model_config = ConfigDict( + json_schema_extra={ + "example": { + "name": "Juan Perez", + "birth_date": "1907-07-06", + "phone_number": "+525500998877", + "user_relationship": "friend", + "percentage": 100, + } + } + )
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
cuenca_validations/types/identities.py (1)
92-108: Fix example in BaseBeneficiary: it shows phone_number which isn’t a field here.
This misleads OpenAPI consumers and docs.Apply this diff to remove the incorrect key:
model_config = ConfigDict( json_schema_extra={ "example": { "name": "Juan Perez", "birth_date": "1907-07-06", - "phone_number": "+525500998877", "user_relationship": "friend", "percentage": 100, } } )cuenca_validations/types/requests.py (3)
333-366: Migration to BaseRequest is fine; tighten required-fields check when manual_curp is absent.
Currently it only checks for missing keys, not empty/None values.Apply this diff inside validate_manual_curp:
- missing = [r for r in required if r not in values.keys()] + missing = [r for r in required if not values.get(r)] if not manual_curp and missing: raise ValueError(f'values required: {",".join(missing)}')
597-611: Broken recipient validator; also duplicates built-ins.
- The field validator signature is invalid (second param isn’t supported in v2).
- It uses the built-in name type instead of the field value.
- Union[EmailStr, PhoneNumber] already handles parsing; add a model-level check to enforce consistency with type.
Apply this diff:
@@ class VerificationRequest(BaseRequest): @@ model_config = ConfigDict( str_strip_whitespace=True, json_schema_extra={ 'example': { 'type': 'email', 'recipient': 'user@example.com', } }, ) - @field_validator('recipient') - def validate_sender(cls, recipient: str, values): - return ( - EmailStr(recipient) - if type == VerificationType.email - else PhoneNumber(recipient) - ) + @model_validator(mode='after') + def _check_recipient_matches_type(self) -> 'VerificationRequest': + if self.type == VerificationType.email and not isinstance(self.recipient, EmailStr): + raise ValueError('recipient must be an email when type=email') + if self.type != VerificationType.email and not isinstance(self.recipient, PhoneNumber): + raise ValueError('recipient must be a phone number when type=phone') + return self
174-189: Only-one-property check is incorrect for falsy values (e.g., is_active=False).
False isn’t counted, allowing multiple updates unintentionally; also doesn’t enforce “at least one”.Apply this diff:
class UserCredentialUpdateRequest(BaseRequest): @@ @model_validator(mode="before") @classmethod def check_one_property_at_a_time(cls, values: DictStrAny) -> DictStrAny: - not_none_count = sum(1 for val in values.values() if val) - if not_none_count > 1: + not_none_count = sum(1 for val in values.values() if val is not None) + if not_none_count > 1: raise ValueError('Only one property can be updated at a time') + if not_none_count == 0: + raise ValueError('At least one property must be provided') return values
🧹 Nitpick comments (3)
cuenca_validations/types/identities.py (1)
111-113: Optional: Provide a correct example on Beneficiary.
This helps schema consumers.For example:
class Beneficiary(BaseBeneficiary): phone_number: Union[PhoneNumber, str] model_config = ConfigDict( json_schema_extra={ "example": { "name": "Juan Perez", "birth_date": "1907-07-06", "phone_number": "+525500998877", "user_relationship": "friend", "percentage": 100, } } )cuenca_validations/types/requests.py (2)
448-481: LGTM; minor naming nit on validator.
The curp validator is named validate_birth_date; consider renaming for clarity.- @field_validator('curp') - @classmethod - def validate_birth_date(cls, curp: Optional[Curp]) -> Optional[Curp]: + @field_validator('curp') + @classmethod + def validate_curp_age(cls, curp: Optional[Curp]) -> Optional[Curp]: if curp: validate_age_requirement(curp) return curp
522-529: Percentage validator is OK; consider edge-case messaging.
Optional: include actual total in error to aid debugging.- if beneficiaries and sum(b.percentage for b in beneficiaries) != 100: - raise ValueError('The total percentage should be 100%') + if beneficiaries: + total = sum(b.percentage for b in beneficiaries) + if total != 100: + raise ValueError(f'The total percentage should be 100%, got {total}%')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cuenca_validations/types/__init__.py(2 hunks)cuenca_validations/types/identities.py(3 hunks)cuenca_validations/types/requests.py(9 hunks)
🧰 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:
cuenca_validations/types/__init__.pycuenca_validations/types/requests.pycuenca_validations/types/identities.py
🧬 Code graph analysis (2)
cuenca_validations/types/__init__.py (1)
cuenca_validations/types/requests.py (1)
BeneficiaryRequest(498-499)
cuenca_validations/types/requests.py (1)
cuenca_validations/types/identities.py (2)
BaseBeneficiary(92-108)AddressRequest(73-89)
🔇 Additional comments (12)
cuenca_validations/types/identities.py (2)
2-2: Import update is fine.
Union import is appropriate for the new phone_number union.
111-113: LGTM; Beneficiary extends the base cleanly.
Allowing Union[PhoneNumber, str] keeps backward compatibility.cuenca_validations/types/__init__.py (2)
17-17: Good export addition.
Adding BeneficiaryRequest to all keeps the public API consistent.
225-225: Re-export looks correct.
Relative import aligns with the package’s import style.cuenca_validations/types/requests.py (8)
76-76: Import alignment looks good.
Reusing BaseBeneficiary across request/identity models reduces duplication.
442-446: LGTM on BaseRequest adoption.
Consistent dump behavior (exclude_none/unset).
498-500: LGTM: Dedicated BeneficiaryRequest with strict PhoneNumber.
Clear separation between persisted model and request payload.
502-521: LGTM on UserUpdateRequest changes.
Switch to BeneficiaryRequest and keeping BaseRequest is correct.
621-629: LGTM.
Consistent with BaseRequest; good example.
662-664: LGTM.
BankAccountValidationRequest now aligns with other requests.
666-721: LGTM; validations read well.
Checks are clear and follow EAFP where needed.
765-767: LGTM.
Simple, consistent BaseRequest usage.
3 participants
Summary by CodeRabbit