From f4ab801b3ad069a726b3984f47d648248d31bcb3 Mon Sep 17 00:00:00 2001
From: Matt Prahl <mprahl@redhat.com>
Date: Dec 06 2019 14:02:13 +0000
Subject: Merge #1535 `Only allow cancelling module builds in the init, wait, and build states`


---

diff --git a/module_build_service/views.py b/module_build_service/views.py
index 4900b94..e0a4af7 100644
--- a/module_build_service/views.py
+++ b/module_build_service/views.py
@@ -206,15 +206,28 @@ class ModuleBuildAPI(AbstractQueryableBuildAPI):
             log.error("Invalid JSON submitted")
             raise ValidationError("Invalid JSON submitted")
 
-        if module.state == models.BUILD_STATES["failed"]:
-            raise Forbidden("You can't cancel a failed module")
+        state = r["state"]
+        valid_input_states = ("failed", str(models.BUILD_STATES["failed"]))
+        if state not in valid_input_states:
+            raise ValidationError(
+                "An invalid state was submitted. Valid states values are: {}"
+                .format(", ".join(valid_input_states))
+            )
 
-        if r["state"] == "failed" or r["state"] == str(models.BUILD_STATES["failed"]):
-            module.transition(
-                db.session, conf, models.BUILD_STATES["failed"], "Canceled by %s." % username)
-        else:
-            log.error('The provided state change of "{}" is not supported'.format(r["state"]))
-            raise ValidationError("The provided state change is not supported")
+        valid_states_to_cancel = ("build", "init", "wait")
+        module_state_name = models.INVERSE_BUILD_STATES[module.state]
+        if module_state_name not in valid_states_to_cancel:
+            log.error(
+                "The user %s attempted to cancel a build in the %s state",
+                username, module_state_name,
+            )
+            raise ValidationError(
+                "To cancel a module build, it must be in one of the following states: {}"
+                .format(", ".join(valid_states_to_cancel))
+            )
+
+        module.transition(
+            db.session, conf, models.BUILD_STATES["failed"], "Canceled by %s." % username)
         db.session.add(module)
         db.session.commit()
 
diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py
index f1125d8..1febc16 100644
--- a/tests/test_views/test_views.py
+++ b/tests/test_views/test_views.py
@@ -1253,18 +1253,25 @@ class TestViews:
         assert data["state"] == 4
         assert data["state_reason"] == "Canceled by some_other_user."
 
+    @pytest.mark.parametrize("module_state", (BUILD_STATES["failed"], BUILD_STATES["ready"]))
     @patch("module_build_service.auth.get_user", return_value=other_user)
-    def test_cancel_build_already_failed(self, mocked_get_user):
+    def test_cancel_build_in_invalid_state(self, mocked_get_user, module_state):
         module = ModuleBuild.get_by_id(db_session, 7)
-        module.state = 4
+        module.state = module_state
         db_session.commit()
 
         rv = self.client.patch(
             "/module-build-service/1/module-builds/7", data=json.dumps({"state": "failed"}))
 
-        data = json.loads(rv.data)
-        assert data["status"] == 403
-        assert data["error"] == "Forbidden"
+        assert rv.status_code == 400
+        assert rv.json == {
+            "error": "Bad Request",
+            "message": (
+                "To cancel a module build, it must be in one of the following states: "
+                "build, init, wait"
+            ),
+            "status": 400,
+        }
 
     @patch("module_build_service.auth.get_user", return_value=("sammy", set()))
     def test_cancel_build_unauthorized_no_groups(self, mocked_get_user):
@@ -1328,11 +1335,13 @@ class TestViews:
     def test_cancel_build_wrong_state(self, mocked_get_user):
         rv = self.client.patch(
             "/module-build-service/1/module-builds/7", data=json.dumps({"state": "some_state"}))
-        data = json.loads(rv.data)
 
-        assert data["status"] == 400
-        assert data["error"] == "Bad Request"
-        assert data["message"] == "The provided state change is not supported"
+        assert rv.status_code == 400
+        assert rv.json == {
+            "error": "Bad Request",
+            "message": "An invalid state was submitted. Valid states values are: failed, 4",
+            "status": 400,
+        }
 
     @patch("module_build_service.auth.get_user", return_value=user)
     def test_submit_build_unsupported_scm_scheme(self, mocked_get_user):