From a71a4329c8b7e51a825dfab35b7715042c5ebf3c Mon Sep 17 00:00:00 2001 From: Adam Lundrigan Date: Thu, 30 Apr 2015 21:33:15 -0230 Subject: [PATCH 1/3] Update RepositoryRetriever to catch EdpGithub exception and return false --- .../Service/RepositoryRetriever.php | 18 ++++++++--- .../Service/RepositoryRetrieverTest.php | 32 +++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/module/Application/src/Application/Service/RepositoryRetriever.php b/module/Application/src/Application/Service/RepositoryRetriever.php index e05f2214..795c7acc 100644 --- a/module/Application/src/Application/Service/RepositoryRetriever.php +++ b/module/Application/src/Application/Service/RepositoryRetriever.php @@ -46,11 +46,16 @@ public function getUserRepositoryMetadata($user, $module) * @param string $user * @param array $params * - * @return RepositoryCollection + * @return RepositoryCollection|bool */ public function getUserRepositories($user, $params = []) { - return $this->githubClient->api('user')->repos($user, $params); + try { + return $this->githubClient->api('user')->repos($user, $params); + } + catch (RuntimeException $e) { + return false; + } } /** @@ -140,10 +145,15 @@ public function getRepositoryFileMetadata($user, $module, $filePath) * * @param array $params * - * @return RepositoryCollection + * @return RepositoryCollection|bool */ public function getAuthenticatedUserRepositories($params = []) { - return $this->githubClient->api('current_user')->repos($params); + try { + return $this->githubClient->api('current_user')->repos($params); + } + catch (RuntimeException $e) { + return false; + } } } diff --git a/module/Application/test/ApplicationTest/Service/RepositoryRetrieverTest.php b/module/Application/test/ApplicationTest/Service/RepositoryRetrieverTest.php index ffea5f22..b751d89c 100644 --- a/module/Application/test/ApplicationTest/Service/RepositoryRetrieverTest.php +++ b/module/Application/test/ApplicationTest/Service/RepositoryRetrieverTest.php @@ -45,6 +45,22 @@ public function testCanRetrieveUserRepositories() $this->assertEquals(count($payload), $count); } + public function testGetUserRepositoriesReturnsFalseOnRemoteFailure() + { + $client = $this->getClientMock( + new Api\User(), + [] + ); + $client->expects($this->any()) + ->method('api') + ->willThrowException(new Exception\RuntimeException()); + + $service = new RepositoryRetriever($client); + + $repositories = $service->getUserRepositories('foo'); + $this->assertFalse($repositories); + } + public function testCanRetrieveUserRepositoryMetadata() { $payload = [ @@ -202,6 +218,22 @@ public function testCanRetrieveAuthenticatedUserRepositories() $this->assertEquals(count($payload), $count); } + public function testGetAuthenticatedUserRepositoriesReturnsFalseOnRemoteFailure() + { + $client = $this->getClientMock( + new Api\CurrentUser(), + [] + ); + $client->expects($this->any()) + ->method('api') + ->willThrowException(new Exception\RuntimeException()); + + $service = new RepositoryRetriever($client); + + $repositories = $service->getAuthenticatedUserRepositories(); + $this->assertFalse($repositories); + } + public function testRepositoryFileContentFails() { $clientMock = $this->getMock('EdpGithub\Client'); From 393f709e9b42892838bf5f4f3e11d63ce1d146e9 Mon Sep 17 00:00:00 2001 From: Adam Lundrigan Date: Thu, 30 Apr 2015 22:09:55 -0230 Subject: [PATCH 2/3] Update ModuleController to detect module fetch failure and display error message to user --- .../view/user/helper/user-organizations.phtml | 12 ++- module/User/view/zfc-user/user/index.phtml | 11 ++- .../ZfModule/Controller/ModuleController.php | 28 ++++-- .../Controller/ModuleControllerTest.php | 87 +++++++++++++++++++ .../view/zf-module/module/index.phtml | 4 +- 5 files changed, 130 insertions(+), 12 deletions(-) diff --git a/module/User/view/user/helper/user-organizations.phtml b/module/User/view/user/helper/user-organizations.phtml index 81399a04..18e1f14a 100644 --- a/module/User/view/user/helper/user-organizations.phtml +++ b/module/User/view/user/helper/user-organizations.phtml @@ -26,12 +26,18 @@ inlineScript()->appendScript(sprintf($jsTemplate, $org->login, $org->login, $this->url('zf-module/list', ['owner' => $org->login]))); + $this->inlineScript()->appendScript(sprintf($jsTemplate, $org->login, $this->url('zf-module/list', ['owner' => $org->login]))); ?> diff --git a/module/User/view/zfc-user/user/index.phtml b/module/User/view/zfc-user/user/index.phtml index 8c4bd425..133ff505 100644 --- a/module/User/view/zfc-user/user/index.phtml +++ b/module/User/view/zfc-user/user/index.phtml @@ -48,4 +48,13 @@ -inlineScript()->appendScript('$("#repositories").load("' . $this->escapeJs($this->url('zf-module')) . '");'); ?> +inlineScript()->appendScript(<<escapeJs($this->url('zf-module'))}", + }).done(function(data) { + $('#repositories').html(data); + }).fail(function(xhr) { + $('#repositories').html(xhr.responseText); + }); +EOB +); diff --git a/module/ZfModule/src/ZfModule/Controller/ModuleController.php b/module/ZfModule/src/ZfModule/Controller/ModuleController.php index d62e0931..def2d962 100644 --- a/module/ZfModule/src/ZfModule/Controller/ModuleController.php +++ b/module/ZfModule/src/ZfModule/Controller/ModuleController.php @@ -83,17 +83,24 @@ public function indexAction() return $this->redirect()->toRoute('zfcuser/login'); } + $viewModel = new ViewModel(); + $viewModel->setTerminal(true); + $currentUserRepositories = $this->repositoryRetriever->getAuthenticatedUserRepositories([ 'type' => 'all', 'per_page' => 100, 'sort' => 'updated', 'direction' => 'desc', ]); + if ($currentUserRepositories === false) { + $this->getResponse()->setStatusCode(503); + $viewModel->setVariable('errorMessage', 'module_fetch_failed'); - $repositories = $this->unregisteredRepositories($currentUserRepositories); + return $viewModel; + } - $viewModel = new ViewModel(['repositories' => $repositories]); - $viewModel->setTerminal(true); + $repositories = $this->unregisteredRepositories($currentUserRepositories); + $viewModel->setVariable('repositories', $repositories); return $viewModel; } @@ -105,18 +112,25 @@ public function listAction() } $owner = $this->params()->fromRoute('owner', null); + + $viewModel = new ViewModel(); + $viewModel->setTerminal(true); + $viewModel->setTemplate('zf-module/module/index.phtml'); $userRepositories = $this->repositoryRetriever->getUserRepositories($owner, [ 'per_page' => 100, 'sort' => 'updated', 'direction' => 'desc', ]); + if ($userRepositories === false) { + $this->getResponse()->setStatusCode(503); + $viewModel->setVariable('errorMessage', 'module_fetch_failed'); + return $viewModel; + } + $repositories = $this->unregisteredRepositories($userRepositories); - - $viewModel = new ViewModel(['repositories' => $repositories]); - $viewModel->setTerminal(true); - $viewModel->setTemplate('zf-module/module/index.phtml'); + $viewModel->setVariable('repositories', $repositories); return $viewModel; } diff --git a/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php b/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php index b5e53be2..d5110e44 100644 --- a/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php +++ b/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php @@ -173,6 +173,46 @@ public function testIndexActionRendersUnregisteredModulesOnly() $this->assertCount(1, $viewVariable); $this->assertSame($unregisteredModule, $viewVariable[0]); } + + public function testIndexActionRendersErrorMessageWhenModuleFetchReturnsFalse() + { + $this->authenticatedAs(new User()); + + $repositoryRetriever = $this->getMockBuilder(RepositoryRetriever::class) + ->disableOriginalConstructor() + ->getMock() + ; + + $repositoryRetriever + ->expects($this->once()) + ->method('getAuthenticatedUserRepositories') + ->willReturn(false) + ; + + $this->getApplicationServiceLocator() + ->setAllowOverride(true) + ->setService( + RepositoryRetriever::class, + $repositoryRetriever + ) + ; + + $this->dispatch('/module'); + + $this->assertMatchedRouteName('zf-module'); + + $this->assertControllerName(Controller\ModuleController::class); + $this->assertActionName('index'); + $this->assertResponseStatusCode(Http\Response::STATUS_CODE_503); + + $viewModel = $this->getApplication()->getMvcEvent()->getViewModel(); + + $this->assertTrue($viewModel->terminate()); + $this->assertSame('zf-module/module/index', $viewModel->getTemplate()); + + $this->assertEquals(null, $viewModel->getVariable('repositories')); + $this->assertEquals('module_fetch_failed', $viewModel->getVariable('errorMessage')); + } public function testListActionRedirectsIfNotAuthenticated() { @@ -439,6 +479,53 @@ public function testListActionRendersUnregisteredModulesOnly() $this->assertCount(1, $viewVariable); $this->assertSame($unregisteredModule, $viewVariable[0]); } + + public function testListActionRendersErrorMessageWhenModuleFetchReturnsFalse() + { + $this->authenticatedAs(new User()); + + $repositoryRetriever = $this->getMockBuilder(RepositoryRetriever::class) + ->disableOriginalConstructor() + ->getMock() + ; + + $vendor = 'suzie'; + + $repositoryRetriever + ->expects($this->once()) + ->method('getUserRepositories') + ->willReturn(false) + ; + + $this->getApplicationServiceLocator() + ->setAllowOverride(true) + ->setService( + RepositoryRetriever::class, + $repositoryRetriever + ) + ; + + $url = sprintf( + '/module/list/%s', + $vendor + ); + + $this->dispatch($url); + + $this->assertMatchedRouteName('zf-module/list'); + + $this->assertControllerName(Controller\ModuleController::class); + $this->assertActionName('list'); + $this->assertResponseStatusCode(Http\Response::STATUS_CODE_503); + + $viewModel = $this->getApplication()->getMvcEvent()->getViewModel(); + + $this->assertTrue($viewModel->terminate()); + $this->assertSame('zf-module/module/index.phtml', $viewModel->getTemplate()); + + $this->assertEquals(null, $viewModel->getVariable('repositories')); + $this->assertEquals('module_fetch_failed', $viewModel->getVariable('errorMessage')); + } public function testAddActionRedirectsIfNotAuthenticated() { diff --git a/module/ZfModule/view/zf-module/module/index.phtml b/module/ZfModule/view/zf-module/module/index.phtml index 882333f9..69c68910 100644 --- a/module/ZfModule/view/zf-module/module/index.phtml +++ b/module/ZfModule/view/zf-module/module/index.phtml @@ -1,5 +1,7 @@ - + +
An error occurred while fetching modules
+
No modules found
From 75a7d8b10271d0a2b8b194f1560ca8f284a7adaf Mon Sep 17 00:00:00 2001 From: Adam Lundrigan Date: Thu, 30 Apr 2015 22:18:16 -0230 Subject: [PATCH 3/3] CS fixes --- .../src/Application/Service/RepositoryRetriever.php | 6 ++---- .../ZfModule/src/ZfModule/Controller/ModuleController.php | 6 +++--- .../Integration/Controller/ModuleControllerTest.php | 6 +++--- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/module/Application/src/Application/Service/RepositoryRetriever.php b/module/Application/src/Application/Service/RepositoryRetriever.php index 795c7acc..cf5c051c 100644 --- a/module/Application/src/Application/Service/RepositoryRetriever.php +++ b/module/Application/src/Application/Service/RepositoryRetriever.php @@ -52,8 +52,7 @@ public function getUserRepositories($user, $params = []) { try { return $this->githubClient->api('user')->repos($user, $params); - } - catch (RuntimeException $e) { + } catch (RuntimeException $e) { return false; } } @@ -151,8 +150,7 @@ public function getAuthenticatedUserRepositories($params = []) { try { return $this->githubClient->api('current_user')->repos($params); - } - catch (RuntimeException $e) { + } catch (RuntimeException $e) { return false; } } diff --git a/module/ZfModule/src/ZfModule/Controller/ModuleController.php b/module/ZfModule/src/ZfModule/Controller/ModuleController.php index def2d962..b84cb12e 100644 --- a/module/ZfModule/src/ZfModule/Controller/ModuleController.php +++ b/module/ZfModule/src/ZfModule/Controller/ModuleController.php @@ -85,7 +85,7 @@ public function indexAction() $viewModel = new ViewModel(); $viewModel->setTerminal(true); - + $currentUserRepositories = $this->repositoryRetriever->getAuthenticatedUserRepositories([ 'type' => 'all', 'per_page' => 100, @@ -112,7 +112,7 @@ public function listAction() } $owner = $this->params()->fromRoute('owner', null); - + $viewModel = new ViewModel(); $viewModel->setTerminal(true); $viewModel->setTemplate('zf-module/module/index.phtml'); @@ -128,7 +128,7 @@ public function listAction() return $viewModel; } - + $repositories = $this->unregisteredRepositories($userRepositories); $viewModel->setVariable('repositories', $repositories); diff --git a/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php b/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php index d5110e44..307f7bed 100644 --- a/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php +++ b/module/ZfModule/test/ZfModuleTest/Integration/Controller/ModuleControllerTest.php @@ -173,7 +173,7 @@ public function testIndexActionRendersUnregisteredModulesOnly() $this->assertCount(1, $viewVariable); $this->assertSame($unregisteredModule, $viewVariable[0]); } - + public function testIndexActionRendersErrorMessageWhenModuleFetchReturnsFalse() { $this->authenticatedAs(new User()); @@ -479,7 +479,7 @@ public function testListActionRendersUnregisteredModulesOnly() $this->assertCount(1, $viewVariable); $this->assertSame($unregisteredModule, $viewVariable[0]); } - + public function testListActionRendersErrorMessageWhenModuleFetchReturnsFalse() { $this->authenticatedAs(new User()); @@ -490,7 +490,7 @@ public function testListActionRendersErrorMessageWhenModuleFetchReturnsFalse() ; $vendor = 'suzie'; - + $repositoryRetriever ->expects($this->once()) ->method('getUserRepositories')