From a3d4a8f0a4118d03eccffccc8bc336b43902b1f4 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 29 Mar 2024 13:57:11 -0400 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Favor=20method=5Fmissing?= =?UTF-8?q?=20over=20a=20raft=20of=20delegates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's like we're flooding a political convention with delegates. Instead of having all of those delegate methods, let's just implement method missing and pass things along to the `SolrDocument`. The benefit is that as we incorporate dynamic metadata, we don't need to keep on adding `delegate :this_property, to: :solr_document`. --- app/presenters/hyrax/work_show_presenter.rb | 30 ++++++++---------- .../hyrax/work_show_presenter_spec.rb | 31 ++++++++++--------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/app/presenters/hyrax/work_show_presenter.rb b/app/presenters/hyrax/work_show_presenter.rb index 957d3b5bfd..2b420f79cb 100644 --- a/app/presenters/hyrax/work_show_presenter.rb +++ b/app/presenters/hyrax/work_show_presenter.rb @@ -16,13 +16,6 @@ class WorkShowPresenter self.collection_presenter_class = CollectionPresenter self.presenter_factory_class = MemberPresenterFactory - # Methods used by blacklight helpers - delegate :has?, :first, :fetch, :export_formats, :export_as, to: :solr_document - - # delegate fields from Hyrax::Works::Metadata to solr_document - delegate :based_near_label, :related_url, :depositor, :identifier, :resource_type, - :keyword, :itemtype, :admin_set, :rights_notes, :access_right, :abstract, to: :solr_document - # @param [SolrDocument] solr_document # @param [Ability] current_ability # @param [ActionDispatch::Request] request the http request context. Used so @@ -33,20 +26,14 @@ def initialize(solr_document, current_ability, request = nil) @request = request end + # We cannot rely on the method missing to catch this delegation. Because + # most all objects implicitly implicitly implement #to_s + delegate :to_s, to: :solr_document + def page_title "#{human_readable_type} | #{title.first} | ID: #{id} | #{I18n.t('hyrax.product_name')}" end - # CurationConcern methods - delegate :stringify_keys, :human_readable_type, :collection?, :to_s, :suppressed?, - to: :solr_document - - # Metadata Methods - delegate :title, :date_created, :description, - :creator, :contributor, :subject, :publisher, :language, :embargo_release_date, - :lease_expiration_date, :license, :source, :rights_statement, :thumbnail_id, :representative_id, - :rendering_ids, :member_of_collection_ids, :alternative_title, :bibliographic_citation, to: :solr_document - def workflow @workflow ||= WorkflowPresenter.new(solr_document, current_ability) end @@ -259,6 +246,15 @@ def valkyrie_presenter? private + def method_missing(method_name, *args, &block) + return solr_document.public_send(method_name, *args, &block) if solr_document.respond_to?(method_name) + super + end + + def respond_to_missing?(method_name, include_private = false) + solr_document.respond_to?(method_name, include_private) || super + end + # list of item ids to display is based on ordered_ids def authorized_item_ids(filter_unreadable: Flipflop.hide_private_items?) @member_item_list_ids ||= diff --git a/spec/presenters/hyrax/work_show_presenter_spec.rb b/spec/presenters/hyrax/work_show_presenter_spec.rb index 94d28bb8f5..8fe296ed87 100644 --- a/spec/presenters/hyrax/work_show_presenter_spec.rb +++ b/spec/presenters/hyrax/work_show_presenter_spec.rb @@ -18,21 +18,22 @@ end it { is_expected.to delegate_method(:to_s).to(:solr_document) } - it { is_expected.to delegate_method(:suppressed?).to(:solr_document) } - it { is_expected.to delegate_method(:human_readable_type).to(:solr_document) } - it { is_expected.to delegate_method(:date_created).to(:solr_document) } - it { is_expected.to delegate_method(:date_modified).to(:solr_document) } - it { is_expected.to delegate_method(:date_uploaded).to(:solr_document) } - it { is_expected.to delegate_method(:rights_statement).to(:solr_document) } - it { is_expected.to delegate_method(:rights_notes).to(:solr_document) } - - it { is_expected.to delegate_method(:based_near_label).to(:solr_document) } - it { is_expected.to delegate_method(:related_url).to(:solr_document) } - it { is_expected.to delegate_method(:depositor).to(:solr_document) } - it { is_expected.to delegate_method(:identifier).to(:solr_document) } - it { is_expected.to delegate_method(:resource_type).to(:solr_document) } - it { is_expected.to delegate_method(:keyword).to(:solr_document) } - it { is_expected.to delegate_method(:itemtype).to(:solr_document) } + + it { is_expected.to respond_to(:suppressed?) } + it { is_expected.to respond_to(:human_readable_type) } + it { is_expected.to respond_to(:date_created) } + it { is_expected.to respond_to(:date_modified) } + it { is_expected.to respond_to(:date_uploaded) } + it { is_expected.to respond_to(:rights_statement) } + it { is_expected.to respond_to(:rights_notes) } + + it { is_expected.to respond_to(:based_near_label) } + it { is_expected.to respond_to(:related_url) } + it { is_expected.to respond_to(:depositor) } + it { is_expected.to respond_to(:identifier) } + it { is_expected.to respond_to(:resource_type) } + it { is_expected.to respond_to(:keyword) } + it { is_expected.to respond_to(:itemtype) } it { is_expected.to delegate_method(:member_presenters).to(:member_presenter_factory) } it { is_expected.to delegate_method(:ordered_ids).to(:member_presenter_factory) }