From 3ea2c3b98d0ab031c0f32063262d36cde71dc13c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jovan=20Kruni=C4=87?= <jovan.krunic@gmail.com>
Date: Fri, 5 Jul 2019 15:25:42 +0200
Subject: [PATCH] refactor: throw an error when removing a non-existing plugin

Related to #2
---
 src/routes/plugin-register-route.ts       |  9 ++++-----
 test/app.spec.ts                          |  4 ++--
 test/routes/plugin-register-route.spec.ts | 14 +++++---------
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/routes/plugin-register-route.ts b/src/routes/plugin-register-route.ts
index 7356a741..199e4bc7 100644
--- a/src/routes/plugin-register-route.ts
+++ b/src/routes/plugin-register-route.ts
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 import {
+  SCNotFoundErrorResponse,
   SCPluginAlreadyRegisteredErrorResponse,
   SCPluginMetaData,
   SCPluginRegisterRequest,
@@ -85,11 +86,9 @@ function addPlugin(plugin: SCPluginMetaData): SCPluginRegisterResponse {
  */
 function removePlugin(route: string): SCPluginRegisterResponse {
   if (!plugins.has(route)) {
-    // TODO: throw error when plugin that is to be removed is not already registered (after @openstapps/core update)
-    // throw new SCNotFoundErrorResponse(
-    //   isTestEnvironment,
-    // );
-    return {success: false};
+    throw new SCNotFoundErrorResponse(
+      isTestEnvironment,
+    );
   }
   // remove the plugin information using its route as a key
   plugins.delete(route);
diff --git a/test/app.spec.ts b/test/app.spec.ts
index f70f0e65..ad308670 100644
--- a/test/app.spec.ts
+++ b/test/app.spec.ts
@@ -102,7 +102,7 @@ export class AppPluginRegisterSpec {
   }
 
   @test
-  async 'should response with false when deleting non previously registered plugin'() {
+  async 'should response with 404 when deleting non previously registered plugin'() {
     // lets simulate that the plugin is already registered
     plugins.set(registerAddRequest.plugin.route, registerAddRequest.plugin);
 
@@ -112,7 +112,7 @@ export class AppPluginRegisterSpec {
       .set('Accept', 'application/json')
       // using a different route for the remove request
       .send({action: 'remove', route: '/not-foo'} as SCPluginRemove)
-      .expect(200, {success: false});
+      .expect(404);
   }
 }
 
diff --git a/test/routes/plugin-register-route.spec.ts b/test/routes/plugin-register-route.spec.ts
index 46f4445e..c48518d7 100644
--- a/test/routes/plugin-register-route.spec.ts
+++ b/test/routes/plugin-register-route.spec.ts
@@ -17,6 +17,7 @@ import {
   SCPluginAdd,
   SCPluginAlreadyRegisteredErrorResponse,
   SCPluginRemove,
+  SCNotFoundErrorResponse,
 } from '@openstapps/core';
 import {should, use} from 'chai';
 import {slow, timeout} from 'mocha-typescript';
@@ -94,17 +95,12 @@ export class PluginRegisterRouteSpec {
   }
 
   @test
-  async 'should return false when removing a plugin whose route doesn\'t exist'() {
+  async 'should throw a "not found" error when removing a plugin whose route doesn\'t exist'() {
     // register one plugin
     await pluginRegisterHandler(registerAddRequest, {});
 
-    const response = await pluginRegisterHandler({...registerRemoveRequest, route: '/not-foo'}, {});
-
-    return response.should.eql({success: false});
-
-    // TODO: use 404 response
-    // return pluginRegisterHandler({...registerRemoveRequest, route: '/bar'}, {})
-    //   .should.eventually.be.rejectedWith('Resource not found')
-    //   .and.be.an.instanceOf(SCNotFoundErrorResponse);
+    return pluginRegisterHandler({...registerRemoveRequest, route: '/not-foo'}, {})
+      .should.eventually.be.rejectedWith('Resource not found')
+      .and.be.an.instanceOf(SCNotFoundErrorResponse);
   }
 }