diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index f2ad52044..f66ad7708 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -61,6 +61,7 @@ @import "pages/sources"; @import "pages/workflows"; @import "groups"; +@import "pages/curator"; // Vendor @import "cytoscape-panzoom"; diff --git a/app/assets/stylesheets/pages/curator.scss b/app/assets/stylesheets/pages/curator.scss new file mode 100644 index 000000000..0570c7bed --- /dev/null +++ b/app/assets/stylesheets/pages/curator.scss @@ -0,0 +1,14 @@ +.curator-floating-actions { + position: fixed; + bottom: 30px; + left: 50%; + transform: translateX(-50%); + z-index: 1050; + background-color: rgba(255, 255, 255, 0.512); + padding: 10px 20px; + border-radius: 25px; + box-shadow: 0 4px 12px rgba(0, 0, 0, 0.25); + border: 1px solid #ddd; + display: flex; + gap: 10px; +} \ No newline at end of file diff --git a/app/controllers/curator_controller.rb b/app/controllers/curator_controller.rb index e325040c2..52d56a633 100644 --- a/app/controllers/curator_controller.rb +++ b/app/controllers/curator_controller.rb @@ -1,11 +1,19 @@ # The controller for actions related to the curator model class CuratorController < ApplicationController - CURATION_ACTIONS = %w(material.add_term event.add_term material.reject_term event.reject_term) + CURATION_ACTIONS = %w(material.add_term event.add_term material.reject_term event.reject_term material.approval_status_changed event.approval_status_changed).freeze + CURATABLE_TYPES = { + 'materials' => Material, + 'events' => Event + }.freeze + + APPROVAL_FEATURE_FLAGS = { + 'materials' => 'material_under_admin_approval', + 'events' => 'event_under_admin_approval' + }.freeze before_action :check_curator before_action :set_breadcrumbs, :only => [:topic_suggestions] - # Hacky stub to make breadcrumbs work def index redirect_to '/curate/topic_suggestions' end @@ -57,15 +65,106 @@ def users end end + def resources + @status = params[:status].presence || 'requested' + @type = params[:type].presence || 'all' + + target_classes = @type == 'all' ? enabled_curatable_classes : [enabled_curatable_types[@type]].compact + + page = (params[:page] || 1).to_i + per_page = (params[:per_page] || 20).to_i + fetch_limit = page * per_page + + # Total count across enabled classes + total_count = target_classes.sum { |klass| fetch_curatable_records(klass).count } + + # Bounded fetch to prevent memory spikes on massive queues + records = target_classes.flat_map do |klass| + fetch_curatable_records(klass).order(updated_at: :desc).limit(fetch_limit).to_a + end + + sorted_records = records.sort_by(&:updated_at).reverse + + @resources = WillPaginate::Collection.create(page, per_page, total_count) do |pager| + pager.replace(sorted_records[pager.offset, pager.per_page] || []) + end + + respond_to(&:html) + end + + def bulk_approve + status = params[:status].presence || 'requested' + type = params[:type].presence || 'all' + action = params[:approve_action] + new_status = action == 'reject' ? 'not_approved' : 'approved' + target_classes = type == 'all' ? enabled_curatable_classes : [enabled_curatable_types[type]].compact + + updated_count = 0 + target_classes.each do |klass| + fetch_curatable_records(klass).find_each do |resource| + updated_count += 1 if resource.update(approval_status: new_status) + end + end + + redirect_to params[:redirect_to].presence || curate_resources_path, + notice: "#{updated_count} resource(s) successfully #{new_status.humanize.downcase}." + end + private + def enabled_curatable_types + CURATABLE_TYPES.select { |type, _| TeSS::Config.feature[APPROVAL_FEATURE_FLAGS[type]] } + end + + def enabled_curatable_classes + enabled_curatable_types.values + end + def action_count_for(action) return PublicActivity::Activity.where(key: action).group_by{|logs| logs.owner}.sort_by{|user, logs| -logs.count}.map{|user,logs| [user, logs.count]}.to_h end + def fetch_curatable_records(klass) + scope = ApplicationPolicy::Scope.new(pundit_user, klass).resolve_curatable + + # Default to 'requested' if params[:status] is blank + status = params[:status].presence || 'requested' + if status != 'all' + status_code = klass::APPROVAL_STATUS_CODES[status.to_sym] || status + scope = scope.where(approval_status: status_code) + end + + scope = scope.where(content_provider_id: params[:content_provider_id]) if params[:content_provider_id].present? + scope = scope.where(user_id: params[:user_id]) if params[:user_id].present? && klass.reflect_on_association(:user) + + if params[:space_id].present? && klass.reflect_on_association(:space) + if params[:space_id] == 'default' && Space.respond_to?(:default) + scope = scope.where(space_id: [Space.default&.id, nil]) + else + scope = scope.where(space_id: params[:space_id]) + end + end + + scope.includes(:user, :content_provider, (:space if klass.reflect_on_association(:space))) + end + def check_curator - unless current_user && (current_user.is_admin? || current_user.is_curator?) + unless current_user && (current_user.is_admin? || current_user.is_curator? || current_user.has_role_in_any_space?('admin')) handle_error(:forbidden, 'This page is only visible to curators.') end end -end + + def recent_resource_approvals + activity_ids = enabled_curatable_classes.flat_map do |klass| + allowed_ids = ApplicationPolicy::Scope.new(pundit_user, klass).resolve_curatable.pluck(:id) + key = "#{klass.name.underscore}.approval_status_changed" + PublicActivity::Activity.where(trackable_type: klass.name, trackable_id: allowed_ids, key: key).pluck(:id) + end + + PublicActivity::Activity.where(id: activity_ids) + .includes(:owner, trackable: [:content_provider, :space]) + .order(created_at: :desc) + .limit(10) + end + helper_method :recent_resource_approvals +end \ No newline at end of file diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index bdfe153fb..efb59965b 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -3,7 +3,7 @@ # The controller for actions related to the Events model class EventsController < ApplicationController before_action :ensure_feature_enabled - before_action :set_event, only: %i[show edit clone update destroy update_collections add_term reject_term + before_action :set_event, only: %i[show edit clone update destroy update_collections request_approval add_term reject_term redirect report update_report add_data reject_data] before_action :set_breadcrumbs before_action :disable_pagination, only: :index, if: ->(controller) { controller.request.format.ics? or controller.request.format.csv? or controller.request.format.rss? } @@ -16,7 +16,7 @@ class EventsController < ApplicationController # GET /events # GET /events.json def index - @bioschemas = @events.flat_map(&:to_bioschemas) + @bioschemas = @events.select { |e| policy(e).show? }.flat_map(&:to_bioschemas) # `policy(m).show?` allows to display JSON-LD metadata of events that are visible in the UI to the user, whether unauth, basic, curator or admin @past_events_count = 0 if TeSS::Config.solr_enabled && @events.none? && @facet_params[:include_expired] != 'true' && request.format.html? @past_events_count = Event.search_and_filter(current_user, @@ -158,6 +158,11 @@ def create @event.user = current_user @event.space = current_space + # apply approval_status from params only if the built record's policy allows it + if params[:event].present? && params[:event].key?(:approval_status) && policy(@event).approve? + @event.approval_status = params[:event][:approval_status] + end + respond_to do |format| if @event.save @event.create_activity :create, owner: current_user @@ -175,9 +180,10 @@ def create def update authorize @event respond_to do |format| - if @event.update(event_params) + if @event.update(event_params(@event)) @event.create_activity(:update, owner: current_user) if @event.log_update_activity? - format.html { redirect_to @event, notice: 'Event was successfully updated.' } + redirect_path = params[:redirect_to].presence || @event + format.html { redirect_to redirect_path, notice: 'Event was successfully updated.' } format.json { render :show, status: :ok, location: @event } else format.html { render :edit } @@ -217,6 +223,24 @@ def update_collections redirect_to @event end + # POST /events/1/request_approval + def request_approval + authorize @event + + if @event.approval_requested? + flash[:error] = 'Approval request has already been submitted.' + elsif @event.approved? + flash[:error] = 'Already approved.' + elsif @event.not_approved? + @event.request_approval + flash[:notice] = 'Approval request was sent successfully.' + end + + respond_to do |format| + format.html { redirect_to @event } + end + end + def redirect @event.widget_logs.create(widget_name: params[:widget], action: "#{controller_name}##{action_name}", @@ -234,21 +258,30 @@ def set_event end # Never trust parameters from the scary internet, only allow the white list through. - def event_params - params.require(:event).permit(:external_id, :title, :subtitle, :url, :organizer, :last_scraped, :scraper_record, - :description, { scientific_topic_names: [] }, { scientific_topic_uris: [] }, - { operation_names: [] }, { operation_uris: [] }, { event_types: [] }, - { keywords: [] }, { fields: [] }, :start, :end, :duration, { sponsors: [] }, - :online, :venue, :city, :county, :country, :postcode, :latitude, :longitude, - :timezone, :content_provider_id, { collection_ids: [] }, { node_ids: [] }, - { node_names: [] }, { target_audience: [] }, { eligibility: [] }, :visible, - { host_institutions: [] }, :capacity, :contact, :recognition, :learning_objectives, - :prerequisites, :tech_requirements, :cost_basis, :cost_value, :cost_currency, :language, - :presence, :origin_uri, - external_resources_attributes: %i[id url title _destroy], - external_resources: %i[url title], material_ids: [], - llm_interaction_attributes: %i[id scrape_or_process model prompt input output needs_processing _destroy], - locked_fields: [], instructors: [:name, :orcid], contributors: [:name, :orcid]) + # When a record is provided, gate approval-specific fields using the record-level policy (policy(record).approve?) + # For create flows where the record is not yet built, approval fields are applied explicitly after building the record. + def event_params(record = nil) + permitted = [:external_id, :title, :subtitle, :url, :organizer, :last_scraped, :scraper_record, + :description, { scientific_topic_names: [] }, { scientific_topic_uris: [] }, + { operation_names: [] }, { operation_uris: [] }, { event_types: [] }, + { keywords: [] }, { fields: [] }, :start, :end, :duration, { sponsors: [] }, + :online, :venue, :city, :county, :country, :postcode, :latitude, :longitude, + :timezone, :content_provider_id, { collection_ids: [] }, { node_ids: [] }, + { node_names: [] }, { target_audience: [] }, { eligibility: [] }, :visible, + { host_institutions: [] }, :capacity, :contact, :recognition, :learning_objectives, + :prerequisites, :tech_requirements, :cost_basis, :cost_value, :cost_currency, :language, + :presence, :origin_uri, + external_resources_attributes: %i[id url title _destroy], + external_resources: %i[url title], material_ids: [], + llm_interaction_attributes: %i[id scrape_or_process model prompt input output needs_processing _destroy], + locked_fields: [], instructors: [:name, :orcid], contributors: [:name, :orcid]] + + # Only permit approval_status when a record is present and the record-level policy allows approval + if record && policy(record).approve? + permitted << :approval_status + end + + params.require(:event).permit(permitted) end def event_report_params diff --git a/app/controllers/materials_controller.rb b/app/controllers/materials_controller.rb index 9a53fa0d3..c4a5ef582 100644 --- a/app/controllers/materials_controller.rb +++ b/app/controllers/materials_controller.rb @@ -1,7 +1,7 @@ # The controller for actions related to the Materials model class MaterialsController < ApplicationController before_action :ensure_feature_enabled - before_action :set_material, only: %i[show edit update destroy update_collections clone + before_action :set_material, only: %i[show edit update destroy update_collections request_approval clone add_term reject_term add_data reject_data] before_action :set_breadcrumbs before_action :set_learning_path_navigation, only: :show @@ -18,7 +18,7 @@ class MaterialsController < ApplicationController def index elearning = @facet_params[:resource_type] == 'e-learning' && feature_enabled?('elearning_materials') - @bioschemas = @materials.flat_map(&:to_bioschemas) + @bioschemas = @materials.select { |m| policy(m).show? }.flat_map(&:to_bioschemas) # `policy(m).show?` allows to display JSON-LD metadata of materials that are visible in the UI to the user, whether unauth, basic, curator or admin respond_to do |format| format.html { render elearning ? 'elearning_materials/index' : 'index' } format.json @@ -97,6 +97,11 @@ def create @material.user = current_user @material.space = current_space + # apply approval_status from params only if the built record's policy allows it + if params[:material].present? && params[:material].key?(:approval_status) && policy(@material).approve? + @material.approval_status = params[:material][:approval_status] + end + respond_to do |format| if @material.save @material.create_activity :create, owner: current_user @@ -114,9 +119,10 @@ def create def update authorize @material respond_to do |format| - if @material.update(material_params) + if @material.update(material_params(@material)) @material.create_activity(:update, owner: current_user) if @material.log_update_activity? - format.html { redirect_to @material, notice: 'Material was successfully updated.' } + redirect_path = params[:redirect_to].presence || @material + format.html { redirect_to redirect_path, notice: 'Material was successfully updated.' } format.json { render :show, status: :ok, location: @material } else format.html { render :edit } @@ -156,6 +162,24 @@ def update_collections redirect_to @material end + # POST /materials/1/request_approval + def request_approval + authorize @material + + if @material.approval_requested? + flash[:error] = 'Approval request has already been submitted.' + elsif @material.approved? + flash[:error] = 'Already approved.' + elsif @material.not_approved? + @material.request_approval + flash[:notice] = 'Approval request was sent successfully.' + end + + respond_to do |format| + format.html { redirect_to @material } + end + end + private # Use callbacks to share common setup or constraints between actions. @@ -164,23 +188,30 @@ def set_material end # Never trust parameters from the scary internet, only allow the white list through. - def material_params - params.require(:material).permit(:id, :title, :url, :contact, :description, :short_description, - :long_description, :doi, :licence, - :last_scraped, :scraper_record, :remote_created_date, :remote_updated_date, - :content_provider_id, :difficulty_level, :version, :status, - :date_created, :date_modified, :date_published, :other_types, - :prerequisites, :syllabus, :visible, :learning_objectives, :origin_uri, { subsets: [] }, - { target_audience: [] }, - { collection_ids: [] }, { keywords: [] }, { resource_type: [] }, - { scientific_topic_names: [] }, { scientific_topic_uris: [] }, - { operation_names: [] }, { operation_uris: [] }, - { node_ids: [] }, { node_names: [] }, { fields: [] }, - { authors: [:name, :orcid] }, { contributors: [:name, :orcid] }, # Structured - { authors: [] }, { contributors: [] }, # as strings - external_resources_attributes: %i[id url title _destroy], - external_resources: %i[url title], - event_ids: [], locked_fields: []) + def material_params(record = nil) + permitted = [:id, :title, :url, :contact, :description, :short_description, + :long_description, :doi, :licence, + :last_scraped, :scraper_record, :remote_created_date, :remote_updated_date, + :content_provider_id, :difficulty_level, :version, :status, + :date_created, :date_modified, :date_published, :other_types, + :prerequisites, :syllabus, :visible, :learning_objectives, :origin_uri, { subsets: [] }, + { target_audience: [] }, + { collection_ids: [] }, { keywords: [] }, { resource_type: [] }, + { scientific_topic_names: [] }, { scientific_topic_uris: [] }, + { operation_names: [] }, { operation_uris: [] }, + { node_ids: [] }, { node_names: [] }, { fields: [] }, + { authors: [:name, :orcid] }, { contributors: [:name, :orcid] }, # Structured + { authors: [] }, { contributors: [] }, # as strings + external_resources_attributes: %i[id url title _destroy], + external_resources: %i[url title], + event_ids: [], locked_fields: []] + + # Only permit approval_status when a record is present and the record-level policy allows approval + if record && policy(record).approve? + permitted << :approval_status + end + + params.require(:material).permit(permitted) end def set_learning_path_navigation diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f390e6f15..19615cc64 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -3,6 +3,7 @@ # The core application helper module ApplicationHelper IGNORED_FILTERS = %w[user].freeze + CURATION_FILTERS = %w[approval_status].freeze # add here the attributes shown as filters to be hidden from users but not from admin and curators # def bootstrap_class_for flash_type # { success: "alert-success", error: "alert-danger", alert: "alert-warning", notice: "alert-info" }[flash_type] || flash_type.to_s @@ -31,6 +32,8 @@ module ApplicationHelper check: { icon: 'fa-check', message: 'This resource is enabled' }, cross: { icon: 'fa-times', message: 'This resource has been disabled' }, exchanged: { icon: 'fa-exchange', message: 'This resource originated from another TeSS registry' }, + approval_requested: { icon: 'fa-eye', message: 'This resource has been requested an approval' }, + not_approved: { icon: 'fa-ban', message: 'This resource has been rejected by an admin' } }.freeze # Countries that have priority in the country selection menu. Using ISO 3166-1 Alpha2 code. @@ -86,6 +89,18 @@ def suggestion_icon(record, size = nil) "#{icon_for(:suggestion, size)}".html_safe end + def approval_requested_icon(record, size = nil) + return unless record.approval_requested? + + "#{icon_for(:approval_requested, size)}".html_safe + end + + def not_approved_icon(record, size = nil) + return unless record.not_approved? + + "#{icon_for(:not_approved, size)}".html_safe + end + def event_status_icon(event, size = nil) if event.started? "#{icon_for(:started, size)}".html_safe @@ -647,12 +662,11 @@ def cookie_consent end def available_facets(resources) - if (selected_facets = TeSS::Config.solr_facets&.fetch(controller_name, nil)) - indices = selected_facets.map { |name| resources.facets.index { |f| f.field_name.to_s == name } }.compact - resources.facets.values_at(*indices) - else - resources.facets - end.select { |f| f.rows.any? && !IGNORED_FILTERS.include?(f.field_name.to_s) } + filter_facets(resources) { |name| !IGNORED_FILTERS.include?(name) && !CURATION_FILTERS.include?(name) } + end + + def curation_facets(resources) + filter_facets(resources) { |name| CURATION_FILTERS.include?(name) } end def render_language_name(code) @@ -742,4 +756,22 @@ def flatten_params(prefix, value) [[prefix, value]] end end + + def filter_facets(resources) + raw_facets = resources.facets + selected_names = TeSS::Config.solr_facets&.fetch(controller_name, nil) + + facets = if selected_names + indices = selected_names.map { |name| raw_facets.index { |f| f.field_name.to_s == name } }.compact + raw_facets.values_at(*indices) + else + raw_facets + end + + facets.select do |f| + name = f.field_name.to_s + f.rows.any? && yield(name) + end + end + end diff --git a/app/helpers/curators_helper.rb b/app/helpers/curators_helper.rb index 3c912f57e..82d828ede 100644 --- a/app/helpers/curators_helper.rb +++ b/app/helpers/curators_helper.rb @@ -52,4 +52,28 @@ def approval_message(role_id) content_tag(:span, text, class: css_class) end + + def curator_space_options(user) + is_global = user.is_admin? || user.is_curator? + + if is_global + options = [] + options << ['Default Space', 'default'] if Space.respond_to?(:default) && Space.default + options + Space.where.not(id: Space.default&.id).order(:title).map { |s| [s.title, s.id] } + else + admin_space_ids = user.space_roles.where(key: 'admin').pluck(:space_id) + Space.where(id: admin_space_ids).order(:title).map { |s| [s.title, s.id] } + end + end + + def resource_approval_badge(status) + case status.to_sym + when :not_approved + content_tag(:span, t("resources.approval_status.#{status}"), class: 'label label-danger') + when :requested + content_tag(:span, t("resources.approval_status.#{status}"), class: 'label label-warning') + when :approved + content_tag(:span, t("resources.approval_status.#{status}"), class: 'label label-success') + end + end end diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 4a72a3a08..bbde9b6b1 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -163,4 +163,12 @@ def neatly_printed_date_range(start, finish = nil) end end end + + def approval_options_for_select_event + Event::APPROVAL_STATUS.values.map { |status| [t("events.approval_status.#{status}"), status] } + end + + def event_enabled_badge(enabled) + content_tag(:span, enabled ? 'Enabled' : 'Disabled', class: "label label-#{enabled ? 'success' : 'danger'}") + end end diff --git a/app/helpers/materials_helper.rb b/app/helpers/materials_helper.rb index c8a91f3cb..a02f57d6e 100644 --- a/app/helpers/materials_helper.rb +++ b/app/helpers/materials_helper.rb @@ -169,4 +169,23 @@ def keywords_and_topics(resource, limit: nil) tags << '…'.html_safe if limit_exceeded safe_join(tags, ' ') end + + def approval_options_for_select_material + Material::APPROVAL_STATUS.values.map { |status| [t("materials.approval_status.#{status}"), status] } + end + + def material_enabled_badge(enabled) + content_tag(:span, enabled ? 'Enabled' : 'Disabled', class: "label label-#{enabled ? 'success' : 'danger'}") + end + + def material_approval_badge(status) + case status.to_sym + when :not_approved + content_tag(:span, t("materials.approval_status.#{status}"), class: 'label label-danger') + when :requested + content_tag(:span, t("materials.approval_status.#{status}"), class: 'label label-warning') + when :approved + content_tag(:span, t("materials.approval_status.#{status}"), class: 'label label-success') + end + end end diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 6073cb1bd..82e409f3c 100644 --- a/app/models/concerns/searchable.rb +++ b/app/models/concerns/searchable.rb @@ -19,6 +19,15 @@ def search_and_facet_keys @search_and_facet_keys ||= ([:q] | facet_keys_with_multiple) end + def user_can_curate_unapproved_in_space?(user, space) + return false unless user + return true if user.is_admin? + return true if space && !space.default? && user.has_space_role?(space, 'admin') + return true if user.is_curator? && (space.nil? || space.default? || !space.is_private) + + false + end + # Searches and filters resources of the including model using Solr (via Sunspot), # applying space-based visibility, facets, sorting and pagination in a single query. # @@ -59,6 +68,13 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s inaccessible_space_ids = Space.where(is_private: true).pluck(:id) end + filter_material_approval = name == 'Material' && + TeSS::Config.feature['material_under_admin_approval'] && + !user_can_curate_unapproved_in_space?(user, space) + filter_event_approval = name == 'Event' && + TeSS::Config.feature['event_under_admin_approval'] && + !user_can_curate_unapproved_in_space?(user, space) + search(include: includes) do if has_space if space && !space.default? @@ -86,20 +102,36 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s if sort_by && sort_by != 'default' case sort_by - when 'early' then order_by(:start, :asc) - when 'late' then order_by(:start, :desc) - when 'rel' then nil - when 'mod' then order_by(:updated_at, :desc) - when 'new' then order_by(:created_at, :desc) - when 'finished' then order_by(:finished_at, :desc) - else order_by(:sort_title, sort_by.to_sym) + when 'early' + # Sort by start date asc + order_by(:start, :asc) + when 'late' + # Sort by start date desc + order_by(:start, :desc) + when 'rel' + # Sort by relevance + when 'mod' + # Sort by last modified + order_by(:updated_at, :desc) + when 'new' + # Sort by newest + order_by(:created_at, :desc) + when 'finished' + # Sort by last finished + order_by(:finished_at, :desc) + else + order_by(:sort_title, sort_by.to_sym) end else case name - when 'Event' then order_by(:start, :asc) - when 'ContentProvider' then order_by(:count, :desc) - when 'Material' then order_by(:created_at, :desc) - else order_by(:sort_title, :asc) + when 'Event' + order_by(:start, :asc) + when 'ContentProvider' + order_by(:count, :desc) + when 'Material' + order_by(:approval_status, :asc) + else + order_by(:sort_title, :asc) end end @@ -114,7 +146,7 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s if name == 'Trainer' || name == 'Profile' any_of { with(:public, true) } - elsif has_public && !user&.is_admin? + elsif has_public && !(user&.is_admin? || user&.is_curator?) any_of do with(:public, true) with(:user_id, user.id) if user @@ -122,6 +154,20 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s end end + if filter_material_approval + any_of do + with(:approval_status, I18n.t('materials.approval_status.approved')) + with(:user_id, user.id) if user.present? + end + end + + if filter_event_approval + any_of do + with(:approval_status, I18n.t('events.approval_status.approved')) + with(:user_id, user.id) if user.present? + end + end + facet_fields.each do |ff| facet ff, exclude: active_facets[ff] end diff --git a/app/models/event.rb b/app/models/event.rb index 030b25f10..100515003 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -32,6 +32,14 @@ class Event < ApplicationRecord before_save :geocoding_cache_lookup, if: :address_will_change? after_save :enqueue_geocoding_worker, if: :address_changed? + APPROVAL_STATUS = { + 0 => :not_approved, + 1 => :requested, + 2 => :approved + }.freeze + + APPROVAL_STATUS_CODES = APPROVAL_STATUS.invert.freeze + if TeSS::Config.solr_enabled # :nocov: searchable do @@ -122,6 +130,9 @@ class Event < ApplicationRecord string :instructors, multiple: true do instructors.map(&:display_name) end + string :approval_status do + I18n.t("events.approval_status.#{approval_status}") + end end # :nocov: end @@ -162,6 +173,11 @@ class Event < ApplicationRecord validate :allowed_url validates :origin_uri, url: { allow_blank: true } + validates :approval_status, inclusion: { in: APPROVAL_STATUS.values } + before_create :set_approval_status + before_update :log_approval_status_change + before_update :reset_approval_status, if: :approval_required_and_content_changed? + clean_array_fields(:keywords, :fields, :event_types, :target_audience, :eligibility, :host_institutions, :sponsors) update_suggestions(:keywords, :target_audience, :host_institutions) @@ -217,7 +233,7 @@ def expired? def self.facet_fields field_list = %w[ content_provider keywords scientific_topics operations tools fields online event_types start venue city country organizer sponsors target_audience eligibility language - user node collections contributors instructors] + user node collections contributors instructors approval_status] field_list.delete('operations') if TeSS::Config.feature['disabled'].include? 'operations' field_list.delete('scientific_topics') if TeSS::Config.feature['disabled'].include? 'topics' @@ -568,6 +584,44 @@ def to_oai_dc xml.target! end + def self.approved + where(approval_status: APPROVAL_STATUS_CODES[:approved]) + end + + def self.approval_requested + where(approval_status: APPROVAL_STATUS_CODES[:requested]) + end + + def approval_status + APPROVAL_STATUS[super.to_i] || APPROVAL_STATUS[0] + end + + def approval_status=(key) + super(APPROVAL_STATUS_CODES[key.to_sym]) + end + + def not_approved? + approval_status == :not_approved + end + + def approved? + approval_status == :approved + end + + def approval_requested? + approval_status == :requested + end + + def request_approval + self.approval_status = :requested + save! + # CurationMailer.materials_require_approval(self, User.current_user).deliver_later + end + + def self.approval_required? + TeSS::Config.feature['event_under_admin_approval'] && !User.current_user&.is_admin? + end + private def allowed_url @@ -626,4 +680,40 @@ def fix_keywords def presence_default self.presence = :onsite if presence.blank? end + + def set_approval_status + # sets to `:approved` when the feature is off + unless TeSS::Config.feature['event_under_admin_approval'] + self.approval_status = :approved + end + # sets to `:requested` by default when the feature is on + if self.class.approval_required? + self.approval_status = :requested + end + end + + def approval_required_and_content_changed? + return false unless self.class.approval_required? + return false if approval_status_changed? # Avoid triggering on status changes itself + + # Ignore standard system/audit columns + ignored_columns = %w[updated_at created_at approval_status user_id] + (changed - ignored_columns).any? + end + + def reset_approval_status + self.approval_status = :requested + end + + def log_approval_status_change + if approval_status_changed? + old = (APPROVAL_STATUS[approval_status_before_last_save.to_i] || APPROVAL_STATUS[0]).to_s + new = approval_status.to_s + create_activity(:approval_status_changed, owner: User.current_user, parameters: { old: old, new: new }) + end + end + + def loggable_changes + super - %w[approval_status] + end end diff --git a/app/models/material.rb b/app/models/material.rb index 934edff78..824419426 100644 --- a/app/models/material.rb +++ b/app/models/material.rb @@ -23,6 +23,14 @@ class Material < ApplicationRecord include InSpace include HasPeople + APPROVAL_STATUS = { + 0 => :not_approved, + 1 => :requested, + 2 => :approved + }.freeze + + APPROVAL_STATUS_CODES = APPROVAL_STATUS.invert.freeze + if TeSS::Config.solr_enabled # :nocov: searchable do @@ -94,6 +102,9 @@ class Material < ApplicationRecord string :status do MaterialStatusDictionary.instance.lookup_value(status, 'title') end + string :approval_status do + I18n.t("materials.approval_status.#{approval_status}") + end end # :nocov: end @@ -125,6 +136,11 @@ class Material < ApplicationRecord validates :keywords, length: { maximum: 20 } validates :origin_uri, url: { allow_blank: true } + validates :approval_status, inclusion: { in: APPROVAL_STATUS.values } + before_create :set_approval_status + before_update :log_approval_status_change + before_update :reset_approval_status, if: :approval_required_and_content_changed? + clean_array_fields(:keywords, :fields, :target_audience, :resource_type, :subsets) @@ -147,7 +163,7 @@ def long_description=(desc) def self.facet_fields field_list = %w[scientific_topics operations tools standard_database_or_policy content_provider keywords difficulty_level fields licence target_audience authors contributors resource_type - related_resources user node collections status] + related_resources user node collections status approval_status] field_list.delete('operations') if TeSS::Config.feature['disabled'].include? 'operations' field_list.delete('scientific_topics') if TeSS::Config.feature['disabled'].include? 'topics' @@ -258,4 +274,80 @@ def to_oai_dc end xml.target! end + + def self.approved + where(approval_status: APPROVAL_STATUS_CODES[:approved]) + end + + def self.approval_requested + where(approval_status: APPROVAL_STATUS_CODES[:requested]) + end + + def approval_status + APPROVAL_STATUS[super.to_i] || APPROVAL_STATUS[0] + end + + def approval_status=(key) + super(APPROVAL_STATUS_CODES[key.to_sym]) + end + + def not_approved? + approval_status == :not_approved + end + + def approved? + approval_status == :approved + end + + def approval_requested? + approval_status == :requested + end + + def request_approval + self.approval_status = :requested + save! + # CurationMailer.materials_require_approval(self, User.current_user).deliver_later + end + + def self.approval_required? + TeSS::Config.feature['material_under_admin_approval'] && !User.current_user&.is_admin? + end + + private + + def set_approval_status + # sets to `:approved` when the feature is off + unless TeSS::Config.feature['material_under_admin_approval'] + self.approval_status = :approved + end + # sets to `:requested` by default when the feature is on + if self.class.approval_required? + self.approval_status = :requested + end + end + + def approval_required_and_content_changed? + return false unless self.class.approval_required? + return false if approval_status_changed? # Avoid triggering on status changes itself + + # Ignore standard system/audit columns + ignored_columns = %w[updated_at created_at approval_status user_id] + (changed - ignored_columns).any? + end + + def reset_approval_status + self.approval_status = :requested + end + + def log_approval_status_change + if approval_status_changed? + old = (APPROVAL_STATUS[approval_status_before_last_save.to_i] || APPROVAL_STATUS[0]).to_s + new = approval_status.to_s + create_activity(:approval_status_changed, owner: User.current_user, parameters: { old: old, new: new }) + end + end + + def loggable_changes + super - %w[approval_status] + end end diff --git a/app/models/user.rb b/app/models/user.rb index dea538778..1fe9de2e3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -135,6 +135,10 @@ def is_curator? self.has_role?('curator') end + def can_approve_resources? + self.is_admin? || self.is_curator? || has_role_in_any_space?(:admin) + end + def maintained_collections Collection.left_outer_joins(:collaborators).where(collaborators: { id: id }).or(Collection.where(user_id: id)).distinct end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index d4bcb5ac8..2136eee71 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -90,34 +90,41 @@ def curators_and_admin user_has_role?(:curator, :admin, :scraper_user) end + def user_can_approve_resources? + @user&.can_approve_resources? + end + # Returns:: the default Pundit policy scope for the record's class. def scope Pundit.policy_scope!(user, record.class) end - # Determines whether the record should be visible to the current user, - # based on the private/public status of its associated space. +# Determines whether the record should be visible to the current user, + # considering space privacy and resource approval status. # # Rules: - # * if the record has no associated space, it is always shown; - # * if the associated space is not private, it is always shown; - # * otherwise, an authenticated user is shown the record only if they are - # an admin, or belong to at least one of the space's groups (and only - # when the space in question is the current space, or the record *is* - # the space itself). + # * Site administrator privileges: Always visible. + # * Space accessibility: + # - Public records or records outside private spaces are accessible. + # - Private space records are accessible to authenticated site admins, + # space admins, or members of the space's assigned groups. + # * Approval status (when approval feature is enabled): + # - Approved records follow standard space accessibility rules. + # - Unapproved records require BOTH space accessibility AND one of: + # * Site administrator privileges; + # * Curator privileges for public/default-space records; + # * Record ownership (@record.user_id == @user.id); + # * Space administrator privileges for the record's space. # # Returns:: +true+ or +false+. def shown? - return true if @space == nil - return true if !@space.is_private - return false unless @user # and so if space is private - if @space == Space.current_space || @record == @space - user_groups = @user.groups.pluck(:id) - space_groups = @space.groups.pluck(:id) - return @user.is_admin? || @user.groups.where(id: @space.groups).any? - end + return true if @user&.is_admin? # Resource is always shown to admin + return false unless space_accessible? # If the resource can be shown in the space continue + return true unless approval_enabled? && # If there is the approval feature enabled continue + @record.respond_to?(:approved?) && + !@record.approved? - return false + unapproved_accessible? # Who can still access the unapproved resource end # Default Pundit policy scope class. @@ -139,6 +146,35 @@ def initialize(context, scope) def resolve scope end + + # Determines the database scope of records a user is authorized to curate/approve. + # + # Rules: + # * Site administrators: All records across all spaces. + # * Curators: Records in public spaces (or with no space) + private spaces where they hold space admin roles. + # * Space administrators: Records strictly within their assigned spaces. + # * Non-admin / Regular users: No records (empty relation). + # + # Returns:: +ActiveRecord::Relation+. + def resolve_curatable + return scope.none unless user + return scope.all if user.is_admin? + + return scope.none unless scope.reflect_on_association(:space) + + admin_space_ids = user.space_roles.where(key: 'admin').pluck(:space_id) + + if user.is_curator? + # Public spaces (or no space) + private spaces where user is admin + scope.left_outer_joins(:space) + .where(spaces: { is_private: [false, nil] }) + .or(scope.where(space_id: admin_space_ids)) + else + # Space admins can only see their explicit spaces + return scope.none if admin_space_ids.empty? + scope.where(space_id: admin_space_ids) + end + end end private @@ -166,7 +202,41 @@ def scraper? def user_has_role?(*roles) return false if @user.nil? roles.any? { |r| @user.has_role?(r) } || - (@space && roles.any? { |r| @user.has_space_role?(@space, r) }) + (@space && roles.any? { |r| @user&.has_space_role?(@space, r) }) end + # Can the resource be shown in the space? + def space_accessible? + return true if @user&.has_space_role?(@space, 'admin') # Continue if it is the admin space + return true if @space == nil # Continue if we're in the default space + return true if !@space.is_private # Continue if the space is not private + return false unless @user + if @space == Space.current_space || @record == @space + return @user.groups.where(id: @space.groups).any? # Continue if the user is part of the space + end + + return false + end + + def approval_enabled? + case @record + when Material + TeSS::Config.feature['material_under_admin_approval'] + when Event + TeSS::Config.feature['event_under_admin_approval'] + else + false + end + end + + # Who can still access the resource when it is unapproved? + def unapproved_accessible? + return false unless @user + + space = @record.respond_to?(:space) ? @record.space : @space + + return true if @user.is_curator? && (space.nil? || !space.is_private) # Curators can see unapproved public resources + return true if @record.respond_to?(:user_id) && @record.user_id == @user.id # Resource can be shown to its owner + return true if space.present? && @user&.has_space_role?(space, 'admin') # Resource can be shown to its space admin + end end \ No newline at end of file diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb index a6510c3c5..0b4cc9d0d 100644 --- a/app/policies/event_policy.rb +++ b/app/policies/event_policy.rb @@ -1,5 +1,29 @@ class EventPolicy < ScrapedResourcePolicy + class Scope < ApplicationPolicy::Scope + def resolve + return scope unless TeSS::Config.feature['event_under_admin_approval'] + return scope if user&.is_admin? + approved_scope = scope.approved + return approved_scope unless user + + user_conditions = scope.where(user_id: user.id) + + if scope.reflect_on_association(:space) + admin_space_ids = user.space_roles.where(key: 'admin').pluck(:space_id) + user_conditions = user_conditions.or(scope.where(space_id: admin_space_ids)) if admin_space_ids.any? + if user.is_curator? + public_space_ids = Space.where(is_private: false).pluck(:id) + user_conditions = user_conditions.or(scope.where(space_id: [nil, *public_space_ids])) + end + end + + approved_scope.or(user_conditions) + end + end + + # Determines whether the record should be visible to the current user, + # considering space privacy and resource approval status. def show? super && shown? end @@ -13,7 +37,35 @@ def view_report? end def clone? - manage? + manage? || curators_and_admin + end + + # For the edit and clone buttons + def update? + manage? || curators_and_admin || user&.has_space_role?(record.space, :admin) end + # For the approve rights + def approve? + approval_enabled? && curatable_by_user? + end + + def request_approval? + approval_enabled? && curatable_by_user? + end + + private + + def approval_enabled? + TeSS::Config.feature['event_under_admin_approval'] + end + + def curatable_by_user? + return false unless user + return true if user.is_admin? + return true if user.has_space_role?(record.space, 'admin') + return true if user.is_curator? && (record.space.nil? || !record.space.is_private) + + false + end end diff --git a/app/policies/material_policy.rb b/app/policies/material_policy.rb index f45c95bb3..81ce3f345 100644 --- a/app/policies/material_policy.rb +++ b/app/policies/material_policy.rb @@ -1,11 +1,63 @@ class MaterialPolicy < ScrapedResourcePolicy + class Scope < ApplicationPolicy::Scope + def resolve + return scope unless TeSS::Config.feature['material_under_admin_approval'] + return scope if user&.is_admin? + approved_scope = scope.approved + return approved_scope unless user + + user_conditions = scope.where(user_id: user.id) + + if scope.reflect_on_association(:space) + admin_space_ids = user.space_roles.where(key: 'admin').pluck(:space_id) + user_conditions = user_conditions.or(scope.where(space_id: admin_space_ids)) if admin_space_ids.any? + if user.is_curator? + public_space_ids = Space.where(is_private: false).pluck(:id) + user_conditions = user_conditions.or(scope.where(space_id: [nil, *public_space_ids])) + end + end + + approved_scope.or(user_conditions) + end + end + + # Determines whether the record should be visible to the current user, + # considering space privacy and resource approval status. def show? super && shown? end def clone? - manage? + manage? || curators_and_admin + end + + # For the edit and clone buttons + def update? + manage? || curators_and_admin || user&.has_space_role?(record.space, :admin) end + # To check if it has approval rights + def approve? + approval_enabled? && curatable_by_user? + end + + def request_approval? + approval_enabled? && curatable_by_user? + end + + private + + def approval_enabled? + TeSS::Config.feature['material_under_admin_approval'] + end + + def curatable_by_user? + return false unless user + return true if user.is_admin? + return true if user.has_space_role?(record.space, 'admin') + return true if user.is_curator? && (record.space.nil? || !record.space.is_private) + + false + end end \ No newline at end of file diff --git a/app/views/curator/_resource_card.html.erb b/app/views/curator/_resource_card.html.erb new file mode 100644 index 000000000..96fa9f35d --- /dev/null +++ b/app/views/curator/_resource_card.html.erb @@ -0,0 +1,76 @@ +
+
+ <%= resource.class.model_name.human %> + <%= link_to resource.title, resource, target: '_blank' %> + + <%= respond_to?("#{resource.model_name.singular}_approval_badge") ? + send("#{resource.model_name.singular}_approval_badge", resource.approval_status) : + resource_approval_badge(resource.approval_status) %> + +
+ <% unless resource.approved? %> + <%= link_to 'Approve', + polymorphic_path(resource, + resource.class.model_name.param_key => { approval_status: 'approved' }, + redirect_to: request.fullpath), + method: :put, + class: 'btn btn-xs btn-success', + data: { confirm: 'Approve this resource?' } %> + <% end %> + <% unless resource.not_approved? %> + <%= link_to 'Reject', + polymorphic_path(resource, + resource.class.model_name.param_key => { approval_status: 'not_approved' }, + redirect_to: request.fullpath), + method: :put, + class: 'btn btn-xs btn-danger', + data: { confirm: 'Reject this resource?' } %> + <% end %> +
+
+ +
+
+
+

+ Created <%= time_ago_in_words(resource.created_at) %> ago + <% if resource.user %> + by <%= link_to resource.user.username, resource.user, target: '_blank' %> + <% end %> + <% if resource.respond_to?(:content_provider) && resource.content_provider %> + | Provider: <%= link_to resource.content_provider.title, resource.content_provider, target: '_blank' %> + <% end %> + <% if resource.respond_to?(:space) && resource.space %> + | + <% if resource.space.is_private %> + + Private + <% end %> + Space: <%= link_to resource.space.title, space_path(resource.space), target: '_blank' %> + <% else %> + | Space: Default + <% end %> +

+ + <% if resource.respond_to?(:url) && resource.url.present? %> +

URL: <%= link_to resource.url, resource.url, target: '_blank', rel: 'nofollow noreferrer' %>

+ <% end %> + + <% if resource.respond_to?(:short_description) && resource.short_description.present? %> +

<%= truncate(resource.short_description, length: 200) %>

+ <% elsif resource.respond_to?(:description) && resource.description.present? %> +

<%= truncate(strip_tags(resource.description), length: 200) %>

+ <% end %> +
+ +
+ + Last updated: <%= time_ago_in_words(resource.updated_at) %> ago
+ <% if resource.respond_to?(:scraper_record) && resource.scraper_record? %> + Ingested Record + <% end %> + +
+
+
+
\ No newline at end of file diff --git a/app/views/curator/materials.html.erb b/app/views/curator/materials.html.erb new file mode 100644 index 000000000..f803c9026 --- /dev/null +++ b/app/views/curator/materials.html.erb @@ -0,0 +1,62 @@ +<% filter_params = params.permit(:status, :content_provider_id).to_h %> + +
+

Material Curation Queue

+ +
+
+ <%= label_tag 'status', 'Approval Status:' %> + <%= select_tag 'status', + options_for_select([['Requested', 'requested'], ['Not Approved', 'not_approved'], ['Approved', 'approved'], ['All', 'all']], @status), + onchange: "window.location = '#{curate_materials_path}?' + $.param($.extend(#{filter_params.to_json}, { status: this.value }));", + class: 'form-control input-sm' %> +
+ +
+ <%= label_tag 'content_provider_id', 'Provider:' %> + <%= select_tag 'content_provider_id', + options_from_collection_for_select(ContentProvider.order(:title), :id, :title, params[:content_provider_id]), + include_blank: 'All Providers', + onchange: "window.location = '#{curate_materials_path}?' + $.param($.extend(#{filter_params.to_json}, { content_provider_id: this.value }));", + class: 'form-control input-sm' %> +
+
+
+ +
+ +
+
+
Recent Approval Activity
+
+ <% activities = recent_resource_approvals %> + <% if activities.any? %> +
    + <% activities.each do |activity| %> + <% next unless activity.trackable %> +
  • + <%= link_to activity.trackable.title, activity.trackable, target: '_blank' %>
    + <%= resource_approval_badge(activity.parameters[:new]) %> + + by <%= activity.owner.try(:username) || 'System' %> <%= time_ago_in_words(activity.created_at) %> ago + +
    • + <% end %> +
+ <% else %> + No recent approval activity. + <% end %> +
+
+
+ + +
+ <% if @materials.any? %> + <%= render partial: 'curator/resource_card', collection: @materials, as: :resource %> + <%= render partial: 'search/common/pagination_bar', locals: { resources: @materials } %> + <% else %> +
No materials found matching this criteria.
+ <% end %> +
+
\ No newline at end of file diff --git a/app/views/curator/resources.html.erb b/app/views/curator/resources.html.erb new file mode 100644 index 000000000..ac37b0fcb --- /dev/null +++ b/app/views/curator/resources.html.erb @@ -0,0 +1,146 @@ +<% filter_params = params.permit(:status, :type, :content_provider_id, :space_id, :user_id).to_h %> +<% is_global = current_user.is_admin? || current_user.is_curator? %> + +
+ <% if @resources.any? %> +
+ <%= link_to bulk_approve_curate_resources_path(filter_params.merge(approve_action: 'approve', redirect_to: request.fullpath)), + method: :post, + class: 'btn btn-success', + data: { confirm: "Approve ALL #{@resources.total_entries} filtered resources?" } do %> + Approve All Filtered (<%= @resources.total_entries %>) + <% end %> + + <%= link_to bulk_approve_curate_resources_path(filter_params.merge(approve_action: 'reject', redirect_to: request.fullpath)), + method: :post, + class: 'btn btn-danger', + data: { confirm: "Reject ALL #{@resources.total_entries} filtered resources?" } do %> + Reject All Filtered (<%= @resources.total_entries %>) + <% end %> +
+ <% end %> + +

Curate Resources

+ +

Filters

+
+ <%= link_to curate_resources_path, class: 'btn btn-default btn-sm', title: 'Reset all filters' do %> + Reset Filters + <% end %> +
+ +
+ <%= label_tag 'type', 'Type:' %> + <%= select_tag 'type', + options_for_select([['All Types', 'all'], ['Materials', 'materials'], ['Events', 'events']], @type), + onchange: "window.location = '#{curate_resources_path}?' + $.param($.extend(#{filter_params.to_json}, { type: this.value, page: 1 }));", + class: 'form-control input-sm' %> +
+ +
+ <%= label_tag 'user_id', 'User:' %> + <%= select_tag 'user_id', + options_from_collection_for_select(User.order(:username), :id, :username, params[:user_id]), + include_blank: 'All Users', + onchange: "window.location = '#{curate_resources_path}?' + $.param($.extend(#{filter_params.to_json}, { user_id: this.value, page: 1 }));", + class: 'form-control input-sm' %> +
+ + <% if TeSS::Config.feature['sources'] %> +
+ <%= label_tag 'content_provider_id', 'Provider:' %> + <%= select_tag 'content_provider_id', + options_from_collection_for_select(ContentProvider.order(:title), :id, :title, params[:content_provider_id]), + include_blank: 'All Providers', + onchange: "window.location = '#{curate_resources_path}?' + $.param($.extend(#{filter_params.to_json}, { content_provider_id: this.value, page: 1 }));", + class: 'form-control input-sm' %> +
+ <% end %> + + <% if TeSS::Config.feature['spaces'] %> +
+ <%= label_tag 'space_id', 'Space:' %> + <%= select_tag 'space_id', + options_for_select(curator_space_options(current_user), params[:space_id]), + include_blank: (is_global ? 'All Spaces' : 'All My Spaces'), + onchange: "window.location = '#{curate_resources_path}?' + $.param($.extend(#{filter_params.to_json}, { space_id: this.value, page: 1 }));", + class: 'form-control input-sm' %> +
+ <% end %> + +
+ <%= label_tag 'status', 'Approval status:' %> + <%= select_tag 'status', + options_for_select([['Requested', 'requested'], ['Not Approved', 'not_approved'], ['Approved', 'approved'], ['All', 'all']], @status), + onchange: "window.location = '#{curate_resources_path}?' + $.param($.extend(#{filter_params.to_json}, { status: this.value, page: 1 }));", + class: 'form-control input-sm' %> +
+
+ +
+ +
+
+
Recent Approval Activity
+
+ <% activities = recent_resource_approvals %> + <% if activities.any? %> +
    + <% activities.each do |activity| %> + <% resource = activity.trackable %> + <% next unless resource %> +
  • + <%= activity.trackable_type %> + <%= link_to resource.title, resource, target: '_blank' %> + + <% if TeSS::Config.feature['sources'] && resource.respond_to?(:content_provider) && resource.content_provider %> + | Provider: <%= link_to resource.content_provider.title, resource.content_provider, target: '_blank' %> + <% end %> + + <% if TeSS::Config.feature['spaces'] %> + <% if resource.respond_to?(:space) && resource.space %> + | + <% if resource.space.is_private %> + + Private + <% end %> + Space: <%= link_to resource.space.title, space_path(resource.space), target: '_blank' %> + <% else %> + | Space: Default + <% end %> + <% end %> +
    + <% + raw_params = activity.parameters + params_hash = if raw_params.is_a?(Hash) + raw_params.with_indifferent_access + elsif raw_params.is_a?(String) + (JSON.parse(raw_params) rescue YAML.safe_load(raw_params) rescue {}).with_indifferent_access + else + {} + end + %> + <%= resource_approval_badge(params_hash[:new]) %> + + by <%= activity.owner.try(:username) || 'System' %> <%= time_ago_in_words(activity.created_at) %> ago + +
    • + <% end %> +
+ <% else %> + No recent approval activity. + <% end %> +
+
+
+ + +
+ <% if @resources.any? %> + <%= render partial: 'curator/resource_card', collection: @resources, as: :resource %> + <%= render partial: 'search/common/pagination_bar', locals: { resources: @resources } %> + <% else %> +
No resources found matching this criteria.
+ <% end %> +
+
\ No newline at end of file diff --git a/app/views/events/_event.html.erb b/app/views/events/_event.html.erb index 8bf1fd2b0..cd1eb43ed 100644 --- a/app/views/events/_event.html.erb +++ b/app/views/events/_event.html.erb @@ -21,6 +21,8 @@ <%= event_status_icon(event) %> <%= exchange_icon(event) %> + <%= approval_requested_icon(event) %> + <%= not_approved_icon(event) %> <% if event.event_types.any? %> <% event.event_types.each do |t| %> diff --git a/app/views/events/_form.html.erb b/app/views/events/_form.html.erb index 7dbb92a6b..ef7836c04 100644 --- a/app/views/events/_form.html.erb +++ b/app/views/events/_form.html.erb @@ -181,6 +181,16 @@ <%= render partial: "common/people_form", locals: { f: f, field_name: 'instructors', role: 'instructor' } %> + + <% if policy(@event).approve? %> + <%= f.input :approval_status, + label: 'Approval Status', + collection: approval_options_for_select_event, + include_blank: false %> + <% end %> + +
+
<%= f.submit(class: 'btn btn-primary') %> <%= link_to 'Back', :back, class: 'btn btn-default' %> diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb index cab70d890..dc1ee3ea5 100644 --- a/app/views/events/index.html.erb +++ b/app/views/events/index.html.erb @@ -1,3 +1,5 @@ +<% can_approve_resources = current_user&.can_approve_resources? %> +
<% if TeSS::Config.solr_enabled %>
+ <% if TeSS::Config.feature['event_under_admin_approval'] %> + <% unless policy(@event).approve? %> +
<%= t('events.info.approval_notice') %>

+ <% end %> + <% end %> + <%= render :partial => 'form' %>
diff --git a/app/views/events/show.html.erb b/app/views/events/show.html.erb index e263ed2b8..b7171a141 100644 --- a/app/views/events/show.html.erb +++ b/app/views/events/show.html.erb @@ -48,6 +48,17 @@
<% if @event.persisted? %>
+ <% if policy(@event).update? %> +

+ Approval status: + <%= resource_approval_badge(@event.approval_status) %> +

+ + <% if policy(@event).approve? && !@event.approved? %> + <%= link_to 'Approve', event_path(@event, event: { approval_status: 'approved' }), + method: :put, class: 'btn btn-success' %> + <% end %> + <% end %> <% if policy(@event).update? %> <%= link_to 'Clone', clone_event_path(@event), class: 'btn btn-default' %> <%= edit_button(@event) %> diff --git a/app/views/layouts/_user_menu.html.erb b/app/views/layouts/_user_menu.html.erb index 89edda352..94c6bab7f 100644 --- a/app/views/layouts/_user_menu.html.erb +++ b/app/views/layouts/_user_menu.html.erb @@ -20,7 +20,8 @@ <% is_curator = current_user.is_curator? %> <% is_owner_in_any_group = current_user.is_owner_in_any_group? %> <% is_current_space_admin = !Space.current_space.default? && current_user.has_space_role?(Space.current_space, :admin) %> - <% if is_admin || is_curator || is_current_space_admin || is_owner_in_any_group %> + <% is_space_admin = current_user.has_role_in_any_space?(:admin) %> + <% if is_admin || is_curator || is_current_space_admin || is_space_admin || is_owner_in_any_group %>
  • <%= t('menu.user.administration') %>
    • @@ -33,7 +34,7 @@ <% end %> - <% if TeSS::Config.feature['sources'] %> + <% if TeSS::Config.feature['sources'] && !is_space_admin %>
  • <%= link_to sources_path, title: t('menu.user.view_ingestion_sources') do %> <%= t('menu.user.view_sources') %> @@ -68,6 +69,14 @@
    • <% end %> + <% if (TeSS::Config.feature['material_under_admin_approval'] || TeSS::Config.feature['event_under_admin_approval']) && (is_admin || is_curator || is_space_admin) %> +
  • + <%= link_to curate_resources_path do %> + <%= t('menu.user.curate_resources', default: 'Curate Resources') %> + <% end %> +
    • + <% end %> + <% if is_admin %>
  • <%= link_to rails_admin_path, title: t('menu.user.view_admin_console') do %> diff --git a/app/views/materials/_form.html.erb b/app/views/materials/_form.html.erb index 6c994e3a0..3e4b379f2 100644 --- a/app/views/materials/_form.html.erb +++ b/app/views/materials/_form.html.erb @@ -187,6 +187,14 @@ title: t('materials.hints.fields') } %> <% end %> + + <% if policy(@material).approve? %> + <%= f.input :approval_status, + label: 'Approval Status', + collection: approval_options_for_select_material, + include_blank: false %> + <% end %> +
    diff --git a/app/views/materials/_material.html.erb b/app/views/materials/_material.html.erb index 154116419..1e337715f 100644 --- a/app/views/materials/_material.html.erb +++ b/app/views/materials/_material.html.erb @@ -19,6 +19,8 @@ <%= suggestion_icon(material) %> <% end %> <%= exchange_icon(material) %> + <%= approval_requested_icon(material) %> + <%= not_approved_icon(material) %>
    <%= display_attribute_no_label(material, :resource_type) { |values| values.join(', ') } %> diff --git a/app/views/materials/index.html.erb b/app/views/materials/index.html.erb index 70ba2f33d..b89690b5b 100644 --- a/app/views/materials/index.html.erb +++ b/app/views/materials/index.html.erb @@ -1,3 +1,5 @@ +<% can_approve_resources = current_user&.can_approve_resources? %> +
    <% if TeSS::Config.solr_enabled %>
    + <% if TeSS::Config.feature['material_under_admin_approval'] %> + <% unless policy(@material).approve? %> +
    <%= t('materials.info.approval_notice') %>

    + <% end %> + <% end %> + <%= render :partial => 'form' %>
    diff --git a/app/views/materials/show.html.erb b/app/views/materials/show.html.erb index 845806bd6..ad193dc65 100644 --- a/app/views/materials/show.html.erb +++ b/app/views/materials/show.html.erb @@ -39,6 +39,17 @@
    <% if @material.persisted? %>
    + <% if policy(@material).update? %> +

    + Approval status: + <%= resource_approval_badge(@material.approval_status) %> +

    + + <% if policy(@material).approve? && !@material.approved? %> + <%= link_to 'Approve', material_path(@material, material: { approval_status: 'approved' }), + method: :put, class: 'btn btn-success' %> + <% end %> + <% end %> <% if policy(@material).update? %> <%= link_to 'Clone', clone_material_path(@material), class: 'btn btn-default' %> <%= edit_button(@material) %> diff --git a/app/views/search/common/_facet_sidebar.html.erb b/app/views/search/common/_facet_sidebar.html.erb index 0411ec3d7..fc6e0aa6c 100644 --- a/app/views/search/common/_facet_sidebar.html.erb +++ b/app/views/search/common/_facet_sidebar.html.erb @@ -109,5 +109,5 @@ Variable that should be available <% if current_user && (current_user.is_curator? || current_user.is_admin?) %> - <%= render partial: 'search/common/facet_sidebar_curation', locals: { resource_type: resource_type } %> + <%= render partial: 'search/common/facet_sidebar_curation', locals: { resources: resources, resource_type: resource_type } %> <% end %> diff --git a/app/views/search/common/_facet_sidebar_curation.html.erb b/app/views/search/common/_facet_sidebar_curation.html.erb index 0a9d7587a..c9c130650 100644 --- a/app/views/search/common/_facet_sidebar_curation.html.erb +++ b/app/views/search/common/_facet_sidebar_curation.html.erb @@ -1,5 +1,6 @@ <% show_last_scraped_filter = resource_type.method_defined?(:last_scraped) %> <% show_hidden_filter = resource_type.method_defined?(:user_requires_approval?) %> +<% resource_name = resource_type.model_name.human.downcase.pluralize %> <% if show_last_scraped_filter || show_hidden_filter %>
      @@ -38,6 +39,16 @@ <% end %> + <% boolean_facets, regular_facets = curation_facets(resources).partition { |f| f.field_name == :online } %> + + <% regular_facets.reject{ |f| f.field_name.to_s == 'start' }.each do |facet| %> + <%= render partial: 'search/common/facet_sidebar_filter', locals: { facet: facet, resources: resources } %> + <% end %> + + <% boolean_facets.each do |facet| %> + <%= render partial: 'search/common/facet_sidebar_boolean_filter', locals: { facet: facet, resources: resources } %> + <% end %> + <% if show_hidden_filter %> <%= render partial: 'search/common/facet_sidebar_boolean_filter', locals: { facet_field: 'include_hidden', diff --git a/config/locales/en.yml b/config/locales/en.yml index e98585567..85c2441a5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -672,6 +672,12 @@ en: show_past_events: one: Show %{count} past event other: Show %{count} past events + info: + approval_notice: This event will need to be approved by an administrator before being public. + approval_status: + not_approved: Not approved + requested: Requested + approved: Approved materials: hints: authors: > @@ -731,6 +737,12 @@ en: A specific iteration or release of the training material document, labeled to track updates (for example, v1.0, v2.1). visible: 'Whether this material should be shown or hidden from the materials page.' + info: + approval_notice: This material will need to be approved by an administrator before being public. + approval_status: + not_approved: Not approved + requested: Requested + approved: Approved learning_paths: hints: authors: > @@ -778,6 +790,11 @@ en: all_topics: All topics select_topic: Select a topic... search_for_topics: Search for topics... + resources: + approval_status: + not_approved: Not approved + requested: Requested + approved: Approved profile: user: disclaimer: > @@ -1000,6 +1017,9 @@ en: assign_scientific_topics: "Assign scientific topics to %{title} resources" curate_topics: 'Curate topics' curate_users: 'Curate users' + curate_materials: 'Curate materials' + curate_events: 'Curate events' + curate_resources: 'Curate resources' view_admin_console: 'View the administrator console' admin_console: 'Admin console' log_out: 'Log out' diff --git a/config/routes.rb b/config/routes.rb index 627892161..1ee1fdb29 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -73,6 +73,7 @@ post :add_data post :reject_term post :reject_data + post :request_approval get :report patch :report, to: 'events#update_report' get :clone @@ -113,6 +114,7 @@ post :reject_data post :add_term post :add_data + post :request_approval get :clone end collection do @@ -158,6 +160,10 @@ get 'curate/topic_suggestions' => 'curator#topic_suggestions' get 'curate/users' => 'curator#users' + get 'curate/materials' => 'curator#resources', defaults: { type: 'materials' }, as: :curate_materials + get 'curate/events' => 'curator#resources', defaults: { type: 'events' }, as: :curate_events + get 'curate/resources' => 'curator#resources', as: :curate_resources + post '/curate/resources/bulk_approve', to: 'curator#bulk_approve', as: :bulk_approve_curate_resources get 'curate' => 'curator#index' get 'fairsharing/search' => 'fairsharing#search' diff --git a/config/tess.example.yml b/config/tess.example.yml index 5d0352295..fb0327e96 100644 --- a/config/tess.example.yml +++ b/config/tess.example.yml @@ -169,6 +169,8 @@ default: &default collection_curation: true auto_parse_vars: [] # available features to auto parse from description: ['keywords', 'target_audience'] controlled_vocabulary_vars: [] # available features: ['target_audience'] + material_under_admin_approval: false # this is the material curation, if true, a material needs to be approved by an admin to be online, if false, all registered materials are automatically public + event_under_admin_approval: false # this is the event curation, if true, an event needs to be approved by an admin to be online, if false, all registered events are automatically public # User login invitation: false diff --git a/db/migrate/20260113122337_add_approval_status_to_materials_and_events.rb b/db/migrate/20260113122337_add_approval_status_to_materials_and_events.rb new file mode 100644 index 000000000..85c779823 --- /dev/null +++ b/db/migrate/20260113122337_add_approval_status_to_materials_and_events.rb @@ -0,0 +1,14 @@ +class AddApprovalStatusToMaterialsAndEvents < ActiveRecord::Migration[7.2] + def change + add_column :materials, :approval_status, :string, default: '2' + add_column :events, :approval_status, :string, default: '2' + + reversible do |dir| + dir.up do + # Backfill existing records (will be NULL on existing rows when adding a column) + Material.unscoped.where(approval_status: nil).update_all(approval_status: '2') + Event.unscoped.where(approval_status: nil).update_all(approval_status: '2') + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index ed474f683..7b9d50add 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -235,6 +235,7 @@ t.integer "user_id" t.text "venue" t.boolean "visible", default: true + t.string "approval_status" t.index ["last_scraped_by_id"], name: "index_events_on_last_scraped_by_id" t.index ["presence"], name: "index_events_on_presence" t.index ["slug"], name: "index_events_on_slug", unique: true @@ -417,6 +418,7 @@ t.integer "user_id" t.string "version" t.boolean "visible", default: true + t.string "approval_status" t.index ["content_provider_id"], name: "index_materials_on_content_provider_id" t.index ["last_scraped_by_id"], name: "index_materials_on_last_scraped_by_id" t.index ["slug"], name: "index_materials_on_slug", unique: true diff --git a/test/controllers/curator_controller_test.rb b/test/controllers/curator_controller_test.rb index a56c600f4..4ffccf702 100644 --- a/test/controllers/curator_controller_test.rb +++ b/test/controllers/curator_controller_test.rb @@ -3,6 +3,26 @@ class CuratorControllerTest < ActionController::TestCase include Devise::Test::ControllerHelpers + setup do + @admin = users(:admin) + sign_in @admin + + @user = users(:regular_user) + @another_user = users(:another_regular_user) + + @requested_material = materials(:requested_material) + @approved_material = materials(:approved_material) + @default_space = Space.default + + @requested_astro_space_material = materials(:requested_astro_space_material) + @astro_space = spaces(:astro) + + @curator = users(:curator) + @space_admin = users(:private_space_owner) + @private_space = spaces(:private_space) + @private_material = materials(:requested_private_material) + end + test 'should get topic suggestions if curator' do sign_in users(:curator) e1 = add_topic_suggestions(events(:one), ['Genomics', 'Animals']) @@ -225,6 +245,184 @@ class CuratorControllerTest < ActionController::TestCase end end + test 'should get resources queue with default requested status' do + with_settings(feature: { material_under_admin_approval: true }) do + get :resources + assert_response :success + assert_not_nil assigns(:resources) + assert_includes assigns(:resources), @requested_material + refute_includes assigns(:resources), @approved_material + end + end + + test 'should filter resources by type and status' do + with_settings(feature: { material_under_admin_approval: true }) do + get :resources, params: { type: 'materials', status: 'approved' } + assert_response :success + assert_includes assigns(:resources), @approved_material + refute_includes assigns(:resources), @requested_material + end + end + + test 'should filter resources by user_id' do + with_settings(feature: { material_under_admin_approval: true }) do + get :resources, params: { user_id: @user.id, status: 'all' } + assert_response :success + assert_includes assigns(:resources), @requested_material + + get :resources, params: { user_id: @another_user.id, status: 'all' } + assert_response :success + refute_includes assigns(:resources), @requested_material + end + end + + test 'should bulk approve requested resources' do + with_settings(feature: { material_under_admin_approval: true }) do + assert_equal :requested, @requested_material.reload.approval_status + + post :bulk_approve, params: { status: 'requested', type: 'materials', approve_action: 'approve' } + + assert_redirected_to curate_resources_path + assert_equal :approved, @requested_material.reload.approval_status + assert_equal :approved, @requested_astro_space_material.reload.approval_status + assert_equal '3 resource(s) successfully approved.', flash[:notice] + end + end + + test 'should bulk reject requested resources' do + with_settings(feature: { material_under_admin_approval: true }) do + post :bulk_approve, params: { status: 'requested', type: 'materials', approve_action: 'reject' } + + assert_redirected_to curate_resources_path + assert_equal :not_approved, @requested_material.reload.approval_status + assert_equal :not_approved, @requested_astro_space_material.reload.approval_status + assert_equal '3 resource(s) successfully not approved.', flash[:notice] + end + end + + test 'should honor user_id filter during bulk approve' do + with_settings(feature: { material_under_admin_approval: true }) do + post :bulk_approve, params: { status: 'requested', type: 'materials', user_id: @another_user.id, approve_action: 'approve' } + + assert_redirected_to curate_resources_path + assert_equal :requested, @requested_material.reload.approval_status + assert_equal '0 resource(s) successfully approved.', flash[:notice] + end + end + + test 'should filter resources by default space_id' do + with_settings(feature: { material_under_admin_approval: true }) do + get :resources, params: { space_id: 'default', status: 'requested' } + + assert_response :success + assert_includes assigns(:resources), @requested_material + refute_includes assigns(:resources), @requested_astro_space_material + end + end + + test 'should filter resources by explicit space_id' do + with_settings(feature: { material_under_admin_approval: true }) do + get :resources, params: { space_id: @astro_space.id, status: 'requested' } + + assert_response :success + assert_includes assigns(:resources), @requested_astro_space_material + refute_includes assigns(:resources), @requested_material + end + end + + test 'should bulk approve resources scoped by default space' do + with_settings(feature: { material_under_admin_approval: true }) do + post :bulk_approve, params: { + status: 'requested', + type: 'materials', + space_id: 'default', + approve_action: 'approve' + } + + assert_redirected_to curate_resources_path + assert_equal :approved, @requested_material.reload.approval_status + assert_equal :requested, @requested_astro_space_material.reload.approval_status + end + end + + test 'should bulk approve resources scoped by explicit space_id' do + with_settings(feature: { material_under_admin_approval: true }) do + post :bulk_approve, params: { + status: 'requested', + type: 'materials', + space_id: @astro_space.id, + approve_action: 'approve' + } + + assert_redirected_to curate_resources_path + assert_equal :approved, @requested_astro_space_material.reload.approval_status + assert_equal :requested, @requested_material.reload.approval_status + end + end + + test 'curator can bulk approve visible public resources but not inaccessible private space resources' do + with_settings(feature: { material_under_admin_approval: true }) do + sign_in @curator + with_settings(feature: { spaces: true, material_under_admin_approval: true }) do + + assert_equal :requested, @requested_material.reload.approval_status + assert_equal :requested, @requested_astro_space_material.reload.approval_status + assert_equal :requested, @private_material.reload.approval_status + + post :bulk_approve, params: { status: 'requested', type: 'materials', approve_action: 'approve' } + + assert_redirected_to curate_resources_path + assert_equal :approved, @requested_material.reload.approval_status + assert_equal :approved, @requested_astro_space_material.reload.approval_status + assert_equal :requested, @private_material.reload.approval_status + assert_equal '2 resource(s) successfully approved.', flash[:notice] + end + end + end + + test 'private space admin can bulk approve and reject only resources in their private space' do + with_settings(feature: { material_under_admin_approval: true }) do + # Ensure user has space admin role so check_curator passes + @space_admin.space_roles.find_or_create_by!(space: @private_space, key: 'admin') + + sign_in @space_admin + + with_settings(feature: { spaces: true, material_under_admin_approval: true }) do + # Test Bulk Approve + post :bulk_approve, params: { status: 'requested', type: 'materials', approve_action: 'approve' } + + assert_redirected_to curate_resources_path + assert_equal :approved, @private_material.reload.approval_status + assert_equal :requested, @requested_material.reload.approval_status + assert_equal '1 resource(s) successfully approved.', flash[:notice] + + # Test Bulk Reject + post :bulk_approve, params: { status: 'approved', type: 'materials', approve_action: 'reject' } + + assert_redirected_to curate_resources_path + assert_equal :not_approved, @private_material.reload.approval_status + assert_equal :requested, @requested_material.reload.approval_status + assert_equal '1 resource(s) successfully not approved.', flash[:notice] + end + end + end + + test 'should ignore disabled approval resource types in resources listing and bulk actions' do + with_settings(feature: { material_under_admin_approval: false }) do + TeSS::Config.feature['material_under_admin_approval'] = false + TeSS::Config.feature['event_under_admin_approval'] = true + + get :resources, params: { type: 'materials', status: 'requested' } + assert_response :success + assert_empty assigns(:resources) + + post :bulk_approve, params: { status: 'requested', type: 'materials', approve_action: 'reject' } + assert_redirected_to curate_resources_path + assert_equal :requested, @requested_material.reload.approval_status + assert_equal '0 resource(s) successfully not approved.', flash[:notice] + end + end + private def add_topic_suggestions(resource, topic_names = []) @@ -241,8 +439,8 @@ def add_topic_activity(resource, topic, user) resource.edit_suggestion.accept_suggestion('scientific_topics', topic) resource.create_activity :add_term, - owner: user, - recipient: resource.user, - parameters: log_params + owner: user, + recipient: resource.user, + parameters: log_params end end diff --git a/test/controllers/events_controller_test.rb b/test/controllers/events_controller_test.rb index 984f6a0a6..aeeb4cc67 100644 --- a/test/controllers/events_controller_test.rb +++ b/test/controllers/events_controller_test.rb @@ -23,7 +23,7 @@ class EventsControllerTest < ActionController::TestCase @mandatory_fields = { online: true, start: @event.start, end: @event.end, organizer: @event.organizer, host_institutions: @event.host_institutions, timezone: @event.timezone, contact: @event.contact, eligibility: @event.eligibility } - end +end # Tests # INDEX, NEW, EDIT, CREATE, SHOW, BREADCRUMBS, TABS, API CHECKS @@ -1719,4 +1719,59 @@ class EventsControllerTest < ActionController::TestCase assert_select '#origin-info a[href=?]', uri end + + test "should request approval when event is not approved" do + sign_in users(:admin) + with_settings(feature: { event_under_admin_approval: true }) do + assert_changes -> { @event.reload.approval_status }, to: :requested do + post :request_approval, params: { + id: @event.id + } + end + assert_redirected_to event_path(@event) + assert_equal 'Approval request was sent successfully.', flash[:notice] + end + end + + test "should not request approval and alert if already requested" do + sign_in users(:admin) + @event.update!(approval_status: :requested) + + with_settings(feature: { event_under_admin_approval: true }) do + assert_no_changes -> { @event.approval_status } do + post :request_approval, params: { + id: @event.id + } + end + assert_redirected_to event_path(@event) + assert_equal 'Approval request has already been submitted.', flash[:error] + end + end + + test "should not request approval and alert if already approved" do + sign_in users(:admin) + @event.update!(approval_status: :approved) + + with_settings(feature: { event_under_admin_approval: true }) do + assert_no_changes -> { @event.approval_status } do + post :request_approval, params: { + id: @event.id + } + end + assert_redirected_to event_path(@event) + assert_equal 'Already approved.', flash[:error] + end + end + + test "should deny request_approval for unauthorized users" do + with_settings(feature: { event_under_admin_approval: true }) do + assert_no_changes -> { @event.reload.approval_status } do + post :request_approval, params: { + id: @event.id + } + end + assert_response :found + assert_equal :not_approved, @event.reload.approval_status + end + end end diff --git a/test/controllers/materials_controller_test.rb b/test/controllers/materials_controller_test.rb index 21064bf9a..936f4b6b3 100644 --- a/test/controllers/materials_controller_test.rb +++ b/test/controllers/materials_controller_test.rb @@ -22,6 +22,9 @@ class MaterialsControllerTest < ActionController::TestCase @failing_material = materials(:failing_material) @failing_material.title = 'Fail!' @monitor = @failing_material.create_link_monitor(url: @failing_material.url, code: 404, fail_count: 5) + + @requested_material = materials(:requested_material) + @approved_material = materials(:approved_material) end # Tests @@ -1860,4 +1863,56 @@ class MaterialsControllerTest < ActionController::TestCase end end end + + test "should request approval when material is not approved" do + sign_in users(:admin) + with_settings(feature: { material_under_admin_approval: true }) do + assert_changes -> { @material.reload.approval_status }, to: :requested do + post :request_approval, params: { + id: @material.id + } + end + assert_redirected_to material_path(@material) + assert_equal 'Approval request was sent successfully.', flash[:notice] + end + end + + test "should not request approval and alert if already requested" do + sign_in users(:admin) + + with_settings(feature: { material_under_admin_approval: true }) do + assert_no_changes -> { @requested_material.reload.approval_status } do + post :request_approval, params: { + id: @requested_material.id + } + end + assert_redirected_to material_path(@requested_material) + assert_equal 'Approval request has already been submitted.', flash[:error] + end + end + + test "should not request approval and alert if already approved" do + sign_in users(:admin) + with_settings(feature: { material_under_admin_approval: true }) do + assert_no_changes -> { @approved_material.reload.approval_status } do + post :request_approval, params: { + id: @approved_material.id + } + end + assert_redirected_to material_path(@approved_material) + assert_equal 'Already approved.', flash[:error] + end + end + + test "should deny request_approval for unauthorized users" do + with_settings(feature: { material_under_admin_approval: true }) do + assert_no_changes -> { @material.reload.approval_status } do + post :request_approval, params: { + id: @material.id + } + end + assert_response :found + assert_equal :not_approved, @material.reload.approval_status + end + end end diff --git a/test/fixtures/materials.yml b/test/fixtures/materials.yml index 34de6f04d..7b64a6c31 100644 --- a/test/fixtures/materials.yml +++ b/test/fixtures/materials.yml @@ -30,6 +30,35 @@ bad_material: remote_updated_date: 2015-09-25 remote_created_date: 2015-09-25 +not_approved_material: + user: regular_user + title: Training Material Example 0 + url: http://training.com/trainers/0 + description: This is a Training Material produced by an example organization 0 + approval_status: '0' + +requested_material: + user: regular_user + title: Training Material Example 1 + url: http://training.com/trainers/1 + description: This is a Training Material produced by an example organization 1 + approval_status: '1' + +approved_material: + user: regular_user + title: Training Material Example 2 + url: http://training.com/trainers/2 + description: This is a Training Material produced by an example organization 2 + approval_status: '2' + +requested_private_material: + user: regular_user + title: Plant Training Material Example 1 + url: http://plant.training.com/trainers/1 + description: This is a Training Material produced by an example organization 1 + approval_status: '1' + space: private_space + biojs: user: regular_user title: BioJS @@ -198,6 +227,32 @@ plant_space_material: status: active space: plants +requested_astro_space_material: + user: regular_user + title: Astro material req + slug: astro-material-req + url: http://example.com/astro/material123-req + description: This is a material in the astro space requested + licence: CC-BY-4.0 + keywords: ['astro'] + content_provider: goblet + status: active + space: astro + approval_status: '1' + +approved_astro_space_material: + user: regular_user + title: Astro material app + slug: astro-material-app + url: http://example.com/astro/material123-app + description: This is a material in the astro space approved + licence: CC-BY-4.0 + keywords: ['astro'] + content_provider: goblet + status: active + space: astro + approval_status: '2' + passing_import_filters_material: user: regular_user title: does_match diff --git a/test/fixtures/spaces.yml b/test/fixtures/spaces.yml index 903f10857..67a99034f 100644 --- a/test/fixtures/spaces.yml +++ b/test/fixtures/spaces.yml @@ -12,3 +12,9 @@ other: title: Other TeSS Community host: other.mytess.training user: other_owner + +private_space: + title: Private Space + host: private.mytess.training + user: private_space_owner + is_private: true diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 8e006a238..41f84fc3f 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -184,3 +184,9 @@ other_owner: email: 'otherboss@example.com' encrypted_password: <%= Devise::Encryptor.digest(User, 'password') %> confirmed_at: <%= Time.zone.now %> + +private_space_owner: + username: 'private_boss' + email: 'privateboss@example.com' + encrypted_password: <%= Devise::Encryptor.digest(User, '12345678') %> + confirmed_at: <%= Time.zone.now %> diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index e7b38ad63..9a1994aa6 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -2,6 +2,8 @@ class ApplicationHelperTest < ActionView::TestCase + MockFacet = Struct.new(:field_name, :rows) + setup do @old_material = materials(:bad_material) @old_material.last_scraped = Time.parse('1912-04-14 23:40') @@ -18,6 +20,15 @@ class ApplicationHelperTest < ActionView::TestCase @failing_material = materials(:failing_material) @failing_material.title = 'Fail!' @monitor = @failing_material.create_link_monitor(url: @failing_material.url, code: 404, fail_count: 5) + + @controller = MaterialsController.new + @facet_standard = MockFacet.new('keywords', ['row1']) + @facet_curation = MockFacet.new('approval_status', ['row1']) + @facet_ignored = MockFacet.new('user', ['row1']) + @facet_empty = MockFacet.new('topics', []) + @all_facets = [@facet_standard, @facet_curation, @facet_ignored, @facet_empty] + @resources = Minitest::Mock.new + @resources.expect(:facets, @all_facets) end test "icon should be correct for material scraped today" do @@ -150,4 +161,75 @@ def current_user end =end + test "approval_requested_icon returns eye icon when material approval is requested" do + with_settings(feature: { material_under_admin_approval: true }) do + @new_material.update!(approval_status: :requested) + expected_result = "#{icon_for(:approval_requested, 'large')}".html_safe + assert_equal(approval_requested_icon(@new_material, 'large'), expected_result) + assert_match /fresh-icon/, expected_result + assert_match /fa-eye/, expected_result + end + end + + test "not_approved_icon returns fa-ban icon when material is not approved" do + with_settings(feature: { material_under_admin_approval: true }) do + @new_material.update!(approval_status: :not_approved) + expected_result = "#{icon_for(:not_approved, 'large')}".html_safe + assert_equal(not_approved_icon(@new_material, 'large'), expected_result) + assert_match /fresh-icon/, expected_result + assert_match /fa-ban/, expected_result + end + end + + test "approval_requested_icon returns eye icon when event approval is requested" do + with_settings(feature: { event_under_admin_approval: true }) do + @old_event.update!(approval_status: :requested) + expected_result = "#{icon_for(:approval_requested, 'large')}".html_safe + assert_equal(approval_requested_icon(@old_event, 'large'), expected_result) + assert_match /fresh-icon/, expected_result + assert_match /fa-eye/, expected_result + end + end + + test "not_approved_icon returns fa-ban icon when event is not approved" do + with_settings(feature: { event_under_admin_approval: true }) do + @old_event.update!(approval_status: :not_approved) + expected_result = "#{icon_for(:not_approved, 'large')}".html_safe + assert_equal(not_approved_icon(@old_event, 'large'), expected_result) + assert_match /fresh-icon/, expected_result + assert_match /fa-ban/, expected_result + end + end + + test "available_facets returns non-empty, non-curation, non-ignored facets" do + TeSS::Config.stub(:solr_facets, nil) do + results = available_facets(@resources) + + assert_includes results, @facet_standard + refute_includes results, @facet_curation + refute_includes results, @facet_ignored + refute_includes results, @facet_empty + end + end + + test "curation_facets returns only non-empty curation facets" do + TeSS::Config.stub(:solr_facets, nil) do + results = curation_facets(@resources) + + assert_includes results, @facet_curation + refute_includes results, @facet_standard + refute_includes results, @facet_ignored + refute_includes results, @facet_empty + end + end + + test "respects configured solr_facets for controller" do + config_mock = { 'materials' => ['approval_status', 'keywords'] } + @resources.expect(:facets, @all_facets) + + TeSS::Config.stub(:solr_facets, config_mock) do + assert_equal [@facet_standard], available_facets(@resources) + end + end + end \ No newline at end of file diff --git a/test/helpers/events_helper_test.rb b/test/helpers/events_helper_test.rb index 5f3d8d4c0..22d01023d 100644 --- a/test/helpers/events_helper_test.rb +++ b/test/helpers/events_helper_test.rb @@ -55,4 +55,29 @@ class EventsHelperTest < ActionView::TestCase assert_equal 'No date given', neatly_printed_date_range(nil, '') assert_equal 'No start date', neatly_printed_date_range(nil, DateTime.new(2024, 5, 16, 17)) end + + test 'approval_options_for_select_event returns mapped array of translated pairs' do + options = approval_options_for_select_event + + assert_kind_of Array, options + assert_equal Event::APPROVAL_STATUS.values.size, options.size + + Event::APPROVAL_STATUS.values.each do |status| + expected_label = I18n.t("events.approval_status.#{status}") + assert_includes options, [expected_label, status] + end + end + + test 'event_enabled_badge renders success label when true' do + badge = event_enabled_badge(true) + + assert_dom_equal 'Enabled', badge + end + + test 'event_enabled_badge renders danger label when false' do + badge = event_enabled_badge(false) + + assert_dom_equal 'Disabled', badge + end + end diff --git a/test/helpers/materials_helper_test.rb b/test/helpers/materials_helper_test.rb index f348b4fd1..36870602c 100644 --- a/test/helpers/materials_helper_test.rb +++ b/test/helpers/materials_helper_test.rb @@ -43,4 +43,50 @@ class MaterialsHelperTest < ActionView::TestCase assert_includes result, '…' end + + test 'approval_options_for_select_material returns mapped array of translated pairs' do + options = approval_options_for_select_material + + assert_kind_of Array, options + assert_equal Material::APPROVAL_STATUS.values.size, options.size + + Material::APPROVAL_STATUS.values.each do |status| + expected_label = I18n.t("materials.approval_status.#{status}") + assert_includes options, [expected_label, status] + end + end + + test 'material_enabled_badge renders success label when true' do + badge = material_enabled_badge(true) + + assert_dom_equal 'Enabled', badge + end + + test 'material_enabled_badge renders danger label when false' do + badge = material_enabled_badge(false) + + assert_dom_equal 'Disabled', badge + end + + test 'material_approval_badge renders correct labels for each status' do + statuses = { + not_approved: 'label-danger', + requested: 'label-warning', + approved: 'label-success' + } + + statuses.each do |status, expected_class| + label_text = I18n.t("materials.approval_status.#{status}") + + [status, status.to_s].each do |input| + badge = material_approval_badge(input) + assert_dom_equal "#{label_text}", badge + end + end + end + + test 'material_approval_badge returns nil for unknown status' do + assert_nil material_approval_badge(:unknown_status) + end end + diff --git a/test/models/content_provider_test.rb b/test/models/content_provider_test.rb index 9e609dc4e..635d6f4c9 100644 --- a/test/models/content_provider_test.rb +++ b/test/models/content_provider_test.rb @@ -124,7 +124,7 @@ class ContentProviderTest < ActiveSupport::TestCase with_settings(feature: { spaces: false }) do assert Space.current_space.default? materials = content_provider.materials.in_current_space - assert_equal 10, materials.length + assert_equal 12, materials.length assert_includes materials, materials(:plant_space_material) assert_includes materials, materials(:good_material) end diff --git a/test/models/event_test.rb b/test/models/event_test.rb index 55ae09150..2ad0f1fd5 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -896,4 +896,85 @@ class EventTest < ActiveSupport::TestCase assert_includes @event.instructors.map(&:name), person1.name assert_includes @event.instructors.map(&:name), person2.name end + + test '.approved scope returns only approved event' do + e1 = events(:one) + e2 = events(:two) + e3 = events(:month_long_event) + + e1.update!(approval_status: :approved) + e2.update!(approval_status: :requested) + e3.update!(approval_status: :not_approved) + + approved = Event.approved + assert_includes approved, e1 + refute_includes approved, e2 + refute_includes approved, e3 + end + + test '.approval_requested scope returns only requested event' do + e1 = events(:one) + e2 = events(:two) + + e1.update!(approval_status: :requested) + e2.update!(approval_status: :approved) + + requested = Event.approval_requested + assert_includes requested, e1 + refute_includes requested, e2 + end + + test 'approval status predicate methods work correctly' do + @event.update!(approval_status: :not_approved) + assert @event.not_approved? + refute @event.approved? + refute @event.approval_requested? + + @event.update!(approval_status: :requested) + assert @event.approval_requested? + refute @event.approved? + refute @event.not_approved? + + @event.update!(approval_status: :approved) + assert @event.approved? + refute @event.not_approved? + refute @event.approval_requested? + end + + test 'request_approval updates status to requested and saves record' do + @event.update!(approval_status: :not_approved) + assert_changes -> { @event.reload.approval_status }, to: :requested do + @event.request_approval + end + end + + test 'initializes approval_status as approved when feature flag is disabled' do + with_settings(feature: { event_under_admin_approval: false }) do + event = Event.create!(title: 'Test', url: 'https://example.org/test2', user: users(:regular_user), description: 'bla') + assert_equal :approved, event.reload.approval_status + end + end + + test 'initializes approval_status as requested when feature flag is enabled' do + with_settings(feature: { event_under_admin_approval: true }) do + event = Event.create!(title: 'Test', url: 'https://example.org/test2', user: users(:regular_user), description: 'bla') + assert_equal :requested, event.reload.approval_status + end + end + + test 'resets status to requested on update when url changes and feature is enabled' do + @event.update!(approval_status: :approved) + with_settings(feature: { event_under_admin_approval: true }) do + @event.update!(url: 'https://example.org/updated-url') + assert_equal :requested, @event.reload.approval_status + end + end + + test 'does not reset status on url change when feature is disabled' do + @event.update!(approval_status: :approved) + with_settings(feature: { event_under_admin_approval: false }) do + @event.update!(url: 'https://example.org/updated-url-2') + assert_equal :approved, @event.reload.approval_status + end + end end diff --git a/test/models/material_test.rb b/test/models/material_test.rb index f5e35e861..4bde87ab5 100644 --- a/test/models/material_test.rb +++ b/test/models/material_test.rb @@ -20,6 +20,10 @@ class MaterialTest < ActiveSupport::TestCase assert_not_nil @user assert_not_nil @event assert_not_nil @material + + @not_approved_material = materials(:not_approved_material) + @requested_material = materials(:requested_material) + @approved_material = materials(:approved_material) end test 'should update optionals' do @@ -768,4 +772,66 @@ class MaterialTest < ActiveSupport::TestCase @material.origin_uri = 'ftp://tess-instance.org/materials/123' refute @material.valid? end + + test '.approved scope returns only approved materials' do + approved = Material.approved + assert_includes approved, @approved_material + refute_includes approved, @not_approved_material + refute_includes approved, @requested_material + end + + test '.approval_requested scope returns only requested materials' do + requested = Material.approval_requested + assert_includes requested, @requested_material + refute_includes requested, @approved_material + refute_includes requested, @not_approved_material + end + + test 'approval status predicate methods work correctly' do + assert @not_approved_material.not_approved? + refute @not_approved_material.approved? + refute @not_approved_material.approval_requested? + + assert @requested_material.approval_requested? + refute @requested_material.approved? + refute @requested_material.not_approved? + + assert @approved_material.approved? + refute @approved_material.not_approved? + refute @approved_material.approval_requested? + end + + test 'request_approval updates status to requested and saves record' do + assert_changes -> { @not_approved_material.reload.approval_status }, to: :requested do + @not_approved_material.request_approval + end + end + + test 'initializes approval_status as approved when feature flag is disabled' do + with_settings(feature: { material_under_admin_approval: false }) do + material = Material.create!(title: 'Test', url: 'https://example.org/test2', user: users(:regular_user), description: 'bla') + assert_equal :approved, material.reload.approval_status + end + end + + test 'initializes approval_status as requested when feature flag is enabled' do + with_settings(feature: { material_under_admin_approval: true }) do + material = Material.create!(title: 'Test', url: 'https://example.org/test2', user: users(:regular_user), description: 'bla') + assert_equal :requested, material.reload.approval_status + end + end + + test 'resets status to requested on update when url changes and feature is enabled' do + with_settings(feature: { material_under_admin_approval: true }) do + @approved_material.update!(url: 'https://example.org/updated-url') + assert_equal :requested, @approved_material.reload.approval_status + end + end + + test 'does not reset status on url change when feature is disabled' do + with_settings(feature: { material_under_admin_approval: false }) do + @approved_material.update!(url: 'https://example.org/updated-url-2') + assert_equal :approved, @approved_material.reload.approval_status + end + end end diff --git a/test/policies/application_policy_test.rb b/test/policies/application_policy_test.rb new file mode 100644 index 000000000..0720450f5 --- /dev/null +++ b/test/policies/application_policy_test.rb @@ -0,0 +1,119 @@ +require 'test_helper' + +class ApplicationPolicyTest < ActiveSupport::TestCase + setup do + @admin = users(:admin) + @curator = users(:curator) # plant space admin + @regular_user = users(:regular_user) # requested_material owner + @another_regular_user = users(:another_regular_user) + @private_space_owner = users(:private_space_owner) + + @space = spaces(:plants) + @private_space = spaces(:private_space) + + @requested_material = materials(:requested_material) # does not belongs to plant space and owned by regular_user + @requested_private_material = materials(:requested_private_material) # belongs to private space and owned by regular_user + end + + def policy_for(user, record) + context = Struct.new(:user, :request).new(user) + ApplicationPolicy.new(context, record) + end + + test 'site admin always sees unapproved resources' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + assert policy_for(@admin, @requested_material).shown? + end + end + + test 'curator can see unapproved resource in public space' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + assert policy_for(@curator, @requested_material).shown? + end + end + + test 'curator cannot see unapproved resource in inaccessible private space' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + refute policy_for(@curator, @requested_private_material).shown? + end + end + + test 'curator with access to a private space cannot see another users unapproved resource' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + group = groups(:one) + @private_space.groups << group + group.users << @curator + + Space.with_current_space(@private_space) do + assert policy_for(@curator, @private_space).shown? + refute policy_for(@curator, @requested_private_material).shown? + end + end + end + + test 'record owner can see their own unapproved resource' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + assert policy_for(@regular_user, @requested_material).shown? + end + end + + test 'another record owner can not see another unapproved resource' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + refute policy_for(@another_regular_user, @requested_material).shown? + end + end + + test 'space admin can see unapproved resource in their managed space' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + assert policy_for(@curator, @requested_material).shown? + end + end + + test 'another regular user cannot see unapproved resource owned by someone else' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + refute policy_for(@another_regular_user, @requested_material).shown? + end + end + + # # --- Scope Tests (resolve_curatable) --- + + test 'resolve_curatable returns all records for site admin' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + context = Struct.new(:user).new(@admin) + scope = ApplicationPolicy::Scope.new(context, Material).resolve_curatable + assert_equal Material.count, scope.count + end + end + + test 'resolve_curatable returns public and managed private spaces for curator' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + context = Struct.new(:user).new(@curator) + scope = ApplicationPolicy::Scope.new(context, Material).resolve_curatable + + # Should include materials in public spaces or without space + scope.each do |material| + assert material.space.nil? || !material.space.is_private + end + end + end + + test 'resolve_curatable returns only assigned space records for space admin' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + @private_space_owner.space_roles.create!(space: @private_space, key: 'admin') + context = Struct.new(:user).new(@private_space_owner) + scope = ApplicationPolicy::Scope.new(context, Material).resolve_curatable + assert scope.exists?(space_id: @private_space.id) + scope.each do |material| + assert_equal @private_space.id, material.space_id + end + end + end + + test 'resolve_curatable returns no records for regular user without space admin role' do + with_settings(feature: { spaces:true, material_under_admin_approval: true }) do + context = Struct.new(:user).new(@regular_user) + scope = ApplicationPolicy::Scope.new(context, Material).resolve_curatable + assert_empty scope + end + end +end \ No newline at end of file diff --git a/test/policies/event_policy_test.rb b/test/policies/event_policy_test.rb new file mode 100644 index 000000000..e88c9a827 --- /dev/null +++ b/test/policies/event_policy_test.rb @@ -0,0 +1,40 @@ +require 'test_helper' + +class EventPolicyTest < ActiveSupport::TestCase + UserContext = Struct.new(:user) + + setup do + @curator = users(:curator) + @private_space = spaces(:private_space) + @requested_private = events(:one) + @requested_private.update_columns( + space_id: @private_space.id, + approval_status: Event::APPROVAL_STATUS_CODES[:requested] + ) + end + + def resolve_scope(user) + EventPolicy::Scope.new(UserContext.new(user), Event.all).resolve + end + + test 'curator scope excludes requested events in a private space' do + with_settings(feature: { event_under_admin_approval: true }) do + refute_includes resolve_scope(@curator), @requested_private + end + end + + test 'space admin curator scope includes requested events in their private space' do + with_settings(feature: { event_under_admin_approval: true }) do + @curator.space_roles.create!(space: @private_space, key: 'admin') + + assert_includes resolve_scope(@curator), @requested_private + assert Event.user_can_curate_unapproved_in_space?(@curator, @private_space) + end + end + + test 'curator can curate unapproved events in a public space' do + with_settings(feature: { event_under_admin_approval: true }) do + assert Event.user_can_curate_unapproved_in_space?(@curator, spaces(:plants)) + end + end +end diff --git a/test/policies/material_policy_test.rb b/test/policies/material_policy_test.rb new file mode 100644 index 000000000..89f09d285 --- /dev/null +++ b/test/policies/material_policy_test.rb @@ -0,0 +1,96 @@ +require 'test_helper' + +class MaterialPolicyTest < ActiveSupport::TestCase + UserContext = Struct.new(:user) + + setup do + @owner = users(:regular_user) + @other_user = users(:another_regular_user) + @admin = users(:admin) + @curator = users(:curator) + + @approved = materials(:approved_material) + @requested = materials(:requested_material) + @requested_private = materials(:requested_private_material) + @private_space = spaces(:private_space) + end + + def resolve_scope(user) + context = UserContext.new(user) + MaterialPolicy::Scope.new(context, Material.all).resolve + end + + test 'anonymous user scope returns only approved materials when feature is enabled' do + with_settings(feature: { material_under_admin_approval: true }) do + results = resolve_scope(nil) + + assert_includes results, @approved + refute_includes results, @requested + end + end + + test 'regular non-owner user scope returns only approved materials' do + with_settings(feature: { material_under_admin_approval: true }) do + results = resolve_scope(@other_user) + + assert_includes results, @approved + refute_includes results, @requested + end + end + + test 'owner user scope includes their own unapproved materials' do + with_settings(feature: { material_under_admin_approval: true }) do + results = resolve_scope(@owner) + + assert_includes results, @approved + assert_includes results, @requested + end + end + + test 'admin user scope includes all materials' do + with_settings(feature: { material_under_admin_approval: true }) do + results = resolve_scope(@admin) + + assert_includes results, @approved + assert_includes results, @requested + end + end + + test 'curator user scope includes all materials' do + with_settings(feature: { material_under_admin_approval: true }) do + results = resolve_scope(@curator) + + assert_includes results, @approved + assert_includes results, @requested + refute_includes results, @requested_private + end + end + + test 'space admin curator scope includes requested resources in their private space' do + with_settings(feature: { material_under_admin_approval: true }) do + @curator.space_roles.create!(space: @private_space, key: 'admin') + + results = resolve_scope(@curator) + + assert_includes results, @requested_private + end + end + + test 'Solr index permits private-space curation only to space admins' do + with_settings(feature: { material_under_admin_approval: true }) do + refute Material.user_can_curate_unapproved_in_space?(@curator, @private_space) + + @curator.space_roles.create!(space: @private_space, key: 'admin') + assert Material.user_can_curate_unapproved_in_space?(@curator, @private_space) + end + end + + test 'returns all materials when approval feature is disabled' do + with_settings(feature: { material_under_admin_approval: false }) do + results = resolve_scope(nil) + + assert_includes results, @approved + assert_includes results, @requested + end + end +end \ No newline at end of file