[ARVADOS] updated: 1.3.0-1028-g91e5cfcbf

Git user git at public.curoverse.com
Wed Jun 5 19:20:22 UTC 2019


Summary of changes:
 apps/workbench/app/helpers/application_helper.rb        |  2 +-
 .../test/integration/pipeline_instances_test.rb         |  3 ++-
 services/api/app/models/collection.rb                   | 10 +++++++++-
 services/api/test/unit/collection_test.rb               | 17 +++++++++++++++++
 4 files changed, 29 insertions(+), 3 deletions(-)

       via  91e5cfcbf7308a4e98ecc63bdf3d83621253cee7 (commit)
      from  985684f7ec46d1749efa686b702605d5ef24a57b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 91e5cfcbf7308a4e98ecc63bdf3d83621253cee7
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Jun 5 15:00:53 2019 -0400

    14287: Make collection name "" equivalent to null.
    
    Regardless of whether name is set/updated to "" or null, it is exempt
    from the unique-name-in-project constraint, and returned as "" in API
    responses.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/apps/workbench/app/helpers/application_helper.rb b/apps/workbench/app/helpers/application_helper.rb
index 3f72d5a2a..e85c74682 100644
--- a/apps/workbench/app/helpers/application_helper.rb
+++ b/apps/workbench/app/helpers/application_helper.rb
@@ -359,7 +359,7 @@ module ApplicationHelper
           display_value = link.name
         elsif value_info[:link_name]
           display_value = value_info[:link_name]
-        elsif value_info[:selection_name]
+        elsif value_info[:selection_name].andand != ""
           display_value = value_info[:selection_name]
         end
       end
diff --git a/apps/workbench/test/integration/pipeline_instances_test.rb b/apps/workbench/test/integration/pipeline_instances_test.rb
index 801609fbb..adfd62bd8 100644
--- a/apps/workbench/test/integration/pipeline_instances_test.rb
+++ b/apps/workbench/test/integration/pipeline_instances_test.rb
@@ -393,6 +393,7 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest
   def create_and_run_pipeline_in_aproject in_aproject, template_name, collection_fixture, choose_file=false
     # collection in aproject to be used as input
     collection = api_fixture('collections', collection_fixture)
+    collection['name'] ||= '' # API response is "" even if fixture attr is null
 
     # create a pipeline instance
     find('.btn', text: 'Run a process').click
@@ -421,7 +422,7 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest
 
       if collection_fixture == 'foo_collection_in_aproject'
         first('span', text: 'foo_tag').click
-      elsif collection['name']
+      elsif collection['name'] != ''
         first('span', text: "#{collection['name']}").click
       else
         collection_uuid = collection['uuid']
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 775ebdb49..f8cca5f07 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -26,6 +26,7 @@ class Collection < ArvadosModel
   before_validation :check_manifest_validity
   before_validation :check_signatures
   before_validation :strip_signatures_and_update_replication_confirmed
+  before_validation :name_null_if_empty
   validate :ensure_pdh_matches_manifest_text
   validate :ensure_storage_classes_desired_is_not_empty
   validate :ensure_storage_classes_contain_non_empty_strings
@@ -36,7 +37,7 @@ class Collection < ArvadosModel
   around_update :manage_versioning, unless: :is_past_version?
 
   api_accessible :user, extend: :common do |t|
-    t.add :name
+    t.add lambda { |x| x.name || "" }, as: :name
     t.add :description
     t.add :properties
     t.add :portable_data_hash
@@ -75,6 +76,7 @@ class Collection < ArvadosModel
                 # correct timestamp in signed_manifest_text.
                 'manifest_text' => ['manifest_text', 'trash_at', 'is_trashed'],
                 'unsigned_manifest_text' => ['manifest_text'],
+                'name' => ['name'],
                 )
   end
 
@@ -193,6 +195,12 @@ class Collection < ArvadosModel
     end
   end
 
+  def name_null_if_empty
+    if name == ""
+      self.name = nil
+    end
+  end
+
   def set_file_names
     if self.manifest_text_changed?
       self.file_names = manifest_files
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 08d5b1fb7..e75ad5d77 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -1012,4 +1012,21 @@ class CollectionTest < ActiveSupport::TestCase
     SweepTrashedObjects.sweep_now
     assert_empty Collection.where(uuid: uuid)
   end
+
+  test "empty names are exempt from name uniqueness" do
+    act_as_user users(:active) do
+      c1 = Collection.new(name: nil, manifest_text: '', owner_uuid: groups(:aproject).uuid)
+      assert c1.save
+      c2 = Collection.new(name: '', manifest_text: '', owner_uuid: groups(:aproject).uuid)
+      assert c2.save
+      c3 = Collection.new(name: '', manifest_text: '', owner_uuid: groups(:aproject).uuid)
+      assert c3.save
+      c4 = Collection.new(name: 'c4', manifest_text: '', owner_uuid: groups(:aproject).uuid)
+      assert c4.save
+      c5 = Collection.new(name: 'c4', manifest_text: '', owner_uuid: groups(:aproject).uuid)
+      assert_raises(ActiveRecord::RecordNotUnique) do
+        c5.save
+      end
+    end
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list