Skip to content

Commit 8f189d5

Browse files
mfnmfn
committed
Return messages directly and not the bag
I don't think the intended output was ever to have the MessageBag being an intermediary for serializing the response, when directly returning the messages yields the same result. The gracious changes in the tests result from not having the MessageBag available anymore and making the shortcut just comparing the whole response. IMHO it also gives a clearer indication how such a thing looks like (the shape) and find the individual probes into the result harder to grasp in the big picture.
1 parent 09eb4ad commit 8f189d5

File tree

8 files changed

+326
-92
lines changed

8 files changed

+326
-92
lines changed

‎src/GraphQL.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ public static function formatError(Error $e): array
435435
}
436436

437437
if ($previous instanceof ValidationError) {
438-
$error['extensions']['validation'] = $previous->getValidatorMessages();
438+
$error['extensions']['validation'] = $previous->getValidatorMessages()->getMessages();
439439
}
440440
}
441441

‎tests/Database/MutationValidationUniqueWithCustomRulesTests/MutationValidationUniqueWithCustomRulesTest.php‎

Lines changed: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
declare(strict_types = 1);
44
namespace Rebing\GraphQL\Tests\Database\MutationValidationUniqueWithCustomRulesTests;
55

6-
use Illuminate\Contracts\Support\MessageBag;
76
use Rebing\GraphQL\Tests\Support\Models\User;
87
use Rebing\GraphQL\Tests\Support\Traits\SqlAssertionTrait;
98
use Rebing\GraphQL\Tests\TestCaseDatabase;
@@ -89,14 +88,31 @@ public function testUniqueFailRulePass(): void
8988
SQL
9089
);
9190

92-
self::assertCount(1, $result['errors']);
93-
self::assertSame('validation', $result['errors'][0]['message']);
94-
/** @var MessageBag $messageBag */
95-
$messageBag = $result['errors'][0]['extensions']['validation'];
96-
$expectedMessages = [
97-
'The arg unique rule pass has already been taken.',
91+
$expected = [
92+
'errors' => [
93+
[
94+
'message' => 'validation',
95+
'extensions' => [
96+
'category' => 'validation',
97+
'validation' => [
98+
'arg_unique_rule_pass' => [
99+
'The arg unique rule pass has already been taken.',
100+
],
101+
],
102+
],
103+
'locations' => [
104+
[
105+
'line' => 2,
106+
'column' => 3,
107+
],
108+
],
109+
'path' => [
110+
'mutationWithCustomRuleWithRuleObject',
111+
],
112+
],
113+
],
98114
];
99-
self::assertSame($expectedMessages, $messageBag->all());
115+
self::assertEquals($expected, $result);
100116
}
101117

102118
public function testUniquePassRuleFail(): void
@@ -122,14 +138,31 @@ public function testUniquePassRuleFail(): void
122138
],
123139
]);
124140

125-
self::assertCount(1, $result['errors']);
126-
self::assertSame('validation', $result['errors'][0]['message']);
127-
/** @var MessageBag $messageBag */
128-
$messageBag = $result['errors'][0]['extensions']['validation'];
129-
$expectedMessages = [
130-
'rule object validation fails',
141+
$expected = [
142+
'errors' => [
143+
[
144+
'message' => 'validation',
145+
'extensions' => [
146+
'category' => 'validation',
147+
'validation' => [
148+
'arg_unique_rule_fail' => [
149+
'rule object validation fails',
150+
],
151+
],
152+
],
153+
'locations' => [
154+
[
155+
'line' => 2,
156+
'column' => 3,
157+
],
158+
],
159+
'path' => [
160+
'mutationWithCustomRuleWithRuleObject',
161+
],
162+
],
163+
],
131164
];
132-
self::assertSame($expectedMessages, $messageBag->all());
165+
self::assertEquals($expected, $result);
133166
}
134167

135168
public function testUniqueFailRuleFail(): void
@@ -161,15 +194,32 @@ public function testUniqueFailRuleFail(): void
161194
SQL
162195
);
163196

164-
self::assertCount(1, $result['errors']);
165-
self::assertSame('validation', $result['errors'][0]['message']);
166-
/** @var MessageBag $messageBag */
167-
$messageBag = $result['errors'][0]['extensions']['validation'];
168-
$expectedMessages = [
169-
'The arg unique rule fail has already been taken.',
170-
'rule object validation fails',
197+
$expected = [
198+
'errors' => [
199+
[
200+
'message' => 'validation',
201+
'extensions' => [
202+
'category' => 'validation',
203+
'validation' => [
204+
'arg_unique_rule_fail' => [
205+
'The arg unique rule fail has already been taken.',
206+
'rule object validation fails',
207+
],
208+
],
209+
],
210+
'locations' => [
211+
[
212+
'line' => 2,
213+
'column' => 3,
214+
],
215+
],
216+
'path' => [
217+
'mutationWithCustomRuleWithRuleObject',
218+
],
219+
],
220+
],
171221
];
172-
self::assertSame($expectedMessages, $messageBag->all());
222+
self::assertEquals($expected, $result);
173223
}
174224

175225
public function testErrorExtension(): void

‎tests/Unit/GraphQLQueryTest.php‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,34 @@ public function testQueryWithValidationError(): void
151151
'expectErrors' => true,
152152
]);
153153

154-
self::assertArrayHasKey('data', $result);
155-
self::assertArrayHasKey('errors', $result);
156-
self::assertArrayHasKey('extensions', $result['errors'][0]);
157-
self::assertArrayHasKey('validation', $result['errors'][0]['extensions']);
158-
self::assertTrue($result['errors'][0]['extensions']['validation']->has('test_validation.args.index'));
154+
$expected = [
155+
'errors' => [
156+
[
157+
'message' => 'validation',
158+
'extensions' => [
159+
'category' => 'validation',
160+
'validation' => [
161+
'test_validation.args.index' => [
162+
'The test validation.args.index field is required.',
163+
],
164+
],
165+
],
166+
'locations' => [
167+
[
168+
'line' => 3,
169+
'column' => 13,
170+
],
171+
],
172+
'path' => [
173+
'examples',
174+
],
175+
],
176+
],
177+
'data' => [
178+
'examples' => null,
179+
],
180+
];
181+
self::assertEquals($expected, $result);
159182
}
160183

161184
public function testQueryWithValidation(): void

‎tests/Unit/GraphQLTest.php‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,21 @@ public function testFormatValidationError(): void
349349
$error = new Error('error', null, null, [], null, $validationError);
350350
$error = GraphQL::formatError($error);
351351

352-
self::assertIsArray($error);
353-
self::assertArrayHasKey('extensions', $error);
354-
self::assertArrayHasKey('validation', $error['extensions']);
355-
self::assertTrue($error['extensions']['validation']->has('test'));
352+
self::assertArrayHasKey('trace', $error);
353+
unset($error['trace']);
354+
355+
$expected = [
356+
'message' => 'error',
357+
'extensions' => [
358+
'category' => 'validation',
359+
'validation' => [
360+
'test' => [
361+
'The test field is required.',
362+
],
363+
],
364+
],
365+
];
366+
self::assertEquals($expected, $error);
356367
}
357368

358369
public function testAddType(): void

‎tests/Unit/MutationCustomRulesTests/MutationCustomRulesTest.php‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
declare(strict_types = 1);
44
namespace Rebing\GraphQL\Tests\Unit\MutationCustomRulesTests;
55

6-
use Illuminate\Contracts\Support\MessageBag;
76
use Rebing\GraphQL\Tests\TestCase;
87

98
class MutationCustomRulesTest extends TestCase
@@ -35,11 +34,31 @@ public function testMutationWithCustomRuleWithClosure(): void
3534
],
3635
]);
3736

38-
self::assertCount(1, $result['errors']);
39-
self::assertSame('validation', $result['errors'][0]['message']);
40-
/** @var MessageBag $messageBag */
41-
$messageBag = $result['errors'][0]['extensions']['validation'];
42-
self::assertSame(['arg1 is invalid'], $messageBag->all());
37+
$expected = [
38+
'errors' => [
39+
[
40+
'message' => 'validation',
41+
'extensions' => [
42+
'category' => 'validation',
43+
'validation' => [
44+
'arg1' => [
45+
'arg1 is invalid',
46+
],
47+
],
48+
],
49+
'locations' => [
50+
[
51+
'line' => 2,
52+
'column' => 3,
53+
],
54+
],
55+
'path' => [
56+
'mutationWithCustomRuleWithClosure',
57+
],
58+
],
59+
],
60+
];
61+
self::assertEquals($expected, $result);
4362
}
4463

4564
public function testMutationWithCustomRuleWithRuleObject(): void
@@ -57,10 +76,30 @@ public function testMutationWithCustomRuleWithRuleObject(): void
5776
],
5877
]);
5978

60-
self::assertCount(1, $result['errors']);
61-
self::assertSame('validation', $result['errors'][0]['message']);
62-
/** @var MessageBag $messageBag */
63-
$messageBag = $result['errors'][0]['extensions']['validation'];
64-
self::assertSame(['arg1 is invalid'], $messageBag->all());
79+
$expected = [
80+
'errors' => [
81+
[
82+
'message' => 'validation',
83+
'extensions' => [
84+
'category' => 'validation',
85+
'validation' => [
86+
'arg1' => [
87+
'arg1 is invalid',
88+
],
89+
],
90+
],
91+
'locations' => [
92+
[
93+
'line' => 2,
94+
'column' => 3,
95+
],
96+
],
97+
'path' => [
98+
'mutationWithCustomRuleWithRuleObject',
99+
],
100+
],
101+
],
102+
];
103+
self::assertEquals($expected, $result);
65104
}
66105
}

0 commit comments

Comments
 (0)