Merge branch 'develop' into demo

app/assets/javascripts/institutions.js

@@ -259,10 +259,10 @@ const InstitutionView = {
             });
         });
 
-        $("#embargoed-items-tab").on("show.bs.tab", function () {
-            const url = ROOT_URL + "/institutions/" + institutionKey + "/embargoed-items";
+        $("#private-items-tab").on("show.bs.tab", function () {
+            const url = ROOT_URL + "/institutions/" + institutionKey + "/private-items";
             $.get(url, function (data) {
-                $("#embargoed-items-tab-content").html(data);
+                $("#private-items-tab-content").html(data);
 
                 const attachResultsEventListeners = function() {
                     $(".page-link").on("click", function(e) {
@@ -273,10 +273,10 @@ const InstitutionView = {
                 attachResultsEventListeners();
 
                 const refreshResults = function(url) {
-                    const container = $("#embargoed-items-tab-content");
+                    const container = $("#private-items-tab-content");
                     container.html(IDEALS.UIUtils.Spinner());
                     if (!url) {
-                        url = ROOT_URL + "/institutions/" + institutionKey + "/embargoed-items";
+                        url = ROOT_URL + "/institutions/" + institutionKey + "/private-items";
                     }
                     $.ajax({
                         method:  "GET",

app/controllers/index_pages_controller.rb

@@ -92,40 +92,12 @@ def new
   #
   def show
     @permitted_params = params.permit(:letter, :q, :start)
-    @start            = [@permitted_params[:start].to_i.abs, max_start].min
+    @start            = @permitted_params[:start].to_i.abs
     @window           = 50
     reg_e_ids         = @index_page.registered_element_ids
     if reg_e_ids.any?
-      # This query includes terms from embargoed items because taking those
-      # into account would greatly slow it down. We assume that there
-      # are few enough embargoed items that it isn't going to matter much.
-      @terms            = AscribedElement.
-        select(:string).
-        distinct.
-        joins(:item).
-        where("items.institution_id": current_institution.id,
-              "items.stage":          Item::Stages::APPROVED,
-              registered_element_id:  reg_e_ids).
-        order(:string)
-      # N.B.: attackers are known to attempt SQL injections here, which will
-      # cause a flood of ArgumentError emails unless we rescue.
-      begin
-        if params[:letter]
-          @terms = @terms.where("UNACCENT(LOWER(string)) LIKE ?", "#{params[:letter].downcase}%")
-        elsif params[:q]
-          @terms = @terms.where("UNACCENT(LOWER(string)) LIKE ?", "%#{params[:q].downcase}%")
-        end
-      rescue ArgumentError => e
-        if e.message.include?("string contains null byte")
-          raise ActionDispatch::Http::Parameters::ParseError
-        else
-          raise e
-        end
-      end
-      @count            = @terms.count
-      @terms            = @terms.offset(@start).
-        limit(@window).
-        pluck(:string)
+      @count            = term_count(reg_e_ids)
+      @terms            = terms(reg_e_ids, @start, @window)
       @current_page     = ((@start / @window.to_f).ceil + 1 if @window > 0) || 1
       # This may give us a little performance boost
       @starting_chars = Rails.cache.fetch("index_page_#{@index_page.id} starting_chars",
@@ -138,7 +110,7 @@ def show
       @terms          = []
       @starting_chars = []
     end
-    @breadcrumbable   = @index_page
+    @breadcrumbable = @index_page
   end
 
   ##
@@ -201,4 +173,72 @@ def starting_chars(reg_e_ids)
     ActiveRecord::Base.connection.exec_query(sql, "SQL", values)
   end
 
+  def term_count(reg_e_ids)
+    values = [current_institution.id, Item::Stages::APPROVED]
+    # This query includes terms from embargoed items because taking those
+    # into account would greatly slow it down. We assume that there
+    # are few enough embargoed items that it isn't going to matter much.
+    sql = "SELECT COUNT(DISTINCT(string)) AS count
+          FROM ascribed_elements ae
+          INNER JOIN items i ON i.id = ae.item_id
+          WHERE i.institution_id = $1
+            AND i.stage = $2
+            AND ae.registered_element_id IN (#{reg_e_ids.join(",")}) "
+    # N.B.: attackers are known to attempt SQL injections here, which will
+    # cause a flood of ArgumentError emails unless we rescue.
+    begin
+      if params[:letter]
+        sql += "AND UNACCENT(LOWER(string)) LIKE $3 "
+        values << "#{params[:letter].downcase}%"
+      elsif params[:q]
+        sql += "AND UNACCENT(LOWER(string)) LIKE $3 "
+        values << "%#{params[:q].downcase}%"
+      end
+    rescue ArgumentError => e
+      if e.message.include?("string contains null byte")
+        raise ActionDispatch::Http::Parameters::ParseError
+      else
+        raise e
+      end
+    end
+    ActiveRecord::Base.connection.exec_query(sql, "SQL", values)[0]['count']
+  end
+
+  def terms(reg_e_ids, start, window)
+    values = [current_institution.id, Item::Stages::APPROVED]
+    # This query includes terms from embargoed items because taking those
+    # into account would greatly slow it down. We assume that there
+    # are few enough embargoed items that it isn't going to matter much.
+    sql = "SELECT string
+            FROM (
+              SELECT DISTINCT(string) AS string
+              FROM ascribed_elements ae
+              INNER JOIN items i ON i.id = ae.item_id
+              WHERE i.institution_id = $1
+                AND i.stage = $2
+                AND ae.registered_element_id IN (#{reg_e_ids.join(",")}) "
+    # N.B.: attackers are known to attempt SQL injections here, which will
+    # cause a flood of ArgumentError emails unless we rescue.
+    begin
+      if params[:letter]
+        sql += "AND UNACCENT(LOWER(string)) LIKE $3 "
+        values << "#{params[:letter].downcase}%"
+      elsif params[:q]
+        sql += "AND UNACCENT(LOWER(string)) LIKE $3 "
+        values << "%#{params[:q].downcase}%"
+      end
+    rescue ArgumentError => e
+      if e.message.include?("string contains null byte")
+        raise ActionDispatch::Http::Parameters::ParseError
+      else
+        raise e
+      end
+    end
+    sql += ") t
+            ORDER BY (string ~ '\\d')::int, string
+            OFFSET #{start}
+            LIMIT #{window};"
+    ActiveRecord::Base.connection.exec_query(sql, "SQL", values).map{ |r| r['string'] }
+  end
+
 end

app/controllers/institutions_controller.rb

@@ -351,7 +351,7 @@ def show
     @review_count                  = review_items(0, 0).count
     @submissions_in_progress_count = submissions_in_progress(0, 0).count
     @buried_items_count            = buried_items(0, 0).count
-    @embargoed_items_count         = embargoed_items(0, 0).count
+    @private_items_count           = private_items(0, 0).count
     @withdrawn_items_count         = withdrawn_items(0, 0).count
   end
 
@@ -431,18 +431,18 @@ def show_element_registry
   end
 
   ##
-  # Renders HTML for the embargoed items tab in show-institution view.
+  # Renders HTML for the private items tab in show-institution view.
   #
-  # Responds to `GET /institutions/:key/embargoed-items` (XHR only)
+  # Responds to `GET /institutions/:key/private-items` (XHR only)
   #
-  def show_embargoed_items
+  def show_private_items
     @permitted_params = params.permit(RESULTS_PARAMS)
     @start            = [@permitted_params[:start].to_i.abs, max_start].min
     @window           = window_size
-    @items            = embargoed_items(@start, @window)
+    @items            = private_items(@start, @window)
     @count            = @items.count
     @current_page     = @items.page
-    render partial: "items/listing", locals: { show_embargoed_normally: true }
+    render partial: "items/listing", locals: { show_private_normally: true }
   end
 
   ##
@@ -1004,11 +1004,13 @@ def buried_items(start, limit)
       limit(limit)
   end
 
-  def embargoed_items(start, limit)
+  def private_items(start, limit)
     Item.search.
       institution(@institution).
       aggregations(false).
-      must_exist(Item::IndexFields::EMBARGOES).
+      filter_range("#{Item::IndexFields::EMBARGOES}.#{Embargo::IndexFields::ALL_ACCESS_EXPIRES_AT}",
+                   :gt,
+                   Time.now.strftime("%Y-%m-%d")).
       must_not(Item::IndexFields::STAGE, Item::Stages::WITHDRAWN).
       order(@institution.title_element.indexed_sort_field).
       start(start).

app/helpers/application_helper.rb

@@ -613,20 +613,19 @@ def metadata_as_meta_tags(ascribed_elements, profile = nil)
   end
 
   ##
-  # @param count [Integer] Total number of results. Note that this will be
-  #              limited internally to {OpenSearchIndex::MAX_RESULT_WINDOW}
-  #              to avoid overwhelming the search server.
+  # @param actual_count [Integer] Total number of results.
   # @param page [Integer]
   # @param per_page [Integer]
   # @param permitted_params [ActionController::Parameters]
   # @param max_links [Integer]
   #
-  def paginate(count:,
+  def paginate(actual_count:,
+               visible_count:    OpenSearchIndex::MAX_RESULT_WINDOW,
                page:,
                per_page:,
                permitted_params:,
-               max_links: MAX_PAGINATION_LINKS)
-    count = [count, OpenSearchIndex::MAX_RESULT_WINDOW].min
+               max_links:        MAX_PAGINATION_LINKS)
+    count      = [actual_count, visible_count].min
     return '' if count <= per_page
     num_pages  = (count / per_page.to_f).ceil
     first_page = [1, page - (max_links / 2.0).floor].max
@@ -715,21 +714,21 @@ def policy(entity)
   #                             primary.
   # @param use_resource_host [Boolean]
   # @param show_institution [Boolean]
-  # @param show_embargoed_normally [Boolean]
+  # @param show_private_normally [Boolean]
   # @return [String] HTML listing.
   #
   def resource_list(resources,
-                    primary_id:              nil,
-                    use_resource_host:       true,
-                    show_institution:        false,
-                    show_embargoed_normally: false) # TODO: split this into separate implementations for items & collections/units
+                    primary_id:            nil,
+                    use_resource_host:     true,
+                    show_institution:      false,
+                    show_private_normally: false) # TODO: split this into separate implementations for items & collections/units
     html = StringIO.new
     resources.each do |resource|
       html << resource_list_row(resource,
-                                primary:                 (primary_id == resource.id),
-                                use_resource_host:       use_resource_host,
-                                show_institution:        show_institution,
-                                show_embargoed_normally: show_embargoed_normally)
+                                primary:               (primary_id == resource.id),
+                                use_resource_host:     use_resource_host,
+                                show_institution:      show_institution,
+                                show_private_normally: show_private_normally)
     end
     raw(html.string)
   end
@@ -739,16 +738,16 @@ def resource_list(resources,
   # @param primary [Boolean] Whether to mark the resource as primary.
   # @param use_resource_host [Boolean]
   # @param show_institution [Boolean]
-  # @param show_embargoed_normally [Boolean]
+  # @param show_private_normally [Boolean]
   # @return [String] HTML string.
   # @private
   #
   def resource_list_row(resource,
-                        primary:                 false,
-                        use_resource_host:       true,
-                        show_institution:        false,
-                        show_embargoed_normally: false)
-    embargoed_item = !show_embargoed_normally &&
+                        primary:               false,
+                        use_resource_host:     true,
+                        show_institution:      false,
+                        show_private_normally: false)
+    embargoed_item = !show_private_normally &&
       resource.kind_of?(Item) &&
       resource.embargoed_for?(user:            current_user,
                               client_hostname: request_context.client_hostname,

app/policies/institution_policy.rb

@@ -152,10 +152,6 @@ def show_element_registry
     show_metadata_profiles
   end
 
-  def show_embargoed_items
-    show
-  end
-
   def show_imports
     show_metadata_profiles
   end
@@ -180,6 +176,10 @@ def show_preservation
     effective_sysadmin(@user, @role_limit)
   end
 
+  def show_private_items
+    show
+  end
+
   def show_properties
     show
   end

app/views/collections/_show_review_submissions_tab.html.haml

@@ -1,7 +1,7 @@
 - if @review_count > 0
-  - pagination = paginate(count: @review_count,
-                          page: @review_current_page,
-                          per_page: @review_window,
+  - pagination = paginate(actual_count:     @review_count,
+                          page:             @review_current_page,
+                          per_page:         @review_window,
                           permitted_params: @review_permitted_params)
 
   .btn-group.float-end{role: "group"}

app/views/collections/_show_submissions_in_progress_tab.html.haml

@@ -1,5 +1,5 @@
 - if @count > 0
-  - pagination = paginate(count:            @count,
+  - pagination = paginate(actual_count:     @count,
                           page:             @current_page,
                           per_page:         @window,
                           permitted_params: @permitted_params)

app/views/events/_events.html.haml

@@ -1,4 +1,4 @@
-- pagination = paginate(count:            @count,
+- pagination = paginate(actual_count:     @count,
                         page:             @current_page,
                         per_page:         @window,
                         permitted_params: @permitted_params)

app/views/imports/_listing.html.haml

@@ -1,4 +1,6 @@
-- pagination = paginate(count: @count, page: @current_page, per_page: @window,
+- pagination = paginate(actual_count:     @count,
+                        page:             @current_page,
+                        per_page:         @window,
                         permitted_params: @permitted_params)
 
 .row.justify-content-md-center

app/views/index_pages/show.html.haml

@@ -30,11 +30,12 @@
 %h1
   = @index_page.name
 
-- pagination = paginate(count:            @count, |
-                        page:             @current_page, |
-                        per_page:         @window, |
-                        permitted_params: @permitted_params, |
-                        max_links:        9) |
+- pagination = paginate(actual_count:     @count,
+                        visible_count:    @count,
+                        page:             @current_page,
+                        per_page:         @window,
+                        permitted_params: @permitted_params,
+                        max_links:        9)
 
 - if @starting_chars.any?
   %ul.nav.nav-tabs{role: "tablist"}