Merge branch 'liw/move-error-msgs'

Reviewed-by: Sam Thursfield
Reviewed-by: Richard Maw

2 files changed, 46 insertions, 22 deletions

diff --git a/lorrycontroller/movetopbottom.py b/lorrycontroller/movetopbottom.py
index dcb79a4..2aeff98 100644
--- a/lorrycontroller/movetopbottom.py
+++ b/lorrycontroller/movetopbottom.py
@@ -21,38 +21,50 @@ import bottle
 import lorrycontroller
 
 
-class MoveToTop(lorrycontroller.LorryControllerRoute):
-
-    http_method = 'POST'
-    path = '/1.0/move-to-top'
+class MoveBase(object):
 
     def run(self, **kwargs):
         logging.info('%s %s called', self.http_method, self.path)
         path = bottle.request.forms.path
+        if not path:
+            return 'Form field path was not given'
         statedb = self.open_statedb()
         with statedb:
+            if not self.lorry_exists(statedb, path):
+                return 'Lorry %s does not exist' % path
             lorry_infos = statedb.get_all_lorries_info()
-            if lorry_infos:
-                topmost = lorry_infos[0]
-                timestamp = min(0, topmost['last_run'] - 1)
-                statedb.set_lorry_last_run(path, timestamp)
-        return 'Lorry %s moved to top of run-queue' % path
+            timestamp = self.get_new_timestamp(lorry_infos)
+            statedb.set_lorry_last_run(path, timestamp)
+            return self.msg % path
+
+    def lorry_exists(self, statedb, path):
+        try:
+            statedb.get_lorry_info(path)
+        except lorrycontroller.LorryNotFoundError:
+            return False
+        else:
+            return True
+
+
+class MoveToTop(MoveBase, lorrycontroller.LorryControllerRoute):
+
+    http_method = 'POST'
+    path = '/1.0/move-to-top'
+    msg = 'Lorry %s moved to the top of the run queue'
+
+    def get_new_timestamp(self, lorry_infos):
+        topmost = lorry_infos[0]
+        timestamp = min(0, topmost['last_run'] - 1)
+        return timestamp
 
 
-class MoveToBottom(lorrycontroller.LorryControllerRoute):
+class MoveToBottom(MoveBase, lorrycontroller.LorryControllerRoute):
 
     http_method = 'POST'
     path = '/1.0/move-to-bottom'
+    msg = 'Lorry %s moved to the bottom of the run queue'
 
-    def run(self, **kwargs):
-        logging.info('%s %s called', self.http_method, self.path)
-        path = bottle.request.forms.path
-        statedb = self.open_statedb()
-        with statedb:
-            lorry_infos = statedb.get_all_lorries_info()
-            if lorry_infos:
-                bottommost = lorry_infos[-1]
-                timestamp = (
-                    bottommost['last_run'] + bottommost['interval'] + 1)
-                statedb.set_lorry_last_run(path, timestamp)
-        return 'Lorry %s moved to bototm of run-queue' % path
+    def get_new_timestamp(self, lorry_infos):
+        bottommost = lorry_infos[-1]
+        timestamp = bottommost['last_run'] + bottommost['interval'] + 1
+        return timestamp
diff --git a/yarns.webapp/030-queue-management.yarn b/yarns.webapp/030-queue-management.yarn
index 9fca4fb..ebf50af 100644
--- a/yarns.webapp/030-queue-management.yarn
+++ b/yarns.webapp/030-queue-management.yarn
@@ -101,6 +101,18 @@ Add two Lorries, then make sure they can reordered at will.
     AND admin makes request GET /1.0/list-queue
     THEN response has queue set to ["upstream/bar", "upstream/foo"]
 
+If trying to move a lorry that doesn't exist, make sure there's an
+appropriate error message.
+
+    WHEN admin makes request POST /1.0/move-to-bottom with path=upstream/alfred
+    THEN response matches "upstream/alfred does not exist"
+
+Likewise, if we forget to give a path argument, there should be an
+error message.
+
+    WHEN admin makes request POST /1.0/move-to-bottom
+    THEN response matches "path.*not given"
+
 Finally, clean up.
 
     FINALLY WEBAPP terminates