Fixed field merging issue for skip/include. (#6621)

ChilliCream · Oct 24, 2023 · 5a52375 · 5a52375
1 parent f0b3707
commit 5a52375
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 6 deletions.
diff --git a/src/HotChocolate/AspNetCore/src/AspNetCore.CommandLine/Command/ExportCommand.cs b/src/HotChocolate/AspNetCore/src/AspNetCore.CommandLine/Command/ExportCommand.cs
@@ -50,7 +50,7 @@ private static async Task ExecuteAsync(
 
         var sdl = schema.Schema.Print();
 
-        if (output is { })
+        if (output is not null)
         {
             await File.WriteAllTextAsync(output.FullName, sdl, Encoding.UTF8, cancellationToken);
         }

diff --git a/src/HotChocolate/Core/src/Execution/Processing/Selection.cs b/src/HotChocolate/Core/src/Execution/Processing/Selection.cs
@@ -2,6 +2,7 @@
 using System.Diagnostics;
 using System.Linq;
 using HotChocolate.Execution.Properties;
+using HotChocolate.Execution.Serialization;
 using HotChocolate.Language;
 using HotChocolate.Resolvers;
 using HotChocolate.Types;
@@ -244,9 +245,9 @@ private static FieldNode MergeField(
                     temp[next++] = directives[i];
                 }
 
-                for (var i = 0; i < first.Directives.Count; i++)
+                for (var i = 0; i < other.Directives.Count; i++)
                 {
-                    temp[next++] = first.Directives[i];
+                    temp[next++] = other.Directives[i];
                 }
 
                 directives = temp;

diff --git a/src/HotChocolate/Core/test/Execution.Tests/Processing/SelectionIncludeConditionTests.cs b/src/HotChocolate/Core/test/Execution.Tests/Processing/SelectionIncludeConditionTests.cs
@@ -1,11 +1,8 @@
-using System;
-using System.Threading.Tasks;
 using CookieCrumble;
 using HotChocolate.Language;
 using HotChocolate.Types;
 using Microsoft.Extensions.DependencyInjection;
 using Moq;
-using Xunit;
 
 namespace HotChocolate.Execution.Processing;
 
@@ -306,6 +303,80 @@ person @skip(if: true) @include(if: true) {
             }
             """);
     }
+
+    [Fact]
+    public async Task Skip_Include_Merge_Issue_6550_True()
+    {
+        var result =
+            await new ServiceCollection()
+                .AddGraphQLServer()
+                .AddQueryType<Query>()
+                .ExecuteRequestAsync(
+                    QueryRequestBuilder.New()
+                        .SetQuery(
+                            """
+                            query($shouldSkip: Boolean! = true) {
+                                person @skip(if: $shouldSkip) {
+                                    a: name
+                                }
+                                person @skip(if: $shouldSkip) {
+                                    b: name
+                                }
+                                person @skip(if: $shouldSkip) {
+                                    c: name
+                                }
+                            }
+                            """)
+                        .SetVariableValue("shouldSkip", true)
+                        .Create());
+
+        result.MatchInlineSnapshot(
+            """
+            {
+                "data": {}
+            }
+            """);
+    }
+
+    [Fact]
+    public async Task Skip_Include_Merge_Issue_6550_False()
+    {
+        var result =
+            await new ServiceCollection()
+                .AddGraphQLServer()
+                .AddQueryType<Query>()
+                .ExecuteRequestAsync(
+                    QueryRequestBuilder.New()
+                        .SetQuery(
+                            """
+                            query($shouldSkip: Boolean! = true) {
+                                person @skip(if: $shouldSkip) {
+                                    a: name
+                                }
+                                person @skip(if: $shouldSkip) {
+                                    b: name
+                                }
+                                person @skip(if: $shouldSkip) {
+                                    c: name
+                                }
+                            }
+                            """)
+                        .SetVariableValue("shouldSkip", false)
+                        .Create());
+
+        result.MatchInlineSnapshot(
+            """
+            {
+              "data": {
+                "person": {
+                  "a": "hello",
+                  "b": "hello",
+                  "c": "hello"
+                }
+              }
+            }
+            """);
+    }
 
     [Fact]
     public async Task Variables_Skip_True_Include_True_Should_Be_Empty_Result()