Merge pull request #2382 from samvera/filter_query

Remove source_type column

app/actors/hyrax/actors/apply_permission_template_actor.rb

@@ -19,14 +19,14 @@ def add_edit_users(env)
 
         def add_admin_set_participants(env)
           return if env.attributes[:admin_set_id].blank?
-          template = Hyrax::PermissionTemplate.find_by!(source_id: env.attributes[:admin_set_id], source_type: 'admin_set')
+          template = Hyrax::PermissionTemplate.find_by!(source_id: env.attributes[:admin_set_id])
           set_curation_concern_access(env, template)
         end
 
         def add_collection_participants(env)
           return if env.attributes[:collection_id].blank?
           collection_id = env.attributes.delete(:collection_id) # delete collection_id from attributes because works do not have a collection_id property
-          template = Hyrax::PermissionTemplate.find_by!(source_id: collection_id, source_type: 'collection')
+          template = Hyrax::PermissionTemplate.find_by!(source_id: collection_id)
           set_curation_concern_access(env, template)
         end
 

app/actors/hyrax/actors/default_admin_set_actor.rb

@@ -41,7 +41,7 @@ def default_admin_set_id
 
         # Creates a Hyrax::PermissionTemplate for the given AdminSet
         def create_permission_template!(source_id:)
-          Hyrax::PermissionTemplate.create!(source_id: source_id, source_type: 'admin_set')
+          Hyrax::PermissionTemplate.create!(source_id: source_id)
         end
     end
   end

app/controllers/hyrax/admin/permission_template_accesses_controller.rb

@@ -34,30 +34,34 @@ def update_access(manage_changed:)
         end
 
         def after_destroy_success
-          if source_type == 'admin_set'
+          if source.admin_set?
             redirect_to hyrax.edit_admin_admin_set_path(source_id,
                                                         anchor: 'participants'),
                         notice: translate('participants', scope: 'hyrax.admin.admin_sets.form.permission_update_notices')
-          elsif source_type == 'collection'
+          elsif source.collection?
             redirect_to hyrax.edit_dashboard_collection_path(source_id,
                                                              anchor: 'sharing'),
                         notice: translate('sharing', scope: 'hyrax.dashboard.collections.form.permission_update_notices')
           end
         end
 
         def after_destroy_error
-          if source_type == 'admin_set'
+          if source.admin_set?
             redirect_to hyrax.edit_admin_admin_set_path(source_id,
                                                         anchor: 'participants'),
                         alert: @permission_template_access.errors.full_messages.to_sentence
-          elsif source_type == 'collection'
+          elsif source.collection?
             redirect_to hyrax.edit_dashboard_collection_path(source_id,
                                                              anchor: 'sharing'),
                         alert: @permission_template_access.errors.full_messages.to_sentence
           end
         end
 
-        delegate :source_type, :source_id, to: :permission_template
+        delegate :source_id, to: :permission_template
+
+        def source
+          @source ||= ::SolrDocument.find(source_id)
+        end
 
         def remove_access!
           permission_template_form.remove_access!(@permission_template_access)

app/models/concerns/hyrax/ability.rb

@@ -69,7 +69,12 @@ def admin?
 
       # @return [Boolean] true if the user has at least one admin set they can deposit into.
       def admin_set_with_deposit?
-        Hyrax::Collections::PermissionsService.admin_set_ids_for_user(access: ['deposit', 'manage'], ability: self).any?
+        ids = PermissionTemplateAccess.for_user(ability: self,
+                                                access: ['deposit', 'manage'])
+                                      .joins(:permission_template)
+                                      .pluck('DISTINCT source_id')
+        query = "_query_:\"{!raw f=has_model_ssim}AdminSet\" AND {!terms f=id}#{ids.join(',')}"
+        ActiveFedora::SolrService.count(query) > 0
       end
 
       # This overrides hydra-head, (and restores the method from blacklight-access-controls)

app/search_builders/hyrax/admin_set_search_builder.rb

@@ -35,8 +35,12 @@ def gated_discovery_filters
 
     private
 
+      # IDs of admin_sets into which a user can deposit.
+      #
+      # @return [Array<String>] IDs of admin_sets into which the user can deposit
+      # @note Several checks get the user's groups from the user's ability.  The same values can be retrieved directly from a passed in ability.
       def admin_set_ids_for_deposit
-        Hyrax::Collections::PermissionsService.admin_set_ids_for_deposit(ability: current_ability)
+        Hyrax::Collections::PermissionsService.source_ids_for_deposit(ability: current_ability)
       end
   end
 end

app/search_builders/hyrax/dashboard/collections_search_builder.rb

@@ -24,7 +24,7 @@ def show_only_managed_collections_for_non_admins(solr_parameters)
         solr_parameters[:fq] += ["(#{clauses.join(' OR ')})"]
       end
 
-      # Include all admin sets the user has deposit permission for.
+      # Include all admin sets and collections the user has deposit permission for.
       # @return [Array{String}] values are lucence syntax term queries suitable for :fq
       def apply_collection_deposit_permissions(_permission_types, _ability = current_ability)
         collection_ids = collection_ids_for_deposit
@@ -35,7 +35,7 @@ def apply_collection_deposit_permissions(_permission_types, _ability = current_a
       private
 
         def collection_ids_for_deposit
-          Hyrax::Collections::PermissionsService.collection_ids_for_deposit(ability: current_ability)
+          Hyrax::Collections::PermissionsService.source_ids_for_deposit(ability: current_ability)
         end
     end
   end

app/services/hyrax/admin_set_create_service.rb

@@ -83,7 +83,7 @@ def admin_group_name
       end
 
       def create_permission_template
-        permission_template = PermissionTemplate.create!(source_id: admin_set.id, source_type: 'admin_set', access_grants_attributes: access_grants_attributes)
+        permission_template = PermissionTemplate.create!(source_id: admin_set.id, access_grants_attributes: access_grants_attributes)
         admin_set.reset_access_controls!
         permission_template
       end

app/services/hyrax/collections/permissions_create_service.rb

@@ -12,7 +12,7 @@ class PermissionsCreateService
       def self.create_default(collection:, creating_user:, grants: [])
         collection_type = Hyrax::CollectionType.find_by_gid!(collection.collection_type_gid)
         access_grants = access_grants_attributes(collection_type: collection_type, creating_user: creating_user, grants: grants)
-        PermissionTemplate.create!(source_id: collection.id, source_type: 'collection',
+        PermissionTemplate.create!(source_id: collection.id,
                                    access_grants_attributes: access_grants.uniq)
         collection.reset_access_controls!
       end

app/services/hyrax/collections/permissions_service.rb

@@ -12,22 +12,39 @@ class PermissionsService
       def self.source_ids_for_user(access:, ability:, source_type: nil)
         scope = PermissionTemplateAccess.for_user(ability: ability, access: access)
                                         .joins(:permission_template)
-        scope = scope.where(permission_templates: { source_type: source_type }) if source_type
-        scope.pluck('DISTINCT source_id')
+        ids = scope.pluck('DISTINCT source_id')
+        return ids unless source_type
+        filter_source(source_type: source_type, ids: ids)
       end
       private_class_method :source_ids_for_user
 
-      # @api public
+      def self.filter_source(source_type:, ids:)
+        return [] if ids.empty?
+        id_clause = "{!terms f=id}#{ids.join(',')}"
+        query = case source_type
+                when 'admin_set'
+                  "_query_:\"{!raw f=has_model_ssim}AdminSet\""
+                when 'collection'
+                  "_query_:\"{!raw f=has_model_ssim}Collection\""
+                end
+        query += " AND #{id_clause}"
+        ActiveFedora::SolrService.query(query, fl: 'id').map { |hit| hit['id'] }
+      end
+      private_class_method :filter_source
+
+      # @api private
       #
       # IDs of admin sets a user can access based on participant roles.
       #
       # @param access [Array<String>] one or more types of access (e.g. Hyrax::PermissionTemplateAccess::MANAGE, Hyrax::PermissionTemplateAccess::DEPOSIT, Hyrax::PermissionTemplateAccess::VIEW)
       # @param ability [Ability] the ability coming from cancan ability check
       # @return [Array<String>] IDs of admin sets for which the user has specified roles
       # @note Several checks get the user's groups from the user's ability.  The same values can be retrieved directly from a passed in ability.
+      # TODO: MOVE TO ABILITY
       def self.admin_set_ids_for_user(access:, ability:)
         source_ids_for_user(access: access, ability: ability, source_type: 'admin_set')
       end
+      private_class_method :admin_set_ids_for_user
 
       # @api public
       #
@@ -67,17 +84,6 @@ def self.source_ids_for_manage(ability:, source_type: nil)
         source_ids_for_user(access: access, ability: ability, source_type: source_type)
       end
 
-      # @api public
-      #
-      # IDs of admin_sets into which a user can deposit.
-      #
-      # @param ability [Ability] the ability coming from cancan ability check
-      # @return [Array<String>] IDs of admin_sets into which the user can deposit
-      # @note Several checks get the user's groups from the user's ability.  The same values can be retrieved directly from a passed in ability.
-      def self.admin_set_ids_for_deposit(ability:)
-        source_ids_for_deposit(ability: ability, source_type: 'admin_set')
-      end
-
       # @api public
       #
       # IDs of collections into which a user can deposit.
@@ -109,6 +115,7 @@ def self.can_view_admin_show_for_any_collection?(ability:)
       # @param ability [Ability] the ability coming from cancan ability check
       # @return [Boolean] true if the user has permission to view the admin show page for at least one admin_set
       # @note Several checks get the user's groups from the user's ability.  The same values can be retrieved directly from a passed in ability.
+      # TODO: MOVE TO ABILITY
       def self.can_view_admin_show_for_any_admin_set?(ability:)
         admin_set_ids_for_user(ability: ability, access: [Hyrax::PermissionTemplateAccess::MANAGE,
                                                           Hyrax::PermissionTemplateAccess::DEPOSIT,
@@ -133,6 +140,8 @@ def self.can_manage_any_collection?(ability:)
       # @param ability [Ability] the ability coming from cancan ability check
       # @return [Boolean] true if the user has permission to manage at least one admin_set
       # @note Several checks get the user's groups from the user's ability.  The same values can be retrieved directly from a passed in ability.
+      # TODO: MOVE TO ABILITY
+
       def self.can_manage_any_admin_set?(ability:)
         admin_set_ids_for_user(ability: ability, access: [Hyrax::PermissionTemplateAccess::MANAGE]).present?
       end

lib/generators/hyrax/templates/db/migrate/20170821152307_rename_admin_set_id_to_source_id.rb.erb

@@ -0,0 +1,5 @@
+class RenameAdminSetIdToSourceId < ActiveRecord::Migration<%= migration_version %>
+  def change
+    rename_column :permission_templates, :admin_set_id, :source_id
+  end
+end

spec/abilities/ability_spec.rb

@@ -33,7 +33,7 @@
     end
 
     context "when user can deposit into an admin set" do
-      let(:permission_template) { create(:permission_template, source_type: 'admin_set') }
+      let(:permission_template) { create(:permission_template, with_admin_set: true) }
 
       before do
         # Grant the user access to deposit into an admin set.

spec/controllers/hyrax/admin/permission_template_accesses_controller_spec.rb

@@ -29,7 +29,7 @@
       end
 
       context 'when source is an admin set' do
-        let(:permission_template) { create(:permission_template, source_id: admin_set.id, source_type: 'admin_set') }
+        let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
         let(:admin_set) { create(:admin_set, edit_users: ['Liz']) }
 
         context 'when deleting the admin users group' do
@@ -64,7 +64,7 @@
       end
 
       context 'when source is a collection' do
-        let(:permission_template) { create(:permission_template, source_id: collection.id, source_type: 'collection') }
+        let(:permission_template) { create(:permission_template, source_id: collection.id) }
         let(:collection) { create(:collection, edit_users: ['Liz']) }
 
         context 'when deleting the admin users group' do

spec/factories/admin_sets.rb

@@ -9,7 +9,7 @@
     # This way, we can go ahead
     after(:create) do |admin_set, evaluator|
       if evaluator.with_permission_template
-        attributes = { source_id: admin_set.id, source_type: 'admin_set' }
+        attributes = { source_id: admin_set.id }
         attributes = evaluator.with_permission_template.merge(attributes) if evaluator.with_permission_template.respond_to?(:merge)
         # There is a unique constraint on permission_templates.source_id; I don't want to
         # create a permission template if one already exists for this admin_set

spec/factories/collections_factory.rb

@@ -25,7 +25,7 @@
       # permissions).  Nested indexing requires that the user's permissions be saved on the Fedora object... if simply in
       # local memory, they are lost when the adapter pulls the object from Fedora to reindex.
       if evaluator.with_permission_template || evaluator.create_access || RSpec.current_example.metadata[:with_nested_reindexing]
-        attributes = { source_id: collection.id, source_type: 'collection' }
+        attributes = { source_id: collection.id }
         attributes[:manage_users] = CollectionFactoryHelper.user_managers(evaluator.with_permission_template, evaluator.user,
                                                                           (evaluator.create_access || RSpec.current_example.metadata[:with_nested_reindexing]))
         attributes = evaluator.with_permission_template.merge(attributes) if evaluator.with_permission_template.respond_to?(:merge)
@@ -85,7 +85,7 @@
       collection.apply_depositor_metadata(evaluator.user.user_key)
       collection.save(validate: false) if evaluator.do_save || evaluator.with_permission_template
       if evaluator.with_permission_template
-        attributes = { source_id: collection.id, source_type: 'collection' }
+        attributes = { source_id: collection.id }
         attributes[:manage_users] = [evaluator.user] if evaluator.create_access
         attributes = evaluator.with_permission_template.merge(attributes) if evaluator.with_permission_template.respond_to?(:merge)
         create(:permission_template, attributes) unless Hyrax::PermissionTemplate.find_by(source_id: collection.id)

spec/factories/permission_templates.rb

@@ -18,7 +18,6 @@
             create(:admin_set)
           end
         permission_template.source_id = admin_set.id
-        permission_template.source_type = 'admin_set'
       elsif evaluator.with_collection
         source_id = permission_template.source_id
         collection =
@@ -32,7 +31,6 @@
             create(:collection)
           end
         permission_template.source_id = collection.id
-        permission_template.source_type = 'collection'
       end
     end
 

spec/forms/hyrax/forms/permission_template_form_spec.rb

@@ -132,7 +132,7 @@ def count_workflow_responsibilities_for(user)
     let(:input_params) do
       ActionController::Parameters.new(access_grants_attributes: grant_attributes).permit!
     end
-    let(:permission_template) { create(:permission_template, source_id: admin_set.id, source_type: 'admin_set') }
+    let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
 
     let(:user) { create(:user) }
     let(:user2) { create(:user) }

spec/search_builders/hyrax/admin_set_search_builder_spec.rb

@@ -24,31 +24,13 @@
 
     context "when access is :deposit" do
       let(:access) { :deposit }
-      let(:admin_set) { create(:admin_set) }
-      let(:permission_template) { create(:permission_template, source_id: admin_set.id, source_type: 'admin_set') }
 
       context "and user has access" do
         before do
-          create(:permission_template_access,
-                 permission_template: permission_template,
-                 agent_type: 'user',
-                 agent_id: user.user_key,
-                 access: 'deposit')
+          allow(Hyrax::Collections::PermissionsService).to receive(:source_ids_for_deposit).and_return([7, 8])
         end
 
-        it { is_expected.to eq ["{!terms f=id}#{admin_set.id}"] }
-      end
-
-      context "and group has access" do
-        before do
-          create(:permission_template_access,
-                 permission_template: permission_template,
-                 agent_type: 'group',
-                 agent_id: 'registered',
-                 access: 'deposit')
-        end
-
-        it { is_expected.to eq ["{!terms f=id}#{admin_set.id}"] }
+        it { is_expected.to eq ["{!terms f=id}7,8"] }
       end
 
       context "and user has no access" do
@@ -60,7 +42,7 @@
   describe ".default_processor_chain" do
     subject { described_class.default_processor_chain }
 
-    it { is_expected.to include :filter_models }
+    it { is_expected.to include(:filter_models, :add_access_controls_to_solr_params) }
   end
 
   describe "#to_h" do
@@ -83,27 +65,9 @@
 
     context "when searching for deposit access" do
       let(:access) { :deposit }
-      let(:permission_template1) { create(:permission_template, source_id: 7, source_type: 'admin_set') }
-      let(:permission_template2) { create(:permission_template, source_id: 8, source_type: 'admin_set') }
-      let(:permission_template3) { create(:permission_template, source_id: 9, source_type: 'admin_set') }
 
       before do
-        create(:permission_template_access,
-               :manage,
-               permission_template: permission_template1,
-               agent_type: 'user',
-               agent_id: user.user_key,
-               access: 'deposit')
-        create(:permission_template_access,
-               :manage,
-               permission_template: permission_template2,
-               agent_type: 'user',
-               agent_id: user.user_key)
-        create(:permission_template_access,
-               :view,
-               permission_template: permission_template3,
-               agent_type: 'user',
-               agent_id: user.user_key)
+        allow(Hyrax::Collections::PermissionsService).to receive(:source_ids_for_deposit).and_return([7, 8])
       end
 
       it 'is successful' do

spec/search_builders/hyrax/dashboard/collections_search_builder_spec.rb

@@ -52,7 +52,7 @@
     subject { builder.gated_discovery_filters }
 
     let(:collection) { create(:collection) }
-    let(:permission_template) { create(:permission_template, source_id: collection.id, source_type: 'collection') }
+    let(:permission_template) { create(:permission_template, source_id: collection.id) }
 
     context "user has deposit access" do
       before do