Hyrax 5 upgrade rubocop fixes & get specs running in CI (#2065)

* 🧹Rubocop Fixes

* Attempt to fix circleci

.circleci/config.yml

@@ -1,7 +1,7 @@
 version: 2.1
 orbs:
   samvera: samvera/circleci-orb@1
-  browser-tools: circleci/browser-tools@1.3
+  browser-tools: circleci/browser-tools@1.4.6
   ruby: circleci/ruby@2
   node: circleci/node@5
 

.rubocop.yml

@@ -5,14 +5,17 @@ inherit_gem:
   bixby: bixby_default.yml
 
 AllCops:
-  TargetRubyVersion: 2.6
+  NewCops: disable
+  TargetRubyVersion: 3.2.2
   DisplayCopNames: true
   Exclude:
     - 'db/**/*'
     - 'script/**/*'
     - 'spec/test_app_templates/**/*'
     - 'vendor/**/*'
     - 'lib/hyrax/specs/**/*'
+    - 'bin/graph'
+    - 'bin/import_from_purl'
 
 Lint/ImplicitStringConcatenation:
   Exclude:
@@ -23,7 +26,7 @@ Rails/Output:
     - 'lib/generators/**/*'
 
 Metrics/BlockLength:
-  ExcludedMethods: ['included']
+  IgnoredMethods: ['included']
   Exclude:
     - 'hyrax.gemspec'
     - 'app/models/concerns/hyrax/content_block_behavior.rb'
@@ -65,9 +68,16 @@ Style/SingleLineBlockParams:
 Rails/ApplicationJob:
   Enabled: false
 
+Rails/ApplicationMailer:
+  Enabled: false
+
 Rails/ApplicationRecord:
   Enabled: false
 
+Rails/HasManyOrHasOneDependent:
+  Exclude:
+    - 'app/models/endpoint.rb'
+
 Rails/RakeEnvironment:
   Enabled: false
 
@@ -94,6 +104,12 @@ Rails/OutputSafety:
     - 'app/renderers/hyrax/renderers/attribute_renderer.rb'
     - 'spec/views/hyrax/my/works/_list_works.html.erb_spec.rb'
 
+Rails/UniqueValidationWithoutIndex:
+  Exclude:
+    - 'app/models/account.rb'
+    - 'app/models/domain_name.rb'
+    - 'app/models/hyrax/group.rb'
+
 RSpec/DescribeClass:
   Exclude:
     - 'spec/abilities/**/*'

Rakefile

@@ -17,7 +17,9 @@ Rails.application.load_tasks
 
 begin
   require 'solr_wrapper/rake_task'
+# rubocop:disable Lint/SuppressedException
 rescue LoadError
+  # rubocop:enable Lint/SuppressedException
 end
 
 task :ci do

app/controllers/admin/users_controller.rb

@@ -20,9 +20,9 @@ def activate
       user.password = ENV.fetch('HYKU_USER_DEFAULT_PASSWORD', 'password')
 
       if user.save && user.accept_invitation!
-        redirect_to hyrax.admin_users_path, notice: t('hyrax.admin.users.activate.success', user: user)
+        redirect_to hyrax.admin_users_path, notice: t('hyrax.admin.users.activate.success', user:)
       else
-        redirect_to hyrax.admin_users_path, flash: { error: t('hyrax.admin.users.activate.failure', user: user) }
+        redirect_to hyrax.admin_users_path, flash: { error: t('hyrax.admin.users.activate.failure', user:) }
       end
     end
 

app/controllers/application_controller.rb

@@ -29,18 +29,24 @@ class ApplicationController < ActionController::Base
 
   protected
 
+  # rubocop:disable Naming/PredicateName
   def is_hidden
+    # rubocop:enable Naming/PredicateName
     current_account.persisted? && !current_account.is_public?
   end
 
+  # rubocop:disable Naming/PredicateName
   def is_api_or_pdf
+    # rubocop:enable Naming/PredicateName
     request.format.to_s.match('json') ||
       params[:print] ||
       request.path.include?('api') ||
       request.path.include?('pdf')
   end
 
+  # rubocop:disable Naming/PredicateName
   def is_staging
+    # rubocop:enable Naming/PredicateName
     ['staging'].include?(Rails.env)
   end
 
@@ -49,10 +55,11 @@ def is_staging
   def authenticate_if_needed
     # Disable this extra authentication in test mode
     return true if Rails.env.test?
-    if (is_hidden || is_staging) && !is_api_or_pdf
-      authenticate_or_request_with_http_basic do |username, password|
-        username == "samvera" && password == "hyku"
-      end
+    # rubocop:disable Naming/PredicateName
+    return unless (is_hidden || is_staging) && !is_api_or_pdf
+    # rubocop:enable Naming/PredicateName
+    authenticate_or_request_with_http_basic do |username, password|
+      username == "samvera" && password == "hyku"
     end
   end
 

app/controllers/concerns/hyrax/works_controller_behavior.rb

@@ -184,7 +184,7 @@ def create_valkyrie_work
         transactions['change_set.create_work']
         .with_step_args(
           'work_resource.add_to_parent' => { parent_id: params[:parent_id], user: current_user },
-          'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] },
+          'work_resource.add_file_sets' => { uploaded_files:, file_set_params: params[hash_key_for_curation_concern][:file_set] },
           'change_set.set_user_as_depositor' => { user: current_user },
           'work_resource.change_depositor' => { user: ::User.find_by_user_key(form.on_behalf_of) }
         )
@@ -198,8 +198,8 @@ def update_valkyrie_work
       return after_update_error(form_err_msg(form)) unless form.validate(params[hash_key_for_curation_concern])
       result =
         transactions['change_set.update_work']
-        .with_step_args('work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] },
-                        'work_resource.update_work_members' => { work_members_attributes: work_members_attributes })
+        .with_step_args('work_resource.add_file_sets' => { uploaded_files:, file_set_params: params[hash_key_for_curation_concern][:file_set] },
+                        'work_resource.update_work_members' => { work_members_attributes: })
         .call(form)
       @curation_concern = result.value_or { return after_update_error(transaction_err_msg(result)) }
       after_update_response
@@ -350,7 +350,7 @@ def attributes_for_actor
       uploaded_files = params.fetch(:uploaded_files, [])
       selected_files = params.fetch(:selected_files, {}).values
       browse_everything_urls = uploaded_files &
-                               selected_files.map { |f| f[:url] }
+                               selected_files.pluck(:url)
 
       # we need the hash of files with url and file_name
       browse_everything_files = selected_files
@@ -389,7 +389,7 @@ def after_create_error(errors, original_input_params_for_form = nil)
           rebuild_form(original_input_params_for_form) if original_input_params_for_form.present?
           render 'new', status: :unprocessable_entity
         end
-        wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: errors }) }
+        wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: }) }
       end
     end
 
@@ -418,7 +418,7 @@ def after_update_error(errors)
           build_form unless @form.is_a? Hyrax::ChangeSet
           render 'edit', status: :unprocessable_entity
         end
-        wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: errors }) }
+        wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: }) }
       end
     end
 
@@ -500,7 +500,7 @@ def available_admin_sets
           .new(admin_set: admin_set_doc, permission_template: template, permit_sharing: sharing)
       end
 
-      AdminSetSelectionPresenter.new(admin_sets: admin_sets)
+      AdminSetSelectionPresenter.new(admin_sets:)
     end
 
     # added to prepend the show theme views into the view_paths

app/controllers/hyrax/contact_form_controller.rb

@@ -127,7 +127,7 @@ def setup_negative_captcha
         # If you wish to override the default CSS styles (position: absolute; left: -2000px;)
         # used to position the fields off-screen
         css: "display: none",
-        params: params
+        params:
       )
     end
   end

app/controllers/hyrax/dashboard/collections_controller.rb

@@ -86,9 +86,8 @@ def edit
         form
         collection_type
         # Gets original filename of an uploaded thumbnail. See #update
-        if ::SolrDocument.find(@collection.id).thumbnail_path.include? "uploaded_collection_thumbnails" && uploaded_thumbnail?
-          @thumbnail_filename = File.basename(uploaded_thumbnail_files.reject { |f| File.basename(f).include? @collection.id }.first)
-        end
+        return unless ::SolrDocument.find(@collection.id).thumbnail_path.include?("uploaded_collection_thumbnails" && uploaded_thumbnail?)
+        @thumbnail_filename = File.basename(uploaded_thumbnail_files.reject { |f| File.basename(f).include? @collection.id }.first)
       end
 
       def uploaded_thumbnail?
@@ -182,7 +181,7 @@ def after_destroy_error(id)
             flash[:notice] = t('hyrax.dashboard.my.action.collection_delete_fail')
             render :edit, status: :unprocessable_entity
           end
-          format.json { render json: { id: id }, status: :unprocessable_entity, location: dashboard_collection_path(@collection) }
+          format.json { render json: { id: }, status: :unprocessable_entity, location: dashboard_collection_path(@collection) }
         end
       end
 
@@ -212,7 +211,7 @@ def collection
       # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
       def files
         params[:q] = '' unless params[:q]
-        builder = Hyrax::CollectionMemberSearchBuilder.new(scope: self, collection: collection, search_includes_models: :works)
+        builder = Hyrax::CollectionMemberSearchBuilder.new(scope: self, collection:, search_includes_models: :works)
         # get the default work image because we do not want to show any works in this
         # dropdown that only have the default work image. this indicates that they have
         # no files attached, and will throw an error if selected.
@@ -223,7 +222,9 @@ def files
         result = response.documents.reject do |document|
                    document["thumbnail_path_ss"].blank? || document["thumbnail_path_ss"].include?(default_work_thumbnail_path) ||
                      document["thumbnail_path_ss"].include?(work_with_no_files_thumbnail_path)
+                   # rubocop:disable Style/MultilineBlockChain
                  end.map do |document|
+          # rubocop:enable Style/MultilineBlockChain
           { id: document["thumbnail_path_ss"].split('/').last.gsub(/\?.*/, ''), text: document["title_tesim"].first }
         end
         reset_thumbnail_option = {
@@ -497,7 +498,7 @@ def extract_old_style_permission_attributes(attributes)
         participants = []
         permissions.each do |p|
           access = access(p)
-          participants << { agent_type: agent_type(p), agent_id: p["name"], access: access } if access
+          participants << { agent_type: agent_type(p), agent_id: p["name"], access: } if access
         end
         participants
       end
@@ -526,7 +527,7 @@ def add_members_to_collection(collection = nil, collection_id: nil)
         collection_id ||= (collection.try(:id) || @collection.id)
 
         Hyrax::Collections::CollectionMemberService
-          .add_members_by_ids(collection_id: collection_id,
+          .add_members_by_ids(collection_id:,
                               new_member_ids: batch,
                               user: current_user)
       end
@@ -595,7 +596,7 @@ def query_collection_members
 
       # Instantiate the membership query service
       def collection_member_service
-        @collection_member_service ||= membership_service_class.new(scope: self, collection: collection, params: params_for_query)
+        @collection_member_service ||= membership_service_class.new(scope: self, collection:, params: params_for_query)
       end
 
       def member_works
@@ -614,7 +615,7 @@ def member_subcollections
       def parent_collections
         page = params[:parent_collection_page].to_i
         query = Hyrax::Collections::NestedCollectionQueryService
-        collection.parent_collections = query.parent_collections(child: collection_object, scope: self, page: page)
+        collection.parent_collections = query.parent_collections(child: collection_object, scope: self, page:)
       end
 
       def collection_object
@@ -675,7 +676,7 @@ def after_create_errors(errors) # for valkyrie
             render 'new', status: :unprocessable_entity
           end
           wants.json do
-            render_json_response(response_type: :unprocessable_entity, options: { errors: errors })
+            render_json_response(response_type: :unprocessable_entity, options: { errors: })
           end
         end
       end
@@ -705,7 +706,7 @@ def after_update_errors(errors) # for valkyrie
             flash[:error] = errors.to_s
             render 'edit', status: :unprocessable_entity
           end
-          wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: errors }) }
+          wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: }) }
         end
       end
 

app/controllers/hyrax/homepage_controller.rb

@@ -106,7 +106,7 @@ def search_action_url(options = {})
 
     # Return 6 collections, sorts by title
     def collections(rows: 6)
-    (response, documents) = Hyrax::CollectionsService.new(self).search_results do |builder|
+      Hyrax::CollectionsService.new(self).search_results do |builder|
         builder.rows(rows)
         builder.merge(sort: "title_ssi")
       end
@@ -150,13 +150,14 @@ def inject_theme_views
     end
 
     # add this method to vary blacklight config and user_params
-    def search_service(*args)
+    def search_service(*_args)
       Hyrax::SearchService.new(
-        config: ::CatalogController.new.blacklight_config, 
-        user_params: params.except(:q, :page), 
+        config: ::CatalogController.new.blacklight_config,
+        user_params: params.except(:q, :page),
         scope: self,
-        current_ability: current_ability,
-        search_builder_class: search_builder_class)
+        current_ability:,
+        search_builder_class:
+      )
     end
   end
 end

app/forms/hyrax/forms/admin/appearance.rb

@@ -226,7 +226,9 @@ def primary_button_focus_border_color
         # The custom css module
         def custom_css_block
           # we want to be able to read the css
+          # rubocop:disable Rails/OutputSafety
           block_for('custom_css_block', '/* custom stylesheet */').html_safe
+          # rubocop:enable Rails/OutputSafety
         end
 
         # DEFAULT BUTTON COLORS
@@ -445,21 +447,23 @@ def default_values
         end
 
         def block_for(name, dynamic_default = nil)
-          ContentBlock.block_for(name: name, fallback_value: default_values[name] || dynamic_default)
+          ContentBlock.block_for(name:, fallback_value: default_values[name] || dynamic_default)
         end
 
         # Persist a key/value tuple as a ContentBlock
         # @param [Symbol] name the identifier for the ContentBlock
         # @param [String] value the value to set
         def update_block(name, value)
-          ContentBlock.update_block(name: name, value: value)
+          ContentBlock.update_block(name:, value:)
         end
 
         def format_font_names(font_style)
           # the fonts come with `Font Name:font-weight` - this removes the weight
           parts = font_style.split(':')
           # Google fonts use `+` in place of spaces. This fixes it for CSS.
+          # rubocop:disable Rails/OutputSafety
           parts[0].tr('+', ' ').html_safe
+          # rubocop:enable Rails/OutputSafety
         end
       end
     end

app/helpers/admin_stats_helper.rb

@@ -5,7 +5,7 @@ def graph_tag(id, data, options)
     content_tag :div, class: 'graph-container',
                       data: { graph_data: data.to_json,
                               graph_options: options } do
-      content_tag :div, nil, class: 'graph', id: id
+      content_tag :div, nil, class: 'graph', id:
     end
   end
 end

app/helpers/application_helper.rb

@@ -8,7 +8,7 @@ module ApplicationHelper
   include HykuKnapsack::ApplicationHelper
 
   def hint_for(term:, record_class: nil)
-    hint = locale_for(type: 'hints', term: term, record_class: record_class)
+    hint = locale_for(type: 'hints', term:, record_class:)
 
     return hint unless missing_translation(hint)
   end

app/helpers/google_tag_manager_helper.rb

@@ -5,7 +5,7 @@ def render_gtm_head(_host)
     return '' if current_account.gtm_id.blank?
 
     # rubocop:disable Rails/OutputSafety
-    <<-HTML.strip_heredoc.html_safe
+    <<~HTML.html_safe
       <!-- Google Tag Manager -->
       <script>
        (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
@@ -23,7 +23,7 @@ def render_gtm_body(_host)
     # render 'layouts/google/gtm_body'
     return '' if current_account.gtm_id.blank?
     # rubocop:disable Rails/OutputSafety
-    <<-HTML.strip_heredoc.html_safe
+    <<~HTML.html_safe
        <!-- Google Tag Manager (noscript) -->
 
       <noscript><iframe src='https://www.googletagmanager.com/ns.html?id="#{current_account.gtm_id}"'

app/helpers/group_navigation_helper.rb

@@ -2,6 +2,6 @@
 
 module GroupNavigationHelper
   def navigation_presenter
-    @navigation_presenter ||= Hyku::Admin::Group::NavigationPresenter.new(params: params)
+    @navigation_presenter ||= Hyku::Admin::Group::NavigationPresenter.new(params:)
   end
 end