Remove webhook when package removed (api) and organization removed (u…

…i) (#404) * Remove webhook when package removed (api) and organization removed (ui) * Add functional tests for invite user to organization flow
repman-io · Feb 3, 2021 · 047f0a5 · 047f0a5
1 parent de86f5f
commit 047f0a5
Show file tree

Hide file tree

Showing 13 changed files with 275 additions and 75 deletions.
diff --git a/composer.lock b/composer.lock
diff --git a/src/Controller/Api/PackageController.php b/src/Controller/Api/PackageController.php
@@ -12,6 +12,9 @@
 use Buddy\Repman\Message\Organization\Package\AddBitbucketHook;
 use Buddy\Repman\Message\Organization\Package\AddGitHubHook;
 use Buddy\Repman\Message\Organization\Package\AddGitLabHook;
+use Buddy\Repman\Message\Organization\Package\RemoveBitbucketHook;
+use Buddy\Repman\Message\Organization\Package\RemoveGitHubHook;
+use Buddy\Repman\Message\Organization\Package\RemoveGitLabHook;
 use Buddy\Repman\Message\Organization\Package\Update;
 use Buddy\Repman\Message\Organization\RemovePackage;
 use Buddy\Repman\Message\Organization\SynchronizePackage;
@@ -31,6 +34,7 @@
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Messenger\Exception\HandlerFailedException;
 use Symfony\Component\Routing\Annotation\Route;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
@@ -150,7 +154,7 @@ public function getPackage(Organization $organization, Package $package): JsonRe
      *
      * @OA\Response(
      *     response=200,
-     *     description="Package removed"
+     *     description="Package removed, if there was a problem with removing the webhook, the 'warning' field will appear"
      * )
      *
      * @OA\Response(
@@ -167,9 +171,12 @@ public function getPackage(Organization $organization, Package $package): JsonRe
      */
     public function removePackage(Organization $organization, Package $package): JsonResponse
     {
+        $warning = $this->tryToRemoveWebhook($package);
         $this->dispatchMessage(new RemovePackage($package->getId(), $organization->id()));
 
-        return new JsonResponse();
+        return new JsonResponse($warning !== null ? [
+            'warning' => $warning,
+        ] : null);
     }
 
     /**
@@ -500,4 +507,33 @@ private function getToken(string $type): string
 
         return $token;
     }
+
+    private function tryToRemoveWebhook(Package $package): ?string
+    {
+        $warning = null;
+        if ($package->getWebhookCreatedAt() !== null) {
+            try {
+                switch ($package->getType()) {
+                    case 'github-oauth':
+                        $this->dispatchMessage(new RemoveGitHubHook($package->getId()));
+                        break;
+                    case 'gitlab-oauth':
+                        $this->dispatchMessage(new RemoveGitLabHook($package->getId()));
+                        break;
+                    case 'bitbucket-oauth':
+                        $this->dispatchMessage(new RemoveBitbucketHook($package->getId()));
+                        break;
+                }
+            } catch (HandlerFailedException $exception) {
+                $reason = current($exception->getNestedExceptions());
+
+                $warning = sprintf(
+                    'Webhook removal failed due to "%s". Please remove it manually.',
+                    $reason !== false ? $reason->getMessage() : $exception->getMessage()
+                );
+            }
+        }
+
+        return $warning;
+    }
 }
diff --git a/src/Controller/IndexController.php b/src/Controller/IndexController.php
@@ -27,16 +27,11 @@ public function __construct(SessionInterface $session, Telemetry $telemetry)
     public function index(): Response
     {
         if ($this->session->has('organization-token')) {
-            $organizationToken = $this->session->get('organization-token');
-            $this->session->remove('organization-token');
-
-            return $this->redirectToRoute('organization_accept_invitation', ['token' => $organizationToken]);
+            return $this->redirectToRoute('organization_accept_invitation', ['token' => $this->session->remove('organization-token')]);
         }
 
-        $showTelemetryPrompt = !$this->telemetry->isInstanceIdPresent();
-
         return $this->render('index.html.twig', [
-            'showTelemetryPrompt' => $showTelemetryPrompt,
+            'showTelemetryPrompt' => !$this->telemetry->isInstanceIdPresent(),
             'telemetryDocsUrl' => $this->telemetry->docsUrl(),
         ]);
     }

diff --git a/src/Controller/OrganizationController.php b/src/Controller/OrganizationController.php
@@ -295,6 +295,12 @@ public function settings(Organization $organization, Request $request): Response
      */
     public function removeOrganization(Organization $organization): Response
     {
+        $offset = 0;
+        while (($packages = $this->packageQuery->findAll($organization->id(), new PackageQuery\Filter($offset, $limit = 100))) !== []) {
+            array_walk($packages, [$this, 'tryToRemoveWebhook']);
+            $offset += $limit;
+        }
+
         $this->dispatchMessage(new RemoveOrganization($organization->id()));
         $this->addFlash('success', sprintf('Organization %s has been successfully removed', $organization->name()));
 

diff --git a/src/Controller/RegistrationController.php b/src/Controller/RegistrationController.php
@@ -60,12 +60,9 @@ public function register(Request $request): Response
             $this->guard->authenticateUser($email, $request);
 
             if ($this->session->has('organization-token')) {
-                $organizationToken = $this->session->get('organization-token');
-                $this->session->remove('organization-token');
-
                 $this->addFlash('success', 'Your account has been created.');
 
-                return $this->redirectToRoute('organization_accept_invitation', ['token' => $organizationToken]);
+                return $this->redirectToRoute('organization_accept_invitation', ['token' => $this->session->remove('organization-token')]);
             }
 
             $this->addFlash('success', 'Your account has been created. Please create a new organization.');

diff --git a/tests/Doubles/FakeBitbucketApi.php b/tests/Doubles/FakeBitbucketApi.php
@@ -11,6 +11,10 @@ final class FakeBitbucketApi implements BitbucketApi
 {
     private string $primaryEmail = '';
     private ?\Throwable $exception = null;
+    /**
+     * @var string[]
+     */
+    private array $removedWebhooks = [];
 
     public function primaryEmail(string $accessToken): string
     {
@@ -46,6 +50,15 @@ public function addHook(string $accessToken, string $fullName, string $hookUrl):
     public function removeHook(string $accessToken, string $fullName, string $hookUrl): void
     {
         $this->throwExceptionIfSet();
+        $this->removedWebhooks[] = $fullName;
+    }
+
+    /**
+     * @return string[]
+     */
+    public function removedWebhooks(): array
+    {
+        return $this->removedWebhooks;
     }
 
     private function throwExceptionIfSet(): void

diff --git a/tests/Doubles/FakeGitHubApi.php b/tests/Doubles/FakeGitHubApi.php
@@ -10,6 +10,10 @@ final class FakeGitHubApi implements GitHubApi
 {
     private string $primaryEmail = '';
     private ?\Throwable $exception = null;
+    /**
+     * @var string[]
+     */
+    private array $removedWebhooks = [];
 
     public function primaryEmail(string $accessToken): string
     {
@@ -48,6 +52,15 @@ public function addHook(string $accessToken, string $repo, string $url): void
     public function removeHook(string $accessToken, string $repo, string $url): void
     {
         $this->throwExceptionIfSet();
+        $this->removedWebhooks[] = $repo;
+    }
+
+    /**
+     * @return string[]
+     */
+    public function removedWebhooks(): array
+    {
+        return $this->removedWebhooks;
     }
 
     private function throwExceptionIfSet(): void