From 8a49054de036bdf9f4122b79db1b48d092ba38fd Mon Sep 17 00:00:00 2001 From: enzo Date: Tue, 21 Jan 2020 16:02:44 -0300 Subject: [PATCH 1/3] Add failling test --- .../HasManyWithRenamedTest.php | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php diff --git a/tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php b/tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php new file mode 100644 index 0000000000..7c5b95f956 --- /dev/null +++ b/tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php @@ -0,0 +1,123 @@ +schema .= ' + input UpsertTaskInput { + id: ID + name: String + } + '; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + upsertUser(input: { + firstName: "foo" + tasks: { + upsert: [{ + name: "bar" + }] + } + }) { + id + name + tasks { + id + name + } + } + } +GRAPHQL + )->assertJson([ + 'data' => [ + 'upsertUser' => [ + 'id' => '1', + 'name' => 'foo', + 'tasks' => [ + [ + 'id' => '1', + 'name' => 'bar', + ], + ], + ], + ], + ]); + } + + public function testUpsertHasManyWithNestedRenameDirective(): void + { + $this->schema .= ' + input UpsertTaskInput { + id: ID + customName: String @rename(attribute: "name") + } + '; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + upsertUser(input: { + firstName: "foo" + tasks: { + upsert: [{ + customName: "bar" + }] + } + }) { + id + name + tasks { + id + name + } + } + } +GRAPHQL + )->assertJson([ + 'data' => [ + 'upsertUser' => [ + 'id' => '1', + 'name' => 'foo', + 'tasks' => [ + [ + 'id' => '1', + 'customName' => 'bar', + ], + ], + ], + ], + ]); + } +} From 6d179106ac088c6627c9f2ee29d21111e21e12ae Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 23 Jan 2020 11:30:16 +0100 Subject: [PATCH 2/3] Add a simpler test, fix implementation, add changelog --- CHANGELOG.md | 1 + src/Execution/Arguments/ArgumentSet.php | 18 ++- .../HasManyWithRenamedTest.php | 123 ------------------ .../Schema/Directives/RenameDirectiveTest.php | 39 ++++++ 4 files changed, 54 insertions(+), 127 deletions(-) delete mode 100644 tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c6f064e3d..95999dab85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix eager-loading relations where the parent type is an `interface` or `union` and may correspond to multiple different models https://github.com/nuwave/lighthouse/pull/1035 +- Fix renaming input fields that are nested within lists using @rename https://github.com/nuwave/lighthouse/pull/1166 ## [4.8.1](https://github.com/nuwave/lighthouse/compare/v4.8.1...4.8.0) diff --git a/src/Execution/Arguments/ArgumentSet.php b/src/Execution/Arguments/ArgumentSet.php index 8621c7d2db..49b8564781 100644 --- a/src/Execution/Arguments/ArgumentSet.php +++ b/src/Execution/Arguments/ArgumentSet.php @@ -7,6 +7,7 @@ use Nuwave\Lighthouse\Schema\Directives\SpreadDirective; use Nuwave\Lighthouse\Support\Contracts\ArgBuilderDirective; use Nuwave\Lighthouse\Support\Contracts\Directive; +use Nuwave\Lighthouse\Support\Utils; class ArgumentSet { @@ -87,10 +88,19 @@ public function rename(): self $argumentSet->directives = $this->directives; foreach ($this->arguments as $name => $argument) { - // Recursively apply the renaming to nested inputs - if ($argument->value instanceof self) { - $argument->value = $argument->value->rename(); - } + // Recursively apply the renaming to nested inputs. + // We look for further ArgumentSet instances, they + // might be contained within an array. + $argument->value = Utils::applyEach( + function ($value) { + if($value instanceof self) { + return $value->rename(); + } + + return $value; + }, + $argument->value + ); /** @var \Nuwave\Lighthouse\Schema\Directives\RenameDirective|null $renameDirective */ $renameDirective = $argument->directives->first(function ($directive) { diff --git a/tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php b/tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php deleted file mode 100644 index 7c5b95f956..0000000000 --- a/tests/Integration/Execution/MutationExecutor/HasManyWithRenamedTest.php +++ /dev/null @@ -1,123 +0,0 @@ -schema .= ' - input UpsertTaskInput { - id: ID - name: String - } - '; - - $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' - mutation { - upsertUser(input: { - firstName: "foo" - tasks: { - upsert: [{ - name: "bar" - }] - } - }) { - id - name - tasks { - id - name - } - } - } -GRAPHQL - )->assertJson([ - 'data' => [ - 'upsertUser' => [ - 'id' => '1', - 'name' => 'foo', - 'tasks' => [ - [ - 'id' => '1', - 'name' => 'bar', - ], - ], - ], - ], - ]); - } - - public function testUpsertHasManyWithNestedRenameDirective(): void - { - $this->schema .= ' - input UpsertTaskInput { - id: ID - customName: String @rename(attribute: "name") - } - '; - - $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' - mutation { - upsertUser(input: { - firstName: "foo" - tasks: { - upsert: [{ - customName: "bar" - }] - } - }) { - id - name - tasks { - id - name - } - } - } -GRAPHQL - )->assertJson([ - 'data' => [ - 'upsertUser' => [ - 'id' => '1', - 'name' => 'foo', - 'tasks' => [ - [ - 'id' => '1', - 'customName' => 'bar', - ], - ], - ], - ], - ]); - } -} diff --git a/tests/Unit/Schema/Directives/RenameDirectiveTest.php b/tests/Unit/Schema/Directives/RenameDirectiveTest.php index dd4c8cba3f..9e315bc79a 100644 --- a/tests/Unit/Schema/Directives/RenameDirectiveTest.php +++ b/tests/Unit/Schema/Directives/RenameDirectiveTest.php @@ -81,4 +81,43 @@ public function testRenameArgument(): void ], ]); } + + public function testRenameListOfInputs(): void + { + $this->mockResolver(function ($root, array $args) { + return $args === [ + 'input' => [ + ['bar' => 'something'] + ] + ]; + }); + + $this->schema = /** @lang GraphQL */ ' + type Query { + foo( + input: [FooInput] + ): Boolean @mock + } + + input FooInput { + baz: String @rename(attribute: "bar") + } + '; + + $this->graphQL(/** @lang GraphQL */ ' + { + foo( + input: [ + { + baz: "something" + } + ] + ) + } + ')->assertJson([ + 'data' => [ + 'foo' => true, + ], + ]); + } } From 97af7c4bd083e773f4f64911b01fe9bce76b8a97 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 23 Jan 2020 10:30:32 +0000 Subject: [PATCH 3/3] Apply fixes from StyleCI --- src/Execution/Arguments/ArgumentSet.php | 2 +- tests/Unit/Schema/Directives/RenameDirectiveTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Execution/Arguments/ArgumentSet.php b/src/Execution/Arguments/ArgumentSet.php index 49b8564781..243eddc01f 100644 --- a/src/Execution/Arguments/ArgumentSet.php +++ b/src/Execution/Arguments/ArgumentSet.php @@ -93,7 +93,7 @@ public function rename(): self // might be contained within an array. $argument->value = Utils::applyEach( function ($value) { - if($value instanceof self) { + if ($value instanceof self) { return $value->rename(); } diff --git a/tests/Unit/Schema/Directives/RenameDirectiveTest.php b/tests/Unit/Schema/Directives/RenameDirectiveTest.php index 9e315bc79a..316b38f89a 100644 --- a/tests/Unit/Schema/Directives/RenameDirectiveTest.php +++ b/tests/Unit/Schema/Directives/RenameDirectiveTest.php @@ -87,8 +87,8 @@ public function testRenameListOfInputs(): void $this->mockResolver(function ($root, array $args) { return $args === [ 'input' => [ - ['bar' => 'something'] - ] + ['bar' => 'something'], + ], ]; });