From bcb86b69fba693af210e82d40d92854e609157c4 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tclegg@veritasgenetics.com>
Date: Tue, 30 Jan 2018 17:56:13 -0500
Subject: [PATCH] 12902: Update state=Final when cancelling a container
 request.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>
---
 .../controllers/container_requests_controller.rb | 11 +++++++++++
 .../container_requests_controller_test.rb        | 16 +++++++++++++++-
 services/api/app/models/container_request.rb     | 11 ++++++-----
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index f61596ecc7..783cafa117 100644
--- 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 206352a2af..261169cd1f 100644
--- 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 0b189141da..bcca40700b 100644
--- 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
-- 
2.30.2