From e718e517e8a05b4228e77bb6be672641561ba880 Mon Sep 17 00:00:00 2001 From: Kirk Wang Date: Thu, 21 Dec 2023 13:12:32 -0800 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A7=B9=20Clean=20up=20services?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit will reconcile the services that are overrides for Hyrax with the Hyrax 5.0.0rc2 version. There were a number of overrides that were changed to the decorator pattern. --- .../admin_set_create_service_decorator.rb | 8 +- ...service.rb => create_service_decorator.rb} | 152 +----------------- .../permissions_create_service_decorator.rb | 4 +- app/services/hyrax/indexes_thumbnails.rb | 37 ----- .../hyrax/indexes_thumbnails_decorator.rb | 18 +++ .../manifest_builder_service_decorator.rb | 2 + .../hyrax/permission_manager_decorator.rb | 4 +- .../hyrax/quick_classification_query.rb | 26 --- .../quick_classification_query_decorator.rb | 29 ++++ app/services/hyrax/thumbnail_path_service.rb | 71 -------- .../hyrax/thumbnail_path_service_decorator.rb | 12 ++ .../hyrax/work_thumbnail_path_service.rb | 12 -- .../work_thumbnail_path_service_decorator.rb | 13 ++ .../create_service_decorator_spec.rb | 29 ++++ .../indexes_thumbnails_decorator_spec.rb | 25 +++ ...ick_classification_query_decorator_spec.rb | 25 +++ .../hyrax/thumbnail_path_service_spec.rb | 21 +++ ...k_thumbnail_path_service_decorator_spec.rb | 21 +++ 18 files changed, 213 insertions(+), 296 deletions(-) rename app/services/hyrax/collection_types/{create_service.rb => create_service_decorator.rb} (52%) delete mode 100644 app/services/hyrax/indexes_thumbnails.rb create mode 100644 app/services/hyrax/indexes_thumbnails_decorator.rb delete mode 100644 app/services/hyrax/quick_classification_query.rb create mode 100644 app/services/hyrax/quick_classification_query_decorator.rb delete mode 100644 app/services/hyrax/thumbnail_path_service.rb create mode 100644 app/services/hyrax/thumbnail_path_service_decorator.rb delete mode 100644 app/services/hyrax/work_thumbnail_path_service.rb create mode 100644 app/services/hyrax/work_thumbnail_path_service_decorator.rb create mode 100644 spec/services/hyrax/collection_types/create_service_decorator_spec.rb create mode 100644 spec/services/hyrax/indexes_thumbnails_decorator_spec.rb create mode 100644 spec/services/hyrax/quick_classification_query_decorator_spec.rb create mode 100644 spec/services/hyrax/thumbnail_path_service_spec.rb create mode 100644 spec/services/hyrax/work_thumbnail_path_service_decorator_spec.rb diff --git a/app/services/hyrax/admin_set_create_service_decorator.rb b/app/services/hyrax/admin_set_create_service_decorator.rb index 8670b22eb..989c8522c 100644 --- a/app/services/hyrax/admin_set_create_service_decorator.rb +++ b/app/services/hyrax/admin_set_create_service_decorator.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Hyrax - # This decorator is used to override logic found in Hyrax v3.4.2 + # This decorator is used to override logic found in Hyrax v5.0.0rc2 # # Because Hyku has converted the Hyrax::Group model from a PORO to a db-backed active record object, # we have to query for existing Hyrax groups instead of initializing empty ones. @@ -27,13 +27,15 @@ def create! updated_admin_set end + private + def workflow_agents [ # OVERRIDE: replace #new with #find_by(:name) - Hyrax::Group.find_by(name: admin_group_name) + Sipity::Agent(Hyrax::Group.find_by(name: admin_group_name)) ].tap do |agent_list| # The default admin set does not have a creating user - agent_list << creating_user if creating_user + agent_list << Sipity::Agent(creating_user) if creating_user end end diff --git a/app/services/hyrax/collection_types/create_service.rb b/app/services/hyrax/collection_types/create_service_decorator.rb similarity index 52% rename from app/services/hyrax/collection_types/create_service.rb rename to app/services/hyrax/collection_types/create_service_decorator.rb index 38950d7df..52d37c576 100644 --- a/app/services/hyrax/collection_types/create_service.rb +++ b/app/services/hyrax/collection_types/create_service_decorator.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# OVERRIDE Hyrax v3.4.2 +# OVERRIDE Hyrax v5.0.0rc2 # - Give the :collection_manager role MANAGE_ACCESS to all non-AdminSet CollectionTypes by default # - Give the :collection_editor role CREATE_ACCESS to all non-AdminSet CollectionTypes by default # - Exclude CREATE_ACCESS from ::Ability.registered_group_name (all registered users) if we are restricting permissions @@ -17,7 +17,7 @@ module CollectionTypes # # @see Hyrax:CollectionType # - class CreateService # rubocop:disable Metrics/ClassLength + module CreateServiceDecorator # rubocop:disable Metrics/ModuleLength DEFAULT_OPTIONS = { description: '', nestable: true, @@ -63,8 +63,6 @@ class CreateService # rubocop:disable Metrics/ClassLength end }.freeze - USER_COLLECTION_MACHINE_ID = Hyrax::CollectionType::USER_COLLECTION_MACHINE_ID - USER_COLLECTION_TITLE = Hyrax::CollectionType::USER_COLLECTION_DEFAULT_TITLE USER_COLLECTION_OPTIONS = { description: I18n.t('hyrax.collection_types.create_service.default_description'), nestable: true, @@ -110,95 +108,6 @@ class CreateService # rubocop:disable Metrics/ClassLength end }.freeze - ADMIN_SET_MACHINE_ID = Hyrax::CollectionType::ADMIN_SET_MACHINE_ID - ADMIN_SET_TITLE = Hyrax::CollectionType::ADMIN_SET_DEFAULT_TITLE - ADMIN_SET_OPTIONS = { - description: I18n.t('hyrax.collection_types.create_service.admin_set_description'), - nestable: false, - brandable: false, - discoverable: false, - sharable: true, - share_applies_to_new_works: true, - allow_multiple_membership: false, - require_membership: true, - assigns_workflow: true, - assigns_visibility: true, - badge_color: "#405060", - participants: [ - { - agent_type: Hyrax::CollectionTypeParticipant::GROUP_TYPE, - agent_id: ::Ability.admin_group_name, - access: Hyrax::CollectionTypeParticipant::MANAGE_ACCESS - }, - { - agent_type: Hyrax::CollectionTypeParticipant::GROUP_TYPE, - agent_id: ::Ability.admin_group_name, - access: Hyrax::CollectionTypeParticipant::CREATE_ACCESS - } - ] - }.freeze - - # @api public - # - # Create a new collection type. - # - # @param machine_id [String] - # @param title [String] short tag identifying the collection type - # @param options [Hash] options to override DEFAULT_OPTIONS - # @option options [String] :description a description to show the user when selecting the collection type - # @option options [Boolean] :nestable if true, collections of this type can be nested - # @option options [Boolean] :brandable if true, collections of this type can be branded - # @option options [Boolean] :discoverable if true, collections of this type can be marked Public - # and found in search results - # @option options [Boolean] :sharable if true, collections of this type can have participants added for - # :manage, :deposit, or :view access - # @option options [Boolean] :share_applies_to_new_works if true, share participant permissions are applied - # to new works created in the collection - # @option options [Boolean] :allow_multiple_membership if true, works can be members of multiple collections - # of this type - # @option options [Boolean] :require_membership if true, all works must belong to at least one collection - # of this type. When combined with allow_multiple_membership=false, works can belong to one and only one - # collection of this type. - # @option options [Boolean] :assigns_workflow if true, collections of this type can be used to assign - # a workflow to a work - # @option options [Boolean] :assigns_visibility if true, collections of this type can be used to assign - # initial visibility to a work - # @option options [String] :badge_color a color for the badge to show the user when selecting the collection type - # @return [Hyrax::CollectionType] the newly created collection type instance - def self.create_collection_type(machine_id:, title:, options: {}) - opts = DEFAULT_OPTIONS.merge(options).except(:participants) - ct = Hyrax::CollectionType.create!(opts.merge(machine_id:, title:)) - participants = options[:participants].presence || DEFAULT_OPTIONS[:participants] - add_participants(ct.id, participants) - ct - end - - # @api public - # - # Create admin set collection type. - # - # @return [Hyrax::CollectionType] the newly created admin set collection type instance - def self.create_admin_set_type - create_collection_type( - machine_id: ADMIN_SET_MACHINE_ID, - title: ADMIN_SET_TITLE, - options: ADMIN_SET_OPTIONS - ) - end - - # @api public - # - # Create user collection type. - # - # @return [Hyrax::CollectionType] the newly created user collection type instance - def self.create_user_collection_type - create_collection_type( - machine_id: USER_COLLECTION_MACHINE_ID, - title: USER_COLLECTION_TITLE, - options: USER_COLLECTION_OPTIONS - ) - end - # @api public # # Add the default participants to a collection_type. @@ -209,7 +118,7 @@ def self.create_user_collection_type # If calling from Abilities, pass the ability. # If you try to get the ability from the user, you end up in an infinite loop. # rubocop:disable Metrics/MethodLength - def self.add_default_participants(collection_type_id) + def add_default_participants(collection_type_id) return unless collection_type_id default_participants = [{ agent_type: Hyrax::CollectionTypeParticipant::GROUP_TYPE, agent_id: ::Ability.admin_group_name, @@ -234,55 +143,10 @@ def self.add_default_participants(collection_type_id) end add_participants(collection_type_id, default_participants) end - # rubocop:enable Metrics/MethodLength - - ## - # @api public - # - # Add a participants to a collection_type. - # - # @param collection_type_id [Integer] the id of the collection type - # @param participants [Array] each element holds agent_type, agent_id, - # and access for a participant to be added - # - # @raise [InvalidParticipantError] if a participant is missing an - # `agent_type`, `agent_id`, or `access`. - def self.add_participants(collection_type_id, participants) - raise(InvalidParticipantError, participants) unless - participants.all? { |p| p.key?(:agent_type) && p.key?(:agent_id) && p.key?(:access) } - - participants.each do |p| - Hyrax::CollectionTypeParticipant.create!( - hyrax_collection_type_id: collection_type_id, - agent_type: p.fetch(:agent_type), - agent_id: p.fetch(:agent_id), - access: p.fetch(:access) - ) - end - rescue InvalidParticipantError => error - Rails.logger.error "Participants not created for collection type " \ - " #{collection_type_id}: #{error.message}" - raise error - end - - ## - # An error class for the case that invalid/incomplete participants are - # added to a collection type. - class InvalidParticipantError < RuntimeError - attr_reader :participants - - def initialize(participants) - @participants = participants - end - - ## - # @return [String] - def message - @participants.map do |participant| - "#{participant[:agent_type]}, #{participant[:agent_id]}, #{participant[:access]}" - end.join("--\n") - end - end - end + end # rubocop:enable Metrics/ModuleLength end end + +Hyrax::CollectionTypes::CreateService.singleton_class.send(:prepend, Hyrax::CollectionTypes::CreateServiceDecorator) +Hyrax::CollectionTypes::CreateService::DEFAULT_OPTIONS = Hyrax::CollectionTypes::CreateServiceDecorator::DEFAULT_OPTIONS +Hyrax::CollectionTypes::CreateService::USER_COLLECTION_OPTIONS = Hyrax::CollectionTypes::CreateServiceDecorator::USER_COLLECTION_OPTIONS diff --git a/app/services/hyrax/collections/permissions_create_service_decorator.rb b/app/services/hyrax/collections/permissions_create_service_decorator.rb index 3f6606761..63b392da4 100644 --- a/app/services/hyrax/collections/permissions_create_service_decorator.rb +++ b/app/services/hyrax/collections/permissions_create_service_decorator.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -# OVERRIDE Hyrax v3.4.2 Grants certain roles access to either all AdminSets or -# all Collections (depending on the role) at create time. +# OVERRIDE Hyrax v5.0.0rc2 Grants certain roles access to either all AdminSets or +# all Collections (depending on the role) at create time. module Hyrax module Collections module PermissionsCreateServiceDecorator diff --git a/app/services/hyrax/indexes_thumbnails.rb b/app/services/hyrax/indexes_thumbnails.rb deleted file mode 100644 index 6dd916c81..000000000 --- a/app/services/hyrax/indexes_thumbnails.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -# OVERRIDE Hyrax 3.4.0 to make collection thumbnails uploadable -module Hyrax - module IndexesThumbnails - extend ActiveSupport::Concern - - included do - class_attribute :thumbnail_path_service - self.thumbnail_path_service = ThumbnailPathService - class_attribute :thumbnail_field - self.thumbnail_field = 'thumbnail_path_ss' - end - - # Adds thumbnail indexing to the solr document - def generate_solr_document - super.tap do |solr_doc| - index_thumbnails(solr_doc) - end - end - - # Write the thumbnail paths into the solr_document - # @param [Hash] solr_document the solr document to add the field to - def index_thumbnails(solr_document) - solr_document[thumbnail_field] = thumbnail_path - end - - # Returns the value for the thumbnail path to put into the solr document - def thumbnail_path - if object.class == Collection && UploadedCollectionThumbnailPathService.uploaded_thumbnail?(object) - UploadedCollectionThumbnailPathService.call(object) - else - self.class.thumbnail_path_service.call(object) - end - end - end -end diff --git a/app/services/hyrax/indexes_thumbnails_decorator.rb b/app/services/hyrax/indexes_thumbnails_decorator.rb new file mode 100644 index 000000000..fe357a890 --- /dev/null +++ b/app/services/hyrax/indexes_thumbnails_decorator.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# OVERRIDE Hyrax v5.0.0rc2 to make collection thumbnails uploadable + +module Hyrax + module IndexesThumbnailsDecorator + # Returns the value for the thumbnail path to put into the solr document + def thumbnail_path + if object.class == Collection && UploadedCollectionThumbnailPathService.uploaded_thumbnail?(object) + UploadedCollectionThumbnailPathService.call(object) + else + super + end + end + end +end + +Hyrax::IndexesThumbnails.prepend(Hyrax::IndexesThumbnailsDecorator) diff --git a/app/services/hyrax/manifest_builder_service_decorator.rb b/app/services/hyrax/manifest_builder_service_decorator.rb index fdfe796ab..297cf9f9a 100644 --- a/app/services/hyrax/manifest_builder_service_decorator.rb +++ b/app/services/hyrax/manifest_builder_service_decorator.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +# OVERRIDE Hyrax v5.0.0rc2 to change `&` to `&` in the Universal Viewer + module Hyrax module ManifestBuilderServiceDecorator private diff --git a/app/services/hyrax/permission_manager_decorator.rb b/app/services/hyrax/permission_manager_decorator.rb index 42732acda..75efca6ae 100644 --- a/app/services/hyrax/permission_manager_decorator.rb +++ b/app/services/hyrax/permission_manager_decorator.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Hyrax - # This decorator is used to override logic found in Hyrax v3.4.1 + # This decorator is used to override logic found in Hyrax v5.0.0rc2 # # Because Hyku has converted the Hyrax::Group model from a PORO to a db-backed active record object, # we have to query for existing Hyrax groups instead of initializing empty ones. @@ -16,6 +16,8 @@ module Hyrax # Because of this, we also add queries for Role permissions in addition to Group permissions # as part of these overrides. module PermissionManagerDecorator + private + def update_groups_for(mode:, groups:) groups = groups.map(&:to_s) diff --git a/app/services/hyrax/quick_classification_query.rb b/app/services/hyrax/quick_classification_query.rb deleted file mode 100644 index 278c11b8e..000000000 --- a/app/services/hyrax/quick_classification_query.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -# OVERRIDE Hryax v3.4.0 -require_dependency Hyrax::Engine.root.join('app', 'services', 'hyrax', 'quick_classification_query').to_s - -Hyrax::QuickClassificationQuery.class_eval do - # OVERRIDE: only use work types that are enabled in the current tenant - # @param [::User] user the current user - # @param [#call] concern_name_normalizer (String#constantize) a proc that translates names to classes - # @param [Array] models the options to display, defaults to everything. - def initialize(user, - models: Site.instance.available_works, - concern_name_normalizer: ->(str) { str.constantize }) - @user = user - @concern_name_normalizer = concern_name_normalizer - @models = models - end - - # OVERRIDE: only use work types that are enabled in the current tenant - # - # @return true if the requested concerns is same as all avaliable concerns - def all? - # OVERRIDE: use Site.instance.available_works instead of Hyrax.config.registered_curation_concern_types - models == Site.instance.available_works - end -end diff --git a/app/services/hyrax/quick_classification_query_decorator.rb b/app/services/hyrax/quick_classification_query_decorator.rb new file mode 100644 index 000000000..733d934af --- /dev/null +++ b/app/services/hyrax/quick_classification_query_decorator.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# OVERRIDE Hryax v5.0.0rc2 + +module Hyrax + module QuickClassificationQueryDecorator + # OVERRIDE: only use work types that are enabled in the current tenant + # @param [::User] user the current user + # @param [#call] concern_name_normalizer (String#constantize) a proc that translates names to classes + # @param [Array] models the options to display, defaults to everything. + def initialize(user, + models: Site.instance.available_works, + concern_name_normalizer: ->(str) { str.constantize }) + @user = user + @concern_name_normalizer = concern_name_normalizer + @models = models + end + + # OVERRIDE: only use work types that are enabled in the current tenant + # + # @return true if the requested concerns is same as all avaliable concerns + def all? + # OVERRIDE: use Site.instance.available_works instead of Hyrax.config.registered_curation_concern_types + models == Site.instance.available_works + end + end +end + +Hyrax::QuickClassificationQuery.prepend(Hyrax::QuickClassificationQueryDecorator) diff --git a/app/services/hyrax/thumbnail_path_service.rb b/app/services/hyrax/thumbnail_path_service.rb deleted file mode 100644 index af3851f72..000000000 --- a/app/services/hyrax/thumbnail_path_service.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -# OVERRIDE Hyrax 3.4.0 - use site defaults instead of app wide defaults -module Hyrax - class ThumbnailPathService - class << self - # @param [#id] object - to get the thumbnail for - # @return [String] a path to the thumbnail - def call(object) - return default_image if object.try(:thumbnail_id).blank? - - thumb = fetch_thumbnail(object) - - return default_image unless thumb - return call(thumb) unless thumb.file_set? - return_path(thumb) - end - - private - - def return_path(thumb) - if audio?(thumb) - audio_image - elsif thumbnail?(thumb) - thumbnail_path(thumb) - else - default_image - end - end - - def audio?(thumb) - service = thumb.respond_to?(:audio?) ? thumb : Hyrax::FileSetTypeService.new(file_set: thumb) - service.audio? - end - - def fetch_thumbnail(object) - return object if object.thumbnail_id == object.id - Hyrax.query_service.find_by(id: object.thumbnail_id) - rescue Valkyrie::Persistence::ObjectNotFoundError, Hyrax::ObjectNotFoundError - Rails.logger.error("Couldn't find thumbnail #{object.thumbnail_id} for #{object.id}") - nil - end - - # @return the network path to the thumbnail - # @param [FileSet] thumbnail the object that is the thumbnail - def thumbnail_path(thumbnail) - Hyrax::Engine.routes.url_helpers.download_path(thumbnail.id, - file: 'thumbnail') - end - - def default_image - Site.instance.default_work_image&.url || ActionController::Base.helpers.image_path('default.png') - end - - def audio_image - ActionController::Base.helpers.image_path 'audio.png' - end - - # @return true if there a file on disk for this object, otherwise false - # @param [FileSet] thumb - the object that is the thumbnail - def thumbnail?(thumb) - File.exist?(thumbnail_filepath(thumb)) - end - - # @param [FileSet] thumb - the object that is the thumbnail - def thumbnail_filepath(thumb) - Hyrax::DerivativePath.derivative_path_for_reference(thumb, 'thumbnail') - end - end - end -end diff --git a/app/services/hyrax/thumbnail_path_service_decorator.rb b/app/services/hyrax/thumbnail_path_service_decorator.rb new file mode 100644 index 000000000..5a9e7d8ca --- /dev/null +++ b/app/services/hyrax/thumbnail_path_service_decorator.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# OVERRIDE Hyrax v5.0.0rc2 - use site defaults instead of app wide defaults +module Hyrax + module ThumbnailPathServiceDecorator + def default_image + Site.instance.default_work_image&.url || super + end + end +end + +Hyrax::ThumbnailPathService.singleton_class.send(:prepend, Hyrax::ThumbnailPathServiceDecorator) diff --git a/app/services/hyrax/work_thumbnail_path_service.rb b/app/services/hyrax/work_thumbnail_path_service.rb deleted file mode 100644 index 55d455096..000000000 --- a/app/services/hyrax/work_thumbnail_path_service.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -# OVERRIDE Hyrax 3.4.0 use site defaults over app default -module Hyrax - class WorkThumbnailPathService < Hyrax::ThumbnailPathService - class << self - def default_image - Site.instance.default_work_image&.url || ActionController::Base.helpers.image_path('work.png') - end - end - end -end diff --git a/app/services/hyrax/work_thumbnail_path_service_decorator.rb b/app/services/hyrax/work_thumbnail_path_service_decorator.rb new file mode 100644 index 000000000..1a021dfee --- /dev/null +++ b/app/services/hyrax/work_thumbnail_path_service_decorator.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# OVERRIDE Hyrax v5.0.0rc2 use site defaults over app default + +module Hyrax + module WorkThumbnailPathServiceDecorator + def default_image + Site.instance.default_work_image&.url || super + end + end +end + +Hyrax::WorkThumbnailPathService.singleton_class.send(:prepend, Hyrax::WorkThumbnailPathServiceDecorator) diff --git a/spec/services/hyrax/collection_types/create_service_decorator_spec.rb b/spec/services/hyrax/collection_types/create_service_decorator_spec.rb new file mode 100644 index 000000000..7b3b7aa47 --- /dev/null +++ b/spec/services/hyrax/collection_types/create_service_decorator_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::CollectionTypes::CreateService, type: :decorator do + let(:added_participants) do + { + agent_type: Hyrax::CollectionTypeParticipant::GROUP_TYPE, + agent_id: 'collection_manager', + access: Hyrax::CollectionTypeParticipant::MANAGE_ACCESS + } + end + + describe '.add_default_participants' do + it 'is overridden by our decorator' do + expect(described_class.method(:add_default_participants).source_location.first).to end_with('create_service_decorator.rb') + end + end + + describe 'DEFAULT_OPTIONS' do + it 'is overridden by our decorator' do + expect(Hyrax::CollectionTypes::CreateService::DEFAULT_OPTIONS[:participants]).to include(added_participants) + end + end + + describe 'USER_COLLECTION_OPTIONS' do + it 'is overridden by our decorator' do + expect(Hyrax::CollectionTypes::CreateService::USER_COLLECTION_OPTIONS[:participants]).to include(added_participants) + end + end +end diff --git a/spec/services/hyrax/indexes_thumbnails_decorator_spec.rb b/spec/services/hyrax/indexes_thumbnails_decorator_spec.rb new file mode 100644 index 000000000..6badadb82 --- /dev/null +++ b/spec/services/hyrax/indexes_thumbnails_decorator_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::IndexesThumbnails, type: :decorator do + describe '#thumbnail_path' do + context 'when the object is a Collection' do + let(:object) { FactoryBot.build(:collection, id: '123') } + + it 'calls the UploadedCollectionThumbnailPathService' do + allow(UploadedCollectionThumbnailPathService).to receive(:uploaded_thumbnail?).with(object).and_return(true) + + expect(UploadedCollectionThumbnailPathService).to receive(:call).with(object) + object.update_index + end + end + + context 'when the object is not a Collection' do + let(:object) { FactoryBot.build(:work, id: '123') } + + it 'calls the UploadedCollectionThumbnailPathService' do + expect(Hyrax::ThumbnailPathService).to receive(:call).with(object) + object.update_index + end + end + end +end diff --git a/spec/services/hyrax/quick_classification_query_decorator_spec.rb b/spec/services/hyrax/quick_classification_query_decorator_spec.rb new file mode 100644 index 000000000..5925b53cd --- /dev/null +++ b/spec/services/hyrax/quick_classification_query_decorator_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::QuickClassificationQuery, type: :decorator do + subject { described_class.new(user) } + + let(:site) { create(:site, available_works:) } + let(:user) { create(:user) } + let(:available_works) { ['GenericWork', 'Image', 'SomeOtherWork'] } + + before do + allow(site).to receive(:available_works).and_return(available_works) + end + + describe '#initialize' do + it 'uses Site.instance.available_works instead of Hyrax.config.registered_curation_concern_types' do + expect(subject.instance_variable_get(:@models)).to eq available_works + end + end + + describe '#all?' do + it 'uses Site.instance.available_works instead of Hyrax.config.registered_curation_concern_types' do + expect(subject.all?).to eq true + end + end +end diff --git a/spec/services/hyrax/thumbnail_path_service_spec.rb b/spec/services/hyrax/thumbnail_path_service_spec.rb new file mode 100644 index 000000000..5aa1889ff --- /dev/null +++ b/spec/services/hyrax/thumbnail_path_service_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::ThumbnailPathService, type: :decorator do + describe '.default_image' do + context 'when the site has a default image' do + let(:image) { '/assets/site_default_work_image.png' } + + it 'returns the default image from the site' do + allow_any_instance_of(Hyrax::AvatarUploader).to receive(:url).and_return(image) + + expect(described_class.default_image).to eq(image) + end + end + + context 'when the site does not have a default image' do + it 'returns the default image from Hyrax' do + expect(described_class.default_image).to eq(ActionController::Base.helpers.image_path('default.png')) + end + end + end +end diff --git a/spec/services/hyrax/work_thumbnail_path_service_decorator_spec.rb b/spec/services/hyrax/work_thumbnail_path_service_decorator_spec.rb new file mode 100644 index 000000000..c72b2eeee --- /dev/null +++ b/spec/services/hyrax/work_thumbnail_path_service_decorator_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::WorkThumbnailPathService, type: :decorator do + describe '.default_image' do + context 'when the site has a default work image' do + let(:image) { '/assets/site_default_work_image.png' } + + it 'returns the default image from the site' do + allow_any_instance_of(Hyrax::AvatarUploader).to receive(:url).and_return(image) + + expect(described_class.default_image).to eq(image) + end + end + + context 'when the site does not have a default work image' do + it 'returns the default image from Hyrax' do + expect(described_class.default_image).to eq(ActionController::Base.helpers.image_path('work.png')) + end + end + end +end From 4942fe4bf45b7a09db239ffa5bd968f0947bed3e Mon Sep 17 00:00:00 2001 From: Kirk Wang Date: Thu, 21 Dec 2023 20:51:19 -0800 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A7=B9=20Address=20PR=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses comments from the review but one thing that is of note is loading the I18n translations in the application.rb file. We needed this because our decorators load prior to I18n loads the locales in our config/locales directory for them to use so we were getting missing translations. --- app/services/hyrax/indexes_thumbnails_decorator.rb | 2 +- .../hyrax/quick_classification_query_decorator.rb | 8 ++------ app/services/hyrax/thumbnail_path_service_decorator.rb | 1 + config/application.rb | 3 +++ 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/services/hyrax/indexes_thumbnails_decorator.rb b/app/services/hyrax/indexes_thumbnails_decorator.rb index fe357a890..6620d5f97 100644 --- a/app/services/hyrax/indexes_thumbnails_decorator.rb +++ b/app/services/hyrax/indexes_thumbnails_decorator.rb @@ -6,7 +6,7 @@ module Hyrax module IndexesThumbnailsDecorator # Returns the value for the thumbnail path to put into the solr document def thumbnail_path - if object.class == Collection && UploadedCollectionThumbnailPathService.uploaded_thumbnail?(object) + if object.try(:collection?) && UploadedCollectionThumbnailPathService.uploaded_thumbnail?(object) UploadedCollectionThumbnailPathService.call(object) else super diff --git a/app/services/hyrax/quick_classification_query_decorator.rb b/app/services/hyrax/quick_classification_query_decorator.rb index 733d934af..34dfcbb76 100644 --- a/app/services/hyrax/quick_classification_query_decorator.rb +++ b/app/services/hyrax/quick_classification_query_decorator.rb @@ -8,12 +8,8 @@ module QuickClassificationQueryDecorator # @param [::User] user the current user # @param [#call] concern_name_normalizer (String#constantize) a proc that translates names to classes # @param [Array] models the options to display, defaults to everything. - def initialize(user, - models: Site.instance.available_works, - concern_name_normalizer: ->(str) { str.constantize }) - @user = user - @concern_name_normalizer = concern_name_normalizer - @models = models + def initialize(user, models: Site.instance.available_works, **kwargs) + super(user, **kwargs.merge(models:)) end # OVERRIDE: only use work types that are enabled in the current tenant diff --git a/app/services/hyrax/thumbnail_path_service_decorator.rb b/app/services/hyrax/thumbnail_path_service_decorator.rb index 5a9e7d8ca..4cbc7d8b9 100644 --- a/app/services/hyrax/thumbnail_path_service_decorator.rb +++ b/app/services/hyrax/thumbnail_path_service_decorator.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true # OVERRIDE Hyrax v5.0.0rc2 - use site defaults instead of app wide defaults + module Hyrax module ThumbnailPathServiceDecorator def default_image diff --git a/config/application.rb b/config/application.rb index c256d9c00..b12574357 100644 --- a/config/application.rb +++ b/config/application.rb @@ -139,6 +139,9 @@ def self.path_for(relative_path) DerivativeRodeo::Generators::HocrGenerator.additional_tessearct_options = nil + # Load locales early so decorators can use them during initialization + I18n.load_path += Dir[Rails.root.join('config', 'locales', '**', '*.yml')] + # Allows us to use decorator files Dir.glob(File.join(File.dirname(__FILE__), "../app/**/*_decorator*.rb")).sort.each do |c| Rails.configuration.cache_classes ? require(c) : load(c)