-
-
Notifications
You must be signed in to change notification settings - Fork 125
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
Forward metadata when processing django type #666
Conversation
Reviewer's Guide by SourceryThis PR fixes a bug where metadata attributes were not being properly forwarded when processing Django types in Strawberry Django. The implementation adds metadata forwarding in the _process_type function and includes a test case demonstrating the fix for nullable foreign key filtering. Sequence diagram for test_filter_none functionsequenceDiagram
actor Tester
participant Query
participant Database
Tester->>Query: Execute query with color filter as null
Query->>Database: Fetch fruits with color as null
Database-->>Query: Return fruits with null color
Query-->>Tester: Return result with fruits having null color
note right of Tester: Verifies that fruits with null color are returned correctly.
Class diagram for updated _process_type functionclassDiagram
class _process_type {
+permission_classes
+default
+default_factory
+metadata
+deprecation_reason
+directives
+pagination
}
note for _process_type "Added metadata attribute to forward metadata when processing Django types."
File-Level Changes
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 @g-as - I've reviewed your changes - here's some feedback:
Overall Comments:
- You've marked that this change requires documentation updates but haven't made them. Please add documentation about the nullable FK filtering functionality.
Here's what I looked at during the review
- 🟡 General issues: 1 issue 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.
Description
When trying to allow None filtering on a nullable FK, I found that metadata attr, where this info is stored, was not transferred to the underlying StrawberryField. So I fixed that.
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Fix the issue where metadata attributes were not being forwarded to the underlying StrawberryField when processing Django types, and add a test to ensure filtering on nullable foreign keys with None values works correctly.
Bug Fixes:
Tests: