Skip to content

Commit

Permalink
deprecate Hyrax::CollectionType#gid
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tamsin johnson committed Jan 11, 2021
1 parent 8ececbe commit 237c0c6
Show file tree
Hide file tree
Showing 29 changed files with 141 additions and 125 deletions.
4 changes: 2 additions & 2 deletions app/controllers/hyrax/dashboard/collections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/hyrax/collection_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 10 additions & 4 deletions app/models/hyrax/collection_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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<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

##
Expand Down
6 changes: 3 additions & 3 deletions app/services/hyrax/collections/migration_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!
Expand Down
2 changes: 1 addition & 1 deletion app/views/hyrax/admin/collection_types/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
17 changes: 9 additions & 8 deletions lib/generators/hyrax/templates/db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.'])
Expand Down Expand Up @@ -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.'])
Expand Down
12 changes: 6 additions & 6 deletions spec/abilities/collection_ability_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions spec/abilities/permission_template_ability_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down Expand Up @@ -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

Expand Down
9 changes: 5 additions & 4 deletions spec/factories/collections.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) { [
Expand All @@ -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
Expand Down Expand Up @@ -103,6 +103,7 @@
transient do
user { create(:user) }

collection_type { nil }
collection_type_settings { nil }
with_permission_template { false }
with_nesting_attributes { nil }
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions spec/factories/collections_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions spec/factories/hyrax_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 { [] }
Expand All @@ -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
Expand Down
Loading

0 comments on commit 237c0c6

Please sign in to comment.