Further cleanup and removal of Nested collection indexing

app/forms/hyrax/forms/dashboard/nest_collection_form.rb

@@ -38,7 +38,6 @@ def initialize(parent: nil,
         validates :parent, presence: true
         validates :child, presence: true
         validate :parent_and_child_can_be_nested
-        validate :nesting_within_maximum_depth
 
         def save
           return false unless valid?
@@ -83,12 +82,11 @@ def available_parent_collections
         # rerouting to new_dashboard_collection_path to add the new collection as
         # a child. Since we don't yet have a child collection, the valid? option can't be used here.
         def validate_add
-          if nestable?(parent)
-            nesting_within_maximum_depth
-          else
+          unless nestable?(parent)
             errors.add(:parent, :cannot_have_child_nested)
-            false
+            return false
           end
+          true
         end
 
         def remove
@@ -104,17 +102,6 @@ def remove
 
         attr_accessor :query_service, :persistence_service, :context, :collection
 
-        # ideally we would love to be able to eliminate collections which exceed the
-        # maximum nesting depth from the lists of available collections, but the queries
-        # needed to make the determination are too expensive to do for every possible
-        # collection, so we only test for this situation prior to saving the new
-        # relationship.
-        def nesting_within_maximum_depth
-          return true if query_service.valid_combined_nesting_depth?(parent: parent, child: child, scope: context)
-          errors.add(:collection, :exceeds_maximum_nesting_depth)
-          false
-        end
-
         def parent_and_child_can_be_nested
           if nestable?(parent) && nestable?(child)
             return true if query_service.parent_and_child_can_nest?(parent: parent, child: child, scope: context)

app/search_builders/hyrax/dashboard/nested_collections_search_builder.rb

@@ -6,16 +6,11 @@ class NestedCollectionsSearchBuilder < ::Hyrax::CollectionSearchBuilder
       # @param access [Symbol] :edit, :read, :discover - With the given :access what all can
       # @param collection [::Collection]
       # @param scope [Object] Typically a controller that responds to #current_ability, #blackligh_config
-      # @param nesting_attributes - no longer used
-      # @param nest_direction [Symbol] (:as_parent or :as_child) the direction we are adding nesting to this collection
-      # rubocop:disable Lint/UnusedMethodArgument
-      def initialize(access:, collection:, scope:, nesting_attributes:, nest_direction:)
+      def initialize(access:, collection:, scope:)
         super(scope)
         @collection = collection
         @discovery_permissions = extract_discovery_permissions(access)
-        @nest_direction = nest_direction
       end
-      # rubocop:enable Lint/UnusedMethodArgument
 
       # Override for Hydra::AccessControlsEnforcement
       attr_reader :discovery_permissions

app/services/hyrax/collections/nested_collection_query_service.rb

@@ -16,7 +16,7 @@ module NestedCollectionQueryService
       def self.available_child_collections(parent:, scope:, limit_to_id: nil)
         return [] unless nestable?(collection: parent)
         return [] unless scope.can?(:deposit, parent)
-        query_solr(collection: parent, access: :read, scope: scope, limit_to_id: limit_to_id, nest_direction: :as_child).documents
+        query_solr(collection: parent, access: :read, scope: scope, limit_to_id: limit_to_id).documents
       end
 
       ##
@@ -34,7 +34,7 @@ def self.available_child_collections(parent:, scope:, limit_to_id: nil)
       def self.available_parent_collections(child:, scope:, limit_to_id: nil)
         return [] unless nestable?(collection: child)
         return [] unless scope.can?(:read, child)
-        query_solr(collection: child, access: :deposit, scope: scope, limit_to_id: limit_to_id, nest_direction: :as_parent).documents
+        query_solr(collection: child, access: :deposit, scope: scope, limit_to_id: limit_to_id).documents
       end
 
       ##
@@ -63,14 +63,11 @@ def self.parent_collections(child:, scope:, page: 1)
       #   to +repository+, +can?+, +blacklight_config+, +current_ability+
       # @param limit_to_id [nil, String] Limit the query to just check if the given
       #   id is in the response. Useful for validation.
-      # @param nest_direction [Symbol] :as_child or :as_parent
-      def self.query_solr(collection:, access:, scope:, limit_to_id:, nest_direction:)
+      def self.query_solr(collection:, access:, scope:, limit_to_id:)
         query_builder = Hyrax::Dashboard::NestedCollectionsSearchBuilder.new(
           access: access,
           collection: collection,
-          scope: scope,
-          nesting_attributes: nil,
-          nest_direction: nest_direction
+          scope: scope
         )
 
         query_builder.where(id: limit_to_id.to_s) if limit_to_id
@@ -103,21 +100,6 @@ def self.parent_and_child_can_nest?(parent:, child:, scope:)
         true
       end
 
-      # @api public
-      #
-      # Does the nesting depth fall within defined limit?
-      #
-      # @param parent [::Collection]
-      # @param child [nil, ::Collection] will be nil if we are nesting a new
-      #   collection under the parent
-      # @param scope [Object] Typically a controller object that responds
-      #   to +repository+, +can?+, +blacklight_config+, +current_ability+
-      #
-      # @return [Boolean] true Always return true because we are no longer limited
-      def self.valid_combined_nesting_depth?(*)
-        true
-      end
-
       # @api private
       #
       # @param collection [Hyrax::PcdmCollection,::Collection]

lib/hyrax/configuration.rb

@@ -64,7 +64,6 @@ def initialize
       @registered_concerns = []
       @role_registry = Hyrax::RoleRegistry.new
       @default_active_workflow_name = DEFAULT_ACTIVE_WORKFLOW_NAME
-      @nested_relationship_reindexer = default_nested_relationship_reindexer
     end
 
     DEFAULT_ACTIVE_WORKFLOW_NAME = 'default'
@@ -842,14 +841,6 @@ def uploader
       @uploader ||= default_uploader_config
     end
 
-    attr_accessor :nested_relationship_reindexer
-
-    # Deprecated
-    # Now a no-op
-    def default_nested_relationship_reindexer
-      ->(id:, extent:) {}
-    end
-
     attr_writer :solr_select_path
     def solr_select_path
       @solr_select_path ||= ActiveFedora.solr_config.fetch(:select_path, 'select')

spec/forms/hyrax/forms/dashboard/nest_collection_form_spec.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 RSpec.describe Hyrax::Forms::Dashboard::NestCollectionForm, type: :form do
-  let(:nesting_depth_result) { true }
   let(:context) { double('Context', current_user: user) }
   let(:user)    { create(:user) }
 
@@ -10,8 +9,7 @@
   end
 
   let(:query_service) do
-    double(Hyrax::Collections::NestedCollectionQueryService,
-           valid_combined_nesting_depth?: nesting_depth_result)
+    double(Hyrax::Collections::NestedCollectionQueryService)
   end
 
   describe '.default_query_service' do
@@ -20,7 +18,6 @@
     it { is_expected.to respond_to(:available_parent_collections) }
     it { is_expected.to respond_to(:available_child_collections) }
     it { is_expected.to respond_to(:parent_and_child_can_nest?) }
-    it { is_expected.to respond_to(:valid_combined_nesting_depth?) }
   end
 
   describe '.default_persistence_service' do
@@ -45,16 +42,13 @@
     it { is_expected.to validate_presence_of(:child) }
 
     context 'parent and child nesting' do
-      let(:nesting_depth_result) { false }
-
       it 'is invalid if child cannot be nested within the parent' do
         expect(query_service).to receive(:parent_and_child_can_nest?).with(parent: parent, child: child, scope: context).and_return(false)
 
         expect { form.valid? }
           .to change { form.errors.to_hash }
           .to include parent: ["cannot have child nested within it"],
-                      child: ["cannot nest within parent"],
-                      collection: ["nesting exceeds the allowed maximum nesting depth."]
+                      child: ["cannot nest within parent"]
       end
     end
 
@@ -136,23 +130,10 @@
       end
 
       context 'when nestable' do
-        context 'when at maximum nesting level' do
-          let(:parent) { double(nestable?: true) }
-          let(:nesting_depth_result) { false }
-
-          it 'validates the parent cannot have additional files nested' do
-            expect { form.validate_add }
-              .to change { form.errors.to_hash }
-              .to include collection: ["nesting exceeds the allowed maximum nesting depth."]
-          end
-        end
-
-        context 'when valid' do
-          let(:parent) { double(nestable?: true) }
+        let(:parent) { double(nestable?: true) }
 
-          it 'validates the parent can contain nested subcollections' do
-            expect(form.validate_add).to eq true
-          end
+        it 'validates the parent can contain nested subcollections' do
+          expect(form.validate_add).to eq true
         end
       end
     end
@@ -220,16 +201,13 @@
     it { is_expected.to validate_presence_of(:child) }
 
     context 'parent and child nesting' do
-      let(:nesting_depth_result) { false }
-
       it 'is invalid if child cannot be nested within the parent' do
         expect(query_service).to receive(:parent_and_child_can_nest?).with(parent: parent, child: child, scope: context).and_return(false)
 
         expect { form.valid? }
           .to change { form.errors.to_hash }
           .to include parent: ["cannot have child nested within it"],
-                      child: ["cannot nest within parent"],
-                      collection: ["nesting exceeds the allowed maximum nesting depth."]
+                      child: ["cannot nest within parent"]
       end
     end
 
@@ -311,23 +289,10 @@
       end
 
       context 'when nestable' do
-        context 'when at maximum nesting level' do
-          let(:parent_nestable) { true }
-          let(:nesting_depth_result) { false }
-
-          it 'validates the parent cannot have additional files nested' do
-            expect { form.validate_add }
-              .to change { form.errors.to_hash }
-              .to include collection: ["nesting exceeds the allowed maximum nesting depth."]
-          end
-        end
-
-        context 'when valid' do
-          let(:parent_nestable) { true }
+        let(:parent_nestable) { true }
 
-          it 'validates the parent can contain nested subcollections' do
-            expect(form.validate_add).to eq true
-          end
+        it 'validates the parent can contain nested subcollections' do
+          expect(form.validate_add).to eq true
         end
       end
     end

spec/lib/hyrax/configuration_spec.rb

@@ -41,7 +41,6 @@
   it { is_expected.to respond_to(:collection_model=) }
   it { is_expected.to respond_to(:contact_email) }
   it { is_expected.to respond_to(:default_admin_set_id) }
-  it { is_expected.to respond_to(:default_nested_relationship_reindexer) }
   it { is_expected.to respond_to(:derivative_services) }
   it { is_expected.to respond_to(:derivative_services=) }
   it { is_expected.to respond_to(:display_media_download_link=) }
@@ -75,7 +74,6 @@
   it { is_expected.to respond_to(:max_days_between_fixity_checks) }
   it { is_expected.to respond_to(:max_days_between_fixity_checks=) }
   it { is_expected.to respond_to(:max_notifications_for_dashboard) }
-  it { is_expected.to respond_to(:nested_relationship_reindexer) }
   it { is_expected.to respond_to(:owner_permission_levels) }
   it { is_expected.to respond_to(:permission_levels) }
   it { is_expected.to respond_to(:permission_options) }

spec/search_builders/hyrax/dashboard/nested_collections_search_builder_spec.rb

@@ -9,14 +9,7 @@
   [false, true].each do |test_valkyrie|
     context "when test_valkyrie is #{test_valkyrie}" do
       let(:builder) do
-        described_class.new(scope: scope, access: access, collection: collection,
-                            nesting_attributes: nesting_attributes, nest_direction: test_nest_direction)
-      end
-      let(:nesting_attributes) do
-        double(parents: ['Parent_1', 'Parent_2'],
-               pathnames: ["Parent_1/#{collection_id}", "Parent_2/#{collection_id}"],
-               ancestors: ['Parent_1', 'Parent_2'],
-               depth: 2)
+        described_class.new(scope: scope, access: access, collection: collection)
       end
       let(:collection_id) { collection.id.to_s }
 
@@ -52,30 +45,13 @@
 
         subject { builder.show_only_other_collections_of_the_same_collection_type(solr_params) }
 
-        context 'when nesting :as_parent' do
-          let(:test_nest_direction) { :as_parent }
-
-          it 'will exclude the given collection and its parents' do
-            subject
-            expect(solr_params.fetch(:fq)).to contain_exactly(
-              "_query_:\"{!field f=collection_type_gid_ssim}#{collection.collection_type_gid}\"",
-              "-{!graph to=id from=member_of_collection_ids_ssim}id:#{collection.id}",
-              "-{!graph from=id to=member_of_collection_ids_ssim}id:#{collection.id}"
-            )
-          end
-        end
-
-        context 'when nesting :as_child' do
-          let(:test_nest_direction) { :as_child }
-
-          it 'will build the search for valid children' do
-            subject
-            expect(solr_params.fetch(:fq)).to contain_exactly(
-              "_query_:\"{!field f=collection_type_gid_ssim}#{collection.collection_type_gid}\"",
-              "-{!graph to=id from=member_of_collection_ids_ssim}id:#{collection.id}",
-              "-{!graph from=id to=member_of_collection_ids_ssim}id:#{collection.id}"
-            )
-          end
+        it 'will exclude the given collection and its parents and children' do
+          subject
+          expect(solr_params.fetch(:fq)).to contain_exactly(
+            "_query_:\"{!field f=collection_type_gid_ssim}#{collection.collection_type_gid}\"",
+            "-{!graph to=id from=member_of_collection_ids_ssim}id:#{collection.id}",
+            "-{!graph from=id to=member_of_collection_ids_ssim}id:#{collection.id}"
+          )
         end
       end
 

spec/services/hyrax/collections/nested_collection_query_service_spec.rb

@@ -80,7 +80,7 @@
         describe 'it prevents circular nesting' do
           it 'returns an array of valid collections of the same collection type' do
             expect(scope).to receive(:can?).with(:deposit, coll_c).and_return(true)
-            expect(described_class).to receive(:query_solr).with(collection: coll_c, access: :read, scope: scope, limit_to_id: nil, nest_direction: :as_child).and_call_original
+            expect(described_class).to receive(:query_solr).with(collection: coll_c, access: :read, scope: scope, limit_to_id: nil).and_call_original
             expect(subject.map(&:id)).to contain_exactly(another.id)
           end
         end
@@ -175,7 +175,7 @@
         describe 'it prevents circular nesting' do
           it 'returns an array of collections of the same collection type excluding the given collection' do
             expect(scope).to receive(:can?).with(:read, coll_c).and_return(true)
-            expect(described_class).to receive(:query_solr).with(collection: coll_c, access: :deposit, scope: scope, limit_to_id: nil, nest_direction: :as_parent).and_call_original
+            expect(described_class).to receive(:query_solr).with(collection: coll_c, access: :deposit, scope: scope, limit_to_id: nil).and_call_original
             expect(subject.map(&:id)).to contain_exactly(another.id)
           end
         end
@@ -260,27 +260,4 @@
       it { is_expected.to eq(false) }
     end
   end
-
-  describe '.valid_combined_nesting_depth?' do
-    subject { described_class.valid_combined_nesting_depth?(parent: parent, child: child, scope: scope) }
-
-    context 'when total depth > limit' do
-      let(:parent) { coll_e }
-      let(:child) { another }
-
-      # Limit no longer applies so it will always return true
-      it 'returns true' do
-        expect(subject).to eq true
-      end
-    end
-
-    context 'when valid combined depth' do
-      let(:parent) { coll_c }
-      let(:child) { coll_e }
-
-      it 'returns true' do
-        expect(subject).to eq true
-      end
-    end
-  end
 end