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 InputFieldsAndArgumentsOfCorrectLength validation rule #3307
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3307 +/- ##
==========================================
+ Coverage 84.00% 84.04% +0.03%
==========================================
Files 376 376
Lines 16178 16179 +1
Branches 2600 2600
==========================================
+ Hits 13590 13597 +7
+ Misses 1971 1964 -7
- Partials 617 618 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -59,7 +59,7 @@ public override ValueTask VisitFieldAsync(ValidationContext context, GraphQLVari | |||
new MatchingNodeVisitor<GraphQLArgument>((arg, context) => CheckLength(arg, arg.Value, context.TypeInfo.GetArgument(), context)), | |||
new MatchingNodeVisitor<GraphQLObjectField>((field, context) => | |||
{ | |||
if (context.TypeInfo.GetInputType(1) is IInputObjectGraphType input) | |||
if (context.TypeInfo.GetInputType(1)?.GetNamedType() is IInputObjectGraphType input) |
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.
🤦♂️
|
||
namespace GraphQL.Tests.Validation; | ||
|
||
public class InputFieldsAndArgumentsOfCorrectLengthRequiredTests : ValidationTestBase<InputFieldsAndArgumentsOfCorrectLength, ValidationSchema> |
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.
You don't want to put tests in InputFieldsAndArgumentsOfCorrectLengthTests.cs?
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.
Either way is fine with me. It just seemed easier at the time to copy the whole file. Go ahead and make any changes to the PR you wish.
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.
It seemed to me we had tests and I really found a file with tests. The next question that I asked myself - how they differ? Only non-null
type modifier?
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.
Right; the only difference is that it is testing fields with non-null modifiers. The tests are all the same, I think.
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.
ok
…TRAC-6694-upgrade-graphql-dotnet-server-to-7.1 * commit '13da37d7c0649cc6186714a6671272af49f06d85': (1255 commits) Add type integrity check when existing types are found (graphql-dotnet#3332) Switch to Nullability.Source (graphql-dotnet#3314) Add missing authorization extensions from field and connection builders (graphql-dotnet#3324) Prevent using graphtype as model (graphql-dotnet#3316) Remove init-only properties from pre-.NET 5 targets (graphql-dotnet#3323) Update UnionGraphType to support CLR references (graphql-dotnet#3320) Bump BenchmarkDotNet from 0.13.1 to 0.13.2 (graphql-dotnet#3313) Bump Microsoft.NET.Test.Sdk from 17.3.0 to 17.3.1 (graphql-dotnet#3311) Fix InputFieldsAndArgumentsOfCorrectLength validation rule (graphql-dotnet#3307) Bump Shouldly from 4.0.3 to 4.1.0 (graphql-dotnet#3304) Add test (graphql-dotnet#3302) !(a is T) -> a is not T (graphql-dotnet#3300) User not duplicated in context (graphql-dotnet#3298) Bump deps (graphql-dotnet#3295) Restore `Field<TGraphType>()` method (graphql-dotnet#3294) Include v7 migration document link in readme (graphql-dotnet#3290) Bump GraphQL-Parser to 8.1.0 (graphql-dotnet#3289) Migration notes updates (graphql-dotnet#3287) Simplify configuration (graphql-dotnet#3286) Introduce ErrorInfoProviderOptions.ExposeExceptionDetailsMode (graphql-dotnet#3276) ... # Conflicts: # docs/package.json # src/GraphQL/GraphQL.csproj # src/GraphQL/Types/GraphTypesLookup.cs
See #3301