Skip to content

Commit

Permalink
Deprecate delegated methods and replace occurances in codebase
Browse files Browse the repository at this point in the history
  • Loading branch information
cjcolvar authored and tamsin johnson committed Sep 7, 2022
1 parent b4d3b90 commit eabd3e9
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 58 deletions.
2 changes: 1 addition & 1 deletion app/actors/hyrax/actors/collections_membership_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def extract_collection_id(env)

# Do not apply permissions to work if collection type is configured not to
collection = Hyrax.config.collection_class.find(collection_id)
return unless collection.share_applies_to_new_works?
return unless Hyrax::CollectionType.for(collection: collection).share_applies_to_new_works?

# Save the collection id in env for use in apply_permission_template_actor
env.attributes[:collection_id] = collection_id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,15 @@ def _prefixes
@_prefixes ||= super + ['catalog', 'hyrax/base']
end

# rubocop:disable Style/GuardClause
def query_collection_members
load_member_works
load_member_subcollections if collection.collection_type.nestable?
load_parent_collections if collection.collection_type.nestable? && action_name == 'show'
if Hyrax::CollectionType.for(collection: collection).nestable?
load_member_subcollections
load_parent_collections if action_name == 'show'
end
end
# rubocop:enable Style/GuardClause

# Instantiate the membership query service
def collection_member_service
Expand Down
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 @@ -190,7 +190,7 @@ def create_active_fedora_collection
@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)
@collection.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE unless @collection.discoverable?
@collection.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE unless Hyrax::CollectionType.for(collection: @collection).discoverable?
if @collection.save
after_create_response
else
Expand Down Expand Up @@ -218,7 +218,7 @@ def update_active_fedora_collection
process_member_changes
process_branding

@collection.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE unless @collection.discoverable?
@collection.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE unless Hyrax::CollectionType.for(collection: @collection).discoverable?
# we don't have to reindex the full graph when updating collection
@collection.reindex_extent = Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX
if @collection.update(collection_params.except(:members))
Expand Down
3 changes: 1 addition & 2 deletions app/forms/hyrax/forms/dashboard/nest_collection_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ def find_child(child_id)
def nestable?(collection)
return false if collection.blank?
return collection.nestable? if collection.respond_to? :nestable?
collection_type = Hyrax::CollectionType.find_by_gid!(collection.collection_type_gid)
collection_type.nestable?
Hyrax::CollectionType.for(collection: collection).nestable?
end
end
end
Expand Down
36 changes: 4 additions & 32 deletions app/helpers/hyrax/collections_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,59 +123,31 @@ def single_item_action_remove_form_fields(form, document)
##
# @param collection [Object]
def collection_type_label_for(collection:)
case collection
when Valkyrie::Resource
CollectionType
.find_by_gid!(collection.collection_type_gid)
.title
else
collection.collection_type.title
end
CollectionType.for(collection: collection).title
end

##
# @param collection [Object]
#
# @return [Boolean]
def collection_brandable?(collection:)
case collection
when Valkyrie::Resource
CollectionType
.find_by_gid!(collection.collection_type_gid)
.brandable?
else
collection.try(:brandable?)
end
CollectionType.for(collection: collection).brandable?
end

##
# @param collection [Object]
#
# @return [Boolean]
def collection_discoverable?(collection:)
case collection
when Valkyrie::Resource
CollectionType
.find_by_gid!(collection.collection_type_gid)
.discoverable?
else
collection.try(:discoverable?)
end
CollectionType.for(collection: collection).discoverable?
end

##
# @param collection [Object]
#
# @return [Boolean]
def collection_sharable?(collection:)
case collection
when Valkyrie::Resource
CollectionType
.find_by_gid!(collection.collection_type_gid)
.sharable?
else
collection.try(:sharable?)
end
CollectionType.for(collection: collection).sharable?
end

##
Expand Down
1 change: 1 addition & 0 deletions app/models/concerns/hyrax/collection_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def collection_type_gid=(new_collection_type_gid, force: false)
end

delegate(*Hyrax::CollectionType.settings_attributes, to: :collection_type)
ActiveSupport::Deprecation.deprecate_methods(self, *Hyrax::CollectionType.settings_attributes)

# Get the collection_type when accessed
def collection_type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,7 @@ def self.parent_nesting_depth(parent:, scope:)
def self.nestable?(collection:)
return false if collection.blank?
return collection.nestable? if collection.respond_to? :nestable?
collection_type = Hyrax::CollectionType.find_by_gid!(collection.collection_type_gid)
collection_type.nestable?
Hyrax::CollectionType.for(collection: collection).nestable?
end
private_class_method :nestable?
end
Expand Down
15 changes: 1 addition & 14 deletions app/services/hyrax/edit_permissions_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,20 +202,7 @@ def qualifies_as_unauthorized_collection?(resource:)
# Hyrax::AdministrativeSet, we need to test both cases.
true
else
if resource.respond_to?(:share_applies_to_new_works?)
# The Collection model has traditionally delegated #share_applies_to_new_works? to
# the underlying collection_type
# (see https://github.com/samvera/hyrax/blob/696da5db/spec/models/collection_spec.rb#L189)
resource.share_applies_to_new_works?
elsif resource.respond_to?(:collection_type_gid)
# This is likely a Hyrax::PcdmCollection object, which means we don't have the delegation
# behavior. Instead we'll query the collection type directly.
collection_type = CollectionType.find_by_gid(resource.collection_type_gid)
collection_type&.share_applies_to_new_works?
else
# How might we get here?
false
end
Hyrax::CollectionType.for(collection: resource).share_applies_to_new_works?
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/factories/collections.rb
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ def self.process_with_permission_template(collection, evaluator, force = false)
# @param [Collection] collection object being built/created by the factory
# @param [Class] evaluator holding the transient properties for the current build/creation process
def self.process_with_nesting_attributes(collection, evaluator)
return unless evaluator.with_nesting_attributes.present? && collection.nestable?
return unless evaluator.with_nesting_attributes.present? && Hyrax::CollectionType.for(collection: collection).nestable?
Hyrax::Adapters::NestingIndexAdapter.add_nesting_attributes(
solr_doc: solr_document_with_permissions(collection, evaluator),
ancestors: evaluator.with_nesting_attributes[:ancestors],
Expand All @@ -303,7 +303,7 @@ def self.process_with_nesting_attributes(collection, evaluator)
# @param [Class] evaluator holding the transient properties for the current build/creation process
def self.process_with_solr_document(collection, evaluator)
return unless evaluator.with_solr_document
return if evaluator.with_nesting_attributes.present? && collection.nestable? # will create the solr document there instead
return if evaluator.with_nesting_attributes.present? && Hyrax::CollectionType.for(collection: collection).nestable? # will create the solr document there instead
Hyrax::SolrService.add(solr_document_with_permissions(collection, evaluator), commit: true)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/factories/collections_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
# if requested, create a solr document and add the nesting fields into it
# when a nestable collection is built. This reduces the need to use
# create and :with_nested_indexing for nested collection testing
if evaluator.with_nesting_attributes.present? && collection.nestable?
if evaluator.with_nesting_attributes.present? && Hyrax::CollectionType.for(collection: collection).nestable?
Hyrax::Adapters::NestingIndexAdapter.add_nesting_attributes(
solr_doc: evaluator.to_solr,
ancestors: evaluator.with_nesting_attributes[:ancestors],
Expand Down

0 comments on commit eabd3e9

Please sign in to comment.