12902: Update state=Final when cancelling a container request.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index f61596ecc766ce7b305f4db3d5e2a4e95c466749..783cafa117d9c23b3641f5f9883b90ea066be384 100644 (file)
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -77,6 +77,17 @@ class ContainerRequestsController < ApplicationController
   end
 
   def cancel
+    if @object.container_uuid
+      c = Container.select(['state']).where(uuid: @object.container_uuid).first
+      if c && c.state != 'Running'
+        # If the container hasn't started yet, setting priority=0
+        # leaves our request in "Committed" state and doesn't cancel
+        # the container (even if no other requests are giving it
+        # priority). To avoid showing this container request as "on
+        # hold" after hitting the Cancel button, set state=Final too.
+        @object.state = 'Final'
+      end
+    end
     @object.update_attributes! priority: 0
     if params[:return_to]
       redirect_to params[:return_to]

diff --git a/apps/workbench/test/controllers/container_requests_controller_test.rb b/apps/workbench/test/controllers/container_requests_controller_test.rb
index 206352a2afa8d77abca7404abf5ad3143fbacf51..261169cd1f954c352aaba6e58e577c9d0a955b4d 100644 (file)
--- a/apps/workbench/test/controllers/container_requests_controller_test.rb
+++ b/apps/workbench/test/controllers/container_requests_controller_test.rb
@@ -42,7 +42,21 @@ class ContainerRequestsControllerTest < ActionController::TestCase
     get :show, {id: uuid}, session_for(:active)
     assert_response :success
 
-   assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
+    assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
+  end
+
+  test "cancel request for queued container" do
+    cr_fixture = api_fixture('container_requests')['queued']
+    post :cancel, {id: cr_fixture['uuid']}, session_for(:active)
+    assert_response 302
+
+    use_token 'active'
+    cr = ContainerRequest.find(cr_fixture['uuid'])
+    assert_equal 'Final', cr.state
+    assert_equal 0, cr.priority
+    c = Container.find(cr_fixture['container_uuid'])
+    assert_equal 'Queued', c.state
+    assert_equal 0, c.priority
   end
 
   [

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 0b189141dadc062d78eccbbb88f67c47ad10d4f6..bcca40700bd9efbaf57c74332a94f3325763299a 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -239,12 +239,13 @@ class ContainerRequest < ArvadosModel
       end
 
     when Final
-      if self.state_changed? and not current_user.andand.is_admin
-        self.errors.add :state, "of container request can only be set to Final by system."
-      end
-
       if self.state_was == Committed
-        permitted.push :output_uuid, :log_uuid
+        # "Cancel" means setting priority=0, state=Committed
+        permitted.push :priority
+
+        if current_user.andand.is_admin
+          permitted.push :output_uuid, :log_uuid
+        end
       end
 
     end