-
Notifications
You must be signed in to change notification settings - Fork 14
Update User class with new fields #427
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
WalkthroughAdds imports for AccountUseType, IncomeType, MonthlyMovementsType, MonthlySpendingType and five optional fields to the public User model: fiscal_address, account_use_type, monthly_spending_type, monthly_movements_type, and income_type. Extends User.create(...) signature with account_use_type, monthly_spending_type, monthly_movements_type, income_type and forwards account_use_type, monthly_spending_type, monthly_movements_type into UserRequest (income_type not forwarded on create). Extends User.update(...) to accept and forward fiscal_address plus account_use_type, monthly_spending_type, monthly_movements_type, income_type into UserUpdateRequest. Bumps cuenca-validations to 2.1.16 in requirements.txt. Updates a test cassette (profession lowercased; address.city single-line). Increments package version to 2.1.10. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #427 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 54 54
Lines 1180 1185 +5
=========================================
+ Hits 1180 1185 +5
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:
|
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 (3)
requirements.txt (1)
2-2: Use the latest stable release, not a dev pre-release: cuenca-validations==2.1.16.dev6 is a pre-release; latest stable is 2.1.9—change to==2.1.9(or>=2.1.9,<3.0.0) and add a reminder to bump to 2.1.16 once it’s officially released.cuenca/resources/users.py (2)
136-139: Avoid sending nulls in payloadsConsider excluding None to prevent overwriting server-side values with nulls and to reduce payload size.
Use:
return cls._create(session=session, **req.model_dump(exclude_none=True))
177-180: Exclude None when updatingSame consideration as create(): avoid sending nulls unless explicitly intended.
Use:
return cls._update(id=user_id, **request.model_dump(exclude_none=True), session=session)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cuenca/resources/users.py(6 hunks)requirements.txt(1 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/resources/users.py
🔇 Additional comments (4)
cuenca/resources/users.py (4)
17-24: Confirm enums’ module path against bumped cuenca-validationsEnsure AccountUseTypes, MonthlySpendingTypes, MonthlyMovementsTypes, Country, Gender, and State are indeed exported from cuenca_validations.types.enums in 2.1.16.dev6 to avoid import errors.
69-72: New optional fields look consistentNames are snake_case and types align with read vs. request models. LGTM.
124-127: Should create() also accept fiscal_address?If the API/UserRequest supports fiscal_address at creation time, consider adding it for symmetry; otherwise ignore.
157-160: Update signature additions look goodTypes are appropriate (AddressRequest for updates, enums optional). LGTM.
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cuenca/resources/users.py(6 hunks)requirements.txt(1 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/resources/users.py
🪛 Ruff (0.12.2)
cuenca/resources/users.py
129-129: Unused class method argument: income_type
(ARG003)
🪛 GitHub Actions: test
requirements.txt
[error] 1-1: pip install -qU -r requirements.txt failed: Could not find a version that satisfies the requirement cuenca-validations==2.1.16.dev8 (No matching distribution found for cuenca-validations==2.1.16.dev8).
🔇 Additional comments (4)
cuenca/resources/users.py (4)
17-25: Enum import path OK; verify version compatibilityImports look correct for cuenca_validations 2.x. Just ensure the chosen version exposes these symbols under types.enums as used here.
Use the script in the requirements.txt comment to confirm symbol presence.
70-74: New optional fields on User: LGTMTypes align with retrieve vs. request patterns (Address vs. AddressRequest). No snake_case issues.
160-165: update() signature extension: LGTMParameters and types align with the new model fields.
181-185: Forwarding in UserUpdateRequest: LGTMAll five new fields are correctly included in the update payload.
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
♻️ Duplicate comments (2)
cuenca/resources/users.py (2)
126-129: create(): income_type is accepted but ignored — avoid silent no-opEither remove it or fail fast until backend supports it.
Apply to fail fast:
def create( cls, curp: Curp, profession: Optional[str] = None, address: Optional[AddressRequest] = None, email_verification_id: Optional[str] = None, phone_verification_id: Optional[str] = None, account_use_type: Optional[AccountUseType] = None, monthly_spending_type: Optional[MonthlySpendingType] = None, monthly_movements_type: Optional[MonthlyMovementsType] = None, income_type: Optional[IncomeType] = None, *, session: Session = global_session, ) -> 'User': + if income_type is not None: + raise ValueError('income_type is not accepted on create(); use update()')
139-142: If keeping income_type in create(), forward it to UserRequestPrevents user-provided value from being dropped.
req = UserRequest( curp=curp, profession=profession, address=address, email_verification_id=email_verification_id, phone_verification_id=phone_verification_id, account_use_type=account_use_type, monthly_spending_type=monthly_spending_type, monthly_movements_type=monthly_movements_type, + income_type=income_type, )
🧹 Nitpick comments (1)
cuenca/resources/users.py (1)
70-74: Add new fields to example payload (optional)Consider extending model_config.example to include fiscal_address/account_use_type/monthly_spending_type/monthly_movements_type/income_type for discoverability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cuenca/resources/users.py(6 hunks)requirements.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- requirements.txt
🧰 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/resources/users.py
🪛 Ruff (0.12.2)
cuenca/resources/users.py
129-129: Unused class method argument: income_type
(ARG003)
🔇 Additional comments (3)
cuenca/resources/users.py (3)
17-25: Enums import update looks correctImports align with usage below (Gender/Country/State and new AccountUseType/Monthly*Type/IncomeType).
160-165: update() parameters LGTMNew optional fields and types look consistent with the model and validation package.
181-185: Forwarding in UserUpdateRequest LGTMAll new fields (including income_type) are correctly passed through.
Summary by CodeRabbit