-
Notifications
You must be signed in to change notification settings - Fork 0
Remove dash underscore from uuid #393
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
…_' with 'A' and 'B'
WalkthroughThis PR changes Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Files to pay extra attention to:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #393 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1405 1409 +4
=========================================
+ Hits 1405 1409 +4
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cuenca_validations/types/helpers.py(1 hunks)cuenca_validations/version.py(1 hunks)tests/test_helpers.py(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:
tests/test_helpers.pycuenca_validations/version.pycuenca_validations/types/helpers.py
🧬 Code graph analysis (1)
tests/test_helpers.py (1)
cuenca_validations/types/helpers.py (1)
uuid_field(13-19)
🔇 Additional comments (2)
cuenca_validations/version.py (1)
1-1: LGTM!Version bump is appropriate for the UUID sanitization feature.
cuenca_validations/types/helpers.py (1)
13-19: Code changes are correct; the review comment's breaking change concern is unsupported by repository evidence.The
uuid_fieldfunction implementation is correct and well-tested. However, the original review comment assumes breaking impact on "existing systems, databases, or APIs" expecting URL-safe base64 format. Repository analysis reveals:
- No production code calls
uuid_fieldinternally- No systems parse or validate UUID format elsewhere in the codebase
- No database schemas or migrations reference these UUIDs
- Tests confirm the sanitized format works as intended
- Function is exported as public API but with no internal dependencies on the old format
The sanitization logic (replacing '-' with 'A' and '_' with 'B') does not create a breaking change within this codebase. If external consumers depend on this function, that would require verification outside this repository scope, but no evidence of internal system breakage exists.
Likely an incorrect or invalid review comment.
…rated UUID string
…for enhanced uniqueness
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: 1
♻️ Duplicate comments (1)
cuenca_validations/types/helpers.py (1)
14-20: Clarify/ensure the “only alphanumeric characters” invariant for the full IDThe new logic guarantees that the base64-derived portion is alphanumeric (
-and_are removed), but the returned value isprefix + sanitized. If callers pass prefixes containing non‑alphanumeric characters, the overall ID will still violate the “clean everything that isn’t alphanumeric” requirement mentioned in the earlier review.If the intent is that the entire returned UUID is strictly
[A-Za-z0-9], consider either:
- Documenting that
prefixmust be alphanumeric, or- Sanitizing/validating
prefixinsideuuid_fieldbefore concatenation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cuenca_validations/types/helpers.py(2 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/helpers.py
🪛 Ruff (0.14.5)
cuenca_validations/types/helpers.py
17-17: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.