feat: add field-level alias - #194
Conversation
📝 WalkthroughWalkthroughThis PR introduces explicit ChangesModel Field Mapping Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
==========================================
+ Coverage 93.06% 93.15% +0.09%
==========================================
Files 69 69
Lines 6215 6257 +42
Branches 821 834 +13
==========================================
+ Hits 5784 5829 +45
+ Misses 288 286 -2
+ Partials 143 142 -1 ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/strawchemy/schema/field.py (1)
429-460:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
model_fieldwhen cloningStrawchemyField.Line 429 reconstructs the field but omits
model_field, so copied instances silently reset this mapping toNone.💡 Proposed fix
new_field = type(self)( python_name=self.python_name, graphql_name=self.graphql_name, type_annotation=self.type_annotation, origin=self.origin, is_subscription=self.is_subscription, description=self.description, base_resolver=self.base_resolver, permission_classes=(self.permission_classes[:] if self.permission_classes is not None else []), default=self.default_value, default_factory=self.default_factory, metadata=self.metadata.copy() if self.metadata is not None else None, deprecation_reason=self.deprecation_reason, directives=self.directives[:] if self.directives is not None else [], extensions=self.extensions[:] if self.extensions is not None else [], filter_statement=self._filter_statement, query_hook=self.query_hook, id_field_name=self.id_field_name, + model_field=self.model_field, repository_type=self._repository_type, root_aggregations=self.root_aggregations, filter_type=self._filter, order_by=self._order_by, distinct_on=self._distinct_on,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/strawchemy/schema/field.py` around lines 429 - 460, The __copy__ implementation for StrawchemyField reconstructs a new instance but omits preserving the model_field, causing it to be reset; update the __copy__ method (in class StrawchemyField) to pass model_field=self.model_field (or a shallow copy if appropriate) into the type(self)(...) constructor so the cloned field retains the original model_field mapping.src/strawchemy/dto/types.py (1)
328-339:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
DTOConfig.union()can now build an invalid config (aliases+alias_generator).After Line 328 merges aliases,
copy_with()preservesself.alias_generator;__post_init__then raisesValueErrorwhen both are set. This breaks composition paths that add alias deltas later (e.g., field-levelmodel_fieldmappings).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/strawchemy/dto/types.py` around lines 328 - 339, DTOConfig.union currently merges aliases and then calls copy_with which preserves self.alias_generator, causing __post_init__ to raise when both aliases and alias_generator are set; update DTOConfig.union to ensure the resulting config does not have both set by clearing alias_generator (or removing aliases) on the result when aliases is non-empty. Specifically, in DTOConfig.union adjust the logic around aliases = {**self.aliases, **other.aliases} before calling copy_with so that you pass alias_generator=None (or remove conflicting aliases) into copy_with when aliases is not empty; reference DTOConfig.union, copy_with, __post_init__, alias_generator, and aliases to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/strawchemy/schema/factories/base.py`:
- Around line 173-185: The DTO factory currently adds aliased model fields into
include via config = config | DTOConfig(...), but it doesn't remove them from
exclude/global_exclude so explicit excludes still override the intended
model_field mapping; update the merge so that when you add extra_include you
also remove those model field names from config.exclude and
config.global_exclude (e.g., compute new_exclude = config.exclude -
extra_include and new_global_exclude = config.global_exclude - extra_include)
and create the new DTOConfig using include=extra_include and exclude=new_exclude
(and global_exclude=new_global_exclude) so the aliased model fields win as
intended while still preserving other config properties (reference symbols:
DTOConfig, config, include, exclude, global_exclude, model_field,
annotation_overrides, extra_include).
---
Outside diff comments:
In `@src/strawchemy/dto/types.py`:
- Around line 328-339: DTOConfig.union currently merges aliases and then calls
copy_with which preserves self.alias_generator, causing __post_init__ to raise
when both aliases and alias_generator are set; update DTOConfig.union to ensure
the resulting config does not have both set by clearing alias_generator (or
removing aliases) on the result when aliases is non-empty. Specifically, in
DTOConfig.union adjust the logic around aliases = {**self.aliases,
**other.aliases} before calling copy_with so that you pass alias_generator=None
(or remove conflicting aliases) into copy_with when aliases is not empty;
reference DTOConfig.union, copy_with, __post_init__, alias_generator, and
aliases to locate the change.
In `@src/strawchemy/schema/field.py`:
- Around line 429-460: The __copy__ implementation for StrawchemyField
reconstructs a new instance but omits preserving the model_field, causing it to
be reset; update the __copy__ method (in class StrawchemyField) to pass
model_field=self.model_field (or a shallow copy if appropriate) into the
type(self)(...) constructor so the cloned field retains the original model_field
mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a056f56-94d2-41d0-95f6-384c906369b0
📒 Files selected for processing (13)
mise.tomlsrc/strawchemy/dto/base.pysrc/strawchemy/dto/types.pysrc/strawchemy/mapper.pysrc/strawchemy/schema/factories/_kwargs.pysrc/strawchemy/schema/factories/base.pysrc/strawchemy/schema/factories/types.pysrc/strawchemy/schema/field.pysrc/strawchemy/validation/pydantic.pytests/unit/mapping/test_model_field.pytests/unit/schemas/model_field/__init__.pytests/unit/schemas/model_field/duplicate_target.pytests/unit/schemas/model_field/missing_model_field.py
💤 Files with no reviewable changes (2)
- src/strawchemy/dto/base.py
- src/strawchemy/validation/pydantic.py
Summary by CodeRabbit
Release Notes
New Features
model_fieldparameter to schema field declarations, enabling explicit mapping of GraphQL fields to underlying model attributes for flexible field renaming and customization.Tests