Do not add uris to transition dropdown items if use is not allowed to apply

Author: yann-eugone

src/Admin/Extension/WorkflowExtension.php

@@ -78,13 +78,7 @@ public function configureSideMenu(
         }
 
         $subject = $admin->getSubject();
-        if (null === $subject) {
-            return;
-        }
-
-        try {
-            $admin->checkAccess('viewTransitions', $subject);
-        } catch (AccessDeniedException $exception) {
+        if (null === $subject || !$this->isGrantedView($admin, $subject)) {
             return;
         }
 
@@ -210,13 +204,16 @@ protected function transitionsDropdown(MenuItemInterface $menu, AdminInterface $
     protected function transitionsItem(MenuItemInterface $menu, AdminInterface $admin, Transition $transition, $subject)
     {
         $options = [
-            'uri' => $this->generateTransitionUri($admin, $transition, $subject),
             'attributes' => [],
             'extras' => [
                 'translation_domain' => $admin->getTranslationDomain(),
             ],
         ];
 
+        if ($this->isGrantedApply($admin, $subject)) {
+            $options['uri'] = $this->generateTransitionUri($admin, $transition, $subject);
+        }
+
         if ($icon = $this->getTransitionIcon($transition)) {
             $options['attributes']['icon'] = $icon;
         }
@@ -256,4 +253,38 @@ protected function generateTransitionUri(AdminInterface $admin, Transition $tran
             ['transition' => $transition->getName()]
         );
     }
+
+    /**
+     * @param AdminInterface $admin
+     * @param object         $subject
+     *
+     * @return bool
+     */
+    protected function isGrantedView(AdminInterface $admin, $subject)
+    {
+        try {
+            $admin->checkAccess('viewTransitions', $subject);
+        } catch (AccessDeniedException $exception) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * @param AdminInterface $admin
+     * @param object         $subject
+     *
+     * @return bool
+     */
+    protected function isGrantedApply(AdminInterface $admin, $subject)
+    {
+        try {
+            $admin->checkAccess('applyTransitions', $subject);
+        } catch (AccessDeniedException $exception) {
+            return false;
+        }
+
+        return true;
+    }
 }

tests/Admin/Extension/WorkflowExtensionTest.php

@@ -123,7 +123,7 @@ public function testConfigureSideMenuWithoutWorkflow()
     /**
      * @dataProvider markingToTransition
      */
-    public function testConfigureSideMenu($marking, array $transitions)
+    public function testConfigureSideMenu($marking, array $transitions, $grantedApply)
     {
         $pullRequest = new PullRequest();
         $pullRequest->setMarking($marking);
@@ -137,14 +137,24 @@ public function testConfigureSideMenu($marking, array $transitions)
         $admin->getLabelTranslatorStrategy()->willReturn($labelStrategy->reveal());
         $admin->getSubject()->willReturn($pullRequest);
         $admin->checkAccess('viewTransitions', $pullRequest)->shouldBeCalled();
+        if ($grantedApply) {
+            $admin->checkAccess('applyTransitions', $pullRequest)->shouldBeCalledTimes(count($transitions));
+        } else {
+            $admin->checkAccess('applyTransitions', $pullRequest)->willThrow(new AccessDeniedException());
+        }
 
         foreach ($transitions as $transition) {
             $labelStrategy->getLabel($transition, 'workflow', 'transition')
                 ->shouldBeCalledTimes(1)
                 ->willReturn('workflow.transition.'.$transition);
-            $admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition])
-                ->shouldBeCalledTimes(1)
-                ->willReturn('/pull-request/42/workflow/transition/'.$transition.'/apply');
+            if ($grantedApply) {
+                $admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition])
+                    ->shouldBeCalledTimes(1)
+                    ->willReturn('/pull-request/42/workflow/transition/'.$transition.'/apply');
+            } else {
+                $admin->generateObjectUrl('workflow_apply_transition', $pullRequest, ['transition' => $transition])
+                    ->shouldNotBeCalled();
+            }
         }
 
         $registry = new Registry();
@@ -182,7 +192,11 @@ public function testConfigureSideMenu($marking, array $transitions)
                 }
 
                 self::assertNotNull($item = $child->getChild('workflow.transition.'.$transition));
-                self::assertSame('/pull-request/42/workflow/transition/'.$transition.'/apply', $item->getUri());
+                if ($grantedApply) {
+                    self::assertSame('/pull-request/42/workflow/transition/'.$transition.'/apply', $item->getUri());
+                } else {
+                    self::assertNull($item->getUri());
+                }
                 self::assertSame('admin', $item->getExtra('translation_domain'));
                 self::assertSame($icon, $item->getAttribute('icon'));
             }
@@ -191,10 +205,12 @@ public function testConfigureSideMenu($marking, array $transitions)
 
     public function markingToTransition()
     {
-        return [
-            'opened' => ['opened', ['start_review']],
-            'pending_review' => ['pending_review', ['merge', 'close']],
-            'closed' => ['closed', []],
-        ];
+        foreach ([true, false] as $grantedApply) {
+            $grantedApplyStr = $grantedApply ? 'with links' : 'without links';
+
+            yield 'opened '.$grantedApplyStr => ['opened', ['start_review'], $grantedApply];
+            yield 'pending_review '.$grantedApplyStr => ['pending_review', ['merge', 'close'], $grantedApply];
+            yield 'closed '.$grantedApplyStr => ['closed', [], $grantedApply];
+        }
     }
 }