Skip to content
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.

Sign up for GitHub

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

Jump to bottom

added new variable that override pydantic fields definition with strawberry fields #3469

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −3
Draft

added new variable that override pydantic fields definition with strawberry fields #3469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions strawberry/experimental/pydantic/object_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,16 @@ def _build_dataclass_creation_fields(
auto_fields_set: Set[str],
use_pydantic_alias: bool,
compat: PydanticCompat,
override: bool = True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code_clarification): Consider documenting the purpose and usage of the 'override' parameter.

Adding a new parameter with a default value is a significant change. It would be beneficial to include inline documentation or update the function's docstring to explain how this parameter affects the function's behavior.

Suggested change
override: bool = True,
override: bool = True,
"""
:param override: A boolean flag that determines whether the default settings should be overridden.
When set to True, the function will override the existing settings with the provided values.
Defaults to True.
"""

) -> DataclassCreationFields:
field_type = (
get_type_for_field(field, is_input, compat=compat)
if field.name in auto_fields_set
else existing_fields[field.name].type
)

if (
field.name in existing_fields
and existing_fields[field.name].base_resolver is not None
if field.name in existing_fields and (
existing_fields[field.name].base_resolver is not None or not override
):
# if the user has defined a resolver for this field, always use it
strawberry_field = existing_fields[field.name]
Expand Down Expand Up @@ -129,6 +129,7 @@ def type(
directives: Optional[Sequence[object]] = (),
all_fields: bool = False,
use_pydantic_alias: bool = True,
override: bool = True,
) -> Callable[..., Type[StrawberryTypeFromPydantic[PydanticModel]]]:
def wrap(cls: Any) -> Type[StrawberryTypeFromPydantic[PydanticModel]]:
compat = PydanticCompat.from_model(model)
Expand Down Expand Up @@ -191,6 +192,7 @@ def wrap(cls: Any) -> Type[StrawberryTypeFromPydantic[PydanticModel]]:
auto_fields_set,
use_pydantic_alias,
compat=compat,
override=override,
)
for field_name, field in model_fields.items()
if field_name in fields_set
Expand Down
21 changes: 21 additions & 0 deletions tests/experimental/pydantic/test_conversion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,27 @@ def age() -> int:
assert User.__strawberry_definition__.fields[2].base_resolver() == 84


def test_can_override_pydantic_with_strawberry_definition():
# Set variable override=False to pydantic type
# That's override pydantic fields with strawberry fields
class UserModel(BaseModel):
new_name: Optional[str] = None
new_age: Optional[int] = None

@strawberry.experimental.pydantic.type(UserModel, override=False)
class User:
new_name: Optional[str] = None
new_age: Optional[int] = strawberry.UNSET

origin_user = UserModel()
user = User()
assert origin_user.new_name is None
assert origin_user.new_age is None

assert user.new_name is None
assert user.new_age is strawberry.UNSET
Comment on lines +951 to +969
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding a test case for when override=True.

The current test only covers the scenario where override=False. It would be beneficial to ensure that the default behavior (override=True) works as expected, especially since this is a new feature.

Comment on lines +951 to +969
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add assertions to verify that the User fields correctly override the UserModel fields.

While the test checks the initial state of UserModel and User, it does not verify if the User fields actually override those from UserModel when set. This is crucial for validating the feature's functionality.



def test_can_convert_both_output_and_input_type():
class Work(BaseModel):
time: float
Expand Down
Loading