From e9d429c388b3256c09459a571fc55012225fd103 Mon Sep 17 00:00:00 2001 From: tamsin johnson Date: Tue, 5 Jan 2021 17:42:15 -0800 Subject: [PATCH 1/4] add a rake task to update collection references to collection types as part of #4696: since we're deprecating our older semi-custom global id syntax, we need to be able to support applications updating their collections. the rake task `hyrax:collections:update_collection_type_global_ids` provides a way to do this, and is a non-destructive if collections all use the standard syntax. note that this task may create substantial load, even if all collections are already up to date; so it shouldn't be run repeatedly. --- .../concerns/hyrax/collection_behavior.rb | 6 ++-- lib/tasks/collection_type_global_id.rake | 22 ++++++++++++ spec/tasks/rake_spec.rb | 35 +++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 lib/tasks/collection_type_global_id.rake diff --git a/app/models/concerns/hyrax/collection_behavior.rb b/app/models/concerns/hyrax/collection_behavior.rb index 9205ba8bfa..eb4045ecaf 100644 --- a/app/models/concerns/hyrax/collection_behavior.rb +++ b/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 diff --git a/lib/tasks/collection_type_global_id.rake b/lib/tasks/collection_type_global_id.rake new file mode 100644 index 0000000000..5655c12d3f --- /dev/null +++ b/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 diff --git a/spec/tasks/rake_spec.rb b/spec/tasks/rake_spec.rb index d44fdc9320..f2f13578b8 100644 --- a/spec/tasks/rake_spec.rb +++ b/spec/tasks/rake_spec.rb @@ -25,4 +25,39 @@ File.delete("abc123.txt") end end + + describe 'hyrax:collections', :clean_repo do + describe ':update_collection_type_global_ids' do + before do + load_rake_environment [File.expand_path('../../../lib/tasks/collection_type_global_id.rake', __FILE__)] + end + + context 'with no collections' do + it 'outputs that 0 collections were updated' do + run_task 'hyrax:collections:update_collection_type_global_ids' + end + end + + context 'with collections' do + let(:collection_type) { FactoryBot.create(:collection_type) } + let(:other_collection_type) { FactoryBot.create(:collection_type) } + + let(:collections_with_legacy_gids) do + [FactoryBot.create(:collection, collection_type_gid: "gid://internal/sometext/#{collection_type.id}"), + FactoryBot.create(:collection, collection_type_gid: "gid://internal/sometext/#{other_collection_type.id}")] + end + + before do + FactoryBot.create_list(:collection, 3, collection_type_gid: collection_type.to_global_id) + FactoryBot.create_list(:collection, 3, collection_type_gid: other_collection_type.to_global_id) + end + + it 'updates collections to use standard GlobalId URI' do + expect { run_task 'hyrax:collections:update_collection_type_global_ids' } + .to change { collections_with_legacy_gids.map { |col| col.reload.collection_type_gid } } + .to eq [collection_type.to_global_id.to_s, other_collection_type.to_global_id.to_s] + end + end + end + end end From d8157d2d16ed5035a2210f97fdd03cef3d84412c Mon Sep 17 00:00:00 2001 From: tamsin johnson Date: Mon, 4 Jan 2021 18:34:58 -0800 Subject: [PATCH 2/4] make CollectionType#find_by_gid work with correct global id format this begins to address #4696 by allowing `CollectionType#find_by_gid` to find "real" GlobalID objects and matching strings. to move away from the old custom format, we'll need to provide a data migration/rake task that can fix existing references. --- app/models/hyrax/collection_type.rb | 9 +++++++++ spec/models/hyrax/collection_type_spec.rb | 18 ++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/models/hyrax/collection_type.rb b/app/models/hyrax/collection_type.rb index 81d91a9a88..b01e37b123 100644 --- a/app/models/hyrax/collection_type.rb +++ b/app/models/hyrax/collection_type.rb @@ -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 diff --git a/spec/models/hyrax/collection_type_spec.rb b/spec/models/hyrax/collection_type_spec.rb index f4c3cac5f6..bd0833c5f3 100644 --- a/spec/models/hyrax/collection_type_spec.rb +++ b/spec/models/hyrax/collection_type_spec.rb @@ -129,14 +129,17 @@ describe '.find_by_gid' do let(:collection_type) { create(:collection_type) } - let(:nonexistent_gid) { 'gid://internal/hyrax-collectiontype/NO_EXIST' } - it 'returns instance of collection type when one with the gid exists' do + it 'returns the same collection type the gid exists' do expect(described_class.find_by_gid(collection_type.gid)).to eq collection_type end + it 'returns the same collection type with `#to_global_id`' do + expect(described_class.find_by_gid(collection_type.to_global_id)).to eq collection_type + end + it 'returns false if collection type with gid does NOT exist' do - expect(described_class.find_by_gid(nonexistent_gid)).to eq false + expect(described_class.find_by_gid('gid://internal/hyrax-collectiontype/NO_EXIST')).to eq false end it 'returns false if gid is nil' do @@ -145,15 +148,18 @@ end describe '.find_by_gid!' do - let(:collection_type) { create(:collection_type) } - let(:nonexistent_gid) { 'gid://internal/hyrax-collectiontype/NO_EXIST' } + let(:collection_type) { FactoryBot.create(:collection_type) } it 'returns instance of collection type when one with the gid exists' do expect(described_class.find_by_gid(collection_type.gid)).to eq collection_type end + it 'returns the same collection type with `#to_global_id`' do + expect(described_class.find_by_gid!(collection_type.to_global_id)).to eq collection_type + end + it 'raises error if collection type with gid does NOT exist' do - expect { described_class.find_by_gid!(nonexistent_gid) } + expect { described_class.find_by_gid!('gid://internal/hyrax-collectiontype/NO_EXIST') } .to raise_error(ActiveRecord::RecordNotFound) end From 8ececbe73aafb934f8b9439b1669da7414367a9f Mon Sep 17 00:00:00 2001 From: tamsin johnson Date: Tue, 5 Jan 2021 09:26:24 -0800 Subject: [PATCH 3/4] disable rubocop classlength for CollectionType make room for deprecations --- app/models/hyrax/collection_type.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/hyrax/collection_type.rb b/app/models/hyrax/collection_type.rb index b01e37b123..52cec4b094 100644 --- a/app/models/hyrax/collection_type.rb +++ b/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 @@ -197,5 +197,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 From 237c0c63297df05b9d483a15b212900c2323a455 Mon Sep 17 00:00:00 2001 From: tamsin johnson Date: Tue, 5 Jan 2021 11:06:56 -0800 Subject: [PATCH 4/4] deprecate Hyrax::CollectionType#gid it's preferable, to use `#to_global_id` (the default `GlobalID` interface) to generate global ids. this ensures we use the correct syntax and respect application level global id configuration. --- .../hyrax/dashboard/collections_controller.rb | 4 +- .../concerns/hyrax/collection_behavior.rb | 2 +- app/models/hyrax/collection_type.rb | 14 +++++-- .../hyrax/collections/migration_service.rb | 6 +-- .../admin/collection_types/index.html.erb | 2 +- lib/generators/hyrax/templates/db/seeds.rb | 17 ++++---- spec/abilities/collection_ability_spec.rb | 12 +++--- .../permission_template_ability_spec.rb | 5 +-- .../collections_membership_actor_spec.rb | 2 +- .../admin/collection_types_controller_spec.rb | 2 +- .../dashboard/collections_controller_spec.rb | 7 ++-- spec/factories/collections.rb | 9 +++-- spec/factories/collections_factory.rb | 3 +- spec/factories/hyrax_collection.rb | 2 + .../collection_multi_membership_spec.rb | 16 ++++---- spec/features/collection_type_spec.rb | 6 ++- spec/features/dashboard/collection_spec.rb | 24 ++++++------ spec/features/work_show_spec.rb | 6 +-- .../forms/admin/collection_type_form_spec.rb | 2 +- spec/helpers/hyrax/collections_helper_spec.rb | 3 +- spec/models/collection_spec.rb | 28 ++++++------- spec/models/hyrax/collection_type_spec.rb | 35 +++++++++-------- .../hyrax/collection_presenter_spec.rb | 2 +- .../collections/migration_service_spec.rb | 2 +- .../nested_collection_query_service_spec.rb | 39 ++++++++++--------- .../permissions_create_service_spec.rb | 2 +- .../hyrax/multiple_membership_checker_spec.rb | 6 +-- .../hyrax/collections/show.html.erb_spec.rb | 2 +- .../_list_collections.html.erb_spec.rb | 6 +-- 29 files changed, 141 insertions(+), 125 deletions(-) diff --git a/app/controllers/hyrax/dashboard/collections_controller.rb b/app/controllers/hyrax/dashboard/collections_controller.rb index e6921ea5f6..64697d781a 100644 --- a/app/controllers/hyrax/dashboard/collections_controller.rb +++ b/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? diff --git a/app/models/concerns/hyrax/collection_behavior.rb b/app/models/concerns/hyrax/collection_behavior.rb index eb4045ecaf..f1771e431f 100644 --- a/app/models/concerns/hyrax/collection_behavior.rb +++ b/app/models/concerns/hyrax/collection_behavior.rb @@ -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. diff --git a/app/models/hyrax/collection_type.rb b/app/models/hyrax/collection_type.rb index 52cec4b094..8c70e874c7 100644 --- a/app/models/hyrax/collection_type.rb +++ b/app/models/hyrax/collection_type.rb @@ -95,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] 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 ## diff --git a/app/services/hyrax/collections/migration_service.rb b/app/services/hyrax/collections/migration_service.rb index 8d69c852c4..57033e5cf7 100644 --- a/app/services/hyrax/collections/migration_service.rb +++ b/app/services/hyrax/collections/migration_service.rb @@ -30,7 +30,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 @@ -74,8 +74,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! diff --git a/app/views/hyrax/admin/collection_types/index.html.erb b/app/views/hyrax/admin/collection_types/index.html.erb index ddcb3191de..b1cb10b47c 100644 --- a/app/views/hyrax/admin/collection_types/index.html.erb +++ b/app/views/hyrax/admin/collection_types/index.html.erb @@ -40,7 +40,7 @@ <% unless collection_type.admin_set? || collection_type.user_collection? %> diff --git a/lib/generators/hyrax/templates/db/seeds.rb b/lib/generators/hyrax/templates/db/seeds.rb index a92571dfa5..a3846eb6d9 100644 --- a/lib/generators/hyrax/templates/db/seeds.rb +++ b/lib/generators/hyrax/templates/db/seeds.rb @@ -114,14 +114,15 @@ def collection_attributes_for(collection_ids) genuser = create_user('general_user@example.com', '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.']) diff --git a/spec/abilities/collection_ability_spec.rb b/spec/abilities/collection_ability_spec.rb index 90e5e13e47..a39268b1cd 100644 --- a/spec/abilities/collection_ability_spec.rb +++ b/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 diff --git a/spec/abilities/permission_template_ability_spec.rb b/spec/abilities/permission_template_ability_spec.rb index c1df868113..c9e88b6919 100644 --- a/spec/abilities/permission_template_ability_spec.rb +++ b/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, diff --git a/spec/actors/hyrax/actors/collections_membership_actor_spec.rb b/spec/actors/hyrax/actors/collections_membership_actor_spec.rb index bd23002a08..2ec16fdb36 100644 --- a/spec/actors/hyrax/actors/collections_membership_actor_spec.rb +++ b/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| diff --git a/spec/controllers/hyrax/admin/collection_types_controller_spec.rb b/spec/controllers/hyrax/admin/collection_types_controller_spec.rb index b6dbf5982f..ba15dbbf5e 100644 --- a/spec/controllers/hyrax/admin/collection_types_controller_spec.rb +++ b/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 } diff --git a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb index b6e713992a..1eb9c12f6d 100644 --- a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb +++ b/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 diff --git a/spec/factories/collections.rb b/spec/factories/collections.rb index 46690128ac..1f9a11c399 100644 --- a/spec/factories/collections.rb +++ b/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 diff --git a/spec/factories/collections_factory.rb b/spec/factories/collections_factory.rb index 18d90ec83e..4e4ca00849 100644 --- a/spec/factories/collections_factory.rb +++ b/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 diff --git a/spec/factories/hyrax_collection.rb b/spec/factories/hyrax_collection.rb index f4fbdf3614..978669a464 100644 --- a/spec/factories/hyrax_collection.rb +++ b/spec/factories/hyrax_collection.rb @@ -8,6 +8,7 @@ collection_type_gid { Hyrax::CollectionType.find_or_create_default_collection_type.to_global_id } transient do + collection_type { nil } edit_groups { [] } edit_users { [] } read_groups { [] } @@ -16,6 +17,7 @@ end after(:build) do |collection, evaluator| + collection.collection_type_gid ||= evaluator.collection_type.to_global_id if evaluator.collection_type&.id.present? collection.member_ids = evaluator.members.map(&:id) if evaluator.members collection.permission_manager.edit_groups = evaluator.edit_groups collection.permission_manager.edit_users = evaluator.edit_users diff --git a/spec/features/collection_multi_membership_spec.rb b/spec/features/collection_multi_membership_spec.rb index dbaee949cc..6a5557e96a 100644 --- a/spec/features/collection_multi_membership_spec.rb +++ b/spec/features/collection_multi_membership_spec.rb @@ -12,11 +12,11 @@ end describe 'when both collections support multiple membership' do - let(:old_collection) { build(:collection_lw, user: admin_user, collection_type_gid: multi_membership_type_1.gid, title: ['OldCollectionTitle']) } + let(:old_collection) { FactoryBot.build(:collection_lw, user: admin_user, collection_type: multi_membership_type_1, title: ['OldCollectionTitle']) } let!(:work) { create(:generic_work, user: admin_user, member_of_collections: [old_collection], title: ['The highly valued work that everyone wants in their collection']) } context 'and are of different types' do - let!(:new_collection) { create(:collection_lw, user: admin_user, collection_type_gid: multi_membership_type_2.gid, title: ['NewCollectionTitle']) } + let!(:new_collection) { FactoryBot.create(:collection_lw, user: admin_user, collection_type: multi_membership_type_2, title: ['NewCollectionTitle']) } it 'then the work is added to both collections' do # Add to second multi-membership collection of a different type @@ -35,7 +35,7 @@ end context 'and are of the same type' do - let!(:new_collection) { create(:collection_lw, user: admin_user, collection_type_gid: multi_membership_type_1.gid, title: ['NewCollectionTitle']) } + let!(:new_collection) { FactoryBot.create(:collection_lw, user: admin_user, collection_type: multi_membership_type_1, title: ['NewCollectionTitle']) } it 'then the work is added to both collections' do # Add to second multi-membership collection of a different type @@ -55,7 +55,7 @@ end describe 'when both collections require single membership' do - let(:old_collection) { build(:collection_lw, user: admin_user, collection_type_gid: single_membership_type_1.gid, title: ['OldCollectionTitle']) } + let(:old_collection) { FactoryBot.build(:collection_lw, user: admin_user, collection_type: single_membership_type_1, title: ['OldCollectionTitle']) } let!(:work) do create(:generic_work, user: admin_user, @@ -66,7 +66,7 @@ end context 'and are of different types' do - let!(:new_collection) { create(:collection_lw, user: admin_user, collection_type_gid: single_membership_type_2.gid, title: ['NewCollectionTitle']) } + let!(:new_collection) { FactoryBot.create(:collection_lw, user: admin_user, collection_type: single_membership_type_2, title: ['NewCollectionTitle']) } it 'then the work is added to both collections' do # Add to second single-membership collection of a different type @@ -85,7 +85,7 @@ end context 'and are of the same type' do - let!(:new_collection) { create(:collection_lw, user: admin_user, collection_type_gid: single_membership_type_1.gid, title: ['NewCollectionTitle']) } + let!(:new_collection) { FactoryBot.create(:collection_lw, user: admin_user, collection_type: single_membership_type_1, title: ['NewCollectionTitle']) } context 'then the work fails to add to the second collection' do it 'from the dashboard->works batch add to collection' do @@ -156,7 +156,7 @@ let!(:work) { create(:generic_work, user: admin_user, member_of_collections: [old_collection], title: ['The highly valued work that everyone wants in their collection']) } context 'allowing multi-membership' do - let(:old_collection) { create(:collection_lw, user: admin_user, collection_type_gid: multi_membership_type_1.gid, title: ['CollectionTitle']) } + let(:old_collection) { FactoryBot.create(:collection_lw, user: admin_user, collection_type: multi_membership_type_1, title: ['CollectionTitle']) } let!(:new_collection) { old_collection } it 'then the add is treated as a success' do @@ -176,7 +176,7 @@ end context 'requiring single-membership' do - let(:old_collection) { create(:collection_lw, user: admin_user, collection_type_gid: single_membership_type_1.gid, title: ['CollectionTitle']) } + let(:old_collection) { FactoryBot.create(:collection_lw, user: admin_user, collection_type: single_membership_type_1, title: ['CollectionTitle']) } let!(:new_collection) { old_collection } it 'then the add is treated as a success' do diff --git a/spec/features/collection_type_spec.rb b/spec/features/collection_type_spec.rb index a9a9ad64d1..8102c7cf89 100644 --- a/spec/features/collection_type_spec.rb +++ b/spec/features/collection_type_spec.rb @@ -292,7 +292,9 @@ end context 'when collections exist of this type' do - let!(:collection1) { create(:public_collection_lw, user: build(:user), collection_type_gid: exhibit_collection_type.gid) } + let!(:collection1) do + FactoryBot.create(:public_collection_lw, user: build(:user), collection_type: exhibit_collection_type) + end before do exhibit_collection_type @@ -352,7 +354,7 @@ context 'when collections exist of this type' do let!(:not_empty_collection_type) { FactoryBot.create(:collection_type, title: 'Not Empty Type', creator_user: admin_user) } - let!(:collection1) { FactoryBot.create(:public_collection_lw, user: admin_user, collection_type_gid: not_empty_collection_type.gid) } + let!(:collection1) { FactoryBot.create(:public_collection_lw, user: admin_user, collection_type: not_empty_collection_type) } let(:deny_delete_modal_text) do 'You cannot delete this collection type because one or more collections of this type have already been created. ' \ diff --git a/spec/features/dashboard/collection_spec.rb b/spec/features/dashboard/collection_spec.rb index e8aacfcba3..637ea849e1 100644 --- a/spec/features/dashboard/collection_spec.rb +++ b/spec/features/dashboard/collection_spec.rb @@ -12,10 +12,10 @@ # Setting Title on admin sets to avoid false positive matches with collections. let(:admin_set_a) { create(:admin_set, creator: [admin_user.user_key], title: ['Set A'], with_permission_template: true) } let(:admin_set_b) { create(:admin_set, creator: [user.user_key], title: ['Set B'], edit_users: [user.user_key], with_permission_template: true) } - let(:collection1) { create(:public_collection_lw, user: user, collection_type_gid: collection_type.gid, with_permission_template: true) } - let(:collection2) { create(:public_collection_lw, user: user, collection_type_gid: collection_type.gid, with_permission_template: true) } - let(:collection3) { create(:public_collection_lw, user: admin_user, collection_type_gid: collection_type.gid, with_permission_template: true) } - let(:collection4) { create(:public_collection_lw, user: admin_user, collection_type_gid: user_collection_type.gid, with_permission_template: true) } + let(:collection1) { FactoryBot.create(:public_collection_lw, user: user, collection_type: collection_type, with_permission_template: true) } + let(:collection2) { FactoryBot.create(:public_collection_lw, user: user, collection_type: collection_type, with_permission_template: true) } + let(:collection3) { FactoryBot.create(:public_collection_lw, user: admin_user, collection_type: collection_type, with_permission_template: true) } + let(:collection4) { FactoryBot.create(:public_collection_lw, user: admin_user, collection_type: user_collection_type, with_permission_template: true) } describe 'Your Collections tab' do context 'when non-admin user' do @@ -57,11 +57,11 @@ href: /visibility_ssi.+#{Regexp.escape(CGI.escape(collection3.visibility))}/ expect(page).to have_button 'Collection Type' expect(page).to have_link collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.to_global_id.to_s))}/ expect(page).to have_link 'AdminSet', href: /#{solr_model_field}.+#{Regexp.escape('AdminSet')}/ expect(page).not_to have_link user_collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.to_global_id.to_s))}/ expect(page).not_to have_link 'Collection', href: /#{solr_model_field}.+#{Regexp.escape('Collection')}/ end @@ -109,9 +109,9 @@ href: /visibility_ssi.+#{Regexp.escape(CGI.escape(collection3.visibility))}/ expect(page).to have_button 'Collection Type' expect(page).to have_link collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.to_global_id.to_s))}/ expect(page).to have_link user_collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.to_global_id.to_s))}/ expect(page).to have_link 'AdminSet', href: /#{solr_model_field}.+#{Regexp.escape('AdminSet')}/ expect(page).not_to have_link 'Collection', @@ -153,9 +153,9 @@ href: /visibility_ssi.+#{Regexp.escape(CGI.escape(collection1.visibility))}/ expect(page).to have_button 'Collection Type' expect(page).to have_link collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.to_global_id.to_s))}/ expect(page).to have_link user_collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.to_global_id.to_s))}/ expect(page).to have_link 'AdminSet', href: /#{solr_model_field}.+#{Regexp.escape('AdminSet')}/ expect(page).not_to have_link 'Collection', @@ -214,9 +214,9 @@ href: /visibility_ssi.+#{Regexp.escape(CGI.escape(collection1.visibility))}/ expect(page).to have_button 'Collection Type' expect(page).to have_link collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(collection_type.to_global_id.to_s))}/ expect(page).to have_link user_collection_type.title, - href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.gid))}/ + href: /#{solr_gid_field}.+#{Regexp.escape(CGI.escape(user_collection_type.to_global_id.to_s))}/ expect(page).not_to have_link 'AdminSet', href: /#{solr_model_field}.+#{Regexp.escape('AdminSet')}/ expect(page).not_to have_link 'Collection', diff --git a/spec/features/work_show_spec.rb b/spec/features/work_show_spec.rb index ba05d19ca6..49de3caf8f 100644 --- a/spec/features/work_show_spec.rb +++ b/spec/features/work_show_spec.rb @@ -23,7 +23,7 @@ let(:file_set) { FactoryBot.create(:file_set, user: user, title: ['A Contained FileSet'], content: file) } let(:file) { File.open(fixture_path + '/world.png') } let(:multi_membership_type_1) { FactoryBot.create(:collection_type, :allow_multiple_membership, title: 'Multi-membership 1') } - let!(:collection) { FactoryBot.create(:collection_lw, user: user, collection_type_gid: multi_membership_type_1.gid) } + let!(:collection) { FactoryBot.create(:collection_lw, user: user, collection_type: multi_membership_type_1) } before do sign_in user @@ -61,7 +61,7 @@ click_button "Add to collection" # opens the modal # Really ensure that this Collection model is persisted Collection.all.map(&:destroy!) - persisted_collection = create(:collection_lw, user: user, collection_type_gid: multi_membership_type_1.gid) + persisted_collection = FactoryBot.create(:collection_lw, user: user, collection_type: multi_membership_type_1) select_member_of_collection(persisted_collection) click_button 'Save changes' @@ -88,7 +88,7 @@ let(:file_set) { create(:file_set, user: user, title: ['A Contained FileSet'], content: file) } let(:file) { File.open(fixture_path + '/world.png') } let(:multi_membership_type_1) { create(:collection_type, :allow_multiple_membership, title: 'Multi-membership 1') } - let!(:collection) { create(:collection_lw, user: viewer, collection_type_gid: multi_membership_type_1.gid) } + let!(:collection) { FactoryBot.create(:collection_lw, user: viewer, collection_type: multi_membership_type_1) } before do sign_in viewer diff --git a/spec/forms/hyrax/forms/admin/collection_type_form_spec.rb b/spec/forms/hyrax/forms/admin/collection_type_form_spec.rb index 2380dea579..136dfa6e74 100644 --- a/spec/forms/hyrax/forms/admin/collection_type_form_spec.rb +++ b/spec/forms/hyrax/forms/admin/collection_type_form_spec.rb @@ -7,7 +7,7 @@ let(:collection_type) { FactoryBot.create(:collection_type) } before do - FactoryBot.valkyrie_create(:hyrax_collection, collection_type_gid: collection_type.gid) + FactoryBot.valkyrie_create(:hyrax_collection, collection_type_gid: collection_type.to_global_id) end end diff --git a/spec/helpers/hyrax/collections_helper_spec.rb b/spec/helpers/hyrax/collections_helper_spec.rb index a9db16e3a2..8d795d88c1 100644 --- a/spec/helpers/hyrax/collections_helper_spec.rb +++ b/spec/helpers/hyrax/collections_helper_spec.rb @@ -131,7 +131,8 @@ let(:test_collection_type) { FactoryBot.create(:collection_type) } it "returns the CollectionType title" do - expect(collection_type_label(test_collection_type.gid)).to eq test_collection_type.title + expect(collection_type_label(test_collection_type.to_global_id)) + .to eq test_collection_type.title end end diff --git a/spec/models/collection_spec.rb b/spec/models/collection_spec.rb index d2d05d0023..1092eea1ef 100644 --- a/spec/models/collection_spec.rb +++ b/spec/models/collection_spec.rb @@ -8,7 +8,7 @@ describe '#bytes' do it 'returns a hard-coded integer and issues a deprecation warning' do - expect(Deprecation).to receive(:warn).once + expect(Deprecation).to receive(:warn).at_least(:once) expect(collection.bytes).to eq(0) end end @@ -119,7 +119,7 @@ class Member < ActiveFedora::Base end let(:member) { Member.create } - let(:collection) { OtherCollection.create(title: ['test title'], collection_type_gid: create(:user_collection_type).gid) } + let(:collection) { OtherCollection.create(title: ['test title'], collection_type: FactoryBot.create(:user_collection_type)) } it "have members that know about the collection", clean_repo: true do member.reload @@ -128,12 +128,12 @@ class Member < ActiveFedora::Base end describe '#collection_type_gid', :clean_repo do - subject(:collection) { described_class.new(collection_type_gid: collection_type.gid) } + subject(:collection) { described_class.new(collection_type_gid: collection_type.to_global_id) } - let(:collection_type) { create(:collection_type) } + let(:collection_type) { FactoryBot.create(:collection_type) } it 'has a collection_type_gid' do - expect(collection.collection_type_gid).to eq collection_type.gid + expect(collection.collection_type_gid).to eq collection_type.to_global_id.to_s end end @@ -142,12 +142,12 @@ class Member < ActiveFedora::Base let(:collection_type) { create(:collection_type) } it 'sets gid' do - collection.collection_type_gid = collection_type.gid - expect(collection.collection_type_gid).to eq collection_type.gid + collection.collection_type_gid = collection_type.to_global_id + expect(collection.collection_type_gid).to eq collection_type.to_global_id.to_s end it 'throws ActiveRecord::RecordNotFound if cannot find collection type for the gid' do - gid = 'gid://internal/hyrax-collectiontype/999' + gid = 'gid://internal/Hyrax::CollectionType/999' expect { collection.collection_type_gid = gid }.to raise_error(ActiveRecord::RecordNotFound) end @@ -156,13 +156,16 @@ class Member < ActiveFedora::Base end it 'updates the collection_type instance variable' do - expect { collection.collection_type_gid = collection_type.gid }.to change { collection.collection_type }.from(create(:user_collection_type)).to(collection_type) + expect { collection.collection_type_gid = collection_type.to_global_id } + .to change { collection.collection_type } + .from(create(:user_collection_type)).to(collection_type) end it 'throws ArgumentError if collection has already been persisted with a collection type' do collection.save! expect(collection.collection_type_gid).not_to be_nil - expect { collection.collection_type_gid = create(:collection_type).gid }.to raise_error(RuntimeError, "Can't modify collection type of this collection") + expect { collection.collection_type_gid = FactoryBot.create(:collection_type).to_global_id } + .to raise_error(RuntimeError, "Can't modify collection type of this collection") end end @@ -173,7 +176,6 @@ class Member < ActiveFedora::Base it 'returns a collection_type instance from the collection_type_gid' do expect(collection.collection_type).to be_kind_of(Hyrax::CollectionType) expect(collection.collection_type).to eq collection_type - expect(collection.collection_type.gid).to eq collection_type.gid end end @@ -201,7 +203,7 @@ class Member < ActiveFedora::Base describe '#reset_access_controls!' do let!(:user) { build(:user) } let(:collection_type) { create(:collection_type) } - let!(:collection) { build(:collection_lw, user: user, collection_type_gid: collection_type.gid) } + let!(:collection) { FactoryBot.build(:collection_lw, user: user, collection_type: collection_type) } let!(:permission_template) { build(:permission_template) } before do @@ -303,7 +305,7 @@ class Member < ActiveFedora::Base let(:coll123) do build(:collection_lw, id: 'Collection123', - collection_type_gid: collection_type.gid, + collection_type_gid: collection_type.to_global_id, with_nesting_attributes: { ancestors: ['Parent_1'], parent_ids: ['Parent_1'], diff --git a/spec/models/hyrax/collection_type_spec.rb b/spec/models/hyrax/collection_type_spec.rb index bd0833c5f3..589cd9a08e 100644 --- a/spec/models/hyrax/collection_type_spec.rb +++ b/spec/models/hyrax/collection_type_spec.rb @@ -4,7 +4,7 @@ shared_context 'with a collection' do let(:collection_type) { FactoryBot.create(:collection_type) } - let!(:collection) { FactoryBot.valkyrie_create(:hyrax_collection, collection_type_gid: collection_type.gid) } + let!(:collection) { FactoryBot.valkyrie_create(:hyrax_collection, collection_type_gid: collection_type.to_global_id) } end describe '.collection_type_settings_methods' do @@ -55,7 +55,7 @@ describe '#gid' do it 'returns the gid when id exists' do collection_type.id = 5 - expect(collection_type.gid.to_s).to eq "gid://#{GlobalID.app}/hyrax-collectiontype/5" + expect(collection_type.gid.to_s).to eq "gid://#{GlobalID.app}/#{described_class}/5" end it 'returns nil when id is nil' do @@ -66,7 +66,7 @@ describe ".any_nestable?" do context "when there is a nestable collection type" do - let!(:collection_type) { create(:collection_type, nestable: true) } + let!(:collection_type) { FactoryBot.create(:collection_type, nestable: true) } it 'returns true' do expect(described_class.any_nestable?).to be true @@ -74,7 +74,7 @@ end context "when there are no nestable collection types" do - let!(:collection_type) { create(:collection_type, nestable: false) } + let!(:collection_type) { FactoryBot.create(:collection_type, nestable: false) } it 'returns false' do expect(described_class.any_nestable?).to be false @@ -92,12 +92,13 @@ end describe ".gids_that_do_not_allow_multiple_membership" do - let!(:type_allows_multiple_membership) { create(:collection_type, allow_multiple_membership: true) } - let!(:type_disallows_multiple_membership) { create(:collection_type, allow_multiple_membership: false) } + let!(:type_allows_multiple_membership) { FactoryBot.create(:collection_type, allow_multiple_membership: true) } + let!(:type_disallows_multiple_membership) { FactoryBot.create(:collection_type, allow_multiple_membership: false) } - subject { described_class.gids_that_do_not_allow_multiple_membership } - - it { is_expected.to match_array(type_disallows_multiple_membership.gid) } + it 'lists the single membership gids' do + expect(described_class.gids_that_do_not_allow_multiple_membership) + .to match_array(type_disallows_multiple_membership.to_global_id.to_s) + end end describe ".find_or_create_admin_set_type" do @@ -112,7 +113,7 @@ end describe "validations", :clean_repo do - let(:collection_type) { create(:collection_type) } + let(:collection_type) { FactoryBot.create(:collection_type) } it "ensures the required fields have values" do collection_type.title = nil @@ -128,7 +129,7 @@ end describe '.find_by_gid' do - let(:collection_type) { create(:collection_type) } + let(:collection_type) { FactoryBot.create(:collection_type) } it 'returns the same collection type the gid exists' do expect(described_class.find_by_gid(collection_type.gid)).to eq collection_type @@ -169,8 +170,8 @@ end describe "collections" do - let!(:collection) { create(:collection_lw, collection_type_gid: collection_type.gid.to_s) } - let(:collection_type) { create(:collection_type) } + let!(:collection) { FactoryBot.create(:collection_lw, collection_type_gid: collection_type.gid.to_s) } + let(:collection_type) { FactoryBot.create(:collection_type) } it 'returns collections of this collection type' do expect(collection_type.collections.to_a).to include collection @@ -183,10 +184,10 @@ end describe "collections?", :clean_repo do - let(:collection_type) { create(:collection_type) } + let(:collection_type) { FactoryBot.create(:collection_type) } it 'returns true if there are any collections of this collection type' do - create(:collection_lw, collection_type_gid: collection_type.gid.to_s) + FactoryBot.create(:collection_lw, collection_type: collection_type) expect(collection_type).to have_collections end it 'returns false if there are not any collections of this collection type' do @@ -235,7 +236,7 @@ end context 'for admin set collection type' do - let(:collection_type) { create(:admin_set_collection_type) } + let(:collection_type) { FactoryBot.create(:admin_set_collection_type) } it 'fails if settings are changed' do expect(collection_type.save).to be false @@ -244,7 +245,7 @@ end context 'for user collection type' do - let(:collection_type) { create(:user_collection_type) } + let(:collection_type) { FactoryBot.create(:user_collection_type) } it 'fails if settings are changed' do expect(collection_type.save).to be false diff --git a/spec/presenters/hyrax/collection_presenter_spec.rb b/spec/presenters/hyrax/collection_presenter_spec.rb index 9376ad58a3..6f61ae43cb 100644 --- a/spec/presenters/hyrax/collection_presenter_spec.rb +++ b/spec/presenters/hyrax/collection_presenter_spec.rb @@ -58,7 +58,7 @@ let(:collection_type) { create(:collection_type) } describe 'when solr_document#collection_type_gid exists' do - let(:collection) { build(:collection_lw, collection_type_gid: collection_type.gid) } + let(:collection) { FactoryBot.build(:collection_lw, collection_type: collection_type) } let(:solr_doc) { SolrDocument.new(collection.to_solr) } it 'finds the collection type based on the solr_document#collection_type_gid if one exists' do diff --git a/spec/services/hyrax/collections/migration_service_spec.rb b/spec/services/hyrax/collections/migration_service_spec.rb index ec6b0a0f82..fd66625199 100644 --- a/spec/services/hyrax/collections/migration_service_spec.rb +++ b/spec/services/hyrax/collections/migration_service_spec.rb @@ -11,7 +11,7 @@ let(:depositor2) { create(:user) } let(:viewer1) { create(:user) } let(:viewer2) { create(:user) } - let(:default_gid) { create(:user_collection_type).gid } + let(:default_gid) { FactoryBot.create(:user_collection_type).to_global_id.to_s } describe ".migrate_all_collections" do context 'when legacy collections are found (e.g. collections created before Hyrax 2.1.0)' do diff --git a/spec/services/hyrax/collections/nested_collection_query_service_spec.rb b/spec/services/hyrax/collections/nested_collection_query_service_spec.rb index 0b0fec7a86..21cc4b91e1 100644 --- a/spec/services/hyrax/collections/nested_collection_query_service_spec.rb +++ b/spec/services/hyrax/collections/nested_collection_query_service_spec.rb @@ -13,7 +13,7 @@ let(:coll_a) do build(:public_collection, id: 'Collection_A', - collection_type_gid: collection_type.gid, + collection_type: collection_type, with_nesting_attributes: { ancestors: [], parent_ids: [], @@ -23,7 +23,7 @@ let(:coll_b) do build(:public_collection, id: 'Collection_B', - collection_type_gid: collection_type.gid, + collection_type: collection_type, member_of_collections: [coll_a], with_nesting_attributes: { ancestors: ['Collection_A'], @@ -34,7 +34,7 @@ let(:coll_c) do build(:public_collection, id: 'Collection_C', - collection_type_gid: collection_type.gid, + collection_type: collection_type, member_of_collections: [coll_b], with_nesting_attributes: { ancestors: ["Collection_A", @@ -46,7 +46,7 @@ let(:coll_d) do build(:public_collection, id: 'Collection_D', - collection_type_gid: collection_type.gid, + collection_type: collection_type, member_of_collections: [coll_c], with_nesting_attributes: { ancestors: ["Collection_A", @@ -59,7 +59,7 @@ let(:coll_e) do build(:public_collection, id: 'Collection_E', - collection_type_gid: collection_type.gid, + collection_type: collection_type, member_of_collections: [coll_d], with_nesting_attributes: { ancestors: ["Collection_A", @@ -73,7 +73,7 @@ let(:another) do build(:public_collection, id: 'Another_One', - collection_type_gid: collection_type.gid, + collection_type: collection_type, with_nesting_attributes: { ancestors: [], parent_ids: [], @@ -83,7 +83,7 @@ let(:wrong) do build(:public_collection, id: 'Wrong_Type', - collection_type_gid: another_collection_type.gid, + collection_type: another_collection_type, with_nesting_attributes: { ancestors: [], parent_ids: [], @@ -156,14 +156,14 @@ let(:coll_a) do build(:public_collection_lw, id: 'Collection_A', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true) end let(:coll_b) do build(:public_collection_lw, id: 'Collection_B', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true, member_of_collections: [coll_a]) @@ -171,7 +171,7 @@ let(:coll_c) do build(:public_collection_lw, id: 'Collection_C', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true, member_of_collections: [coll_b]) @@ -179,7 +179,7 @@ let(:coll_d) do build(:public_collection_lw, id: 'Collection_D', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true, member_of_collections: [coll_c]) @@ -187,7 +187,7 @@ let(:coll_e) do create(:public_collection_lw, id: 'Collection_E', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true, member_of_collections: [coll_d]) @@ -195,14 +195,14 @@ let(:another) do create(:public_collection_lw, id: 'Another_One', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true) end let(:wrong) do build(:public_collection_lw, id: 'Wrong_Type', - collection_type_gid: another_collection_type.gid, + collection_type: another_collection_type, user: user, with_permission_template: true) end @@ -248,14 +248,14 @@ let!(:parent) do create(:public_collection_lw, id: 'Parent_Collecton', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true) end let!(:child) do create(:public_collection_lw, id: 'Child_Collection', - collection_type_gid: collection_type.gid, + collection_type: collection_type, user: user, with_permission_template: true) end @@ -326,12 +326,13 @@ describe 'nesting attributes object', with_nested_reindexing: true do let(:user) { create(:user) } - let!(:parent) { build(:collection_lw, id: 'Parent_Coll', collection_type_gid: collection_type.gid, user: user) } - let!(:child) { create(:user_collection_lw, id: 'Child_Coll', collection_type_gid: collection_type.gid, user: user) } + let(:parent) { FactoryBot.build(:collection_lw, id: 'Parent_Coll', collection_type: collection_type, user: user) } + let(:child) { FactoryBot.create(:collection_lw, id: 'Child_Coll', collection_type: collection_type, user: user) } let(:nesting_attributes) { Hyrax::Collections::NestedCollectionQueryService::NestingAttributes.new(id: child.id, scope: scope) } before do - Hyrax::Collections::NestedCollectionPersistenceService.persist_nested_collection_for(parent: parent, child: child) + Hyrax::Collections::NestedCollectionPersistenceService + .persist_nested_collection_for(parent: parent, child: child) end it 'will respond to expected methods' do diff --git a/spec/services/hyrax/collections/permissions_create_service_spec.rb b/spec/services/hyrax/collections/permissions_create_service_spec.rb index 969f785132..758ff0df6d 100644 --- a/spec/services/hyrax/collections/permissions_create_service_spec.rb +++ b/spec/services/hyrax/collections/permissions_create_service_spec.rb @@ -25,7 +25,7 @@ end let!(:collection_type_participant) { create(:collection_type_participant, user_manage_attributes) } let!(:collection_type_participant2) { create(:collection_type_participant, group_manage_attributes) } - let(:collection) { build(:collection_lw, id: 'collection1', collection_type_gid: collection_type.gid) } + let(:collection) { FactoryBot.build(:collection_lw, id: 'collection1', collection_type: collection_type) } before do subject diff --git a/spec/services/hyrax/multiple_membership_checker_spec.rb b/spec/services/hyrax/multiple_membership_checker_spec.rb index cdb9bc807c..96d9e8b43e 100644 --- a/spec/services/hyrax/multiple_membership_checker_spec.rb +++ b/spec/services/hyrax/multiple_membership_checker_spec.rb @@ -79,7 +79,7 @@ context 'with multiple single membership collection types' do let!(:collection_type_2) { create(:collection_type, title: 'Doc', allow_multiple_membership: false) } - let(:collection_type_gids) { [collection_type.gid, collection_type_2.gid] } + let(:collection_type_gids) { [collection_type.to_global_id, collection_type_2.to_global_id] } it 'returns an error' do expect(item).not_to receive(:member_of_collection_ids) @@ -110,7 +110,7 @@ context 'with multiple single membership collection types' do let!(:collection_type_2) { create(:collection_type, title: 'Doc', allow_multiple_membership: false) } - let(:collection_type_gids) { [collection_type.gid, collection_type_2.gid] } + let(:collection_type_gids) { [collection_type.to_global_id, collection_type_2.to_global_id] } it 'returns an error' do expect(item).to receive(:member_of_collection_ids) @@ -127,7 +127,7 @@ let(:collections) { [collection1, collection2] } let(:collection_ids) { collections.map(&:id) } let(:collection_type_2) { create(:collection_type, title: 'Doc', allow_multiple_membership: false) } - let(:collection_type_gids) { [collection_type.gid, collection_type_2.gid] } + let(:collection_type_gids) { [collection_type.to_global_id, collection_type_2.to_global_id] } it 'returns nil' do expect(item).not_to receive(:member_of_collection_ids) diff --git a/spec/views/hyrax/collections/show.html.erb_spec.rb b/spec/views/hyrax/collections/show.html.erb_spec.rb index 591673c585..9e49dbf491 100644 --- a/spec/views/hyrax/collections/show.html.erb_spec.rb +++ b/spec/views/hyrax/collections/show.html.erb_spec.rb @@ -2,7 +2,7 @@ RSpec.describe 'hyrax/collections/show.html.erb', type: :view do let(:document) do SolrDocument.new(id: 'xyz123z4', - 'collection_type_gid_ssim' => [collection_type.gid], + 'collection_type_gid_ssim' => [collection_type.to_global_id.to_s], 'title_tesim' => ['Make Collections Great Again'], 'rights_tesim' => ["http://creativecommons.org/licenses/by-sa/3.0/us/"]) end diff --git a/spec/views/hyrax/my/collections/_list_collections.html.erb_spec.rb b/spec/views/hyrax/my/collections/_list_collections.html.erb_spec.rb index 28d114ab90..5b019ef585 100644 --- a/spec/views/hyrax/my/collections/_list_collections.html.erb_spec.rb +++ b/spec/views/hyrax/my/collections/_list_collections.html.erb_spec.rb @@ -18,13 +18,13 @@ def check_tr_data_attributes "title_tesim" => ["Collection Title"], "description_tesim" => ["Collection Description"], "thumbnail_path_ss" => Hyrax::CollectionIndexer.thumbnail_path_service.default_image, - "collection_type_gid_ssim" => [collection_type.gid], + "collection_type_gid_ssim" => [collection_type.to_global_id.to_s], "system_modified_dtsi" => modified_date } end let(:doc) { SolrDocument.new(attributes) } - let(:collection_type) { build(:collection_type) } + let(:collection_type) { FactoryBot.build(:collection_type, id: 'coltype_id') } let(:collection_presenter) { Hyrax::CollectionPresenter.new(doc, Ability.new(build(:user)), nil) } before do @@ -67,7 +67,7 @@ def check_tr_data_attributes "title_tesim" => ["AdminSet Title"], "description_tesim" => ["Admin Description"], "thumbnail_path_ss" => Hyrax::AdminSetIndexer.thumbnail_path_service.default_image, - "collection_type_gid_ssim" => [collection_type.gid], + "collection_type_gid_ssim" => [collection_type.to_global_id.to_s], "system_modified_dtsi" => modified_date } end