-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
fix(printer): Fix printer ignoring input arguments using snake_case #3780
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request fixes an issue with printer behavior where input arguments in snake_case were being ignored. It introduces a new parameter 'name_converter' in the serialization function to correctly process snake_case keys, propagates this change through the directive printing functions, adds a corresponding test, and documents the change in the release notes. Sequence diagram for schema directive printing with name conversionsequenceDiagram
participant SDP as SchemaDirectivePrinter
participant SD as _serialize_dataclasses
participant NC as NameConverter
participant AV as ast_from_value
participant PA as print_ast
SDP->>SD: Call _serialize_dataclasses(value, name_converter=schema.config.name_converter.apply_naming_config)
SD->>NC: For each key in dataclass, convert key (name_converter(key))
NC-->>SD: Return converted key
SD-->>SD: Build and return serialized object
SD-->>SD: (Recursive calls for list/tuple and dict branches)
SD-->>SD: Return serialized value
SD-->>SD: (Return serialized result)
SDP->>AV: Call ast_from_value(serialized_value, arg.type)
AV-->>SDP: Return AST
SDP->>PA: Call print_ast(AST)
PA-->>SDP: Return printed directive argument
SDP-->>SDP: Combine params to form final printed directive
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @bellini666 - I've reviewed your changes - here's some feedback:
Overall Comments:
- It might be worth adding a test case that uses a schema directive on a type, to ensure that the name conversion works in that case as well.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Apollo Federation Subgraph Compatibility Results
Learn more: |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Thanks for adding the Here's a preview of the changelog: This release fixes an issue where directives with input types using snake_case For example, the following: @strawberry.input
class FooInput:
hello: str
hello_world: str
@strawberry.schema_directive(locations=[Location.FIELD_DEFINITION])
class FooDirective:
input: FooInput
@strawberry.type
class Query:
@strawberry.field(
directives=[
FooDirective(input=FooInput(hello="hello", hello_world="hello world")),
]
)
def foo(self, info) -> str: ... Would previously print as: directive @fooDirective(
input: FooInput!
optionalInput: FooInput
) on FIELD_DEFINITION
type Query {
foo: String! @fooDirective(input: { hello: "hello" })
}
input FooInput {
hello: String!
hello_world: String!
} Now it will be correctly printed as: directive @fooDirective(
input: FooInput!
optionalInput: FooInput
) on FIELD_DEFINITION
type Query {
foo: String!
@fooDirective(input: { hello: "hello", helloWorld: "hello world" })
}
input FooInput {
hello: String!
hello_world: String!
} Here's the tweet text:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3780 +/- ##
==========================================
- Coverage 97.24% 97.23% -0.01%
==========================================
Files 502 502
Lines 33460 33501 +41
Branches 1702 1716 +14
==========================================
+ Hits 32537 32575 +38
- Misses 706 707 +1
- Partials 217 219 +2 |
CodSpeed Performance ReportMerging #3780 will not alter performanceComparing Summary
|
Fix #3760
Summary by Sourcery
Bug Fixes: