road to 100% code coverage

.rubocop_todo.yml

@@ -1,26 +1,26 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2022-02-18 11:32:31 -0500 using RuboCop version 0.52.1.
+# on 2022-05-24 16:48:44 -0400 using RuboCop version 0.52.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 203
+# Offense count: 187
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 217
+  Max: 181
 
 # Offense count: 1
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
   Max: 215
 
-# Offense count: 5
+# Offense count: 4
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
-  Max: 30
+  Max: 27
 
 # Offense count: 2
 RSpec/AnyInstance:
@@ -33,16 +33,13 @@ RSpec/DescribeClass:
   Exclude:
     - 'spec/features/hyrax_callbacks_spec.rb'
 
-# Offense count: 14
+# Offense count: 5
 # Configuration parameters: AssignmentOnly.
 RSpec/InstanceVariable:
   Exclude:
-    - 'spec/features/spot_workflow_spec.rb'
-    - 'spec/services/spot/mappers/base_mapper_spec.rb'
-    - 'spec/services/spot/validators/bag_validator_spec.rb'
+    - 'spec/presenters/spot/iiif_manifest_presenter_spec.rb'
     - 'spec/support/shared_examples/indexing/indexes_language_and_label.rb'
     - 'spec/support/shared_examples/logs_a_warning.rb'
-    - 'spec/support/shared_examples/mappers/language_tagged_titles.rb'
     - 'spec/support/shared_examples/spot_workflow_notification.rb'
 
 # Offense count: 1
@@ -55,6 +52,13 @@ RSpec/NamedSubject:
   Exclude:
     - 'spec/support/shared_examples/logs_a_warning.rb'
 
+# Offense count: 3
+# Configuration parameters: IgnoreSymbolicNames.
+RSpec/VerifiedDoubles:
+  Exclude:
+    - 'spec/inputs/multi_authority_controlled_vocabulary_input_spec.rb'
+    - 'spec/jobs/characterize_job_spec.rb'
+
 # Offense count: 1
 # Cop supports --auto-correct.
 Security/YAMLLoad:

app/controllers/concerns/spot/redirection_helpers.rb

@@ -7,7 +7,7 @@ module RedirectionHelpers
     # collections but not other work types. There's probably something I'm missing, but this will at least
     # allow us to resolve Handles and legacy URLs for collections.
     def redirect_params_for(solr_document:)
-      controller = solr_document['has_model_ssim'].first.downcase.pluralize
+      controller = solr_document['has_model_ssim'].first.underscore.pluralize
       params = { controller: "hyrax/#{controller}", action: 'show', id: solr_document['id'] }
 
       return Hyrax::Engine.routes.url_for(**params, only_path: true) if controller == 'collections'

app/controllers/handle_controller.rb

@@ -20,10 +20,6 @@ def show
 
   private
 
-  def id_from_params
-    URI.decode(params[:id])
-  end
-
   # @return [String]
   def identifier_solr_field
     'identifier_ssim'

app/indexers/concerns/indexes_sortable_date.rb

@@ -45,11 +45,6 @@ def generate_solr_document
 
   private
 
-  # @return [String, nil]
-  def raw_date_value
-    object.send(sortable_date_property).sort.first
-  end
-
   # Converts whatever our +date_value+ is to a YYYY-MM-DDT00:00:00Z
   # string for Solr to use in sorting.
   #

app/jobs/characterize_job.rb

@@ -23,13 +23,19 @@ def characterize(file_set, _file_id, filepath)
     Hydra::Works::CharacterizationService.run(file_set.characterization_proxy, filepath, ch12n_tool: tool)
     Rails.logger.debug "Ran characterization on #{file_set.characterization_proxy.id} (#{file_set.characterization_proxy.mime_type})"
 
-    alpha_channels(file_set) if file_set.image? && Hyrax.config.iiif_image_server?
+    alpha_channels(file_set, filepath) if file_set.image? && Hyrax.config.iiif_image_server?
     file_set.characterization_proxy.save!
 
     file_set.update_index
     file_set.parent&.in_collections&.each(&:update_index)
   end
 
+  def alpha_channels(file_set, filepath)
+    return unless file_set.characterization_proxy.respond_to?(:alpha_channels=)
+
+    file_set.characterization_proxy.alpha_channels = channels(filepath)
+  end
+
   def channels(filepath)
     ch = MiniMagick::Tool::Identify.new do |cmd|
       cmd.format '%[channels]'
@@ -38,12 +44,6 @@ def channels(filepath)
     [ch]
   end
 
-  def alpha_channels(file_set)
-    return unless file_set.characterization_proxy.respond_to?(:alpha_channels=)
-
-    file_set.characterization_proxy.alpha_channels = channels(filepath)
-  end
-
   def tool
     ENV['FITS_SERVLET_URL'].present? ? :fits_servlet : :fits
   end

app/models/concerns/spot/nested_collection_behavior.rb

@@ -32,7 +32,7 @@ def add_member_objects(new_member_ids)
       collections_to_add = gather_collections_to_add
       collection_ids_to_add = collections_to_add.map(&:id)
 
-      Array(new_member_ids).collect do |member_id|
+      Array.wrap(new_member_ids).collect do |member_id|
         member = member_query_service(member_id)
         message = check_multiple_membership(item: member, collection_ids: collection_ids_to_add)
 

app/presenters/concerns/spot/presents_attributes.rb

@@ -69,9 +69,9 @@ def find_renderer_class(name)
       end
 
       return renderer unless renderer.nil?
-      return super if defined?(:super)
 
-      raise NameError, "unknown renderer type: #{name}"
+      # super will check the Hyrax scope, which is necessary for their renderers
+      super
     end
   end
 end

app/services/spot/iiif_service.rb

@@ -11,6 +11,16 @@ class IiifService
     COMPLIANCE_LEVEL_URI = 'http://iiif.io/api/image/2/level2.json'
     DEFAULT_SIZE = '600,'
 
+    # Class method for providing a download url (one where the content-disposition is set to 'attachment')
+    #
+    # @param [String] file_id
+    # @param [String] size
+    # @param [String] filename (must include extension)
+    # @return [String]
+    def self.download_url(file_id:, size:, filename:)
+      new(file_id: file_id).download_url(size: size, filename: filename)
+    end
+
     # Class method to be used via Hyrax initializer for generating an image's IIIF URL.
     # We're not using the +base_url+ parameter provided and instead relying on
     # the default, which is the environment value for 'IIIF_BASE_URL'.
@@ -24,7 +34,7 @@ class IiifService
     # @return [String]
     # @see config/initializers/hyrax.rb
     def self.image_url(file_id, _base_url, size)
-      new(file_id: file_id, base_url: ENV['IIIF_BASE_URL']).image_url(size: size)
+      new(file_id: file_id).image_url(size: size)
     end
 
     # Class method to be used via Hyrax initializer for generating an info.json URL.
@@ -41,17 +51,7 @@ def self.image_url(file_id, _base_url, size)
     # @note this produces a URL _without_ the final 'info.json' of the path.
     #       Somewhere in the pipeline this is added (possibly by the viewer?)
     def self.info_url(file_id, _base_url)
-      new(file_id: file_id, base_url: ENV['IIIF_BASE_URL']).info_url
-    end
-
-    # Class method for providing a download url (one where the content-disposition is set to 'attachment')
-    #
-    # @param [String] file_id
-    # @param [String] size
-    # @param [String] filename (must include extension)
-    # @return [String]
-    def self.download_url(file_id:, size:, filename:)
-      new(file_id: file_id, base_url: ENV['IIIF_BASE_URL']).download_url(size: size, filename: filename)
+      new(file_id: file_id).info_url
     end
 
     attr_reader :file_id, :base_url

app/validators/spot/required_local_authority_validator.rb

@@ -30,7 +30,7 @@ def validate(record)
 
     def authority_for(name)
       # try the name
-      return Qa::Authorities::Local::FileBasedAuthority.new(name) if authority_exists?(name)
+      return Qa::Authorities::Local::FileBasedAuthority.new(name.to_s) if authority_exists?(name.to_s)
 
       # otherwise oops!
       raise "Authority doesn't exist: #{name}"

config/routes.rb

@@ -33,7 +33,8 @@
   }
 
   # handle uri catching: ldr.lafayette.edu/handle/:id
-  resources :handle, only: :show, constraints: { id: %r{[0-9]+/[a-zA-Z0-9]+} }
+  # allows slash encoding ('/' => '%2F')
+  resources :handle, only: :show, constraints: { id: %r{10385/[a-zA-Z0-9%]+} }
 
   ##
   # routes for engines + hyrax

spec/authorities/qa/authorities/solr_suggest_spec.rb

@@ -55,4 +55,36 @@
 
     it { is_expected.to eq [] }
   end
+
+  describe '#build_dictionary!' do
+    before do
+      allow(ActiveFedora::SolrService.instance.conn)
+        .to receive(:get)
+        .with('/solr/spot-test/suggest', params: params)
+
+      authority.build_dictionary!
+    end
+
+    context 'when acting on an individual field' do
+      let(:dictionary) { 'keyword' }
+      let(:params) { { 'suggest' => true, 'suggest.dictionary' => dictionary, 'suggest.build' => true } }
+
+      it 'sends individaul dictionary params' do
+        expect(ActiveFedora::SolrService.instance.conn)
+          .to have_received(:get)
+          .with('/solr/spot-test/suggest', params: params)
+      end
+    end
+
+    context 'when building all' do
+      let(:dictionary) { described_class::BUILD_ALL_KEYWORD }
+      let(:params) { { 'suggest' => true, 'suggest.buildAll' => true } }
+
+      it 'sends the build_all params' do
+        expect(ActiveFedora::SolrService.instance.conn)
+          .to have_received(:get)
+          .with('/solr/spot-test/suggest', params: params)
+      end
+    end
+  end
 end

spec/controllers/handle_controller_spec.rb

@@ -42,8 +42,21 @@
       it { is_expected.to redirect_to hyrax_image_path(solr_data[:id]) }
     end
 
-    context 'when a Handle exists for a Collection' do
+    context 'when a Handle exists for a StudentWork' do
       let(:handle) { '10385/9012' }
+      let(:solr_data) do
+        {
+          id: 'existing-student_work',
+          has_model_ssim: ['StudentWork'],
+          identifier_ssim: ["hdl:#{handle}"]
+        }
+      end
+
+      it { is_expected.to redirect_to hyrax_student_work_path(solr_data[:id]) }
+    end
+
+    context 'when a Handle exists for a Collection' do
+      let(:handle) { '10385/col123' }
       let(:solr_data) do
         {
           id: 'existing-col',
@@ -56,7 +69,7 @@
     end
 
     context 'when a handle does not exist for an item' do
-      let(:handle) { '1234/nothere' }
+      let(:handle) { '10385/nothere' }
 
       let(:solr_data) { { id: 'unrelated' } }
 

spec/forms/spot/forms/collection_form_spec.rb

@@ -87,6 +87,14 @@
         it { is_expected.to eq [RDF::Literal('A lengthier explanation of a collection', language: :en)] }
       end
     end
+
+    describe 'stores a "slug" identifier' do
+      subject { attributes[:identifier] }
+
+      let(:params) { { 'slug' => 'a-cool-collection' } }
+
+      it { is_expected.to include 'slug:a-cool-collection' }
+    end
   end
 
   describe '#initialize_field' do

spec/helpers/spot/facet_helper_spec.rb

@@ -58,4 +58,54 @@
 
     it { is_expected.to eq '<span class="label label-success">Public</span>' }
   end
+
+  describe '#admin_facets?' do
+    subject { helper.admin_facets? }
+
+    let(:user) { create(:user) }
+    let(:admin_user) { create(:admin_user) }
+    let(:current_user) { admin_user }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(current_user)
+    end
+
+    context 'when current_user is not an admin' do
+      let(:current_user) { user }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when no admin_facets are defined' do
+      before do
+        allow(helper).to receive(:admin_facet_names).and_return []
+      end
+
+      it { is_expected.to be false }
+    end
+
+    context 'when facets are in the request' do
+      let(:facet) { double }
+
+      before do
+        allow(helper).to receive(:facets_from_request).and_return([facet])
+      end
+
+      context 'when any of the facets should be rendered' do
+        before do
+          allow(helper).to receive(:should_render_facet?).with(facet).and_return true
+        end
+
+        it { is_expected.to be true }
+      end
+
+      context 'when none of the facets should be rendered' do
+        before do
+          allow(helper).to receive(:should_render_facet?).with(facet).and_return false
+        end
+
+        it { is_expected.to be false }
+      end
+    end
+  end
 end