Browse Source

Refactor button visibility business logic to template

During a code review by @Stof he indicated that it is desirable to follow suit
with the rest of the application and have the business logic determining
whether the 'delete' button is visible in the template instead of the Controller.
Mike van Riel 13 years ago
parent
commit
28dd136ed9

+ 2 - 15
src/Packagist/WebBundle/Controller/WebController.php

@@ -396,19 +396,9 @@ class WebController extends Controller
         /** @var \Packagist\WebBundle\Entity\VersionRepository $repo  */
         /** @var \Packagist\WebBundle\Entity\VersionRepository $repo  */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Version');
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Version');
 
 
-        /** @var Version $version  */
-        $version = $repo->getFullVersion($versionId);
-        $package = $version->getPackage();
-
-        $isMaintainer   = $package->getMaintainers()->contains($this->getUser());
-        $mayEditPackage = $this->get('security.context')->isGranted('ROLE_EDIT_PACKAGES');
-
         $html = $this->renderView(
         $html = $this->renderView(
             'PackagistWebBundle:Web:versionDetails.html.twig',
             'PackagistWebBundle:Web:versionDetails.html.twig',
-            array(
-                'version'    => $version,
-                'mayDelete' => $isMaintainer || $mayEditPackage,
-            )
+            array('version' => $repo->getFullVersion($versionId))
         );
         );
 
 
         return new JsonResponse(array('content' => $html));
         return new JsonResponse(array('content' => $html));
@@ -432,10 +422,7 @@ class WebController extends Controller
         $version = $repo->getFullVersion($versionId);
         $version = $repo->getFullVersion($versionId);
         $package = $version->getPackage();
         $package = $version->getPackage();
 
 
-        $isMaintainer   = $package->getMaintainers()->contains($this->getUser());
-        $mayEditPackage = $this->get('security.context')->isGranted('ROLE_EDIT_PACKAGES');
-
-        if (!$isMaintainer || !$mayEditPackage) {
+        if (!$package->getMaintainers()->contains($this->getUser()) && !$this->get('security.context')->isGranted('ROLE_EDIT_PACKAGES')) {
             throw new AccessDeniedException;
             throw new AccessDeniedException;
         }
         }
 
 

+ 1 - 1
src/Packagist/WebBundle/Resources/views/Web/versionDetails.html.twig

@@ -1,6 +1,6 @@
 {% import "PackagistWebBundle::macros.html.twig" as packagist %}
 {% import "PackagistWebBundle::macros.html.twig" as packagist %}
 
 
-{% if mayDelete is defined and mayDelete %}
+{% if is_granted('ROLE_EDIT_PACKAGES') or version.package.maintainers.contains(app.user) %}
 <form class="action" action="{{ path("delete_version", {"versionId": version.id}) }}" method="post">
 <form class="action" action="{{ path("delete_version", {"versionId": version.id}) }}" method="post">
     <input type="hidden" name="_method" value="DELETE" />
     <input type="hidden" name="_method" value="DELETE" />
     <input type="submit" value="Delete">
     <input type="submit" value="Delete">