Use bixby branch with rubocop 1; Add exceptions and run rubocop autoc…

…orrect

.rubocop.yml

@@ -5,7 +5,7 @@ inherit_gem:
   bixby: bixby_default.yml
 
 AllCops:
-  TargetRubyVersion: 2.4
+  TargetRubyVersion: 2.5
   DisplayCopNames: true
   Exclude:
     - 'db/**/*'

.rubocop_fixme.yml

@@ -171,14 +171,31 @@ RSpec/RepeatedDescription:
     - 'spec/models/sipity/workflow_state_action_spec.rb'
     - 'spec/models/sipity/workflow_state_spec.rb'
 
-# Offense count: 1
+# Offense count: 2
 # Configuration parameters: Include.
 # Include: app/models/**/*.rb
 Rails/HasManyOrHasOneDependent:
   Exclude:
     - 'app/models/admin_set.rb'
+    - 'app/models/hyrax/permission_template.rb'
 
 # Offense count: 1
-Style/MethodMissingSuper:
+Rails/SkipsModelValidations:
+  Exclude:
+    - 'app/services/hyrax/works/migration_service.rb'
+
+# Offense count: 12
+Lint/MissingSuper:
   Exclude:
+    - 'app/actors/hyrax/actors/interpret_visibility_actor.rb'
+    - 'app/actors/hyrax/actors/ordered_members_actor.rb'
     - 'app/models/concerns/hyrax/file_set/characterization.rb'
+    - 'app/presenters/hyrax/file_usage.rb'
+    - 'app/presenters/hyrax/work_usage.rb'
+    - 'app/services/hyrax/batch_create_failure_service.rb'
+    - 'app/services/hyrax/batch_create_success_service.rb'
+    - 'app/services/hyrax/collection_types/create_service.rb'
+    - 'app/services/hyrax/solr_query_service.rb'
+    - 'lib/hyrax/form_fields.rb'
+    - 'lib/hyrax/health_checks/solr_check.rb'
+    - 'lib/hyrax/schema.rb'

app/controllers/concerns/hyrax/collections_controller_behavior.rb

@@ -41,9 +41,7 @@ def collection
     private
 
     def presenter
-      @presenter ||= begin
-        presenter_class.new(curation_concern, current_ability)
-      end
+      @presenter ||= presenter_class.new(curation_concern, current_ability)
     end
 
     def curation_concern

app/controllers/hyrax/dashboard/collections_controller.rb

@@ -376,9 +376,7 @@ def remove_select_something_first_flash
       end
 
       def presenter
-        @presenter ||= begin
-          presenter_class.new(curation_concern, current_ability)
-        end
+        @presenter ||= presenter_class.new(curation_concern, current_ability)
       end
 
       def curation_concern

app/controllers/hyrax/single_use_links_viewer_controller.rb

@@ -75,7 +75,7 @@ def current_ability
 
     def render_single_use_error(exception)
       logger.error("Rendering PAGE due to exception: #{exception.inspect} - #{exception.backtrace if exception.respond_to? :backtrace}")
-      render 'single_use_error', layout: "error", status: 404
+      render 'single_use_error', layout: "error", status: :not_found
     end
 
     def _prefixes

app/forms/hyrax/forms/widgets/admin_set_visibility.rb

@@ -6,7 +6,7 @@ class AdminSetVisibility
         # Visibility options for permission templates
         def options
           i18n_prefix = "hyrax.admin.admin_sets.form_visibility.visibility"
-          # Note: Visibility 'varies' = '' implies no constraints
+          # NOTE: Visibility 'varies' = '' implies no constraints
           [[Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC, I18n.t('.everyone', scope: i18n_prefix)],
            ['', I18n.t('.varies', scope: i18n_prefix)],
            [Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED, I18n.t('.institution', scope: i18n_prefix)],

app/jobs/import_url_job.rb

@@ -80,12 +80,10 @@ def copy_remote_file(name)
     Rails.logger.debug("ImportUrlJob: Copying <#{uri}> to #{dir}")
 
     File.open(File.join(dir, filename), 'wb') do |f|
-      begin
-        write_file(f)
-        yield f
-      rescue StandardError => e
-        send_error(e.message)
-      end
+      write_file(f)
+      yield f
+    rescue StandardError => e
+      send_error(e.message)
     end
     Rails.logger.debug("ImportUrlJob: Closing #{File.join(dir, filename)}")
   end

app/models/concerns/hyrax/collection_behavior.rb

@@ -87,7 +87,7 @@ def to_s
 
     module ClassMethods
       # This governs which partial to draw when you render this type of object
-      def _to_partial_path #:nodoc:
+      def _to_partial_path # :nodoc:
         @_to_partial_path ||= begin
           element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(name))
           collection = ActiveSupport::Inflector.tableize(name)

app/models/concerns/hyrax/work_behavior.rb

@@ -34,7 +34,7 @@ module WorkBehavior
 
     module ClassMethods
       # This governs which partial to draw when you render this type of object
-      def _to_partial_path #:nodoc:
+      def _to_partial_path # :nodoc:
         @_to_partial_path ||= begin
           element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(name))
           collection = ActiveSupport::Inflector.tableize(name)

app/models/hyrax/file_set.rb

@@ -34,9 +34,7 @@ class FileSet < Hyrax::Resource
     include Hyrax::Schema(:basic_metadata)
 
     def self.model_name(name_class: Hyrax::Name)
-      @_model_name ||= begin
-        name_class.new(self, nil, 'FileSet')
-      end
+      @_model_name ||= name_class.new(self, nil, 'FileSet')
     end
 
     class_attribute :characterization_proxy

app/models/job_io_wrapper.rb

@@ -94,7 +94,7 @@ def file
 
   def extracted_original_name
     eon = uploaded_file.uploader.filename if uploaded_file
-    eon ||= File.basename(path) if path.present? # note: uploader.filename is `nil` with uncached remote files (e.g. AWSFile)
+    eon ||= File.basename(path) if path.present? # NOTE: uploader.filename is `nil` with uncached remote files (e.g. AWSFile)
     eon
   end
 

app/presenters/hyrax/member_presenter_factory.rb

@@ -47,13 +47,11 @@ def ordered_ids
     # in order.
     # Arbitrarily maxed at 10 thousand; had to specify rows due to solr's default of 10
     def file_set_ids
-      @file_set_ids ||= begin
-                          Hyrax::SolrService.query("{!field f=has_model_ssim}FileSet",
+      @file_set_ids ||= Hyrax::SolrService.query("{!field f=has_model_ssim}FileSet",
                                                    rows: 10_000,
                                                    fl: Hyrax.config.id_field,
                                                    fq: "{!join from=ordered_targets_ssim to=id}id:\"#{id}/list_source\"")
-                                            .flat_map { |x| x.fetch(Hyrax.config.id_field, []) }
-                        end
+                                          .flat_map { |x| x.fetch(Hyrax.config.id_field, []) }
     end
 
     def presenter_factory_arguments

app/services/hyrax/adapters/nesting_index_adapter.rb

@@ -60,7 +60,7 @@ def self.each_perservation_document_id_and_parent_ids(&block) # rubocop:disable
           object = ActiveFedora::Base.find(id)
           parent_ids = object.try(:member_of_collection_ids) || []
 
-          # note: we do not yield when the object has parents. Calling the nested indexer for the
+          # NOTE: we do not yield when the object has parents. Calling the nested indexer for the
           # top id will reindex all descendants as well.
           if object.try(:use_nested_reindexing?)
             yield(id, parent_ids) if parent_ids.empty?
@@ -119,11 +119,11 @@ def self.add_nesting_attributes(solr_doc:, ancestors:, parent_ids:, pathnames:,
       # @yield Samvera::NestingIndexer::Documents::IndexDocument
       #
       # @return [void]
-      def self.each_child_document_of(document:, extent:, &block)
+      def self.each_child_document_of(document:, extent:, &block) # rubocop:disable Lint/UnusedMethodArgument
         raw_child_solr_documents_of(parent_document: document).each do |solr_document|
           child_document = coerce_solr_document_to_index_document(original_solr_document: solr_document, id: solr_document.fetch('id'))
           # during light reindexing, we want to reindex the child only if fields aren't already there
-          block.call(child_document) if full_reindex?(extent: extent) || child_document.pathnames.empty?
+          yield(child_document) if full_reindex?(extent: extent) || child_document.pathnames.empty?
         end
       end
 

app/services/hyrax/characterization/valkyrie_characterization_service.rb

@@ -91,11 +91,9 @@ def characterization_terms(doc)
     h = {}
 
     doc.class.terminology.terms.each_pair do |key, _target|
-      begin
-        h[key] = doc.public_send(key)
-      rescue NoMethodError
-        next
-      end
+      h[key] = doc.public_send(key)
+    rescue NoMethodError
+      next
     end
 
     h.compact

app/services/hyrax/collections/collection_member_service.rb

@@ -83,11 +83,9 @@ def add_members_by_ids(collection_id:, new_member_ids:, user:)
         def add_members(collection_id:, new_members:, user:)
           messages = []
           new_members.map do |new_member|
-            begin
-              add_member(collection_id: collection_id, new_member: new_member, user: user)
-            rescue Hyrax::SingleMembershipError => err
-              messages += [err.message]
-            end
+            add_member(collection_id: collection_id, new_member: new_member, user: user)
+          rescue Hyrax::SingleMembershipError => err
+            messages += [err.message]
           end
           raise Hyrax::SingleMembershipError, messages if messages.present?
         end

app/services/hyrax/listeners/member_cleanup_listener.rb

@@ -13,15 +13,13 @@ def on_object_deleted(event)
         return if event[:object].is_a?(ActiveFedora::Base) # handled by legacy code
 
         Hyrax.custom_queries.find_child_file_sets(resource: event[:object]).each do |file_set|
-          begin
-            Hyrax.persister.delete(resource: file_set)
-            Hyrax.publisher
-                 .publish('object.deleted', object: file_set, id: file_set.id, user: event[:user])
-          rescue StandardError # we don't uncaught errors looping filesets
-            Hyrax.logger.warn "Failed to delete #{file_set.class}:#{file_set.id} " \
-                              "during cleanup for resource: #{event[:object]}. " \
-                              'This member may now be orphaned.'
-          end
+          Hyrax.persister.delete(resource: file_set)
+          Hyrax.publisher
+               .publish('object.deleted', object: file_set, id: file_set.id, user: event[:user])
+        rescue StandardError # we don't uncaught errors looping filesets
+          Hyrax.logger.warn "Failed to delete #{file_set.class}:#{file_set.id} " \
+                            "during cleanup for resource: #{event[:object]}. " \
+                            'This member may now be orphaned.'
         end
       end
 
@@ -33,15 +31,13 @@ def on_collection_deleted(event)
         return if event[:collection].is_a?(ActiveFedora::Base) # handled by legacy code
 
         Hyrax.custom_queries.find_members_of(collection: event[:collection]).each do |resource|
-          begin
-            resource.member_of_collection_ids -= [event[:collection].id]
-            Hyrax.persister.save(resource: resource)
-            Hyrax.publisher
-                 .publish('collection.membership.updated', collection: event[:collection], user: event[:user])
-          rescue StandardError
-            Hyrax.logger.warn "Failed to remove collection reference from #{work.class}:#{work.id} " \
-                              "during cleanup for collection: #{event[:collection]}. "
-          end
+          resource.member_of_collection_ids -= [event[:collection].id]
+          Hyrax.persister.save(resource: resource)
+          Hyrax.publisher
+               .publish('collection.membership.updated', collection: event[:collection], user: event[:user])
+        rescue StandardError
+          Hyrax.logger.warn "Failed to remove collection reference from #{work.class}:#{work.id} " \
+                            "during cleanup for collection: #{event[:collection]}. "
         end
       end
     end

app/services/hyrax/workflow/workflow_importer.rb

@@ -128,16 +128,14 @@ def validate!
       def call
         self.errors = []
         Array.wrap(data.fetch(:workflows)).map do |configuration|
-          begin
-            find_or_create_from(configuration: configuration)
-          rescue InvalidStateRemovalException => e
-            e.states.each do |state|
-              error = I18n.t('hyrax.workflow.load.state_error', workflow_name: state.workflow.name, state_name: state.name, entity_count: state.entities.count)
-              Rails.logger.error(error)
-              errors << error
-            end
-            Sipity::Workflow.find_by(name: configuration[:name])
+          find_or_create_from(configuration: configuration)
+        rescue InvalidStateRemovalException => e
+          e.states.each do |state|
+            error = I18n.t('hyrax.workflow.load.state_error', workflow_name: state.workflow.name, state_name: state.name, entity_count: state.entities.count)
+            Rails.logger.error(error)
+            errors << error
           end
+          Sipity::Workflow.find_by(name: configuration[:name])
         end
       end
 

app/services/hyrax/workflow/workflow_schema.rb

@@ -20,11 +20,9 @@ class Types
         include Dry::Types()
 
         Constant = Types::Class.constructor do |v|
-          begin
-            v.constantize
-          rescue NameError => _err
-            v
-          end
+          v.constantize
+        rescue NameError => _err
+          v
         end
       end
 

app/strategies/hyrax/strategies/yaml_strategy.rb

@@ -34,12 +34,10 @@ def key_exists?(feature)
 
     def yaml_file
       @yaml_file ||=
-        begin
-          if File.exist?(@config_file)
-            YAML.load_file(@config_file)
-          else
-            {}
-          end
+        if File.exist?(@config_file)
+          YAML.load_file(@config_file)
+        else
+          {}
         end
     end
   end