closes #11840

Merge branch '11840-unique-constraint-untrash-coll'

Arvados-DCO-1.1-Signed-off-by: Radhika Chippada <radhika@curoverse.com>

diff --git a/apps/workbench/app/models/collection.rb b/apps/workbench/app/models/collection.rb
index 305ea015306fe898c97bd16b443a13d9ff230781..025c136d41bed69883a80d0cc12cbc3bfdf43ec9 100644 (file)
--- a/apps/workbench/app/models/collection.rb
+++ b/apps/workbench/app/models/collection.rb
@@ -99,6 +99,6 @@ class Collection < ArvadosBase
   end
 
   def untrash
-    arvados_api_client.api(self.class, "/#{self.uuid}/untrash", {})
+    arvados_api_client.api(self.class, "/#{self.uuid}/untrash", {"ensure_unique_name" => true})
   end
 end

diff --git a/apps/workbench/test/controllers/trash_items_controller_test.rb b/apps/workbench/test/controllers/trash_items_controller_test.rb
new file mode 100644 (file)
index 0000000..c530ba9
--- /dev/null
+++ b/apps/workbench/test/controllers/trash_items_controller_test.rb
@@ -0,0 +1,14 @@
+require 'test_helper'
+
+class TrashItemsControllerTest < ActionController::TestCase
+  test "untrash collection with same name as another collection" do
+    collection = api_fixture('collections')['trashed_collection_to_test_name_conflict_on_untrash']
+    items = [collection['uuid']]
+    post :untrash_items, {
+      selection: items,
+      format: :js
+    }, session_for(:active)
+
+    assert_response :success
+  end
+end

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 5c09b1fccdf09f0af508490ac34803efd5b61237..b5dd07e064a71e003b66e81030134dce460e7d4f 100644 (file)
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -75,7 +75,13 @@ class Arvados::V1::CollectionsController < ApplicationController
 
   def untrash
     if @object.is_trashed
-      @object.update_attributes!(trash_at: nil)
+      @object.trash_at = nil
+
+      if params[:ensure_unique_name]
+        @object.save_with_unique_name!
+      else
+        @object.save!
+      end
     else
       raise InvalidStateTransitionError
     end

diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 6543f5422a861b3430c285402a5ae1d8ac1d3dd4..240306059d139ffd9a53d97b04cdd6e69ecd6061 100644 (file)
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -674,6 +674,33 @@ collection_with_tags_owned_by_active:
       existing tag 2: value for existing tag 2
     some other property: value for the other property
 
+trashed_collection_to_test_name_conflict_on_untrash:
+  uuid: zzzzz-4zz18-trashedcolnamec
+  portable_data_hash: 80cf6dd2cf079dd13f272ec4245cb4a8+48
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-02-03T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2014-02-03T17:22:54Z
+  updated_at: 2014-02-03T17:22:54Z
+  manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:file1 0:0:file2\n"
+  name: same name for trashed and persisted collections
+  is_trashed: true
+  trash_at: 2001-01-01T00:00:00Z
+  delete_at: 2038-01-01T00:00:00Z
+
+same_name_as_trashed_coll_to_test_name_conflict_on_untrash:
+  uuid: zzzzz-4zz18-namesameastrash
+  portable_data_hash: 80cf6dd2cf079dd13f272ec4245cb4a8+48
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-02-03T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2014-02-03T17:22:54Z
+  updated_at: 2014-02-03T17:22:54Z
+  manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:file1 0:0:file2\n"
+  name: same name for trashed and persisted collections
+
 
 # Test Helper trims the rest of the file
 

diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 17af916b3df244821bc596bbbc58fad799d8d380..70f35f3dd2b54f51f00edee2deed4aa8dd265e3b 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1065,4 +1065,25 @@ EOS
       end
     end
   end
+
+  test 'untrash collection with same name as another with no ensure unique name' do
+    authorize_with :active
+    post :untrash, {
+      id: collections(:trashed_collection_to_test_name_conflict_on_untrash).uuid,
+    }
+    assert_response 422
+  end
+
+  test 'untrash collection with same name as another with ensure unique name' do
+    authorize_with :active
+    post :untrash, {
+      id: collections(:trashed_collection_to_test_name_conflict_on_untrash).uuid,
+      ensure_unique_name: true
+    }
+    assert_response 200
+    assert_equal false, json_response['is_trashed']
+    assert_nil json_response['trash_at']
+    assert_nil json_response['delete_at']
+    assert_match /^same name for trashed and persisted collections \(\d{4}-\d\d-\d\d.*?Z\)$/, json_response['name']
+  end
 end