Skip to content

Commit

Permalink
♻️ Favor Hyrax::ModelRegistry
Browse files Browse the repository at this point in the history
For those reading along, I removed an assertion from
`spec/features/dashboard/collection_spec.rb`.  Why?  Because the test
was creating one type of admin set, and the queries for what to show was
filtering on other kinds of admin sets.

Ultimatley creating a false assertion.
  • Loading branch information
jeremyf committed Feb 13, 2024
1 parent 5597e2a commit 108abdb
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 16 deletions.
1 change: 1 addition & 0 deletions app/controllers/hyrax/file_sets_controller.rb
Expand Up @@ -137,6 +137,7 @@ def parent(file_set: curation_concern)
@parent ||=
case file_set
when Hyrax::FileSet
# TODO: Add Hyrax::FileSet#parent method
Hyrax.query_service.find_parents(resource: file_set).first
else
file_set.parent
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/hyrax/my/collections_controller.rb
Expand Up @@ -12,7 +12,7 @@ def self.configure_facets
config.add_facet_field Hyrax.config.collection_type_index_field,
helper_method: :collection_type_label, limit: 5,
pivot: ['has_model_ssim', Hyrax.config.collection_type_index_field],
skip_item: ->(item) { item.value == Hyrax.config.collection_class.to_s },
skip_item: ->(item) { Hyrax::ModelRegistry.collection_rdf_representations.include?(item.value) },
label: I18n.t('hyrax.dashboard.my.heading.collection_type')
# This causes AdminSets to also be shown with the Collection Type label
config.add_facet_field 'has_model_ssim',
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/hyrax/ability.rb
Expand Up @@ -421,7 +421,7 @@ def user_is_depositor?(document_id)
end

def curation_concerns_models
[::FileSet, ::Hyrax::FileSet, Hyrax.config.collection_class] + Hyrax.config.curation_concerns
Hyrax::ModelRegistry.collection_classes + Hyrax::ModelRegistry.file_set_classes + Hyrax::ModelRegistry.work_classes
end

def can_review_submissions?
Expand Down
12 changes: 5 additions & 7 deletions app/models/concerns/hyrax/solr_document_behavior.rb
Expand Up @@ -53,28 +53,25 @@ def to_model
##
# @return [Boolean]
def collection?
hydra_model == Hyrax.config.collection_class ||
("Collection".safe_constantize == hydra_model)
Hyrax::ModelRegistry.collection_classes.include?(hydra_model)
end

##
# @return [Boolean]
def file_set?
hydra_model == Hyrax::FileSet ||
("::FileSet".safe_constantize == hydra_model)
Hyrax::ModelRegistry.file_set_classes.include?(hydra_model)
end

##
# @return [Boolean]
def admin_set?
(hydra_model == Hyrax.config.admin_set_class) ||
("AdminSet".safe_constantize == hydra_model)
Hyrax::ModelRegistry.admin_set_classes.include?(hydra_model)
end

##
# @return [Boolean]
def work?
Hyrax.config.curation_concerns.include? hydra_model
Hyrax::ModelRegistry.work_classes.include?(hydra_model)
end

# Method to return the model
Expand All @@ -89,6 +86,7 @@ def depositor(default = '')
end

def creator
# TODO: should we replace "hydra_model == AdminSet" with by #admin_set?
solr_term = hydra_model == AdminSet ? "creator_ssim" : "creator_tesim"
fetch(solr_term, [])
end
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/hyrax/pcdm_member_presenter_factory.rb
Expand Up @@ -97,7 +97,7 @@ def work_presenters
# @return
def presenter_for(document:, ability:)
case document['has_model_ssim'].first
when Hyrax::FileSet.name, ::FileSet.name # ActiveFedora FileSet within a Valkyrie Resource
when Hyrax::ModelRegistry.file_set_rdf_representations
Hyrax::FileSetPresenter.new(document, ability)
else
Hyrax::WorkShowPresenter.new(document, ability)
Expand Down
2 changes: 1 addition & 1 deletion app/search_builders/hyrax/exposed_models_relation.rb
Expand Up @@ -3,7 +3,7 @@ module Hyrax
# A relation that scopes to all user visible models (e.g. works + collections + file sets)
class ExposedModelsRelation < AbstractTypeRelation
def allowable_types
(Hyrax.config.curation_concerns + [Hyrax.config.collection_class, ::Collection, ::FileSet]).uniq
Hyrax::ModelRegistry.work_classes + Hyrax::ModelRegistry.collection_classes + Hyrax::ModelRegistry.file_set_classes
end
end
end
2 changes: 1 addition & 1 deletion app/services/hyrax/edit_permissions_service.rb
Expand Up @@ -215,7 +215,7 @@ def object_unauthorized_collection_ids
# @todo Refactor inner working of code as there's lots of branching logic with potential hidden assumptions.
def qualifies_as_unauthorized_collection?(resource:)
case resource
when AdminSet, Hyrax::AdministrativeSet
when Hyrax::ModelRegistry.admin_set_classes
# Prior to this refactor, we looked at AdminSet only; However with the advent of the
# Hyrax::AdministrativeSet, we need to test both cases.
true
Expand Down
2 changes: 1 addition & 1 deletion app/services/hyrax/statistics/collections/over_time.rb
Expand Up @@ -7,7 +7,7 @@ class OverTime < Statistics::OverTime

def relation
AbstractTypeRelation
.new(allowable_types: [Hyrax.config.collection_class])
.new(allowable_types: Hyrax::ModelRegistry.collection_classes)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/hyrax/resource_sync/change_list_writer.rb
Expand Up @@ -62,7 +62,7 @@ def build_changes(xml)
def build_resources(xml, doc_set)
doc_set.each do |doc|
model = doc.fetch('has_model_ssim', []).first.safe_constantize
if model.try(:collection?) || model == Hyrax.config.collection_class
if model.try(:collection?) || Hyrax::ModelRegistry.collection_classes.include?(model)
build_resource(xml, doc, model, hyrax_routes)
else
build_resource(xml, doc, model, main_app_routes)
Expand Down
2 changes: 1 addition & 1 deletion lib/hyrax/resource_sync/resource_list_writer.rb
Expand Up @@ -32,7 +32,7 @@ def builder
end
end

def build_collections(xml, searcher: AbstractTypeRelation.new(allowable_types: [::Collection, Hyrax.config.collection_class]))
def build_collections(xml, searcher: AbstractTypeRelation.new(allowable_types: Hyrax::ModelRegistry.collection_classes))
searcher.search_in_batches(public_access) do |doc_set|
build_resources(xml, doc_set, hyrax_routes)
end
Expand Down
3 changes: 2 additions & 1 deletion spec/features/dashboard/collection_spec.rb
Expand Up @@ -35,8 +35,9 @@
it "has page title, does not have tabs, lists only user's collections, and displays number of collections in the respository" do
expect(page).to have_content 'Collections'
expect(page).not_to have_link 'All Collections'

within('section.tabs-row') do
expect(page).not_to have_link 'Your Collections'
expect(page).to have_link 'Your Collections'
end
expect(page).to have_link(collection1.title.first)
expect(page).to have_link(collection2.title.first)
Expand Down

0 comments on commit 108abdb

Please sign in to comment.