Fix warnings and errors from rubocop

Gemfile

@@ -86,7 +86,7 @@ group :development, :test do
   gem 'pry-byebug'
   gem 'pry-rails'
   gem 'rspec-rails'
-  gem 'rubocop'
+  gem 'rubocop', '~> 0.93'
   gem 'rubocop-rspec'
   gem 'selenium-webdriver', '~> 3'
   gem 'webmock'

Gemfile.lock

@@ -615,7 +615,6 @@ GEM
       kaminari-core (= 1.2.2)
     kaminari-core (1.2.2)
     language_list (1.2.1)
-    language_server-protocol (3.17.0.3)
     ld-patch (3.3.0)
       ebnf (~> 2.4)
       rdf (~> 3.3)
@@ -989,20 +988,20 @@ GEM
     rspec-support (3.13.2)
     rspec_junit_formatter (0.6.0)
       rspec-core (>= 2, < 4, != 2.12.0)
-    rubocop (1.69.2)
-      json (~> 2.3)
-      language_server-protocol (>= 3.17.0)
+    rubocop (0.93.1)
       parallel (~> 1.10)
-      parser (>= 3.3.0.2)
+      parser (>= 2.7.1.5)
       rainbow (>= 2.2.2, < 4.0)
-      regexp_parser (>= 2.9.3, < 3.0)
-      rubocop-ast (>= 1.36.2, < 2.0)
+      regexp_parser (>= 1.8)
+      rexml
+      rubocop-ast (>= 0.6.0)
       ruby-progressbar (~> 1.7)
-      unicode-display_width (>= 2.4.0, < 4.0)
+      unicode-display_width (>= 1.4.0, < 2.0)
     rubocop-ast (1.37.0)
       parser (>= 3.3.1.0)
-    rubocop-rspec (3.3.0)
-      rubocop (~> 1.61)
+    rubocop-rspec (1.44.1)
+      rubocop (~> 0.87)
+      rubocop-ast (>= 0.7.1)
     ruby-box (1.15.0)
       addressable
       json
@@ -1137,7 +1136,7 @@ GEM
     uber (0.1.0)
     uglifier (4.2.1)
       execjs (>= 0.3.0, < 3)
-    unicode-display_width (2.6.0)
+    unicode-display_width (1.8.0)
     unicode-types (1.10.0)
     validatable (1.6.7)
     valkyrie (3.0.3)
@@ -1268,7 +1267,7 @@ DEPENDENCIES
   rspec-mocks
   rspec-rails
   rspec_junit_formatter
-  rubocop
+  rubocop (~> 0.93)
   rubocop-rspec
   ruby-oembed
   rubyzip (~> 2)

app/controllers/concerns/oregon_digital/work_relation_pagination_behavior.rb

@@ -31,7 +31,7 @@ def search_builder
     def parent_results
       @search_builder_class = OregonDigital::ParentsOfWorkSearchBuilder
       params[:page] = params[:parent_page]
-      (@parent_response, @parent_doc_list) = search_service.search_results do |builder|
+      (@parent_response, @parent_doc_list) = search_service.search_results do
         search_builder
       end
     end
@@ -40,7 +40,7 @@ def parent_results
     def sibling_results
       @search_builder_class = OregonDigital::SiblingsOfWorkSearchBuilder
       params[:page] = params[:sibling_page]
-      (@sibling_response, @sibling_doc_list) = search_service.search_results do |builder|
+      (@sibling_response, @sibling_doc_list) = search_service.search_results do
         search_builder
       end
     end
@@ -49,7 +49,7 @@ def sibling_results
     def child_results
       @search_builder_class = OregonDigital::ChildrenOfWorkSearchBuilder
       params[:page] = params[:child_page]
-      (@child_response, @child_doc_list) = search_service.search_results do |builder|
+      (@child_response, @child_doc_list) = search_service.search_results do
         search_builder
       end
     end

app/controllers/oregon_digital/explore_collections_controller.rb

@@ -40,31 +40,31 @@ def page_title
     def all
       @tab = TABS[:all]
       blacklight_config.search_builder_class = OregonDigital::NonUserCollectionsSearchBuilder
-      (@response, @document_list) = search_service.search_results()
+      (@response, @document_list) = search_service.search_results
       build_breadcrumbs
       render :index
     end
 
     def osu
       @tab = TABS[:osu]
       blacklight_config.search_builder_class = OregonDigital::OsuCollectionsSearchBuilder
-      (@response, @document_list) = search_service.search_results()
+      (@response, @document_list) = search_service.search_results
       build_breadcrumbs
       render :index
     end
 
     def uo
       @tab = TABS[:uo]
       blacklight_config.search_builder_class = OregonDigital::UoCollectionsSearchBuilder
-      (@response, @document_list) = search_service.search_results()
+      (@response, @document_list) = search_service.search_results
       build_breadcrumbs
       render :index
     end
 
     def my
       @tab = TABS[:my]
       blacklight_config.search_builder_class = OregonDigital::MyCollectionsSearchBuilder
-      (@response, @document_list) = search_service.search_results()
+      (@response, @document_list) = search_service.search_results
       build_breadcrumbs
       render :index
     end

lib/hyrax/controlled_vocabularies/location.rb

@@ -100,7 +100,7 @@ def fetch(*_args, &_block)
           fetch_parents unless top_level_element?
           return self
         end
-        resource = super
+        resource = super()
         fetch_parents unless top_level_element?
         store_graph
         resource

spec/controllers/hyrax/homepage_controller_spec.rb

@@ -18,22 +18,14 @@
     context 'with existing featured researcher' do
       let!(:frodo) { ContentBlock.create!(name: ContentBlock::NAME_REGISTRY[:researcher], value: 'Frodo Baggins', created_at: Time.zone.now) }
 
-      it 'finds a featured researcher' do
-        get :index
-        expect(response).to be_success
-      end
       it 'finds the featured researcher' do
         get :index
+        expect(response).to be_successful
         expect(assigns(:featured_researcher)).to eq frodo
       end
     end
 
     context 'with no featured researcher' do
-      it 'responds' do
-        get :index
-        expect(response).to be_success
-      end
-
       it 'creates a featured researcher' do
         get :index
         assigns(:featured_researcher).tap do |researcher|
@@ -42,17 +34,14 @@
       end
       it 'sets featured researcher' do
         get :index
+        expect(response).to be_successful
         assigns(:featured_researcher).tap do |researcher|
+          expect(researcher).to be_kind_of ContentBlock
           expect(researcher.name).to eq 'featured_researcher'
         end
       end
     end
 
-    it 'responds' do
-      get :index
-      expect(response).to be_success
-    end
-
     it 'creates marketing text' do
       get :index
       assigns(:marketing_text).tap do |marketing|
@@ -61,13 +50,16 @@
     end
     it 'sets marketing text' do
       get :index
+      expect(response).to be_successful
       assigns(:marketing_text).tap do |marketing|
+        expect(marketing).to be_kind_of ContentBlock
         expect(marketing.name).to eq 'marketing_text'
       end
     end
 
     it 'does not include other user\'s private documents in recent documents' do
       get :index
+      expect(response).to be_successful
       titles = assigns(:recent_documents).map { |d| d['title_tesim'][0] }
       expect(titles).not_to include('Test Private Document')
     end
@@ -94,37 +86,26 @@
         gw3.save
       end
 
-      it 'responds' do
-        get :index
-        expect(response).to be_success
-      end
-
       it 'does not exceed the maximum recent documents' do
         get :index
         expect(assigns(:recent_documents).length).to be <= 6
       end
       it 'sets recent documents in the right order' do
         get :index
+        expect(response).to be_successful
+        expect(assigns(:recent_documents).length).to be <= 6
         create_times = assigns(:recent_documents).map { |d| d['date_uploaded_dtsi'] }
         expect(create_times).to eq create_times.sort.reverse
       end
     end
 
     context 'with collections' do
       let(:presenter) { double }
-      let(:repository) { double }
-      let(:collection_results) { instance_double('Blacklight::Solr::Response', documents: ['collection results']) }
+      let(:collection_results) { instance_double(Object, documents: ['collection results']) }
 
       before do
-        allow(controller).to receive(:repository).and_return(repository)
         allow(controller).to receive(:search_results).and_return([nil, ['recent document']])
-        allow(controller.repository).to receive(:search).with(an_instance_of(OregonDigital::NonUserCollectionsSearchBuilder))
-                                                        .and_return(collection_results)
-      end
-
-      it 'responds' do
-        get :index
-        expect(response).to be_success
+        allow_any_instance_of(Hyrax::CollectionsService).to receive(:search_results).and_return(collection_results.documents)
       end
 
       it 'initializes the presenter with ability and a list of collections' do
@@ -149,13 +130,9 @@
         FeaturedWork.create!(work_id: my_work.id)
       end
 
-      it 'responds' do
-        get :index
-        expect(response).to be_success
-      end
-
       it 'sets featured works' do
         get :index
+        expect(response).to be_successful
         expect(assigns(:featured_work_list)).to be_kind_of FeaturedWorkList
       end
     end
@@ -168,15 +145,16 @@
     end
     it 'sets announcement content block' do
       get :index
+      expect(response).to be_successful
       assigns(:announcement_text).tap do |announcement|
+        expect(announcement).to be_kind_of ContentBlock
         expect(announcement.name).to eq 'announcement_text'
       end
     end
 
     context 'without solr' do
       before do
-        allow(controller).to receive(:repository).and_return(instance_double(Blacklight::Solr::Repository))
-        allow(controller.repository).to receive(:search).and_raise Blacklight::Exceptions::InvalidRequest
+        allow_any_instance_of(Hyrax::SearchService).to receive(:search_results).and_raise Blacklight::Exceptions::InvalidRequest
       end
 
       it 'errors admin_sets gracefully' do

spec/factories/admin_sets.rb

@@ -23,5 +23,11 @@
       # false, true, or Hash with keys for permission_template
       with_permission_template { false }
     end
+
+    factory :complete_admin_set do
+      alternative_title { ['alternative admin set title'] }
+      creator           { %w[moomin snufkin] }
+      description       { ['Before a revolution happens', 'it is perceived as impossible'] }
+    end
   end
 end

spec/factories/permission_templates.rb

@@ -1,24 +1,23 @@
-# frozen_string_literal:true
+# frozen_string_literal: true
 
 FactoryBot.define do
   factory :permission_template, class: Hyrax::PermissionTemplate do
     # Given that there is a one to one strong relation between permission_template and admin_set,
     # with a unique index on the source_id, I don't want to have duplication in source_id
     sequence(:source_id) { |n| format('%010d', n) }
 
+    trait :with_immediate_release do
+      release_period { Hyrax::PermissionTemplate::RELEASE_TEXT_VALUE_NO_DELAY }
+    end
+
+    trait :with_delayed_release do
+      release_period { Hyrax::PermissionTemplate::RELEASE_TEXT_VALUE_6_MONTHS }
+    end
+
     before(:create) do |permission_template, evaluator|
       if evaluator.with_admin_set
         source_id = permission_template.source_id
-        admin_set =
-          if source_id.present?
-            begin
-              AdminSet.find(source_id)
-            rescue ActiveFedora::ObjectNotFoundError
-              create(:admin_set, id: source_id)
-            end
-          else
-            create(:admin_set)
-          end
+        admin_set = SourceFinder.find_or_create_admin_set(source_id)
         permission_template.source_id = admin_set.id
       elsif evaluator.with_collection
         source_id = permission_template.source_id
@@ -39,7 +38,7 @@
     after(:create) do |permission_template, evaluator|
       if evaluator.with_workflows
         Hyrax::Workflow::WorkflowImporter.load_workflow_for(permission_template: permission_template)
-        Sipity::Workflow.activate!(permission_template: permission_template, workflow_id: permission_template.available_workflows.pluck(:id).first)
+        Sipity::Workflow.activate!(permission_template: permission_template, workflow_id: permission_template.available_workflows.pick(:id))
       end
       if evaluator.with_active_workflow
         workflow = create(:workflow, active: true, permission_template: permission_template)
@@ -78,4 +77,36 @@ def self.create_access(permission_template_id, agent_type, access, agent_ids)
       end
     end
   end
+
+  class SourceFinder
+    def self.find_or_create_admin_set(source_id)
+      Hyrax.config.use_valkyrie? ? find_or_create_admin_set_valkyrie(source_id) : find_or_create_admin_set_active_fedora(source_id)
+    end
+
+    def self.find_or_create_admin_set_active_fedora(source_id)
+      if source_id.present?
+        begin
+          AdminSet.find(source_id)
+        rescue ActiveFedora::ObjectNotFoundError
+          FactoryBot.create(:admin_set, id: source_id)
+        end
+      else
+        FactoryBot.create(:admin_set)
+      end
+    end
+
+    def self.find_or_create_admin_set_valkyrie(source_id)
+      if source_id.present?
+        begin
+          Hyrax.query_service.find_by(id: source_id)
+        rescue Valkyrie::Persistence::ObjectNotFoundError
+          # Creating an Administrative set with a pre-determined id will not work for all adapters
+          # so we're letting the adapter assign the id
+          FactoryBot.valkyrie_create(:hyrax_admin_set)
+        end
+      else
+        FactoryBot.valkyrie_create(:hyrax_admin_set)
+      end
+    end
+  end
 end