From 638217b55841062593f3c0b21bbf883ca19fab07 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Wed, 18 Nov 2020 19:33:57 +0100 Subject: [PATCH 1/2] Adding return type hints --- src/Service/GitHubRequestHandler.php | 1 - src/Service/RepositoryProvider.php | 7 +++++-- src/Service/TaskHandler/CloseDraftHandler.php | 12 ++++++++++-- tests/Controller/WebhookControllerTest.php | 6 +----- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Service/GitHubRequestHandler.php b/src/Service/GitHubRequestHandler.php index 5ecc168d..40adde74 100644 --- a/src/Service/GitHubRequestHandler.php +++ b/src/Service/GitHubRequestHandler.php @@ -46,7 +46,6 @@ public function handle(Request $request) $this->logger->debug(sprintf('Handling from repository %s', $repositoryFullName)); $repository = $this->repositoryProvider->getRepository($repositoryFullName); - if (!$repository) { throw new PreconditionFailedHttpException(sprintf('Unsupported repository "%s".', $repositoryFullName)); } diff --git a/src/Service/RepositoryProvider.php b/src/Service/RepositoryProvider.php index ccf661cd..eaf640a7 100644 --- a/src/Service/RepositoryProvider.php +++ b/src/Service/RepositoryProvider.php @@ -28,14 +28,17 @@ public function __construct(array $repositories) } } - public function getRepository($repositoryName) + public function getRepository(string $repositoryName): ?Repository { $repository = strtolower($repositoryName); return $this->repositories[$repository] ?? null; } - public function getAllRepositories() + /** + * @return Repository[] + */ + public function getAllRepositories(): array { return array_values($this->repositories); } diff --git a/src/Service/TaskHandler/CloseDraftHandler.php b/src/Service/TaskHandler/CloseDraftHandler.php index ad64afe8..f8eb5132 100644 --- a/src/Service/TaskHandler/CloseDraftHandler.php +++ b/src/Service/TaskHandler/CloseDraftHandler.php @@ -8,6 +8,7 @@ use App\Api\PullRequest\PullRequestApi; use App\Entity\Task; use App\Service\RepositoryProvider; +use Psr\Log\LoggerInterface; /** * @author Tobias Nyholm @@ -17,19 +18,26 @@ class CloseDraftHandler implements TaskHandlerInterface private $issueApi; private $repositoryProvider; private $pullRequestApi; + private $logger; - public function __construct(PullRequestApi $pullRequestApi, IssueApi $issueApi, RepositoryProvider $repositoryProvider) + public function __construct(PullRequestApi $pullRequestApi, IssueApi $issueApi, RepositoryProvider $repositoryProvider, LoggerInterface $logger) { $this->issueApi = $issueApi; $this->repositoryProvider = $repositoryProvider; $this->pullRequestApi = $pullRequestApi; + $this->logger = $logger; } public function handle(Task $task): void { $repository = $this->repositoryProvider->getRepository($task->getRepositoryFullName()); - $pr = $this->pullRequestApi->show($repository, $task->getNumber()); + if (!$repository) { + $this->logger->error(sprintf('RepositoryProvider returned nothing for "%s" ', $task->getRepositoryFullName())); + + return; + } + $pr = $this->pullRequestApi->show($repository, $task->getNumber()); if ($pr['draft'] ?? false) { $this->issueApi->close($repository, $task->getNumber()); } diff --git a/tests/Controller/WebhookControllerTest.php b/tests/Controller/WebhookControllerTest.php index dfb3baaa..60e5f2b0 100644 --- a/tests/Controller/WebhookControllerTest.php +++ b/tests/Controller/WebhookControllerTest.php @@ -21,11 +21,7 @@ public function setup() $statusApi = self::$container->get(StatusApi::class); // the labels need to be off this issue for one test to pass - $statusApi->setIssueStatus( - 2, - null, - $repository->getRepository('carsonbot-playground/symfony') - ); + $statusApi->setIssueStatus(2, null, $repository->getRepository('carsonbot-playground/symfony')); } /** From 65d911863948e29d418677d49d1773f1e09acbc3 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Wed, 18 Nov 2020 19:36:13 +0100 Subject: [PATCH 2/2] Fixed tests --- tests/Service/TaskHandler/CloseDraftHandlerTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Service/TaskHandler/CloseDraftHandlerTest.php b/tests/Service/TaskHandler/CloseDraftHandlerTest.php index c5744b88..f9f86978 100644 --- a/tests/Service/TaskHandler/CloseDraftHandlerTest.php +++ b/tests/Service/TaskHandler/CloseDraftHandlerTest.php @@ -10,6 +10,7 @@ use App\Service\RepositoryProvider; use App\Service\TaskHandler\CloseDraftHandler; use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; class CloseDraftHandlerTest extends TestCase { @@ -29,7 +30,7 @@ public function testHandleStillInDraft() $repoProvider = new RepositoryProvider(['carsonbot-playground/symfony' => []]); - $handler = new CloseDraftHandler($prApi, $issueApi, $repoProvider); + $handler = new CloseDraftHandler($prApi, $issueApi, $repoProvider, new NullLogger()); $handler->handle(new Task('carsonbot-playground/symfony', 4711, Task::ACTION_CLOSE_DRAFT, new \DateTimeImmutable())); } @@ -49,7 +50,7 @@ public function testHandleNotDraft() $repoProvider = new RepositoryProvider(['carsonbot-playground/symfony' => []]); - $handler = new CloseDraftHandler($prApi, $issueApi, $repoProvider); + $handler = new CloseDraftHandler($prApi, $issueApi, $repoProvider, new NullLogger()); $handler->handle(new Task('carsonbot-playground/symfony', 4711, Task::ACTION_CLOSE_DRAFT, new \DateTimeImmutable())); } }