Skip to content

Commit a35b581

Browse files
fabpotfabpot
committed
feature #29813 [FrameworkBundle] Remove ControllerTrait::isFormValid() (lyrixx)
This PR was squashed before being merged into the 4.3-dev branch (closes #29813). Discussion ---------- [FrameworkBundle] Remove ControllerTrait::isFormValid() | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#24576 (comment) | License | MIT | Doc PR | **edit**: During the first draft I made, I did not use the request stack. I finally used it to mimic other shortcut! It was a bad idea. Now there is less code, it's simpler. Love it more Commits ------- 2be1987ad1 [FrameworkBundle] Remove ControllerTrait::isFormValid()
2 parents 2aede9c + 1ae3b9f commit a35b581

File tree

3 files changed

+19
-153
lines changed

3 files changed

+19
-153
lines changed

‎CHANGELOG.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ CHANGELOG
66

77
* Not passing the project directory to the constructor of the `AssetsInstallCommand` is deprecated. This argument will
88
be mandatory in 5.0.
9-
* Added `ControllerTrait::isFormValid()`
109

1110
4.2.0
1211
-----

‎Controller/ControllerTrait.php‎

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ protected function get(string $id)
7373
*
7474
* @final
7575
*/
76-
protected function generateUrl(string $route, array $parameters = array(), int $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH): string
76+
protected function generateUrl(string $route, array $parameters = [], int $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH): string
7777
{
7878
return $this->container->get('router')->generate($route, $parameters, $referenceType);
7979
}
@@ -85,7 +85,7 @@ protected function generateUrl(string $route, array $parameters = array(), int $
8585
*
8686
* @final
8787
*/
88-
protected function forward(string $controller, array $path = array(), array $query = array()): Response
88+
protected function forward(string $controller, array $path = [], array $query = []): Response
8989
{
9090
$request = $this->container->get('request_stack')->getCurrentRequest();
9191
$path['_controller'] = $controller;
@@ -109,7 +109,7 @@ protected function redirect(string $url, int $status = 302): RedirectResponse
109109
*
110110
* @final
111111
*/
112-
protected function redirectToRoute(string $route, array $parameters = array(), int $status = 302): RedirectResponse
112+
protected function redirectToRoute(string $route, array $parameters = [], int $status = 302): RedirectResponse
113113
{
114114
return $this->redirect($this->generateUrl($route, $parameters), $status);
115115
}
@@ -119,12 +119,12 @@ protected function redirectToRoute(string $route, array $parameters = array(), i
119119
*
120120
* @final
121121
*/
122-
protected function json($data, int $status = 200, array $headers = array(), array $context = array()): JsonResponse
122+
protected function json($data, int $status = 200, array $headers = [], array $context = []): JsonResponse
123123
{
124124
if ($this->container->has('serializer')) {
125-
$json = $this->container->get('serializer')->serialize($data, 'json', array_merge(array(
125+
$json = $this->container->get('serializer')->serialize($data, 'json', array_merge([
126126
'json_encode_options' => JsonResponse::DEFAULT_ENCODING_OPTIONS,
127-
), $context));
127+
], $context));
128128

129129
return new JsonResponse($json, $status, $headers, true);
130130
}
@@ -203,7 +203,7 @@ protected function denyAccessUnlessGranted($attributes, $subject = null, string
203203
*
204204
* @final
205205
*/
206-
protected function renderView(string $view, array $parameters = array()): string
206+
protected function renderView(string $view, array $parameters = []): string
207207
{
208208
if ($this->container->has('templating')) {
209209
return $this->container->get('templating')->render($view, $parameters);
@@ -221,7 +221,7 @@ protected function renderView(string $view, array $parameters = array()): string
221221
*
222222
* @final
223223
*/
224-
protected function render(string $view, array $parameters = array(), Response $response = null): Response
224+
protected function render(string $view, array $parameters = [], Response $response = null): Response
225225
{
226226
if ($this->container->has('templating')) {
227227
$content = $this->container->get('templating')->render($view, $parameters);
@@ -245,7 +245,7 @@ protected function render(string $view, array $parameters = array(), Response $r
245245
*
246246
* @final
247247
*/
248-
protected function stream(string $view, array $parameters = array(), StreamedResponse $response = null): StreamedResponse
248+
protected function stream(string $view, array $parameters = [], StreamedResponse $response = null): StreamedResponse
249249
{
250250
if ($this->container->has('templating')) {
251251
$templating = $this->container->get('templating');
@@ -311,7 +311,7 @@ protected function createAccessDeniedException(string $message = 'Access Denied.
311311
*
312312
* @final
313313
*/
314-
protected function createForm(string $type, $data = null, array $options = array()): FormInterface
314+
protected function createForm(string $type, $data = null, array $options = []): FormInterface
315315
{
316316
return $this->container->get('form.factory')->create($type, $data, $options);
317317
}
@@ -321,33 +321,11 @@ protected function createForm(string $type, $data = null, array $options = array
321321
*
322322
* @final
323323
*/
324-
protected function createFormBuilder($data = null, array $options = array()): FormBuilderInterface
324+
protected function createFormBuilder($data = null, array $options = []): FormBuilderInterface
325325
{
326326
return $this->container->get('form.factory')->createBuilder(FormType::class, $data, $options);
327327
}
328328

329-
/**
330-
* Handles request and check form validity.
331-
*
332-
* @final
333-
*/
334-
protected function isFormValid(FormInterface $form, Request $request = null): bool
335-
{
336-
if ($form->isSubmitted()) {
337-
throw new \LogicException('The form is already submitted, use $form->isValid() directly.');
338-
}
339-
340-
if (!$request) {
341-
$request = $this->container->get('request_stack')->getCurrentRequest();
342-
}
343-
344-
if (!$request) {
345-
throw new \LogicException('You must pass a request as second argument because the request stack is empty.');
346-
}
347-
348-
return $form->handleRequest($request)->isSubmitted() && $form->isValid();
349-
}
350-
351329
/**
352330
* Shortcut to return the Doctrine Registry service.
353331
*
@@ -441,7 +419,7 @@ protected function addLink(Request $request, Link $link)
441419
}
442420

443421
if (null === $linkProvider = $request->attributes->get('_links')) {
444-
$request->attributes->set('_links', new GenericLinkProvider(array($link)));
422+
$request->attributes->set('_links', new GenericLinkProvider([$link]));
445423

446424
return;
447425
}

‎Tests/Controller/ControllerTraitTest.php‎

Lines changed: 7 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function testForward()
6060
public function testGetUser()
6161
{
6262
$user = new User('user', 'pass');
63-
$token = new UsernamePasswordToken($user, 'pass', 'default', array('ROLE_USER'));
63+
$token = new UsernamePasswordToken($user, 'pass', 'default', ['ROLE_USER']);
6464

6565
$controller = $this->createController();
6666
$controller->setContainer($this->getContainerWithTokenStorage($token));
@@ -122,7 +122,7 @@ public function testJson()
122122
$controller = $this->createController();
123123
$controller->setContainer(new Container());
124124

125-
$response = $controller->json(array());
125+
$response = $controller->json([]);
126126
$this->assertInstanceOf(JsonResponse::class, $response);
127127
$this->assertEquals('[]', $response->getContent());
128128
}
@@ -135,15 +135,15 @@ public function testJsonWithSerializer()
135135
$serializer
136136
->expects($this->once())
137137
->method('serialize')
138-
->with(array(), 'json', array('json_encode_options' => JsonResponse::DEFAULT_ENCODING_OPTIONS))
138+
->with([], 'json', ['json_encode_options' => JsonResponse::DEFAULT_ENCODING_OPTIONS])
139139
->will($this->returnValue('[]'));
140140

141141
$container->set('serializer', $serializer);
142142

143143
$controller = $this->createController();
144144
$controller->setContainer($container);
145145

146-
$response = $controller->json(array());
146+
$response = $controller->json([]);
147147
$this->assertInstanceOf(JsonResponse::class, $response);
148148
$this->assertEquals('[]', $response->getContent());
149149
}
@@ -156,15 +156,15 @@ public function testJsonWithSerializerContextOverride()
156156
$serializer
157157
->expects($this->once())
158158
->method('serialize')
159-
->with(array(), 'json', array('json_encode_options' => 0, 'other' => 'context'))
159+
->with([], 'json', ['json_encode_options' => 0, 'other' => 'context'])
160160
->will($this->returnValue('[]'));
161161

162162
$container->set('serializer', $serializer);
163163

164164
$controller = $this->createController();
165165
$controller->setContainer($container);
166166

167-
$response = $controller->json(array(), 200, array(), array('json_encode_options' => 0, 'other' => 'context'));
167+
$response = $controller->json([], 200, [], ['json_encode_options' => 0, 'other' => 'context']);
168168
$this->assertInstanceOf(JsonResponse::class, $response);
169169
$this->assertEquals('[]', $response->getContent());
170170
$response->setEncodingOptions(JSON_FORCE_OBJECT);
@@ -389,7 +389,7 @@ public function testAddFlash()
389389
$controller->setContainer($container);
390390
$controller->addFlash('foo', 'bar');
391391

392-
$this->assertSame(array('bar'), $flashBag->get('foo'));
392+
$this->assertSame(['bar'], $flashBag->get('foo'));
393393
}
394394

395395
public function testCreateAccessDeniedException()
@@ -517,117 +517,6 @@ public function testCreateFormBuilder()
517517
$this->assertEquals($formBuilder, $controller->createFormBuilder('foo'));
518518
}
519519

520-
/**
521-
* @expectedException \LogicException
522-
* @expectedExceptionMessage The form is already submitted, use $form->isValid() directly.
523-
*/
524-
public function testIsFormValidWhenAlreadySubmitted()
525-
{
526-
$requestStack = new RequestStack();
527-
$requestStack->push($request = new Request());
528-
529-
$container = new Container();
530-
$container->set('request_stack', $requestStack);
531-
532-
$controller = $this->createController();
533-
$controller->setContainer($container);
534-
535-
$form = $this->getMockBuilder('Symfony\Component\Form\FormInterface')->getMock();
536-
$form
537-
->expects($this->once())
538-
->method('isSubmitted')
539-
->willReturn(true)
540-
;
541-
542-
$controller->isFormValid($form);
543-
}
544-
545-
public function testIsFormValidWhenInvalid()
546-
{
547-
$requestStack = new RequestStack();
548-
$requestStack->push($request = new Request());
549-
550-
$container = new Container();
551-
$container->set('request_stack', $requestStack);
552-
553-
$controller = $this->createController();
554-
$controller->setContainer($container);
555-
556-
$form = $this->getMockBuilder('Symfony\Component\Form\FormInterface')->getMock();
557-
$form
558-
->expects($this->at(0))
559-
->method('isSubmitted')
560-
->willReturn(false)
561-
;
562-
$form
563-
->expects($this->once())
564-
->method('handleRequest')
565-
->with($request)
566-
->willReturn($form)
567-
;
568-
$form
569-
->expects($this->at(2))
570-
->method('isSubmitted')
571-
->willReturn(false)
572-
;
573-
574-
$this->assertFalse($controller->isFormValid($form));
575-
}
576-
577-
public function testIsFormValidWhenValid()
578-
{
579-
$requestStack = new RequestStack();
580-
$requestStack->push($request = new Request());
581-
582-
$container = new Container();
583-
$container->set('request_stack', $requestStack);
584-
585-
$controller = $this->createController();
586-
$controller->setContainer($container);
587-
588-
$form = $this->getMockBuilder('Symfony\Component\Form\FormInterface')->getMock();
589-
$form
590-
->expects($this->at(0))
591-
->method('isSubmitted')
592-
->willReturn(false)
593-
;
594-
$form
595-
->expects($this->once())
596-
->method('handleRequest')
597-
->with($request)
598-
->willReturn($form)
599-
;
600-
$form
601-
->expects($this->at(2))
602-
->method('isSubmitted')
603-
->willReturn(true)
604-
;
605-
$form
606-
->expects($this->once())
607-
->method('isValid')
608-
->willReturn(true)
609-
;
610-
611-
$this->assertTrue($controller->isFormValid($form));
612-
}
613-
614-
/**
615-
* @expectedException \LogicException
616-
* @expectedExceptionMessage You must pass a request as second argument because the request stack is empty.
617-
*/
618-
public function testIsFormValidWhenRequestStackIsEmpty()
619-
{
620-
$container = new Container();
621-
$container->set('request_stack', new RequestStack());
622-
623-
$controller = $this->createController();
624-
$controller->setContainer($container);
625-
626-
$form = $this->getMockBuilder('Symfony\Component\Form\FormInterface')->getMock();
627-
628-
$this->assertTrue($controller->isFormValid($form));
629-
}
630-
631520
public function testGetDoctrine()
632521
{
633522
$doctrine = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')->getMock();

0 commit comments

Comments
 (0)