diff --git a/app/assets/javascripts/institutions.js b/app/assets/javascripts/institutions.js index 5fad6892..1650bd5d 100644 --- a/app/assets/javascripts/institutions.js +++ b/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", diff --git a/app/controllers/index_pages_controller.rb b/app/controllers/index_pages_controller.rb index c47a6fb8..de0b9ad6 100644 --- a/app/controllers/index_pages_controller.rb +++ b/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 diff --git a/app/controllers/institutions_controller.rb b/app/controllers/institutions_controller.rb index 08be4185..98bb3bea 100644 --- a/app/controllers/institutions_controller.rb +++ b/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). diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 49f3d808..a07cb721 100644 --- a/app/helpers/application_helper.rb +++ b/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, diff --git a/app/policies/institution_policy.rb b/app/policies/institution_policy.rb index b5f8ee01..5c16bec7 100644 --- a/app/policies/institution_policy.rb +++ b/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 diff --git a/app/views/collections/_show_review_submissions_tab.html.haml b/app/views/collections/_show_review_submissions_tab.html.haml index f5ab545e..91dc2c29 100644 --- a/app/views/collections/_show_review_submissions_tab.html.haml +++ b/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"} diff --git a/app/views/collections/_show_submissions_in_progress_tab.html.haml b/app/views/collections/_show_submissions_in_progress_tab.html.haml index e92edc64..20c4fecc 100644 --- a/app/views/collections/_show_submissions_in_progress_tab.html.haml +++ b/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) diff --git a/app/views/events/_events.html.haml b/app/views/events/_events.html.haml index d514b155..a1cdc705 100644 --- a/app/views/events/_events.html.haml +++ b/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) diff --git a/app/views/imports/_listing.html.haml b/app/views/imports/_listing.html.haml index 3b85c78c..b9849f9d 100644 --- a/app/views/imports/_listing.html.haml +++ b/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 diff --git a/app/views/index_pages/show.html.haml b/app/views/index_pages/show.html.haml index 5d7cf806..66635b4e 100644 --- a/app/views/index_pages/show.html.haml +++ b/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"} diff --git a/app/views/institutions/show.html.haml b/app/views/institutions/show.html.haml index 25c5ece8..4f0a66e6 100644 --- a/app/views/institutions/show.html.haml +++ b/app/views/institutions/show.html.haml @@ -25,13 +25,13 @@ - show_element_mappings = policy(@institution).show_element_mappings? - show_element_namespaces = policy(@institution).show_element_namespaces? - show_element_registry = policy(@institution).show_element_registry? -- show_embargoed_items = policy(@institution).show_embargoed_items? - show_imports = policy(@institution).show_imports? - show_index_pages = policy(@institution).show_index_pages? - show_invitees = policy(@institution).show_invitees? - show_metadata_profiles = policy(@institution).show_metadata_profiles? - show_prebuilt_searches = policy(@institution).show_prebuilt_searches? - show_preservation = policy(@institution).show_preservation? +- show_private_items = policy(@institution).show_private_items? - show_properties = policy(@institution).show_properties? - show_review_submissions = policy(@institution).show_review_submissions? - show_settings = policy(@institution).show_settings? @@ -128,17 +128,17 @@ Submissions in Progress - if @submissions_in_progress_count > 0 %span.badge.rounded-pill.text-bg-primary= @submissions_in_progress_count - - if show_embargoed_items - %button#embargoed-items-tab.nav-link{"data-bs-target": "#embargoed-items-tab-content", - "data-bs-toggle": "pill", - role: "tab", - type: "button", - "aria-controls": "embargoed-items-tab-content", - "aria-selected": "false"} + - if show_private_items + %button#private-items-tab.nav-link{"data-bs-target": "#private-items-tab-content", + "data-bs-toggle": "pill", + role: "tab", + type: "button", + "aria-controls": "private-items-tab-content", + "aria-selected": "false"} = icon_for(Embargo) - Embargoed Items - - if @embargoed_items_count > 0 - %span.badge.rounded-pill.text-bg-primary= @embargoed_items_count + Private Items + - if @private_items_count > 0 + %span.badge.rounded-pill.text-bg-primary= @private_items_count - if show_withdrawn_items %button#withdrawn-items-tab.nav-link{"data-bs-target": "#withdrawn-items-tab-content", "data-bs-toggle": "pill", @@ -330,9 +330,9 @@ "aria-labelledby": "deleted-items-tab"} = spinner - - if show_embargoed_items - #embargoed-items-tab-content.tab-pane.fade{role: "tabpanel", - "aria-labelledby": "embargoed-items-tab"} + - if show_private_items + #private-items-tab-content.tab-pane.fade{role: "tabpanel", + "aria-labelledby": "private-items-tab"} = spinner - if show_element_mappings diff --git a/app/views/invitees/_all_invitees.html.haml b/app/views/invitees/_all_invitees.html.haml index 6678bc57..4eac456c 100644 --- a/app/views/invitees/_all_invitees.html.haml +++ b/app/views/invitees/_all_invitees.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 diff --git a/app/views/invitees/_invitees.html.haml b/app/views/invitees/_invitees.html.haml index 43e244b7..59bcb16c 100644 --- a/app/views/invitees/_invitees.html.haml +++ b/app/views/invitees/_invitees.html.haml @@ -1,4 +1,4 @@ -- 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 diff --git a/app/views/items/_listing.html.haml b/app/views/items/_listing.html.haml index 17a930da..a5eca12b 100644 --- a/app/views/items/_listing.html.haml +++ b/app/views/items/_listing.html.haml @@ -1,9 +1,9 @@ -- show_embargoed_normally = defined?(show_embargoed_normally) ? show_embargoed_normally : false +- show_private_normally = defined?(show_private_normally) ? show_private_normally : false -- pagination = paginate(count: @count, | - page: @current_page, | - per_page: @window, | - permitted_params: @permitted_params) | +- pagination = paginate(actual_count: @count, + page: @current_page, + per_page: @window, + permitted_params: @permitted_params) .card.mb-3 #search-status.card-body.pt-3.pb-3 @@ -14,9 +14,9 @@ = pagination = resource_list(@items, - use_resource_host: false, - show_institution: !institution_host?, - show_embargoed_normally: show_embargoed_normally) + use_resource_host: false, + show_institution: !institution_host?, + show_private_normally: show_private_normally) .row.justify-content-md-center .col-md-auto diff --git a/app/views/items/_recent_listing.html.haml b/app/views/items/_recent_listing.html.haml index 982cb1ea..9090376d 100644 --- a/app/views/items/_recent_listing.html.haml +++ b/app/views/items/_recent_listing.html.haml @@ -1,7 +1,7 @@ -- pagination = paginate(count: @count, | - page: @current_page, | - per_page: @window, | - permitted_params: @permitted_params) | +- pagination = paginate(actual_count: @count, + page: @current_page, + per_page: @window, + permitted_params: @permitted_params) .row.justify-content-md-center .col-md-auto diff --git a/app/views/items/_review_listing.html.haml b/app/views/items/_review_listing.html.haml index 7238bf3f..7e3bad09 100644 --- a/app/views/items/_review_listing.html.haml +++ b/app/views/items/_review_listing.html.haml @@ -1,5 +1,5 @@ - if @items.any? - - pagination = paginate(count: @count, + - pagination = paginate(actual_count: @count, page: @current_page, per_page: @window, permitted_params: @permitted_params) diff --git a/app/views/messages/index.html.haml b/app/views/messages/index.html.haml index 4c2b0be2..c4881cb5 100644 --- a/app/views/messages/index.html.haml +++ b/app/views/messages/index.html.haml @@ -1,7 +1,7 @@ - provide(:body_id, "messages") - provide(:title, "Messages") -- pagination = paginate(count: @count, +- pagination = paginate(actual_count: @count, page: @current_page, per_page: @window, permitted_params: @permitted_params) diff --git a/app/views/tasks/_tasks.html.haml b/app/views/tasks/_tasks.html.haml index 1f68b975..68fd39ea 100644 --- a/app/views/tasks/_tasks.html.haml +++ b/app/views/tasks/_tasks.html.haml @@ -4,7 +4,9 @@ -# show_institution [Boolean] -# -- 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 diff --git a/app/views/users/_show_submittable_collections_tab.html.haml b/app/views/users/_show_submittable_collections_tab.html.haml index caad7b4e..9c7602c8 100644 --- a/app/views/users/_show_submittable_collections_tab.html.haml +++ b/app/views/users/_show_submittable_collections_tab.html.haml @@ -1,8 +1,8 @@ - if @count.positive? - - pagination = paginate(count: @count, | - page: @current_page, | - per_page: @window, | - permitted_params: @permitted_params) | + - pagination = paginate(actual_count: @count, + page: @current_page, + per_page: @window, + permitted_params: @permitted_params) .card.mb-3 #search-status.card-body.pt-3.pb-3 diff --git a/app/views/users/_users.html.haml b/app/views/users/_users.html.haml index b89345ee..c6fa969a 100644 --- a/app/views/users/_users.html.haml +++ b/app/views/users/_users.html.haml @@ -4,7 +4,7 @@ -# institution_column [Boolean] -# -- pagination = paginate(count: @count, +- pagination = paginate(actual_count: @count, page: @current_page, per_page: @window, permitted_params: @permitted_params) diff --git a/app/views/vocabularies/_terms.html.haml b/app/views/vocabularies/_terms.html.haml index 79e440cd..c70843f7 100644 --- a/app/views/vocabularies/_terms.html.haml +++ b/app/views/vocabularies/_terms.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) diff --git a/app/views/vocabularies/_vocabularies.html.haml b/app/views/vocabularies/_vocabularies.html.haml index 86fc7351..16ffb23d 100644 --- a/app/views/vocabularies/_vocabularies.html.haml +++ b/app/views/vocabularies/_vocabularies.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) diff --git a/config/routes.rb b/config/routes.rb index 6dc39c42..6ab4b916 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,8 +141,6 @@ constraints: lambda { |request| request.xhr? } match "/elements", to: "institutions#show_element_registry", via: :get, constraints: lambda { |request| request.xhr? } - match "/embargoed-items", to: "institutions#show_embargoed_items", via: :get, - constraints: lambda { |request| request.xhr? } match "/imports", to: "institutions#show_imports", via: :get, constraints: lambda { |request| request.xhr? } match "/index-pages", to: "institutions#show_index_pages", via: :get, @@ -155,6 +153,8 @@ constraints: lambda { |request| request.xhr? } match "/preservation", to: "institutions#show_preservation", via: :get, constraints: lambda { |request| request.xhr? } + match "/private-items", to: "institutions#show_private_items", via: :get, + constraints: lambda { |request| request.xhr? } match "/properties", to: "institutions#show_properties", via: :get, constraints: lambda { |request| request.xhr? } match "/review-submissions", to: "institutions#show_review_submissions", via: :get, diff --git a/test/controllers/institutions_controller_test.rb b/test/controllers/institutions_controller_test.rb index ea3b3e36..96db693c 100644 --- a/test/controllers/institutions_controller_test.rb +++ b/test/controllers/institutions_controller_test.rb @@ -1068,31 +1068,6 @@ class InstitutionsControllerTest < ActionDispatch::IntegrationTest assert_response :ok end - # show_embargoed_items() - - test "show_embargoed_items() returns HTTP 404 for unscoped requests" do - host! ::Configuration.instance.main_host - get institution_embargoed_items_path(@institution), xhr: true - assert_response :not_found - end - - test "show_embargoed_items() returns HTTP 403 for logged-out users" do - get institution_embargoed_items_path(@institution), xhr: true - assert_response :forbidden - end - - test "show_embargoed_items() returns HTTP 403 for unauthorized users" do - log_in_as(users(:southwest)) - get institution_embargoed_items_path(@institution), xhr: true - assert_response :forbidden - end - - test "show_embargoed_items() returns HTTP 200 for authorized users" do - log_in_as(users(:southwest_admin)) - get institution_embargoed_items_path(@institution), xhr: true - assert_response :ok - end - # show_imports() test "show_imports() returns HTTP 404 for unscoped requests" do @@ -1243,6 +1218,31 @@ class InstitutionsControllerTest < ActionDispatch::IntegrationTest assert_response :ok end + # show_private_items() + + test "show_private_items() returns HTTP 404 for unscoped requests" do + host! ::Configuration.instance.main_host + get institution_private_items_path(@institution), xhr: true + assert_response :not_found + end + + test "show_private_items() returns HTTP 403 for logged-out users" do + get institution_private_items_path(@institution), xhr: true + assert_response :forbidden + end + + test "show_private_items() returns HTTP 403 for unauthorized users" do + log_in_as(users(:southwest)) + get institution_private_items_path(@institution), xhr: true + assert_response :forbidden + end + + test "show_private_items() returns HTTP 200 for authorized users" do + log_in_as(users(:southwest_admin)) + get institution_private_items_path(@institution), xhr: true + assert_response :ok + end + # show_properties() test "show_properties() returns HTTP 404 for unscoped requests" do diff --git a/test/policies/institution_policy_test.rb b/test/policies/institution_policy_test.rb index 26e5d409..28dd7d32 100644 --- a/test/policies/institution_policy_test.rb +++ b/test/policies/institution_policy_test.rb @@ -1486,59 +1486,6 @@ class InstitutionPolicyTest < ActiveSupport::TestCase assert !policy.show_element_registry? end - # show_embargoed_items?() - - test "show_embargoed_items?() returns false with a nil request context" do - context = RequestContext.new(user: nil, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert !policy.show_embargoed_items? - end - - test "show_embargoed_items?() does not authorize non-sysadmins" do - user = users(:southwest) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert !policy.show_embargoed_items? - end - - test "show_embargoed_items?() authorizes sysadmins" do - user = users(:southwest_sysadmin) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert policy.show_embargoed_items? - end - - test "show_embargoed_items?() authorizes administrators of the same - institution" do - user = users(:southwest_admin) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert policy.show_embargoed_items? - end - - test "show_embargoed_items?() does not authorize administrators of a different - institution" do - user = users(:southwest_admin) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, institutions(:northeast)) - assert !policy.show_embargoed_items? - end - - test "show_embargoed_items?() respects role limits" do - # sysadmin user limited to an insufficient role - user = users(:southwest_sysadmin) - context = RequestContext.new(user: user, - institution: user.institution, - role_limit: Role::LOGGED_IN) - policy = InstitutionPolicy.new(context, @institution) - assert !policy.show_embargoed_items? - end - # show_imports?() test "show_imports?() returns false with a nil request context" do @@ -1766,6 +1713,59 @@ class InstitutionPolicyTest < ActiveSupport::TestCase assert !policy.show_preservation? end + # show_private_items?() + + test "show_private_items?() returns false with a nil request context" do + context = RequestContext.new(user: nil, + institution: @institution) + policy = InstitutionPolicy.new(context, @institution) + assert !policy.show_private_items? + end + + test "show_private_items?() does not authorize non-sysadmins" do + user = users(:southwest) + context = RequestContext.new(user: user, + institution: @institution) + policy = InstitutionPolicy.new(context, @institution) + assert !policy.show_private_items? + end + + test "show_private_items?() authorizes sysadmins" do + user = users(:southwest_sysadmin) + context = RequestContext.new(user: user, + institution: @institution) + policy = InstitutionPolicy.new(context, @institution) + assert policy.show_private_items? + end + + test "show_private_items?() authorizes administrators of the same + institution" do + user = users(:southwest_admin) + context = RequestContext.new(user: user, + institution: @institution) + policy = InstitutionPolicy.new(context, @institution) + assert policy.show_private_items? + end + + test "show_private_items?() does not authorize administrators of a different + institution" do + user = users(:southwest_admin) + context = RequestContext.new(user: user, + institution: @institution) + policy = InstitutionPolicy.new(context, institutions(:northeast)) + assert !policy.show_private_items? + end + + test "show_private_items?() respects role limits" do + # sysadmin user limited to an insufficient role + user = users(:southwest_sysadmin) + context = RequestContext.new(user: user, + institution: user.institution, + role_limit: Role::LOGGED_IN) + policy = InstitutionPolicy.new(context, @institution) + assert !policy.show_private_items? + end + # show_properties?() test "show_properties?() returns false with a nil request context" do