Merge pull request #3105 from samvera/backport/fixes_from_master

Backport/fixes from master

app/controllers/concerns/hyrax/controller.rb

@@ -6,6 +6,7 @@ module Hyrax::Controller
 
     # Adds Hydra behaviors into the application controller
     include Hydra::Controller::ControllerBehavior
+    helper_method :create_work_presenter
     before_action :set_locale
   end
 
@@ -14,6 +15,20 @@ def user_root_path
     hyrax.dashboard_path
   end
 
+  ##
+  # @deprecated this helper is no longer used by Hyrax; if you need access to
+  #   this presenter on every page, you may need to readd it manually.
+  #
+  # A presenter for selecting a work type to create this is needed here because
+  # the selector is in the header on every page.
+  def create_work_presenter
+    Deprecation.warn(self, "The `create_work_presenter` helper is deprecated " \
+                           "for removal in Hyrax 3.0. The work selector has " \
+                           "been removed the masthead in Hyrax 2.1.")
+
+    Hyrax::SelectTypeListPresenter.new(current_user)
+  end
+
   # Ensure that the locale choice is persistent across requests
   def default_url_options
     super.merge(locale: I18n.locale)

app/helpers/hyrax/hyrax_helper_behavior.rb

@@ -257,6 +257,14 @@ def collection_thumbnail(_document, _image_options = {}, _url_options = {})
       content_tag(:span, "", class: [Hyrax::ModelIcon.css_class_for(Collection), "collection-icon-search"])
     end
 
+    def collection_title_by_id(id)
+      solr_docs = controller.repository.find(id).docs
+      return nil if solr_docs.empty?
+      solr_field = solr_docs.first[Solrizer.solr_name("title", :stored_searchable)]
+      return nil if solr_field.nil?
+      solr_field.first
+    end
+
     private
 
       def user_agent

app/presenters/hyrax/work_show_presenter.rb

@@ -225,7 +225,7 @@ def total_items
       end
 
       def rows_from_params
-        request.params[:rows].nil? ? 10 : request.params[:rows].to_i
+        request.params[:rows].nil? ? Hyrax.config.show_work_item_rows : request.params[:rows].to_i
       end
 
       def current_page

app/search_builders/hyrax/catalog_search_builder.rb

@@ -2,7 +2,8 @@ class Hyrax::CatalogSearchBuilder < Hyrax::SearchBuilder
   self.default_processor_chain += [
     :add_access_controls_to_solr_params,
     :show_works_or_works_that_contain_files,
-    :show_only_active_records
+    :show_only_active_records,
+    :filter_collection_facet_for_access
   ]
 
   # show both works that match the query and works that contain files that match the query
@@ -19,6 +20,18 @@ def show_only_active_records(solr_parameters)
     solr_parameters[:fq] << '-suppressed_bsi:true'
   end
 
+  # only return facet counts for collections that this user has access to see
+  def filter_collection_facet_for_access(solr_parameters)
+    return if current_ability.admin?
+
+    collection_ids = Hyrax::Collections::PermissionsService.collection_ids_for_view(ability: current_ability).map { |id| "^#{id}$" }
+    solr_parameters['f.member_of_collection_ids_ssim.facet.matches'] = if collection_ids.present?
+                                                                         collection_ids.join('|')
+                                                                       else
+                                                                         "^$"
+                                                                       end
+  end
+
   private
 
     # the {!lucene} gives us the OR syntax

app/services/hyrax/collections/permissions_service.rb

@@ -97,6 +97,19 @@ def self.collection_ids_for_deposit(ability:)
         source_ids_for_deposit(ability: ability, source_type: 'collection')
       end
 
+      # @api public
+      #
+      # IDs of collections which the user can view.
+      #
+      # @param ability [Ability] the ability coming from cancan ability check
+      # @return [Array<String>] IDs of collections into which the user can view
+      # @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.collection_ids_for_view(ability:)
+        collection_ids_for_user(ability: ability, access: [Hyrax::PermissionTemplateAccess::MANAGE,
+                                                           Hyrax::PermissionTemplateAccess::DEPOSIT,
+                                                           Hyrax::PermissionTemplateAccess::VIEW])
+      end
+
       # @api public
       #
       # Determine if the given user has permissions to view the admin show page for at least one collection

lib/generators/hyrax/templates/catalog_controller.rb

@@ -44,7 +44,7 @@ def self.modified_field
     config.add_facet_field solr_name("based_near_label", :facetable), limit: 5
     config.add_facet_field solr_name("publisher", :facetable), limit: 5
     config.add_facet_field solr_name("file_format", :facetable), limit: 5
-    config.add_facet_field solr_name('member_of_collections', :symbol), limit: 5, label: 'Collections'
+    config.add_facet_field solr_name('member_of_collection_ids', :symbol), limit: 5, label: 'Collections', helper_method: :collection_title_by_id
 
     # The generic_type isn't displayed on the facet list
     # It's used to give a label to the filter that comes from the user profile

lib/generators/hyrax/templates/config/initializers/hyrax.rb

@@ -116,6 +116,10 @@
   # The default is true.
   # config.work_requires_files = true
 
+  # How many rows of items should appear on the work show view?
+  # The default is 10
+  # config.show_work_item_rows = 10
+
   # Enable IIIF image service. This is required to use the
   # UniversalViewer-ified show page
   #

lib/hyrax/configuration.rb

@@ -340,6 +340,11 @@ def work_requires_files?
       @work_requires_files
     end
 
+    attr_writer :show_work_item_rows
+    def show_work_item_rows
+      @show_work_item_rows ||= 10 # rows on show view
+    end
+
     attr_writer :batch_user_key
     def batch_user_key
       @batch_user_key ||= '[email protected]'

spec/features/catalog_search_spec.rb

@@ -28,4 +28,45 @@
       expect(page).to have_content(collection.title.first)
     end
   end
+
+  context 'with public works and private collections', clean_repo: true do
+    let!(:collection) { create(:private_collection) }
+
+    let!(:jills_work) do
+      create(:public_work, title: ["Jill's Research"], keyword: ['jills_keyword'], member_of_collections: [collection])
+    end
+
+    it "hides collection facet values the user doesn't have access to view when performing a search" do
+      within('#search-form-header') do
+        fill_in('search-field-header', with: 'jills_keyword')
+        click_button('Go')
+      end
+
+      expect(page).to have_content('Search Results')
+      expect(page).to have_content(jills_work.title.first)
+      expect(page).not_to have_content(collection.title.first)
+      expect(page).not_to have_css('.blacklight-member_of_collection_ids_ssim')
+    end
+
+    context 'as an admin' do
+      let(:admin_user) { create :admin }
+
+      before do
+        sign_in admin_user
+        visit '/'
+      end
+
+      it "shows collection facet values the user has access to view when performing a search" do
+        within('#search-form-header') do
+          fill_in('search-field-header', with: 'jills_keyword')
+          click_button('Go')
+        end
+
+        expect(page).to have_content('Search Results')
+        expect(page).to have_content(jills_work.title.first)
+        find('.blacklight-member_of_collection_ids_ssim').click
+        expect(page).to have_content(collection.title.first)
+      end
+    end
+  end
 end

spec/helpers/hyrax_helper_spec.rb

@@ -366,4 +366,34 @@ def new_state
       expect(helper.banner_image).to eq(Hyrax.config.banner_image)
     end
   end
+
+  describe "#collection_title_by_id" do
+    let(:solr_doc) { double(id: "abcd12345") }
+    let(:bad_solr_doc) { double(id: "efgh67890") }
+    let(:solr_response) { double(docs: [solr_doc]) }
+    let(:bad_solr_response) { double(docs: [bad_solr_doc]) }
+    let(:empty_solr_response) { double(docs: []) }
+    let(:repository) { double }
+
+    before do
+      allow(controller).to receive(:repository).and_return(repository)
+      allow(solr_doc).to receive(:[]).with("title_tesim").and_return(["Collection of Awesomeness"])
+      allow(bad_solr_doc).to receive(:[]).with("title_tesim").and_return(nil)
+      allow(repository).to receive(:find).with("abcd12345").and_return(solr_response)
+      allow(repository).to receive(:find).with("efgh67890").and_return(bad_solr_response)
+      allow(repository).to receive(:find).with("bad-id").and_return(empty_solr_response)
+    end
+
+    it "returns the first title of the collection" do
+      expect(helper.collection_title_by_id("abcd12345")).to eq "Collection of Awesomeness"
+    end
+
+    it "returns nil if collection doesn't have title_tesim field" do
+      expect(helper.collection_title_by_id("efgh67890")).to eq nil
+    end
+
+    it "returns nil if collection not found" do
+      expect(helper.collection_title_by_id("bad-id")).to eq nil
+    end
+  end
 end

spec/lib/hyrax/configuration_spec.rb

@@ -72,6 +72,7 @@
   it { is_expected.to respond_to(:rights_statement_service_class=) }
   it { is_expected.to respond_to(:redis_namespace) }
   it { is_expected.to respond_to(:subject_prefix) }
+  it { is_expected.to respond_to(:show_work_item_rows) }
   it { is_expected.to respond_to(:translate_id_to_uri) }
   it { is_expected.to respond_to(:translate_uri_to_id) }
   it { is_expected.to respond_to(:upload_path) }

spec/search_builders/hyrax/catalog_search_builder_spec.rb

@@ -48,4 +48,39 @@
       expect(solr_params[:fq]).to eq ["-suppressed_bsi:true"]
     end
   end
+
+  describe "#filter_collection_facet_for_access" do
+    let(:user) { build(:user) }
+    let(:ability) { Ability.new(user) }
+    let(:context) { double(current_ability: ability) }
+
+    subject { builder.filter_collection_facet_for_access(solr_params) }
+
+    context 'with an admin' do
+      let(:user) { build(:admin) }
+
+      it "does nothing if user is an admin" do
+        subject
+        expect(solr_params['f.member_of_collection_ids_ssim.facet.matches']).to be_blank
+      end
+    end
+
+    context 'when the user has view access to collections' do
+      let(:collection_ids) { ['abcd12345', 'efgh67890'] }
+
+      before do
+        allow(Hyrax::Collections::PermissionsService).to receive(:collection_ids_for_view).with(ability: ability).and_return(collection_ids)
+      end
+
+      it "includes a regex of the ids of collections" do
+        subject
+        expect(solr_params['f.member_of_collection_ids_ssim.facet.matches']).to eq '^abcd12345$|^efgh67890$'
+      end
+    end
+
+    it "includes an empty regex when user doesn't have access to view any collections" do
+      subject
+      expect(solr_params['f.member_of_collection_ids_ssim.facet.matches']).to eq '^$'
+    end
+  end
 end

spec/services/hyrax/collections/permissions_service_spec.rb

@@ -299,6 +299,20 @@
       end
     end
 
+    describe '.collection_ids_for_view' do
+      it 'returns collection ids where user has view access' do
+        expect(described_class.collection_ids_for_view(ability: ability)).to match_array [col_du.id, col_dg.id, col_mu.id, col_mg.id, col_vu.id, col_vg.id]
+      end
+
+      context 'when user has no access' do
+        let(:ability) { Ability.new(user2) }
+
+        it 'returns empty array' do
+          expect(described_class.collection_ids_for_view(ability: ability)).to match_array []
+        end
+      end
+    end
+
     describe '.can_manage_any_collection?' do
       it 'returns true when user has manage access to at least one collection' do
         expect(described_class.can_manage_any_collection?(ability: ability)).to be true