make CollectionType#find_by_gid work with correct global id format

app/controllers/hyrax/dashboard/collections_controller.rb

@@ -60,7 +60,7 @@ def new
         # Coming from the UI, a collection type id should always be present.  Coming from the API, if a collection type id is not specified,
         # use the default collection type (provides backward compatibility with versions < Hyrax 2.1.0)
         collection_type_id = params[:collection_type_id].presence || default_collection_type.id
-        @collection.collection_type_gid = CollectionType.find(collection_type_id).gid
+        @collection.collection_type_gid = CollectionType.find(collection_type_id).to_global_id
         add_breadcrumb t(:'hyrax.controls.home'), root_path
         add_breadcrumb t(:'hyrax.dashboard.breadcrumbs.admin'), hyrax.dashboard_path
         add_breadcrumb t('.header', type_title: @collection.collection_type.title), request.path
@@ -111,7 +111,7 @@ def create
         authorize! :create, @collection
         # Coming from the UI, a collection type gid should always be present.  Coming from the API, if a collection type gid is not specified,
         # use the default collection type (provides backward compatibility with versions < Hyrax 2.1.0)
-        @collection.collection_type_gid = params[:collection_type_gid].presence || default_collection_type.gid
+        @collection.collection_type_gid = params[:collection_type_gid].presence || default_collection_type.to_global_id
         @collection.attributes = collection_params.except(:members, :parent_id, :collection_type_gid)
         @collection.apply_depositor_metadata(current_user.user_key)
         add_members_to_collection unless batch.empty?

app/models/concerns/hyrax/collection_behavior.rb

@@ -29,11 +29,11 @@ module CollectionBehavior
       validates :collection_type_gid, presence: true
 
       # Need to define here in order to override setter defined by ActiveTriples
-      def collection_type_gid=(new_collection_type_gid)
+      def collection_type_gid=(new_collection_type_gid, force: false)
         new_collection_type_gid = new_collection_type_gid&.to_s
-        raise "Can't modify collection type of this collection" if persisted? && !collection_type_gid_was.nil? && collection_type_gid_was != new_collection_type_gid
+        raise "Can't modify collection type of this collection" if !force && persisted? && !collection_type_gid_was.nil? && collection_type_gid_was != new_collection_type_gid
         new_collection_type = Hyrax::CollectionType.find_by_gid!(new_collection_type_gid)
-        super
+        super(new_collection_type_gid)
         @collection_type = new_collection_type
         collection_type_gid
       end
@@ -47,7 +47,7 @@ def collection_type
     end
 
     def collection_type=(new_collection_type)
-      self.collection_type_gid = new_collection_type.gid
+      self.collection_type_gid = new_collection_type.to_global_id
     end
 
     # Add members using the members association.

app/models/hyrax/collection_type.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 module Hyrax
-  class CollectionType < ActiveRecord::Base
+  class CollectionType < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
     # @!method id
     #   @return [Integer]
     # @!method description
@@ -75,6 +75,15 @@ def self.gids_that_do_not_allow_multiple_membership
     # @return [Hyrax::CollectionType] an instance of Hyrax::CollectionType with id = the model_id portion of the gid (e.g. 3)
     # @raise [ActiveRecord::RecordNotFound] if record matching gid is not found
     def self.find_by_gid!(gid)
+      raise(URI::InvalidURIError) if gid.nil?
+      GlobalID::Locator.locate(gid)
+    rescue NameError
+      Rails.logger.warn "#{self.class}##{__method__} called with #{gid}, which is " \
+                        "a legacy collection type global id format. If this " \
+                        "collection type gid is attached to a collection in " \
+                        "your repository you'll want to run " \
+                        "`hyrax:collections:update_collection_type_global_ids` to " \
+                        "update references. see: https://github.com/samvera/hyrax/issues/4696"
       find(GlobalID.new(gid).model_id)
     end
 
@@ -86,18 +95,24 @@ def self.settings_attributes
       SETTINGS_ATTRIBUTES
     end
 
+    ##
+    # @deprecation use #to_global_id
+    #
     # Return the Global Identifier for this collection type.
-    # @return [String] Global Identifier (gid) for this collection_type (e.g. gid://internal/hyrax-collectiontype/3)
+    # @return [String, nil] Global Identifier (gid) for this collection_type (e.g. gid://internal/hyrax-collectiontype/3)
+    #
+    # @see https://github.com/rails/globalid#usage
     def gid
-      URI::GID.build(app: GlobalID.app, model_name: model_name.name.parameterize.to_sym, model_id: id).to_s if id
+      Deprecation.warn('use #to_global_id.')
+      to_global_id.to_s if id
     end
 
     ##
     # @return [Enumerable<Collection, PcdmCollection>]
     def collections(use_valkyrie: false)
       return [] unless id
-      return Hyrax.custom_queries.find_collections_by_type(global_id: gid) if use_valkyrie
-      ActiveFedora::Base.where(Hyrax.config.collection_type_index_field.to_sym => gid.to_s)
+      return Hyrax.custom_queries.find_collections_by_type(global_id: to_global_id) if use_valkyrie
+      ActiveFedora::Base.where(Hyrax.config.collection_type_index_field.to_sym => to_global_id.to_s)
     end
 
     ##
@@ -188,5 +203,5 @@ def collection_type_settings_changed?
     def exists_for_machine_id?(machine_id)
       self.class.exists?(machine_id: machine_id)
     end
-  end
+  end # rubocop:enable Metrics/ClassLength
 end

app/services/hyrax/collections/migration_service.rb

@@ -34,7 +34,7 @@ def self.migrate_all_collections
       # @param collection [::Collection] collection object to be migrated
       def self.migrate_collection(collection)
         return if collection.collection_type_gid.present? # already migrated
-        collection.collection_type_gid = Hyrax::CollectionType.find_or_create_default_collection_type.gid
+        collection.collection_type_gid = Hyrax::CollectionType.find_or_create_default_collection_type.to_global_id
         create_permissions(collection)
         collection.save
       end
@@ -80,8 +80,8 @@ def self.repair_migrated_collections
       #
       # @param collection [::Collection] collection object to be migrated/repaired
       def self.repair_migrated_collection(collection)
-        return if collection.collection_type_gid.present? && collection.collection_type_gid != Hyrax::CollectionType.find_or_create_default_collection_type.gid
-        collection.collection_type_gid = Hyrax::CollectionType.find_or_create_default_collection_type.gid
+        return if collection.collection_type_gid.present? && collection.collection_type_gid != Hyrax::CollectionType.find_or_create_default_collection_type.to_global_id
+        collection.collection_type_gid = Hyrax::CollectionType.find_or_create_default_collection_type.to_global_id
         permission_template = Hyrax::PermissionTemplate.find_by(source_id: collection.id)
         if permission_template.present?
           collection.reset_access_controls!

app/views/hyrax/admin/collection_types/index.html.erb

@@ -40,7 +40,7 @@
                  <% unless collection_type.admin_set? || collection_type.user_collection? %>
                   <button class="btn btn-danger btn-sm delete-collection-type"
                           data-collection-type="<%= collection_type.to_json %>"
-                          data-collection-type-index="<%= hyrax.dashboard_collections_path({ "f[collection_type_gid_ssim][]" => collection_type.gid, 'f[has_model_ssim][]' => 'Collection' }) %>"
+                          data-collection-type-index="<%= hyrax.dashboard_collections_path({ 'f[collection_type_gid_ssim][]' => collection_type.to_global_id.to_s, 'f[has_model_ssim][]' => 'Collection' }) %>"
                           data-has-collections="<%= collection_type.collections? %>">
                     <%= t('helpers.action.delete') %>
                   </button>

lib/generators/hyrax/templates/db/seeds.rb

@@ -114,14 +114,15 @@ def collection_attributes_for(collection_ids)
 genuser = create_user('[email protected]', 'pppppp')
 
 puts 'Create collection types for QA'
-_discoverable_gid = create_collection_type('discoverable_collection_type', title: 'Discoverable', description: 'Sample collection type allowing collections to be discovered.', discoverable: true).gid
-_sharable_gid = create_collection_type('sharable_collection_type', title: 'Sharable', description: 'Sample collection type allowing collections to be shared.', sharable: true).gid
+_discoverable_gid = create_collection_type('discoverable_collection_type', title: 'Discoverable', description: 'Sample collection type allowing collections to be discovered.', discoverable: true)
+                    .to_global_id
+_sharable_gid = create_collection_type('sharable_collection_type', title: 'Sharable', description: 'Sample collection type allowing collections to be shared.', sharable: true).to_global_id
 options = { title: 'Multi-membership', description: 'Sample collection type allowing works to belong to multiple collections.', allow_multiple_membership: true }
 _multi_membership_gid = create_collection_type('multi_membership_collection_type', options)
-_nestable_1_gid = create_collection_type('nestable_1_collection_type', title: 'Nestable 1', description: 'A sample collection type allowing nesting.', nestable: true).gid
-_nestable_2_gid = create_collection_type('nestable_2_collection_type', title: 'Nestable 2', description: 'Another sample collection type allowing nesting.', nestable: true).gid
-_empty_gid = create_collection_type('empty_collection_type', title: 'Test Empty Collection Type', description: 'A collection type with 0 collections of this type').gid
-inuse_gid = create_collection_type('inuse_collection_type', title: 'Test In-Use Collection Type', description: 'A collection type with at least one collection of this type').gid
+_nestable_1_gid = create_collection_type('nestable_1_collection_type', title: 'Nestable 1', description: 'A sample collection type allowing nesting.', nestable: true).to_global_id
+_nestable_2_gid = create_collection_type('nestable_2_collection_type', title: 'Nestable 2', description: 'Another sample collection type allowing nesting.', nestable: true).to_global_id
+_empty_gid = create_collection_type('empty_collection_type', title: 'Test Empty Collection Type', description: 'A collection type with 0 collections of this type').to_global_id
+inuse_gid = create_collection_type('inuse_collection_type', title: 'Test In-Use Collection Type', description: 'A collection type with at least one collection of this type').to_global_id
 
 puts 'Create collections for QA'
 inuse_col = create_public_collection(genuser, inuse_gid, 'inuse_col1', title: ['Public Collection of type In-Use'], description: ['Public collection of the type Test In-Use Collection Type.'])
@@ -158,11 +159,11 @@ def collection_attributes_for(collection_ids)
 puts 'Create collection types for collection nesting ad hoc testing'
 options = { title: 'Nestable Collection', description: 'Sample collection type that allows nesting of collections.',
             nestable: true, discoverable: true, sharable: true, allow_multiple_membership: true }
-nestable_gid = create_collection_type('nestable_collection', options).gid
+nestable_gid = create_collection_type('nestable_collection', options).to_global_id
 
 options = { title: 'Non-Nestable Collection', description: 'Sample collection type that DOES NOT allow nesting of collections.',
             nestable: false, discoverable: true, sharable: true, allow_multiple_membership: true }
-_nonnestable_gid = create_collection_type('nonnestable_collection', options).gid
+_nonnestable_gid = create_collection_type('nonnestable_collection', options).to_global_id
 
 puts 'Create collections for collection nesting ad hoc testing'
 pnc = create_public_collection(user, nestable_gid, 'public_nestable', title: ['Public Nestable Collection'], description: ['Public nestable collection for use in ad hoc tests.'])

lib/tasks/collection_type_global_id.rake

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+namespace :hyrax do
+  namespace :collections do
+    desc 'Update CollectionType global id references for Hyrax 3.0.0'
+    task :update_collection_type_global_ids do
+      puts 'Updating collection -> collection type GlobalId references.'
+
+      count = 0
+
+      Collection.all.each do |collection|
+        next if collection.collection_type_gid == collection.collection_type.to_global_id.to_s
+
+        collection.public_send(:collection_type_gid=, collection.collection_type.to_global_id, force: true)
+
+        collection.save &&
+          count += 1
+      end
+
+      puts "Updated #{count} collections."
+    end
+  end
+end

spec/abilities/collection_ability_spec.rb

@@ -7,11 +7,11 @@
   let(:ability) { Ability.new(current_user) }
   let(:user) { create(:user) }
   let(:current_user) { user }
-  let(:collection_type_gid) { create(:collection_type).gid }
+  let(:collection_type) { FactoryBot.create(:collection_type) }
 
   context 'when admin user' do
     let(:user) { FactoryBot.create(:admin) }
-    let!(:collection) { build(:collection_lw, id: 'col_au', with_permission_template: true, collection_type_gid: collection_type_gid) }
+    let!(:collection) { FactoryBot.build(:collection_lw, id: 'col_au', with_permission_template: true, collection_type: collection_type) }
     let!(:solr_document) { SolrDocument.new(collection.to_solr) }
 
     it 'allows all abilities' do # rubocop:disable RSpec/ExampleLength
@@ -35,7 +35,7 @@
   end
 
   context 'when collection manager' do
-    let!(:collection) { build(:collection_lw, id: 'col_mu', with_permission_template: true, collection_type_gid: collection_type_gid) }
+    let!(:collection) { FactoryBot.build(:collection_lw, id: 'col_mu', with_permission_template: true, collection_type: collection_type) }
     let!(:solr_document) { SolrDocument.new(collection.to_solr) }
 
     before do
@@ -70,7 +70,7 @@
   end
 
   context 'when collection depositor' do
-    let!(:collection) { build(:collection_lw, id: 'col_du', with_permission_template: true, collection_type_gid: collection_type_gid) }
+    let!(:collection) { FactoryBot.build(:collection_lw, id: 'col_du', with_permission_template: true, collection_type: collection_type) }
     let!(:solr_document) { SolrDocument.new(collection.to_solr) }
 
     before do
@@ -105,7 +105,7 @@
   end
 
   context 'when collection viewer' do
-    let!(:collection) { build(:collection_lw, id: 'col_vu', with_permission_template: true, collection_type_gid: collection_type_gid) }
+    let!(:collection) { FactoryBot.build(:collection_lw, id: 'col_vu', with_permission_template: true, collection_type: collection_type) }
     let!(:solr_document) { SolrDocument.new(collection.to_solr) }
 
     before do
@@ -140,7 +140,7 @@
   end
 
   context 'when user has no special access' do
-    let!(:collection) { create(:collection_lw, id: 'as', with_permission_template: true, collection_type_gid: collection_type_gid) }
+    let!(:collection) { FactoryBot.create(:collection_lw, id: 'as', with_permission_template: true, collection_type: collection_type) }
     let!(:solr_document) { SolrDocument.new(collection.to_solr) }
 
     it 'denies all abilities' do # rubocop:disable RSpec/ExampleLength

spec/abilities/permission_template_ability_spec.rb

@@ -5,9 +5,8 @@
   subject(:ability) { Ability.new(current_user) }
   let(:user) { create(:user) }
   let(:current_user) { user }
-  let(:collection_type_gid) { create(:collection_type).gid }
-
-  let!(:collection) { create(:collection_lw, with_permission_template: true, collection_type_gid: collection_type_gid) }
+  let(:collection_type) { FactoryBot.create(:collection_type) }
+  let!(:collection) { create(:collection_lw, with_permission_template: true, collection_type: collection_type) }
   let(:permission_template) { collection.permission_template }
   let!(:permission_template_access) do
     create(:permission_template_access,

spec/actors/hyrax/actors/collections_membership_actor_spec.rb

@@ -130,7 +130,7 @@
 
     context "updates env" do
       let!(:collection_type) { create(:collection_type) }
-      let!(:collection) { create(:collection_lw, collection_type_gid: collection_type.gid, with_permission_template: true) }
+      let!(:collection) { FactoryBot.create(:collection_lw, collection_type: collection_type, with_permission_template: true) }
 
       subject(:middleware) do
         stack = ActionDispatch::MiddlewareStack.new.tap do |middleware|

spec/controllers/hyrax/admin/collection_types_controller_spec.rb

@@ -307,7 +307,7 @@
       end
 
       context "when collections exist of this type" do
-        let!(:collection) { create(:collection_lw, collection_type_gid: collection_type_to_destroy.gid) }
+        let!(:collection) { FactoryBot.create(:collection_lw, collection_type: collection_type_to_destroy) }
 
         it "doesn't delete the collection type or collection" do
           delete :destroy, params: { id: collection_type_to_destroy }

spec/controllers/hyrax/dashboard/collections_controller_spec.rb

@@ -3,7 +3,7 @@
   routes { Hyrax::Engine.routes }
   let(:user)  { create(:user) }
   let(:other) { build(:user) }
-  let(:collection_type_gid) { create(:user_collection_type).gid }
+  let(:collection_type_gid) { FactoryBot.create(:user_collection_type).to_global_id.to_s }
 
   let(:collection) do
     create(:public_collection_lw, title: ["My collection"],
@@ -105,10 +105,11 @@
       it "creates a Collection of specified type" do
         expect do
           post :create, params: {
-            collection: collection_attrs, collection_type_gid: collection_type.gid
+            collection: collection_attrs, collection_type_gid: collection_type.to_global_id.to_s
           }
         end.to change { Collection.count }.by(1)
-        expect(assigns[:collection].collection_type_gid).to eq collection_type.gid
+
+        expect(assigns[:collection].collection_type_gid).to eq collection_type.to_global_id.to_s
       end
     end
 

spec/factories/collections.rb

@@ -47,7 +47,7 @@
   #
   # @example Build a collection generating its collection type with specific settings. Light Weight.
   #          NOTE: Do not use this approach if you need access to the collection type in the test.
-  #          DEFAULT: If `collection_type_settings` and `collection_type_gid` are not specified, then the default
+  #          DEFAULT: If `collection_type_settings` and `collection_type` are not specified, then the default
   #          User Collection type will be used.
   #   # Any not specified default to ON.  At least one setting should be specified.
   #   let(:settings) { [
@@ -70,7 +70,7 @@
   #                      :allow_multiple_membership, # OR :not_allow_multiple_membership
   #                    ] }
   #   let(:collection_type) { create(:collection_lw_type, settings) }
-  #   let(:collection) { build(:collection_lw, collection_type_gid: collection_type.gid) }
+  #   let(:collection) { build(:collection_lw, collection_type: collection_type) }
   #
   # @example Build a collection with nesting fields set in the solr document.  Light weight.
   #          NOTE: The property `with_nesting_attributes` is only supported for building collections.  The attributes will
@@ -103,6 +103,7 @@
     transient do
       user { create(:user) }
 
+      collection_type { nil }
       collection_type_settings { nil }
       with_permission_template { false }
       with_nesting_attributes { nil }
@@ -111,6 +112,7 @@
     sequence(:title) { |n| ["Collection Title #{n}"] }
 
     after(:build) do |collection, evaluator|
+      collection.collection_type_gid = evaluator.collection_type.to_global_id if evaluator.collection_type&.id.present?
       collection.apply_depositor_metadata(evaluator.user.user_key)
 
       CollectionLwFactoryHelper.process_collection_type_settings(collection, evaluator)
@@ -163,14 +165,13 @@
   factory :user_collection_lw, class: Collection do
     transient do
       user { create(:user) }
+      collection_type { create(:user_collection_type) }
     end
 
     sequence(:title) { |n| ["User Collection Title #{n}"] }
 
     after(:build) do |collection, evaluator|
       collection.apply_depositor_metadata(evaluator.user.user_key)
-      collection_type = create(:user_collection_type)
-      collection.collection_type_gid = collection_type.gid
     end
   end
 

spec/factories/collections_factory.rb

@@ -87,14 +87,13 @@
   factory :user_collection, class: Collection do
     transient do
       user { create(:user) }
+      collection_type { create(:user_collection_type) }
     end
 
     sequence(:title) { |n| ["User Collection Title #{n}"] }
 
     after(:build) do |collection, evaluator|
       collection.apply_depositor_metadata(evaluator.user.user_key)
-      collection_type = create(:user_collection_type)
-      collection.collection_type_gid = collection_type.gid
     end
   end