Use camel case for arguments and variables at `CRUDController::histor…

…yCompareRevisionsAction()`
sonata-project · Mar 10, 2021 · 79e9849 · 79e9849
1 parent 32402b9
commit 79e9849
Show file tree

Hide file tree

Showing 6 changed files with 139 additions and 16 deletions.
diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
@@ -1,6 +1,30 @@
 UPGRADE 3.x
 ===========
 
+UPGRADE FROM 3.x to 3.x
+=======================
+
+### `Sonata\AdminBundle\Controller\CRUDController::historyCompareRevisionsAction()`
+
+- Deprecated route parameter "base_revision" in favor of "baseRevision";
+- Deprecated route parameter "compare_revision" in favor of "compareRevision".
+
+Before:
+```php
+$admin->generateObjectUrl('history_compare_revisions', $subject, [
+    'base_revision' => $currentRev,
+    'compare_revision' => $rev,
+]);
+```
+
+After:
+```php
+$admin->generateObjectUrl('history_compare_revisions', $subject, [
+    'baseRevision' => $currentRev,
+    'compareRevision' => $rev,
+]);
+```
+
 UPGRADE FROM 3.89 to 3.90
 =========================
 

diff --git a/src/Controller/CRUDController.php b/src/Controller/CRUDController.php
@@ -837,15 +837,15 @@ public function historyViewRevisionAction($id = null, $revision = null) // NEXT_
      * Compare history revisions of object.
      *
      * @param int|string|null $id
-     * @param int|string|null $base_revision
-     * @param int|string|null $compare_revision
+     * @param int|string|null $baseRevision
+     * @param int|string|null $compareRevision
      *
      * @throws AccessDeniedException If access is not granted
      * @throws NotFoundHttpException If the object or revision does not exist or the audit reader is not available
      *
      * @return Response
      */
-    public function historyCompareRevisionsAction($id = null, $base_revision = null, $compare_revision = null) // NEXT_MAJOR: Remove the unused $id parameter
+    public function historyCompareRevisionsAction($id = null, $baseRevision = null, $compareRevision = null) // NEXT_MAJOR: Remove the unused $id parameter
     {
         $this->admin->checkAccess('historyCompareRevisions');
 
@@ -866,38 +866,62 @@ public function historyCompareRevisionsAction($id = null, $base_revision = null,
 
         $reader = $manager->getReader($this->admin->getClass());
 
+        // NEXT_MAJOR: Remove this condition.
+        if ($request->attributes->has('base_revision')) {
+            // BC layer for "base_revision" route parameter.
+            $baseRevision = $baseRevision ?? $request->attributes->get('base_revision');
+
+            @trigger_error(sprintf(
+                'Route parameter "base_revision" for action "%s()" is deprecated since sonata-project/admin-bundle 3.x.'
+                .' Use "baseRevision" parameter instead.',
+                __METHOD__
+            ), \E_USER_DEPRECATED);
+        }
+
+        // NEXT_MAJOR: Remove this condition.
+        if ($request->attributes->has('compare_revision')) {
+            // BC layer for "compare_revision" route parameter.
+            $compareRevision = $compareRevision ?? $request->attributes->get('compare_revision');
+
+            @trigger_error(sprintf(
+                'Route parameter "compare_revision" for action "%s()" is deprecated since sonata-project/admin-bundle 3.x.'
+                .' Use "compareRevision" parameter instead.',
+                __METHOD__
+            ), \E_USER_DEPRECATED);
+        }
+
         // retrieve the base revision
-        $base_object = $reader->find($this->admin->getClass(), $id, $base_revision);
-        if (!$base_object) {
+        $baseObject = $reader->find($this->admin->getClass(), $id, $baseRevision);
+        if (!$baseObject) {
             throw $this->createNotFoundException(sprintf(
                 'unable to find the targeted object `%s` from the revision `%s` with classname : `%s`',
                 $id,
-                $base_revision,
+                $baseRevision,
                 $this->admin->getClass()
             ));
         }
 
         // retrieve the compare revision
-        $compare_object = $reader->find($this->admin->getClass(), $id, $compare_revision);
-        if (!$compare_object) {
+        $compareObject = $reader->find($this->admin->getClass(), $id, $compareRevision);
+        if (!$compareObject) {
             throw $this->createNotFoundException(sprintf(
                 'unable to find the targeted object `%s` from the revision `%s` with classname : `%s`',
                 $id,
-                $compare_revision,
+                $compareRevision,
                 $this->admin->getClass()
             ));
         }
 
-        $this->admin->setSubject($base_object);
+        $this->admin->setSubject($baseObject);
 
         // NEXT_MAJOR: Remove this line and use commented line below it instead
         $template = $this->admin->getTemplate('show_compare');
         // $template = $this->templateRegistry->getTemplate('show_compare');
 
         return $this->renderWithExtraParams($template, [
             'action' => 'show',
-            'object' => $base_object,
-            'object_compare' => $compare_object,
+            'object' => $baseObject,
+            'object_compare' => $compareObject,
             'elements' => $this->admin->getShow(),
         ], null);
     }

diff --git a/src/Resources/views/CRUD/base_history.html.twig b/src/Resources/views/CRUD/base_history.html.twig
@@ -42,7 +42,7 @@ file that was distributed with this source code.
                                     {% if (currentRevision == false or revision.rev == currentRevision.rev) %}
                                         /
                                     {% else %}
-                                        <a href="{{ admin.generateObjectUrl('history_compare_revisions', object, {'base_revision': currentRevision.rev, 'compare_revision': revision.rev }) }}" class="revision-compare-link" rel="{{ revision.rev }}">{{ 'label_compare_revision'|trans({}, 'SonataAdminBundle') }}</a>
+                                        <a href="{{ admin.generateObjectUrl('history_compare_revisions', object, {'baseRevision': currentRevision.rev, 'compareRevision': revision.rev }) }}" class="revision-compare-link" rel="{{ revision.rev }}">{{ 'label_compare_revision'|trans({}, 'SonataAdminBundle') }}</a>
                                     {% endif %}
                                 </td>
                             </tr>

diff --git a/src/Route/PathInfoBuilder.php b/src/Route/PathInfoBuilder.php
@@ -47,7 +47,7 @@ public function build(AdminInterface $admin, RouteCollection $collection)
         if ($this->manager->hasReader($admin->getClass())) {
             $collection->add('history', sprintf('%s/history', $admin->getRouterIdParameter()));
             $collection->add('history_view_revision', sprintf('%s/history/{revision}/view', $admin->getRouterIdParameter()));
-            $collection->add('history_compare_revisions', sprintf('%s/history/{base_revision}/{compare_revision}/compare', $admin->getRouterIdParameter()));
+            $collection->add('history_compare_revisions', sprintf('%s/history/{baseRevision}/{compareRevision}/compare', $admin->getRouterIdParameter()));
         }
 
         if ($admin->isAclEnabled()) {

diff --git a/tests/Controller/CRUDControllerTest.php b/tests/Controller/CRUDControllerTest.php
@@ -3562,6 +3562,81 @@ public function testHistoryCompareRevisionsActionAction(): void
         $this->assertSame('@SonataAdmin/CRUD/show_compare.html.twig', $this->template);
     }
 
+    /**
+     * NEXT_MAJOR: Remove this test.
+     *
+     * @group legacy
+     */
+    public function testHistoryCompareRevisionsActionWithDeprecatedRouteParams()
+    {
+        $subjectId = 123;
+        $baseRevision = 456;
+        $compareRevision = 789;
+
+        $this->request->query->set('id', $subjectId);
+        $this->request->attributes->set('base_revision', $baseRevision);
+        $this->request->attributes->set('compare_revision', $compareRevision);
+
+        $this->admin->expects($this->once())
+            ->method('checkAccess')
+            ->with($this->equalTo('historyCompareRevisions'))
+            ->willReturn(true);
+
+        $object = new \stdClass();
+
+        $this->admin->expects($this->exactly(2))
+            ->method('getObject')
+            ->willReturn($object);
+
+        $this->admin
+            ->method('getClass')
+            ->willReturn('Foo');
+
+        $this->auditManager->expects($this->once())
+            ->method('hasReader')
+            ->with($this->equalTo('Foo'))
+            ->willReturn(true);
+
+        $reader = $this->createMock(AuditReaderInterface::class);
+
+        $this->auditManager->expects($this->once())
+            ->method('getReader')
+            ->with($this->equalTo('Foo'))
+            ->willReturn($reader);
+
+        $objectRevision = new \stdClass();
+        $objectRevision->revision = $baseRevision;
+
+        $compareObjectRevision = new \stdClass();
+        $compareObjectRevision->revision = $compareRevision;
+
+        $reader->expects($this->exactly(2))->method('find')->willReturnMap([
+            ['Foo', $subjectId, $baseRevision, $objectRevision],
+            ['Foo', $subjectId, $compareRevision, $compareObjectRevision],
+        ]);
+
+        $this->admin->expects($this->once())
+            ->method('setSubject')
+            ->with($this->equalTo($objectRevision));
+
+        $fieldDescriptionCollection = new FieldDescriptionCollection();
+        $this->admin->expects($this->once())
+            ->method('getShow')
+            ->willReturn($fieldDescriptionCollection);
+
+        $this->expectDeprecation(
+            'Route parameter "base_revision" for action "Sonata\AdminBundle\Controller\CRUDController::historyCompareRevisionsAction()"'.
+            ' is deprecated since sonata-project/admin-bundle 3.x. Use "baseRevision" parameter instead.'
+        );
+
+        $this->expectDeprecation(
+            'Route parameter "compare_revision" for action "Sonata\AdminBundle\Controller\CRUDController::historyCompareRevisionsAction()"'.
+            ' is deprecated since sonata-project/admin-bundle 3.x. Use "compareRevision" parameter instead.'
+        );
+
+        $this->assertInstanceOf(Response::class, $this->controller->historyCompareRevisionsAction());
+    }
+
     public function testBatchActionWrongMethod(): void
     {
         $this->expectException(NotFoundHttpException::class);

diff --git a/tests/Form/Type/Operator/StringOperatorTypeTest.php b/tests/Form/Type/Operator/StringOperatorTypeTest.php
@@ -23,7 +23,7 @@ public function testConfigureOptions(): void
     {
         $formType = new StringOperatorType();
         $optionsResolver = new OptionsResolver();
-        $expected_choices = [
+        $expectedChoices = [
             'label_type_contains' => StringOperatorType::TYPE_CONTAINS,
             'label_type_not_contains' => StringOperatorType::TYPE_NOT_CONTAINS,
             'label_type_equals' => StringOperatorType::TYPE_EQUAL,
@@ -33,7 +33,7 @@ public function testConfigureOptions(): void
         ];
         $formType->configureOptions($optionsResolver);
         $options = $optionsResolver->resolve([]);
-        $this->assertSame($expected_choices, $options['choices']);
+        $this->assertSame($expectedChoices, $options['choices']);
         $this->assertSame('SonataAdminBundle', $options['choice_translation_domain']);
     }
 }