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 sorting custom field variables (issue #6545) #6660
base: main
Are you sure you want to change the base?
Fix sorting custom field variables (issue #6545) #6660
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6660 +/- ##
==========================================
+ Coverage 78.98% 80.67% +1.68%
==========================================
Files 2903 2541 -362
Lines 139771 127082 -12689
==========================================
- Hits 110397 102521 -7876
+ Misses 29374 24561 -4813
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
I do not thing that this is correct... the formatter tries to get the data hooked onto the underlying type ... the property is captured on the field runtime type which takes it directly from the PropertInfo. The type runtime type could be completly different and cause unnecessary conversion. |
Can you add a test so we have you error case captured? |
Thank you for the review; I will try to add tests to capture the issue and the fix. What is actually happening is exactly what you describe: the formatter tries to hook the data onto the underlying type, but for sorting the type is wrong - the type that is tried is the underlying type, while the actual type is the input type that accepts the sorting direction enum values, not any raw value. |
OK, lets get a test in that shows this error and see what the actual issue is. I need something that we can debug. Thanks so much for your work on this! |
In the meantime if you want you may check the sample project that I made to showcase issue #6545 at https://github.com/alex-mazzariol/hotchocolate-issue-6545, I'll try converting it into tests in the weekend. |
We can use the project as test case. Its really easy to get this in and than we do a snapshot test on top of your project. |
Summary of the changes (Less than 80 chars)
Closes #6545