Skip to content

Commit

Permalink
♻️ Extracting naming container (#6694)
Browse files Browse the repository at this point in the history
* ♻️ Extracting naming container

Throughout the code we have quite a bit of conditionals regarding what
is a work, collection, file_set, and adminsitrative set.  This model
attempts to provide a common point to interrogate the application.

There are, at present, no refactors to use this model.

Consider how our specs and our application are inconsistent in their
declaration/configuration/stubbing.  This model should help with that.

* ♻️ Favor Hyrax::ModelRegistry for search builders

Instead of the myriad of ways of asking about which models to use, let's
leverage a consolidated central place for information.

This is but one step in addressing other issues.

* ♻️ Leverage Hyrax::ModelRegistry for ability tests

* 💄 endless and ever appeasing of the coppers

* ♻️ Favor Hyrax::ModelRegistry

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.

* ♻️ Fixing spec

* Adding case statement

* ☑️ Fix stubbing

* ♻️ Favor helper method

* ♻️ Favor dynamic value

* Favor helper method over instance variable

* 🐛 Favor helper method over instance variable

Prior to this commit, if the `update` failed, we would not have set the
`@collection_type` instance variable.  Which would mean that we'd raise
a `NoMethodError` on `NilClass` in the
`app/views/hyrax/dashboard/collections/_form_share_table.html.erb` view.

By favoring a helper method, we no longer require that the edit (nor
create) call the collection_type method.

Below is the the only view references for `@collection_type`, so the
helper_method appears to be adequate.

```shell
❯ rg "@collection_type[\. ]" app/views
app/views/hyrax/dashboard/collections/_form_share_table.html.erb
5:  <p><%= t(".#{access}.help_with_works", type_title: @collection_type.title) if @collection_type.share_applies_to_new_works? && access != 'depositors' %></p>

app/views/hyrax/admin/collection_types/_form.html.erb
22:      <% if @collection_type.admin_set? %>
```

* Adding Hyrax.config.file_set_model

* Update app/models/hyrax/model_registry.rb

* 🧹 An empty commit

---------

Co-authored-by: Daniel Pierce <dlpierce@indiana.edu>
  • Loading branch information
jeremyf and dlpierce authored Feb 18, 2024
1 parent 715f599 commit 76b215a
Show file tree
Hide file tree
Showing 28 changed files with 228 additions and 102 deletions.
1 change: 1 addition & 0 deletions app/controllers/hyrax/file_sets_controller.rb
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
41 changes: 13 additions & 28 deletions app/models/concerns/hyrax/ability/admin_set_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,34 @@ module Hyrax
module Ability
module AdminSetAbility
def admin_set_abilities # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
models = Hyrax::ModelRegistry.admin_set_classes
if admin?
can :manage, [AdminSet, Hyrax::AdministrativeSet]
can :manage_any, AdminSet
can :manage_any, Hyrax::AdministrativeSet
can :create_any, AdminSet
can :create_any, Hyrax::AdministrativeSet
can :view_admin_show_any, AdminSet
can :view_admin_show_any, Hyrax::AdministrativeSet
can :manage, models
can :manage_any, models
can :create_any, models
can :view_admin_show_any, models
else
if Hyrax::Collections::PermissionsService.can_manage_any_admin_set?(ability: self)
can :manage_any, AdminSet
can :manage_any, Hyrax::AdministrativeSet
end
can :manage_any, models if Hyrax::Collections::PermissionsService.can_manage_any_admin_set?(ability: self)
if Hyrax::CollectionTypes::PermissionsService.can_create_admin_set_collection_type?(ability: self)
can :create, [AdminSet, Hyrax::AdministrativeSet]
can :create_any, AdminSet
can :create_any, Hyrax::AdministrativeSet
end
if Hyrax::Collections::PermissionsService.can_view_admin_show_for_any_admin_set?(ability: self)
can :view_admin_show_any, AdminSet
can :view_admin_show_any, Hyrax::AdministrativeSet
can :create, models
can :create_any, models
end
can :view_admin_show_any, models if Hyrax::Collections::PermissionsService.can_view_admin_show_for_any_admin_set?(ability: self)
# [:edit, :update, :destroy] for AdminSet is controlled by Hydra::Ability #edit_permissions
can [:edit, :update, :destroy], Hyrax::AdministrativeSet do |admin_set| # for test by solr_doc, see solr_document_ability.rb
can [:edit, :update, :destroy], models do |admin_set| # for test by solr_doc, see solr_document_ability.rb
test_edit(admin_set.id)
end

can :deposit, AdminSet do |admin_set| # for test by solr_doc, see collection_ability.rb
Hyrax::Collections::PermissionsService.can_deposit_in_collection?(ability: self, collection_id: admin_set.id)
end
can :deposit, Hyrax::AdministrativeSet do |admin_set| # for test by solr_doc, see collection_ability.rb
can :deposit, models do |admin_set| # for test by solr_doc, see collection_ability.rb
Hyrax::Collections::PermissionsService.can_deposit_in_collection?(ability: self, collection_id: admin_set.id)
end

can :view_admin_show, AdminSet do |admin_set| # admin show page # for test by solr_doc, see collection_ability.rb
Hyrax::Collections::PermissionsService.can_view_admin_show_for_collection?(ability: self, collection_id: admin_set.id)
end
can :view_admin_show, Hyrax::AdministrativeSet do |admin_set| # admin show page # for test by solr_doc, see collection_ability.rb
can :view_admin_show, models do |admin_set| # admin show page # for test by solr_doc, see collection_ability.rb
Hyrax::Collections::PermissionsService.can_view_admin_show_for_collection?(ability: self, collection_id: admin_set.id)
end

# [:read] for AdminSet is controlled by Hydra::Ability #read_permissions
can :read, Hyrax::AdministrativeSet do |admin_set| # admin show page # for test by solr_doc, see collection_ability.rb
can :read, models do |admin_set| # admin show page # for test by solr_doc, see collection_ability.rb
test_read(admin_set.id)
end
end
Expand Down
45 changes: 19 additions & 26 deletions app/models/concerns/hyrax/ability/collection_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,33 @@ module Hyrax
module Ability
module CollectionAbility
def collection_abilities # rubocop:disable Metrics/MethodLength, Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
models = [Hyrax::PcdmCollection, Hyrax.config.collection_class].uniq
models = Hyrax::ModelRegistry.collection_classes
if admin?
models.each do |collection_model|
can :manage, collection_model
can :manage_any, collection_model
can :create_any, collection_model
can :view_admin_show_any, collection_model
end
can :manage, models
can :manage_any, models
can :create_any, models
can :view_admin_show_any, models
else
models.each { |collection_model| can :manage_any, collection_model } if
Hyrax::Collections::PermissionsService.can_manage_any_collection?(ability: self)
can :manage_any, models if Hyrax::Collections::PermissionsService.can_manage_any_collection?(ability: self)

models.each { |collection_model| can :create_any, collection_model } if
Hyrax::CollectionTypes::PermissionsService.can_create_any_collection_type?(ability: self)
can :create_any, models if Hyrax::CollectionTypes::PermissionsService.can_create_any_collection_type?(ability: self)

models.each { |collection_model| can :view_admin_show_any, collection_model } if
Hyrax::Collections::PermissionsService.can_view_admin_show_for_any_collection?(ability: self)
can :view_admin_show_any, models if Hyrax::Collections::PermissionsService.can_view_admin_show_for_any_collection?(ability: self)

models.each do |collection_model|
can [:edit, :update, :destroy], collection_model do |collection|
test_edit(collection.id)
end
can [:edit, :update, :destroy], models do |collection|
test_edit(collection.id)
end

can :deposit, collection_model do |collection|
Hyrax::Collections::PermissionsService.can_deposit_in_collection?(ability: self, collection_id: collection.id)
end
can :deposit, models do |collection|
Hyrax::Collections::PermissionsService.can_deposit_in_collection?(ability: self, collection_id: collection.id)
end

can :view_admin_show, collection_model do |collection| # admin show page
Hyrax::Collections::PermissionsService.can_view_admin_show_for_collection?(ability: self, collection_id: collection.id)
end
can :view_admin_show, models do |collection| # admin show page
Hyrax::Collections::PermissionsService.can_view_admin_show_for_collection?(ability: self, collection_id: collection.id)
end

can :read, collection_model do |collection| # public show page
test_read(collection.id)
end
can :read, models do |collection| # public show page
test_read(collection.id)
end

can :deposit, ::SolrDocument do |solr_doc|
Expand Down
12 changes: 5 additions & 7 deletions app/models/concerns/hyrax/solr_document_behavior.rb
Original file line number Diff line number Diff line change
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
111 changes: 111 additions & 0 deletions app/models/hyrax/model_registry.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# frozen_string_literal: true

module Hyrax
##
# There are four conceptual high-level models for the metadata modeling:
#
# - AdminSet :: Where things are "physically" stored.
# - Collection :: What things "logically" belong to.
# - Work :: What the thing is about; these are often sub-divided into different work types
# (e.g. an Article, a Monograph, etc.)
# - FileSet :: Artifacts that further describe the thing.
#
# The purpose of this module is to provide an overview and map of the modeling concepts. This is
# envisioned as a module to interrogate to see how Hyrax conceptually organizes your models.
#
# For each of the above high-level models the {Hyrax::ModelRegistry} implements two methods:
#
# - <model_type>_classes :: These are the Ruby constants that represent the conceptual models.
# - <model_type>_rdf_representations :: These are the stored strings that help us describe what #
# - the data models. Very useful in our Solr queries when we want to filter/query on the
# - "has_model_ssim" attribute.
#
# Consider that an {AdminSet} and a {Hyrax::AdministrativeSet} conceptually model the same thing;
# the latter implements via Valkyrie and the former via ActiveFedora.
#
# @note Due to the shift from ActiveFedora to Valkyrie, we are at a crossroads where we might have
# multiple models for the versions: an ActiveFedora AdminSet and a Valkyrie AdminSet.
class ModelRegistry
##
# @return [Array<String>]
def self.admin_set_class_names
["::AdminSet", "::Hyrax::AdministrativeSet", Hyrax.config.admin_set_model].uniq
end

##
# @return [Array<Class>]
def self.admin_set_classes
classes_from(admin_set_class_names)
end

##
# @return [Array<String>]
def self.admin_set_rdf_representations
rdf_representations_from(admin_set_classes)
end

##
# @return [Array<String>]
def self.collection_class_names
["::Collection", "::Hyrax::PcdmCollection", Hyrax.config.collection_model].uniq
end

##
# @return [Array<String>]
def self.collection_classes
classes_from(collection_class_names)
end

def self.collection_rdf_representations
rdf_representations_from(collection_classes)
end

def self.collection_has_model_ssim
collection_rdf_representations
end

##
# @return [Array<String>]
def self.file_set_class_names
["::FileSet", "::Hyrax::FileSet", Hyrax.config.file_set_model].uniq
end

def self.file_set_classes
classes_from(file_set_class_names)
end

##
# @return [Array<String>]
def self.file_set_rdf_representations
rdf_representations_from(file_set_classes)
end

##
# @return [Array<Class>]
#
# @todo Consider the Wings::ModelRegistry and how we perform mappings.
def self.work_class_names
Hyrax.config.registered_curation_concern_types
end

def self.work_classes
classes_from(work_class_names)
end

##
# @return [Array<String>]
def self.work_rdf_representations
rdf_representations_from(work_classes)
end

def self.classes_from(strings)
strings.map(&:safe_constantize).flatten.uniq
end
private_class_method :classes_from

def self.rdf_representations_from(klasses)
klasses.map { |klass| klass.respond_to?(:to_rdf_represntation) ? klass.to_rdf_represntation : klass.name }.uniq
end
private_class_method :rdf_representations_from
end
end
2 changes: 1 addition & 1 deletion app/presenters/hyrax/pcdm_member_presenter_factory.rb
Original file line number Diff line number Diff line change
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/admin_set_search_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def initialize(context, access)

# This overrides the models in FilterByType
def models
[::AdminSet, Hyrax::AdministrativeSet]
Hyrax::ModelRegistry.admin_set_classes
end

# Overrides Hydra::AccessControlsEnforcement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class CollectionsSearchBuilder < Hyrax::CollectionSearchBuilder

# This overrides the models in FilterByType
def models
[::AdminSet, Hyrax::AdministrativeSet, "Collection".safe_constantize, Hyrax.config.collection_class].uniq.compact
Hyrax::ModelRegistry.admin_set_classes + Hyrax::ModelRegistry.collection_classes
end

# adds a filter to exclude collections and admin sets created by the
Expand All @@ -18,9 +18,11 @@ def models
def show_only_managed_collections_for_non_admins(solr_parameters)
return if current_ability.admin?
clauses = [
'-' + ActiveFedora::SolrQueryBuilder.construct_query_for_rel(depositor: current_user_key),
'-' + ActiveFedora::SolrQueryBuilder.construct_query_for_rel(has_model: Hyrax.config.admin_set_model, creator: current_user_key)
'-' + ActiveFedora::SolrQueryBuilder.construct_query_for_rel(depositor: current_user_key)
]
Hyrax::ModelRegistry.admin_set_rdf_representations.each do |has_model|
clauses += ['-' + ActiveFedora::SolrQueryBuilder.construct_query_for_rel(has_model: has_model, creator: current_user_key)]
end
solr_parameters[:fq] ||= []
solr_parameters[:fq] += ["(#{clauses.join(' OR ')})"]
end
Expand Down
2 changes: 1 addition & 1 deletion app/search_builders/hyrax/exposed_models_relation.rb
Original file line number Diff line number Diff line change
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/search_builders/hyrax/file_set_search_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class FileSetSearchBuilder < ::SearchBuilder

# This overrides the models in FilterByType
def models
[::FileSet, ::Hyrax::FileSet]
Hyrax::ModelRegistry.file_set_classes
end
end
end
5 changes: 3 additions & 2 deletions app/search_builders/hyrax/filter_by_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def generic_type_field
# types from appearing in search results
# @return [Array<Class>] the list of work types to include in searches
def work_types
Hyrax.config.curation_concerns
Hyrax::ModelRegistry.work_classes
end

def work_classes
Expand All @@ -53,7 +53,8 @@ def work_classes

def collection_classes
return [] if only_works?
["Collection".safe_constantize, Hyrax.config.collection_class].uniq.compact

Hyrax::ModelRegistry.collection_classes
end
end
end
4 changes: 2 additions & 2 deletions app/search_builders/hyrax/my/collections_search_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ def show_only_collections_deposited_by_current_user(solr_parameters)
# This overrides the models in FilterByType
# @return [Array<Class>] a list of classes to include
def models
[::AdminSet, Hyrax::AdministrativeSet, "Collection".safe_constantize, Hyrax.config.collection_class].uniq.compact
Hyrax::ModelRegistry.admin_set_classes + Hyrax::ModelRegistry.collection_classes
end

private

def query_for_my_collections
query_service = Hyrax::SolrQueryService.new
query_service.with_field_pairs(field_pairs: { depositor_ssim: current_user_key }, type: 'terms')
query_service.with_field_pairs(field_pairs: { has_model_ssim: Hyrax.config.admin_set_model,
query_service.with_field_pairs(field_pairs: { has_model_ssim: Hyrax::ModelRegistry.admin_set_rdf_representations.join(','),
creator_ssim: current_user_key }, type: 'terms')
query_service.build(join_with: 'OR')
end
Expand Down
Loading

0 comments on commit 76b215a

Please sign in to comment.