From 01e84b33fe992f36c317b01bc9fc4a49352d59a3 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Thu, 21 Nov 2024 18:57:46 +0200 Subject: [PATCH 01/20] health check sends an email on service status changes --- app/controllers/health_controller.rb | 57 ++++++++++++++----- .../stash_engine/notifications_mailer.rb | 13 +++++ .../health_status_change.html.erb | 21 +++++++ 3 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 app/mailers/stash_engine/notifications_mailer.rb create mode 100644 app/views/stash_engine/notifications_mailer/health_status_change.html.erb diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index 1682c44c0..71cb3d72d 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -1,42 +1,69 @@ class HealthController < ApplicationController def check - health_status = { status: 'OK' } + @health_status = {} + populate_statuses + status_code = @health_status.map{|_k, v| v[:status]}.include?('not connected') ? :service_unavailable : :ok + notify_health_status_change(status_code, @health_status) + + render json: { status: status_code }.merge(@health_status), status: status_code and return if params.key?(:advanced) + + render json: { status: status_code }.merge(simple_response_hash), status: status_code + end + + private + + def simple_response_hash + return @simple_response_hash if @simple_response_hash + + @simple_response_hash = @health_status.each_with_object({}) do |(key, value), result| + result[key] = value[:status] + end + end + + def populate_statuses # Check database connectivity + @health_status[:database] = {} begin StashEngine::Identifier.last - health_status[:database] = 'connected' + @health_status[:database][:status] = 'connected' rescue StandardError => e - health_status[:database] = 'not connected' - health_status[:database_error] = e.message if params[:advanced] + @health_status[:database][:status] = 'not connected' + @health_status[:database][:error] = e.message end # Check Solr connectivity + @health_status[:solr] = {} begin solr = RSolr.connect(url: Blacklight.connection_config[:url]) solr.get('select', params: { fl: 'dc_identifier_s', rows: 1 }) - health_status[:solr] = 'connected' + @health_status[:solr][:status] = 'connected' rescue StandardError => e - health_status[:solr] = 'not connected' - health_status[:solr_error] = e.message if params[:advanced] + @health_status[:solr][:status] = 'not connected' + @health_status[:solr][:error] = e.message end # Check AWS S3 connectivity + @health_status[:aws_s3] = {} begin if Stash::Aws::S3.new.exists?(s3_key: 's3_status_check.txt') - health_status[:aws_s3] = 'connected' + @health_status[:aws_s3][:status] = 'connected' else - health_status[:aws_s3] = 'not connected' - health_status[:aws_s3_error] = 'file does not exist' if params[:advanced] + @health_status[:aws_s3][:status] = 'not connected' + @health_status[:aws_s3][:error] = 'file does not exist' end rescue StandardError => e - health_status[:aws_s3] = 'not connected' - health_status[:aws_s3_error] = e.message if params[:advanced] + @health_status[:aws_s3][:status] = 'not connected' + @health_status[:aws_s3][:error] = e.message end + end + + def notify_health_status_change(status_code, health_status) + old_statuses = Rails.cache.read('health_status') + Rails.cache.write('health_status', simple_response_hash, expires_in: 10.minute) + return if old_statuses == simple_response_hash - status_code = health_status.values.include?('not connected') ? :service_unavailable : :ok - health_status[:status] = status_code - render json: health_status, status: status_code + StashEngine::NotificationsMailer.health_status_change(status_code, health_status).deliver_now end end diff --git a/app/mailers/stash_engine/notifications_mailer.rb b/app/mailers/stash_engine/notifications_mailer.rb new file mode 100644 index 000000000..4bab679c7 --- /dev/null +++ b/app/mailers/stash_engine/notifications_mailer.rb @@ -0,0 +1,13 @@ +module StashEngine + + # Mails users about submissions + class NotificationsMailer < ApplicationMailer + + def health_status_change(status_code, health_status) + @status_code = status_code + @health_status = health_status + + mail(to: 'devs@datadryad.org', subject: "#{rails_env}Health check status changed - #{status_code}") + end + end +end diff --git a/app/views/stash_engine/notifications_mailer/health_status_change.html.erb b/app/views/stash_engine/notifications_mailer/health_status_change.html.erb new file mode 100644 index 000000000..26e4f6643 --- /dev/null +++ b/app/views/stash_engine/notifications_mailer/health_status_change.html.erb @@ -0,0 +1,21 @@ +

Server status changed is <%= @status_code.upcase %>

+ +

Services status:

+ + <% @health_status.each do |k, v| %> + + + + + <% if v[:error] %> + + + + + <% end %> + <% end %> +
<%= k %> + status: <%= v[:status] %> +
+ error:  <%= v[:error] %> +
From e7d9be7e9cfb3665fad900f21312b47f4630d003 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Thu, 21 Nov 2024 19:38:25 +0200 Subject: [PATCH 02/20] fixed rubocop --- app/controllers/health_controller.rb | 2 +- config/environments/local.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index 71cb3d72d..24c4e378d 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -4,7 +4,7 @@ def check @health_status = {} populate_statuses - status_code = @health_status.map{|_k, v| v[:status]}.include?('not connected') ? :service_unavailable : :ok + status_code = @health_status.map { |_k, v| v[:status] }.include?('not connected') ? :service_unavailable : :ok notify_health_status_change(status_code, @health_status) render json: { status: status_code }.merge(@health_status), status: status_code and return if params.key?(:advanced) diff --git a/config/environments/local.rb b/config/environments/local.rb index c7009b557..1a3c1ed72 100644 --- a/config/environments/local.rb +++ b/config/environments/local.rb @@ -59,4 +59,5 @@ config.action_mailer.perform_deliveries = true Rails.application.default_url_options = { host: 'localhost', port: 3000} + # Rack::Attack.enabled = false end From d14aed740f79b779ec2ec6ea1f662943b5e9d399 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Tue, 26 Nov 2024 20:22:52 +0200 Subject: [PATCH 03/20] allow superusers to withdraw any dataset --- .../admin_dashboard_controller.rb | 4 +-- .../stash_engine/admin_datasets_helper.rb | 2 +- app/models/stash_engine/curation_activity.rb | 6 +++-- .../admin_dashboard/_curation_form.html.erb | 2 +- .../stash_engine/curation_activity_spec.rb | 27 ++++++++++++++----- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/controllers/stash_engine/admin_dashboard_controller.rb b/app/controllers/stash_engine/admin_dashboard_controller.rb index b374a4d41..44f1e5292 100644 --- a/app/controllers/stash_engine/admin_dashboard_controller.rb +++ b/app/controllers/stash_engine/admin_dashboard_controller.rb @@ -347,10 +347,10 @@ def load end def curation_activity_change - return publishing_error if @resource.id != @identifier.last_submitted_resource.id && + return publishing_error if @resource.id != @identifier.last_submitted_resource&.id && %w[embargoed published].include?(params.dig(:curation_activity, :status)) - return state_error unless CurationActivity.allowed_states(@last_state).include?(@status) + return state_error unless CurationActivity.allowed_states(@last_state, current_user).include?(@status) decipher_curation_activity @note = params.dig(:curation_activity, :note) diff --git a/app/helpers/stash_engine/admin_datasets_helper.rb b/app/helpers/stash_engine/admin_datasets_helper.rb index 7c8d15a57..9af9f3d7f 100644 --- a/app/helpers/stash_engine/admin_datasets_helper.rb +++ b/app/helpers/stash_engine/admin_datasets_helper.rb @@ -15,7 +15,7 @@ def status_select(statuses = []) end def filter_status_select(current_status) - statuses = StashEngine::CurationActivity.allowed_states(current_status) + statuses = StashEngine::CurationActivity.allowed_states(current_status, current_user) statuses.delete(current_status) # because we don't show the current state as an option, it is implied by leaving state blank diff --git a/app/models/stash_engine/curation_activity.rb b/app/models/stash_engine/curation_activity.rb index 742ffc431..fa304ef98 100644 --- a/app/models/stash_engine/curation_activity.rb +++ b/app/models/stash_engine/curation_activity.rb @@ -181,8 +181,10 @@ def first_time_in_status? resource.curation_activities.where('id < ?', id).where(status: status).empty? end - def self.allowed_states(current_state) - CURATOR_ALLOWED_STATES[current_state].dup + def self.allowed_states(current_state, current_user) + statuses = CURATOR_ALLOWED_STATES[current_state].dup + statuses << 'withdrawn' if current_user.superuser? # superusers can withdraw a datasets from any status + statuses.uniq end # Private methods diff --git a/app/views/stash_engine/admin_dashboard/_curation_form.html.erb b/app/views/stash_engine/admin_dashboard/_curation_form.html.erb index 19a8ce444..5f3412bc0 100644 --- a/app/views/stash_engine/admin_dashboard/_curation_form.html.erb +++ b/app/views/stash_engine/admin_dashboard/_curation_form.html.erb @@ -6,7 +6,7 @@

Edit curation status of <%= @resource.title %>

<%= hidden_field_tag :identifier_id, @resource.identifier_id %> <%# Users cannot change the status or publication date once the files are published %> - <% if @resource.curatable? && filter_status_select(@resource.current_curation_status) %> + <% if (@resource.curatable? || current_user.superuser?) && filter_status_select(@resource.current_curation_status) %> <%= form.fields_for :curation_activity, @curation_activity do |ca| %>
<%= ca.label :status, 'Status', class: 'c-input__label' %> diff --git a/spec/models/stash_engine/curation_activity_spec.rb b/spec/models/stash_engine/curation_activity_spec.rb index a389da161..fee66f2f2 100644 --- a/spec/models/stash_engine/curation_activity_spec.rb +++ b/spec/models/stash_engine/curation_activity_spec.rb @@ -186,14 +186,29 @@ module StashEngine end describe 'self.allowed_states(current_state)' do - it 'indicates the states that are allowed from each' do - expect(CurationActivity.allowed_states('curation')).to \ - eq(%w[processing peer_review curation action_required withdrawn embargoed published]) + context 'when user is an admin' do + let(:user) { create(:user, role: 'admin') } - expect(CurationActivity.allowed_states('withdrawn')).to \ - eq(%w[withdrawn curation]) + it 'indicates the states that are allowed from each' do + expect(CurationActivity.allowed_states('in_progress', user)).to eq(%w[in_progress]) + + expect(CurationActivity.allowed_states('curation', user)).to \ + eq(%w[processing peer_review curation action_required withdrawn embargoed published]) + + expect(CurationActivity.allowed_states('withdrawn', user)).to \ + eq(%w[withdrawn curation]) + end end - end + context 'when user is an superuser' do + let(:user) { create(:user, role: 'superuser') } + + CurationActivity::CURATOR_ALLOWED_STATES.keys.each do |status| + it "allows withdrawn for #{status} status" do + expect(CurationActivity.allowed_states(status, user)).to include('withdrawn') + end + end + end + end end end From 3c57571744f3dddf188468b030223d61d160a589 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Tue, 26 Nov 2024 20:24:32 +0200 Subject: [PATCH 04/20] upddated specs --- spec/models/stash_engine/curation_activity_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/stash_engine/curation_activity_spec.rb b/spec/models/stash_engine/curation_activity_spec.rb index fee66f2f2..d2363d8ab 100644 --- a/spec/models/stash_engine/curation_activity_spec.rb +++ b/spec/models/stash_engine/curation_activity_spec.rb @@ -203,7 +203,7 @@ module StashEngine context 'when user is an superuser' do let(:user) { create(:user, role: 'superuser') } - CurationActivity::CURATOR_ALLOWED_STATES.keys.each do |status| + CurationActivity::CURATOR_ALLOWED_STATES.each_key do |status| it "allows withdrawn for #{status} status" do expect(CurationActivity.allowed_states(status, user)).to include('withdrawn') end From 2d1227883362ee9e2317e2fb838728c39279af4e Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Fri, 29 Nov 2024 17:54:21 +0200 Subject: [PATCH 05/20] added option for superuser to delete a dataset --- .../stash_engine/admin_datasets_controller.rb | 15 +++++++++++++++ app/helpers/stash_engine/identifiers_helper.rb | 9 +++++++++ .../stash_engine/admin_datasets_policy.rb | 4 ++++ .../admin_datasets/activity_log.html.erb | 12 ++++++++++++ config/routes.rb | 1 + 5 files changed, 41 insertions(+) create mode 100644 app/helpers/stash_engine/identifiers_helper.rb diff --git a/app/controllers/stash_engine/admin_datasets_controller.rb b/app/controllers/stash_engine/admin_datasets_controller.rb index 793b808ca..6bf9f19c7 100644 --- a/app/controllers/stash_engine/admin_datasets_controller.rb +++ b/app/controllers/stash_engine/admin_datasets_controller.rb @@ -58,6 +58,21 @@ def create_salesforce_case redirect_to(sf_url, allow_other_host: true) end + def destroy + pp current_user + authorize %i[stash_engine admin_datasets], :delete_dataset? + identifier = Identifier.find(params[:id]) + unless helpers.can_user_delete_identifier?(identifier) + redirect_to activity_log_path(identifier.id), alert: 'This dataset can not be deleted.' and return + end + + # if identifier.destroy + # redirect_to admin_dashboard_path, notice: "Dataset #{identifier.identifier} has been deleted." + # else + redirect_to activity_log_path(identifier.id), alert: 'Dataset could not be deleted.' + # end + end + private def load diff --git a/app/helpers/stash_engine/identifiers_helper.rb b/app/helpers/stash_engine/identifiers_helper.rb new file mode 100644 index 000000000..2a60a0487 --- /dev/null +++ b/app/helpers/stash_engine/identifiers_helper.rb @@ -0,0 +1,9 @@ +module StashEngine + module IdentifiersHelper + + def can_user_delete_identifier?(identifier) + identifier.resources.count == 1 && + identifier.resources.first.curation_activities.pluck(:status).uniq == ['in_progress'] + end + end +end diff --git a/app/policies/stash_engine/admin_datasets_policy.rb b/app/policies/stash_engine/admin_datasets_policy.rb index c959ca8b4..73a505a55 100644 --- a/app/policies/stash_engine/admin_datasets_policy.rb +++ b/app/policies/stash_engine/admin_datasets_policy.rb @@ -28,5 +28,9 @@ def waiver_add? def change_delete_schedule? @user.superuser? end + + def delete_dataset? + @user.superuser? + end end end diff --git a/app/views/stash_engine/admin_datasets/activity_log.html.erb b/app/views/stash_engine/admin_datasets/activity_log.html.erb index 7df538442..0910bcafc 100644 --- a/app/views/stash_engine/admin_datasets/activity_log.html.erb +++ b/app/views/stash_engine/admin_datasets/activity_log.html.erb @@ -156,6 +156,18 @@ Please only do this if you know the author is unable/unwilling to submit it.

+ <% if policy([:stash_engine, :admin_datasets]).delete_dataset? && can_user_delete_identifier?(@identifier) %> +
+ <%= form_with(url: stash_url_helpers.ds_admin_destroy_path(id: @identifier&.id), + method: :delete, :html => { onsubmit: "document.body.classList.add('prevent-clicks')" }) do -%> + + <%= hidden_field_tag :id, @identifier.id, id: "identifier_id_#{@identifier.id}" %> + <% end %> +

Deleting a dataset will delete all its data and files.

+
+
+ <% end %> + <% if policy([:stash_engine, :admin_datasets]).waiver_add? && @identifier.payment_type != 'waiver' %>
<%= form_with(url: ds_admin_popup_path(id: @identifier&.id, field: 'waiver'), method: :get, local: false) do %> diff --git a/config/routes.rb b/config/routes.rb index 4bf889e83..957666e6e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -329,6 +329,7 @@ get 'ds_admin/:id/activity_log', to: 'admin_datasets#activity_log', as: 'activity_log' get 'ds_admin/:id/edit/:field', to: 'admin_datasets#popup', as: 'ds_admin_popup' post 'ds_admin/:id', to: 'admin_datasets#edit', as: 'ds_admin_edit' + delete 'ds_admin/:id', to: 'admin_datasets#destroy', as: 'ds_admin_destroy' # curation notes post 'curation_note/:id', to: 'curation_activity#curation_note', as: 'curation_note' From 2eaf867fcbfca69c2c924ebd3763fb2e1b9faa21 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Mon, 2 Dec 2024 17:10:52 +0200 Subject: [PATCH 06/20] added tests --- .../stash_engine/admin_datasets_controller.rb | 12 +-- spec/rails_helper.rb | 1 + .../admin_datasets_controller_spec.rb | 91 +++++++++++++++++++ 3 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 spec/requests/stash_engine/admin_datasets_controller_spec.rb diff --git a/app/controllers/stash_engine/admin_datasets_controller.rb b/app/controllers/stash_engine/admin_datasets_controller.rb index 6bf9f19c7..2afadaa38 100644 --- a/app/controllers/stash_engine/admin_datasets_controller.rb +++ b/app/controllers/stash_engine/admin_datasets_controller.rb @@ -59,18 +59,18 @@ def create_salesforce_case end def destroy - pp current_user authorize %i[stash_engine admin_datasets], :delete_dataset? + identifier = Identifier.find(params[:id]) unless helpers.can_user_delete_identifier?(identifier) redirect_to activity_log_path(identifier.id), alert: 'This dataset can not be deleted.' and return end - # if identifier.destroy - # redirect_to admin_dashboard_path, notice: "Dataset #{identifier.identifier} has been deleted." - # else - redirect_to activity_log_path(identifier.id), alert: 'Dataset could not be deleted.' - # end + if identifier.destroy + redirect_to admin_dashboard_path, notice: "Dataset with DOI #{identifier.identifier} has been deleted." + else + redirect_to activity_log_path(identifier.id), alert: 'Dataset could not be deleted. Please try again later.' + end end private diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index fc79d466e..39869fef7 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -107,4 +107,5 @@ end config.include React::Rails::TestHelper + config.include Rails.application.routes.url_helpers, type: :request end diff --git a/spec/requests/stash_engine/admin_datasets_controller_spec.rb b/spec/requests/stash_engine/admin_datasets_controller_spec.rb new file mode 100644 index 000000000..b3b38a7d4 --- /dev/null +++ b/spec/requests/stash_engine/admin_datasets_controller_spec.rb @@ -0,0 +1,91 @@ +module StashEngine + RSpec.describe AdminDatasetsController, type: :request do + include Mocks::Salesforce + + before do + mock_salesforce! + end + + describe '#destroy' do + let!(:tenant) { create(:tenant) } + let(:resource) { create(:resource) } + let(:identifier) { resource.identifier } + + subject { delete ds_admin_destroy_path(identifier.id) } + + before do + allow_any_instance_of(AdminDatasetsController).to receive(:session).and_return({ user_id: user.id }.to_ostruct) + allow_any_instance_of(AdminDashboardController).to receive(:session).and_return({ user_id: user.id }.to_ostruct) + allow_any_instance_of(DashboardController).to receive(:session).and_return({ user_id: user.id }.to_ostruct) + end + + context 'as superuser' do + let!(:user) { create(:user, role: 'superuser', tenant_id: 'dryad') } + + context 'when identifier can be deleted' do + it 'deletes the resource and identifier' do + allow_any_instance_of(Stash::Aws::S3).to receive(:delete_dir).and_return(true) + subject + + expect(response).to have_http_status(302) + expect(response.headers['Location']).to eq(admin_dashboard_url) + follow_redirect! + expect(response.body).to include("Dataset with DOI #{identifier.identifier} has been deleted.") + end + end + + context 'when dataset status changed' do + before do + create(:curation_activity, status: 'processing', user: resource.submitter, resource: resource) + end + + it 'does not delete the resource and identifier' do + subject + + expect(response).to have_http_status(302) + expect(response.headers['Location']).to eq(activity_log_url(identifier.id)) + expect(controller.flash[:alert]).to eq('This dataset can not be deleted.') + end + end + + context 'when identifier destroy fails' do + before do + expect_any_instance_of(StashEngine::Identifier).to receive(:destroy).and_return(false) + end + + it 'does not delete the resource and identifier' do + subject + + expect(response).to have_http_status(302) + expect(response.headers['Location']).to eq(activity_log_url(identifier.id)) + expect(controller.flash[:alert]).to eq('Dataset could not be deleted. Please try again later.') + end + end + end + + context 'as curator' do + let(:user) { create(:user, role: 'curator', tenant_id: 'dryad') } + + it 'can not delete the resource and identifier' do + subject + + expect(response).to have_http_status(302) + expect(response.headers['Location']).to include(choose_dashboard_url) + expect(controller.flash[:alert]).to eq('You are not authorized to view this information.') + end + end + + context 'as an admin' do + let(:user) { create(:user, role: 'admin', tenant_id: 'dryad') } + + it 'can not delete the resource and identifier' do + subject + + expect(response).to have_http_status(302) + expect(response.headers['Location']).to include(choose_dashboard_path) + expect(controller.flash[:alert]).to eq('You are not authorized to view this information.') + end + end + end + end +end From 0b2c081e4faed86770abdb240436dfc6911409cd Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Mon, 2 Dec 2024 19:03:16 +0200 Subject: [PATCH 07/20] updates to feature tests --- spec/features/stash_engine/dataset_queuing_spec.rb | 1 - .../stash_engine/frictionless_validation_spec.rb | 1 - spec/features/stash_engine/review_and_submit_spec.rb | 1 - spec/support/capybara.rb | 4 ++-- spec/support/helpers/collection_helper.rb | 6 +++++- .../helpers/{ajax_helper.rb => page_load_helper.rb} | 9 ++++----- 6 files changed, 11 insertions(+), 11 deletions(-) rename spec/support/helpers/{ajax_helper.rb => page_load_helper.rb} (71%) diff --git a/spec/features/stash_engine/dataset_queuing_spec.rb b/spec/features/stash_engine/dataset_queuing_spec.rb index 504ab828f..25d5829a5 100644 --- a/spec/features/stash_engine/dataset_queuing_spec.rb +++ b/spec/features/stash_engine/dataset_queuing_spec.rb @@ -10,7 +10,6 @@ include Mocks::RSolr include Mocks::Stripe include Mocks::Salesforce - include AjaxHelper before(:each) do FileUtils.rm_f(hold_submissions_path) diff --git a/spec/features/stash_engine/frictionless_validation_spec.rb b/spec/features/stash_engine/frictionless_validation_spec.rb index 0dfd150c0..849e4f2f7 100644 --- a/spec/features/stash_engine/frictionless_validation_spec.rb +++ b/spec/features/stash_engine/frictionless_validation_spec.rb @@ -14,7 +14,6 @@ include Mocks::Salesforce include Mocks::Stripe include Mocks::Aws - include AjaxHelper before(:each) do mock_repository! diff --git a/spec/features/stash_engine/review_and_submit_spec.rb b/spec/features/stash_engine/review_and_submit_spec.rb index 04f20747b..c22ffe0b4 100644 --- a/spec/features/stash_engine/review_and_submit_spec.rb +++ b/spec/features/stash_engine/review_and_submit_spec.rb @@ -11,7 +11,6 @@ include Mocks::Stripe include Mocks::Aws include Mocks::Salesforce - include AjaxHelper before(:each) do mock_repository! diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index d01af7467..7d5c344c4 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative 'solr' -require_relative 'helpers/ajax_helper' +require_relative 'helpers/page_load_helper' require_relative 'helpers/capybara_helper' require_relative 'helpers/tinymce_helper' require_relative 'helpers/routes_helper' @@ -73,7 +73,7 @@ end RSpec.configure do |config| - # config.include(AjaxHelper, type: :feature) + config.include(PageLoadHelper, type: :feature) config.include(CapybaraHelper, type: :feature) config.before(:all, type: :feature) do diff --git a/spec/support/helpers/collection_helper.rb b/spec/support/helpers/collection_helper.rb index e6e47a9d7..70504e3e6 100644 --- a/spec/support/helpers/collection_helper.rb +++ b/spec/support/helpers/collection_helper.rb @@ -11,13 +11,17 @@ def start_new_collection # Make sure you switch to the Selenium driver for the test calling this helper method # e.g. `it 'should test this amazing thing', js: true do` visit('/stash/resources/new?collection') + wait_for_ajax + wait_for_page_load('Describe collection') navigate_to_metadata end + + def navigate_to_metadata # Make sure you switch to the Selenium driver for the test calling this helper method # e.g. `it 'should test this amazing thing', js: true do` - click_link 'Describe collection', wait: 15 + click_link 'Describe collection' expect(page).to have_content('Collection: Basic information') end diff --git a/spec/support/helpers/ajax_helper.rb b/spec/support/helpers/page_load_helper.rb similarity index 71% rename from spec/support/helpers/ajax_helper.rb rename to spec/support/helpers/page_load_helper.rb index d80abd846..f88678010 100644 --- a/spec/support/helpers/ajax_helper.rb +++ b/spec/support/helpers/page_load_helper.rb @@ -1,4 +1,4 @@ -module AjaxHelper +module PageLoadHelper def wait_for_ajax(seconds = Capybara.default_max_wait_time) Timeout.timeout(seconds) do @@ -10,8 +10,7 @@ def finished_all_ajax_requests? page.evaluate_script('jQuery.active').zero? end -end - -RSpec.configure do |config| - config.include(AjaxHelper, type: :system) + def wait_for_page_load(text) + expect(page).to have_content text + end end From b18863fa0c88b105fbd0927e9501db37723eece2 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Mon, 2 Dec 2024 19:18:15 +0200 Subject: [PATCH 08/20] fixed rubocop --- spec/support/helpers/collection_helper.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/support/helpers/collection_helper.rb b/spec/support/helpers/collection_helper.rb index 70504e3e6..1f3000f0b 100644 --- a/spec/support/helpers/collection_helper.rb +++ b/spec/support/helpers/collection_helper.rb @@ -16,8 +16,6 @@ def start_new_collection navigate_to_metadata end - - def navigate_to_metadata # Make sure you switch to the Selenium driver for the test calling this helper method # e.g. `it 'should test this amazing thing', js: true do` From 2724739288e87af5aa8d54df4c55eb05fd49467d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 01:51:47 +0000 Subject: [PATCH 09/20] Bump actionpack from 7.0.8.5 to 7.0.8.7 Bumps [actionpack](https://github.com/rails/rails) from 7.0.8.5 to 7.0.8.7. - [Release notes](https://github.com/rails/rails/releases) - [Changelog](https://github.com/rails/rails/blob/v8.0.0.1/actionpack/CHANGELOG.md) - [Commits](https://github.com/rails/rails/compare/v7.0.8.5...v7.0.8.7) --- updated-dependencies: - dependency-name: actionpack dependency-type: indirect ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 114 +++++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 041fc38fa..47018b863 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -17,67 +17,67 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (7.0.8.5) - actionpack (= 7.0.8.5) - activesupport (= 7.0.8.5) + actioncable (7.0.8.7) + actionpack (= 7.0.8.7) + activesupport (= 7.0.8.7) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (7.0.8.5) - actionpack (= 7.0.8.5) - activejob (= 7.0.8.5) - activerecord (= 7.0.8.5) - activestorage (= 7.0.8.5) - activesupport (= 7.0.8.5) + actionmailbox (7.0.8.7) + actionpack (= 7.0.8.7) + activejob (= 7.0.8.7) + activerecord (= 7.0.8.7) + activestorage (= 7.0.8.7) + activesupport (= 7.0.8.7) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.0.8.5) - actionpack (= 7.0.8.5) - actionview (= 7.0.8.5) - activejob (= 7.0.8.5) - activesupport (= 7.0.8.5) + actionmailer (7.0.8.7) + actionpack (= 7.0.8.7) + actionview (= 7.0.8.7) + activejob (= 7.0.8.7) + activesupport (= 7.0.8.7) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.0) - actionpack (7.0.8.5) - actionview (= 7.0.8.5) - activesupport (= 7.0.8.5) + actionpack (7.0.8.7) + actionview (= 7.0.8.7) + activesupport (= 7.0.8.7) rack (~> 2.0, >= 2.2.4) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (7.0.8.5) - actionpack (= 7.0.8.5) - activerecord (= 7.0.8.5) - activestorage (= 7.0.8.5) - activesupport (= 7.0.8.5) + actiontext (7.0.8.7) + actionpack (= 7.0.8.7) + activerecord (= 7.0.8.7) + activestorage (= 7.0.8.7) + activesupport (= 7.0.8.7) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.0.8.5) - activesupport (= 7.0.8.5) + actionview (7.0.8.7) + activesupport (= 7.0.8.7) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (7.0.8.5) - activesupport (= 7.0.8.5) + activejob (7.0.8.7) + activesupport (= 7.0.8.7) globalid (>= 0.3.6) - activemodel (7.0.8.5) - activesupport (= 7.0.8.5) - activerecord (7.0.8.5) - activemodel (= 7.0.8.5) - activesupport (= 7.0.8.5) - activestorage (7.0.8.5) - actionpack (= 7.0.8.5) - activejob (= 7.0.8.5) - activerecord (= 7.0.8.5) - activesupport (= 7.0.8.5) + activemodel (7.0.8.7) + activesupport (= 7.0.8.7) + activerecord (7.0.8.7) + activemodel (= 7.0.8.7) + activesupport (= 7.0.8.7) + activestorage (7.0.8.7) + actionpack (= 7.0.8.7) + activejob (= 7.0.8.7) + activerecord (= 7.0.8.7) + activesupport (= 7.0.8.7) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (7.0.8.5) + activesupport (7.0.8.7) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -410,7 +410,7 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2024.1001) mini_mime (1.1.5) - minitest (5.25.1) + minitest (5.25.4) mize (0.6.0) mocha (2.4.5) ruby2_keywords (>= 0.0.5) @@ -442,11 +442,11 @@ GEM netrc (0.11.0) nio4r (2.7.3) noid (0.9.0) - nokogiri (1.16.7-arm64-darwin) + nokogiri (1.17.1-arm64-darwin) racc (~> 1.4) - nokogiri (1.16.7-x86_64-darwin) + nokogiri (1.17.1-x86_64-darwin) racc (~> 1.4) - nokogiri (1.16.7-x86_64-linux) + nokogiri (1.17.1-x86_64-linux) racc (~> 1.4) notiffany (0.1.3) nenv (~> 0.1) @@ -510,29 +510,29 @@ GEM rack rack-test (2.1.0) rack (>= 1.3) - rails (7.0.8.5) - actioncable (= 7.0.8.5) - actionmailbox (= 7.0.8.5) - actionmailer (= 7.0.8.5) - actionpack (= 7.0.8.5) - actiontext (= 7.0.8.5) - actionview (= 7.0.8.5) - activejob (= 7.0.8.5) - activemodel (= 7.0.8.5) - activerecord (= 7.0.8.5) - activestorage (= 7.0.8.5) - activesupport (= 7.0.8.5) + rails (7.0.8.7) + actioncable (= 7.0.8.7) + actionmailbox (= 7.0.8.7) + actionmailer (= 7.0.8.7) + actionpack (= 7.0.8.7) + actiontext (= 7.0.8.7) + actionview (= 7.0.8.7) + activejob (= 7.0.8.7) + activemodel (= 7.0.8.7) + activerecord (= 7.0.8.7) + activestorage (= 7.0.8.7) + activesupport (= 7.0.8.7) bundler (>= 1.15.0) - railties (= 7.0.8.5) + railties (= 7.0.8.7) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) rails-html-sanitizer (1.5.0) loofah (~> 2.19, >= 2.19.1) - railties (7.0.8.5) - actionpack (= 7.0.8.5) - activesupport (= 7.0.8.5) + railties (7.0.8.7) + actionpack (= 7.0.8.7) + activesupport (= 7.0.8.7) method_source rake (>= 12.2) thor (~> 1.0) From 2855585e1dae52c0df67fdf3efe1dc15511939fd Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 14:25:46 +0200 Subject: [PATCH 10/20] updated policy --- app/policies/stash_engine/resource_policy.rb | 4 ++++ .../stash_engine/admin_dashboard/_curation_form.html.erb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/policies/stash_engine/resource_policy.rb b/app/policies/stash_engine/resource_policy.rb index 0d3e1c0e8..9cb4ff1f0 100644 --- a/app/policies/stash_engine/resource_policy.rb +++ b/app/policies/stash_engine/resource_policy.rb @@ -23,6 +23,10 @@ def curator_edit? (@resource.current_resource_state&.resource_state == 'in_progress' && @resource&.user_id == @user.id) end + def change_status? + @resource.curatable? || @user.superuser? + end + class VersionScope def initialize(user, scope) @user = user diff --git a/app/views/stash_engine/admin_dashboard/_curation_form.html.erb b/app/views/stash_engine/admin_dashboard/_curation_form.html.erb index 5f3412bc0..95510a636 100644 --- a/app/views/stash_engine/admin_dashboard/_curation_form.html.erb +++ b/app/views/stash_engine/admin_dashboard/_curation_form.html.erb @@ -6,7 +6,7 @@

Edit curation status of <%= @resource.title %>

<%= hidden_field_tag :identifier_id, @resource.identifier_id %> <%# Users cannot change the status or publication date once the files are published %> - <% if (@resource.curatable? || current_user.superuser?) && filter_status_select(@resource.current_curation_status) %> + <% if policy(@resource).change_status? && filter_status_select(@resource.current_curation_status) %> <%= form.fields_for :curation_activity, @curation_activity do |ca| %>
<%= ca.label :status, 'Status', class: 'c-input__label' %> From dbc2a4db633eabce47257201d01b9dc01e32fb8f Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 17:09:26 +0200 Subject: [PATCH 11/20] removed empty helper files --- app/helpers/stash_engine/dashboard_helper.rb | 4 ---- app/helpers/stash_engine/edit_histories_helper.rb | 4 ---- app/helpers/stash_engine/embargoes_helper.rb | 4 ---- app/helpers/stash_engine/file_uploads_helper.rb | 4 ---- app/helpers/stash_engine/pages_helper.rb | 4 ---- app/helpers/stash_engine/resources_helper.rb | 4 ---- app/helpers/stash_engine/searches_helper.rb | 4 ---- app/helpers/stash_engine/shares_helper.rb | 4 ---- app/helpers/stash_engine/tenants_helper.rb | 4 ---- app/helpers/stash_engine/zenodo_queue_helper.rb | 4 ---- 10 files changed, 40 deletions(-) delete mode 100644 app/helpers/stash_engine/dashboard_helper.rb delete mode 100644 app/helpers/stash_engine/edit_histories_helper.rb delete mode 100644 app/helpers/stash_engine/embargoes_helper.rb delete mode 100644 app/helpers/stash_engine/file_uploads_helper.rb delete mode 100644 app/helpers/stash_engine/pages_helper.rb delete mode 100644 app/helpers/stash_engine/resources_helper.rb delete mode 100644 app/helpers/stash_engine/searches_helper.rb delete mode 100644 app/helpers/stash_engine/shares_helper.rb delete mode 100644 app/helpers/stash_engine/tenants_helper.rb delete mode 100644 app/helpers/stash_engine/zenodo_queue_helper.rb diff --git a/app/helpers/stash_engine/dashboard_helper.rb b/app/helpers/stash_engine/dashboard_helper.rb deleted file mode 100644 index a272b3650..000000000 --- a/app/helpers/stash_engine/dashboard_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module DashboardHelper - end -end diff --git a/app/helpers/stash_engine/edit_histories_helper.rb b/app/helpers/stash_engine/edit_histories_helper.rb deleted file mode 100644 index 79069ef7b..000000000 --- a/app/helpers/stash_engine/edit_histories_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module EditHistoriesHelper - end -end diff --git a/app/helpers/stash_engine/embargoes_helper.rb b/app/helpers/stash_engine/embargoes_helper.rb deleted file mode 100644 index 805b8c292..000000000 --- a/app/helpers/stash_engine/embargoes_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module EmbargoesHelper - end -end diff --git a/app/helpers/stash_engine/file_uploads_helper.rb b/app/helpers/stash_engine/file_uploads_helper.rb deleted file mode 100644 index fc974cc3f..000000000 --- a/app/helpers/stash_engine/file_uploads_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module FileUploadsHelper - end -end diff --git a/app/helpers/stash_engine/pages_helper.rb b/app/helpers/stash_engine/pages_helper.rb deleted file mode 100644 index 13b2b76c2..000000000 --- a/app/helpers/stash_engine/pages_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module PagesHelper - end -end diff --git a/app/helpers/stash_engine/resources_helper.rb b/app/helpers/stash_engine/resources_helper.rb deleted file mode 100644 index 3864a080c..000000000 --- a/app/helpers/stash_engine/resources_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module ResourcesHelper - end -end diff --git a/app/helpers/stash_engine/searches_helper.rb b/app/helpers/stash_engine/searches_helper.rb deleted file mode 100644 index f313602d1..000000000 --- a/app/helpers/stash_engine/searches_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module SearchesHelper - end -end diff --git a/app/helpers/stash_engine/shares_helper.rb b/app/helpers/stash_engine/shares_helper.rb deleted file mode 100644 index 22f97cb08..000000000 --- a/app/helpers/stash_engine/shares_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module SharesHelper - end -end diff --git a/app/helpers/stash_engine/tenants_helper.rb b/app/helpers/stash_engine/tenants_helper.rb deleted file mode 100644 index 0b82a1539..000000000 --- a/app/helpers/stash_engine/tenants_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module TenantsHelper - end -end diff --git a/app/helpers/stash_engine/zenodo_queue_helper.rb b/app/helpers/stash_engine/zenodo_queue_helper.rb deleted file mode 100644 index 79716457f..000000000 --- a/app/helpers/stash_engine/zenodo_queue_helper.rb +++ /dev/null @@ -1,4 +0,0 @@ -module StashEngine - module ZenodoQueueHelper - end -end From c99732719083324b301cbca234527adb9a09fc8d Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 17:10:24 +0200 Subject: [PATCH 12/20] updated policies record/resource naming --- app/policies/stash_engine/admin_search_policy.rb | 2 +- app/policies/stash_engine/application_policy.rb | 6 +++--- app/policies/stash_engine/curation_activity_policy.rb | 2 +- app/policies/stash_engine/resource_policy.rb | 4 ++-- app/policies/stash_engine/saved_search_policy.rb | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/policies/stash_engine/admin_search_policy.rb b/app/policies/stash_engine/admin_search_policy.rb index 1f80466a1..b1b1a439d 100644 --- a/app/policies/stash_engine/admin_search_policy.rb +++ b/app/policies/stash_engine/admin_search_policy.rb @@ -6,7 +6,7 @@ def new_search? end def save_search? - @user.id == @resource.user_id + @user.id == @record.user_id end end diff --git a/app/policies/stash_engine/application_policy.rb b/app/policies/stash_engine/application_policy.rb index b56ac052a..b4acee5d4 100644 --- a/app/policies/stash_engine/application_policy.rb +++ b/app/policies/stash_engine/application_policy.rb @@ -1,10 +1,10 @@ module StashEngine class ApplicationPolicy - attr_reader :user, :resource + attr_reader :user, :record - def initialize(user, resource) + def initialize(user, record) @user = user - @resource = resource + @record = record end def index? diff --git a/app/policies/stash_engine/curation_activity_policy.rb b/app/policies/stash_engine/curation_activity_policy.rb index 2acef4fa5..2427b8346 100644 --- a/app/policies/stash_engine/curation_activity_policy.rb +++ b/app/policies/stash_engine/curation_activity_policy.rb @@ -11,7 +11,7 @@ def curation_note? def file_note? @user.min_app_admin? || - @resource.users.include?(@user) + @record.users.include?(@user) end end diff --git a/app/policies/stash_engine/resource_policy.rb b/app/policies/stash_engine/resource_policy.rb index 0d3e1c0e8..62db586f2 100644 --- a/app/policies/stash_engine/resource_policy.rb +++ b/app/policies/stash_engine/resource_policy.rb @@ -19,8 +19,8 @@ def curate? def curator_edit? (curate? && - (@resource.current_resource_state&.resource_state == 'submitted')) || - (@resource.current_resource_state&.resource_state == 'in_progress' && @resource&.user_id == @user.id) + (@record.current_resource_state&.resource_state == 'submitted')) || + (@record.current_resource_state&.resource_state == 'in_progress' && @record&.user_id == @user.id) end class VersionScope diff --git a/app/policies/stash_engine/saved_search_policy.rb b/app/policies/stash_engine/saved_search_policy.rb index 2e99ceeb8..a1adddd81 100644 --- a/app/policies/stash_engine/saved_search_policy.rb +++ b/app/policies/stash_engine/saved_search_policy.rb @@ -2,7 +2,7 @@ module StashEngine class SavedSearchPolicy < ApplicationPolicy def index? - @user.id == @resource.user_id + @user.id == @record.user_id end def create? From 8d15db927e834ae59d9bd065890b9467e1965e24 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 17:12:57 +0200 Subject: [PATCH 13/20] updated destroy identifier policy --- .../stash_engine/admin_datasets_controller.rb | 6 +----- app/helpers/stash_engine/identifiers_helper.rb | 9 --------- app/policies/stash_engine/identifier_policy.rb | 6 ++++++ .../stash_engine/admin_datasets/activity_log.html.erb | 2 +- .../stash_engine/admin_datasets_controller_spec.rb | 4 ++-- 5 files changed, 10 insertions(+), 17 deletions(-) delete mode 100644 app/helpers/stash_engine/identifiers_helper.rb diff --git a/app/controllers/stash_engine/admin_datasets_controller.rb b/app/controllers/stash_engine/admin_datasets_controller.rb index 2afadaa38..5e2e0fc86 100644 --- a/app/controllers/stash_engine/admin_datasets_controller.rb +++ b/app/controllers/stash_engine/admin_datasets_controller.rb @@ -59,12 +59,8 @@ def create_salesforce_case end def destroy - authorize %i[stash_engine admin_datasets], :delete_dataset? - identifier = Identifier.find(params[:id]) - unless helpers.can_user_delete_identifier?(identifier) - redirect_to activity_log_path(identifier.id), alert: 'This dataset can not be deleted.' and return - end + authorize identifier if identifier.destroy redirect_to admin_dashboard_path, notice: "Dataset with DOI #{identifier.identifier} has been deleted." diff --git a/app/helpers/stash_engine/identifiers_helper.rb b/app/helpers/stash_engine/identifiers_helper.rb deleted file mode 100644 index 2a60a0487..000000000 --- a/app/helpers/stash_engine/identifiers_helper.rb +++ /dev/null @@ -1,9 +0,0 @@ -module StashEngine - module IdentifiersHelper - - def can_user_delete_identifier?(identifier) - identifier.resources.count == 1 && - identifier.resources.first.curation_activities.pluck(:status).uniq == ['in_progress'] - end - end -end diff --git a/app/policies/stash_engine/identifier_policy.rb b/app/policies/stash_engine/identifier_policy.rb index c5575d781..817fd23b4 100644 --- a/app/policies/stash_engine/identifier_policy.rb +++ b/app/policies/stash_engine/identifier_policy.rb @@ -1,6 +1,12 @@ module StashEngine class IdentifierPolicy < ApplicationPolicy + def destroy? + @user.superuser? && + @record.resources.count == 1 && + @record.resources.first.curation_activities.pluck(:status).uniq == ['in_progress'] + end + class Scope def initialize(user, scope) @user = user diff --git a/app/views/stash_engine/admin_datasets/activity_log.html.erb b/app/views/stash_engine/admin_datasets/activity_log.html.erb index 0910bcafc..7750344ff 100644 --- a/app/views/stash_engine/admin_datasets/activity_log.html.erb +++ b/app/views/stash_engine/admin_datasets/activity_log.html.erb @@ -156,7 +156,7 @@ Please only do this if you know the author is unable/unwilling to submit it.

- <% if policy([:stash_engine, :admin_datasets]).delete_dataset? && can_user_delete_identifier?(@identifier) %> + <% if policy(@identifier).destroy? %>
<%= form_with(url: stash_url_helpers.ds_admin_destroy_path(id: @identifier&.id), method: :delete, :html => { onsubmit: "document.body.classList.add('prevent-clicks')" }) do -%> diff --git a/spec/requests/stash_engine/admin_datasets_controller_spec.rb b/spec/requests/stash_engine/admin_datasets_controller_spec.rb index b3b38a7d4..8943aa36b 100644 --- a/spec/requests/stash_engine/admin_datasets_controller_spec.rb +++ b/spec/requests/stash_engine/admin_datasets_controller_spec.rb @@ -43,8 +43,8 @@ module StashEngine subject expect(response).to have_http_status(302) - expect(response.headers['Location']).to eq(activity_log_url(identifier.id)) - expect(controller.flash[:alert]).to eq('This dataset can not be deleted.') + expect(response.headers['Location']).to include(choose_dashboard_url) + expect(controller.flash[:alert]).to eq('You are not authorized to view this information.') end end From f860dbfd87be57a31614da9fc405532688206305 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Mon, 2 Dec 2024 19:03:16 +0200 Subject: [PATCH 14/20] updates to feature tests --- spec/features/stash_engine/dataset_queuing_spec.rb | 1 - .../stash_engine/frictionless_validation_spec.rb | 1 - spec/features/stash_engine/review_and_submit_spec.rb | 1 - spec/support/capybara.rb | 4 ++-- spec/support/helpers/collection_helper.rb | 6 +++++- .../helpers/{ajax_helper.rb => page_load_helper.rb} | 9 ++++----- 6 files changed, 11 insertions(+), 11 deletions(-) rename spec/support/helpers/{ajax_helper.rb => page_load_helper.rb} (71%) diff --git a/spec/features/stash_engine/dataset_queuing_spec.rb b/spec/features/stash_engine/dataset_queuing_spec.rb index 504ab828f..25d5829a5 100644 --- a/spec/features/stash_engine/dataset_queuing_spec.rb +++ b/spec/features/stash_engine/dataset_queuing_spec.rb @@ -10,7 +10,6 @@ include Mocks::RSolr include Mocks::Stripe include Mocks::Salesforce - include AjaxHelper before(:each) do FileUtils.rm_f(hold_submissions_path) diff --git a/spec/features/stash_engine/frictionless_validation_spec.rb b/spec/features/stash_engine/frictionless_validation_spec.rb index 0dfd150c0..849e4f2f7 100644 --- a/spec/features/stash_engine/frictionless_validation_spec.rb +++ b/spec/features/stash_engine/frictionless_validation_spec.rb @@ -14,7 +14,6 @@ include Mocks::Salesforce include Mocks::Stripe include Mocks::Aws - include AjaxHelper before(:each) do mock_repository! diff --git a/spec/features/stash_engine/review_and_submit_spec.rb b/spec/features/stash_engine/review_and_submit_spec.rb index 04f20747b..c22ffe0b4 100644 --- a/spec/features/stash_engine/review_and_submit_spec.rb +++ b/spec/features/stash_engine/review_and_submit_spec.rb @@ -11,7 +11,6 @@ include Mocks::Stripe include Mocks::Aws include Mocks::Salesforce - include AjaxHelper before(:each) do mock_repository! diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index d01af7467..7d5c344c4 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative 'solr' -require_relative 'helpers/ajax_helper' +require_relative 'helpers/page_load_helper' require_relative 'helpers/capybara_helper' require_relative 'helpers/tinymce_helper' require_relative 'helpers/routes_helper' @@ -73,7 +73,7 @@ end RSpec.configure do |config| - # config.include(AjaxHelper, type: :feature) + config.include(PageLoadHelper, type: :feature) config.include(CapybaraHelper, type: :feature) config.before(:all, type: :feature) do diff --git a/spec/support/helpers/collection_helper.rb b/spec/support/helpers/collection_helper.rb index e6e47a9d7..70504e3e6 100644 --- a/spec/support/helpers/collection_helper.rb +++ b/spec/support/helpers/collection_helper.rb @@ -11,13 +11,17 @@ def start_new_collection # Make sure you switch to the Selenium driver for the test calling this helper method # e.g. `it 'should test this amazing thing', js: true do` visit('/stash/resources/new?collection') + wait_for_ajax + wait_for_page_load('Describe collection') navigate_to_metadata end + + def navigate_to_metadata # Make sure you switch to the Selenium driver for the test calling this helper method # e.g. `it 'should test this amazing thing', js: true do` - click_link 'Describe collection', wait: 15 + click_link 'Describe collection' expect(page).to have_content('Collection: Basic information') end diff --git a/spec/support/helpers/ajax_helper.rb b/spec/support/helpers/page_load_helper.rb similarity index 71% rename from spec/support/helpers/ajax_helper.rb rename to spec/support/helpers/page_load_helper.rb index d80abd846..f88678010 100644 --- a/spec/support/helpers/ajax_helper.rb +++ b/spec/support/helpers/page_load_helper.rb @@ -1,4 +1,4 @@ -module AjaxHelper +module PageLoadHelper def wait_for_ajax(seconds = Capybara.default_max_wait_time) Timeout.timeout(seconds) do @@ -10,8 +10,7 @@ def finished_all_ajax_requests? page.evaluate_script('jQuery.active').zero? end -end - -RSpec.configure do |config| - config.include(AjaxHelper, type: :system) + def wait_for_page_load(text) + expect(page).to have_content text + end end From 30c01afd9661ede619b5d9a57458f2e89beac788 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 17:17:59 +0200 Subject: [PATCH 15/20] fixed rubocop --- spec/support/helpers/collection_helper.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/support/helpers/collection_helper.rb b/spec/support/helpers/collection_helper.rb index 70504e3e6..1f3000f0b 100644 --- a/spec/support/helpers/collection_helper.rb +++ b/spec/support/helpers/collection_helper.rb @@ -16,8 +16,6 @@ def start_new_collection navigate_to_metadata end - - def navigate_to_metadata # Make sure you switch to the Selenium driver for the test calling this helper method # e.g. `it 'should test this amazing thing', js: true do` From b63ab8220b767a2e914cf539835e24872080c3a4 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 17:24:38 +0200 Subject: [PATCH 16/20] bumped rails version in Gemfile --- Gemfile | 2 +- Gemfile.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 234a5cb86..966695390 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,7 @@ source 'https://rubygems.org' gem 'irb', '~> 1.4.1' gem 'mail', '~> 2.8' gem 'mysql2', '~> 0.5.3' -gem 'rails', '~> 7.0.8.5' +gem 'rails', '~> 7.0.8.7' gem 'react-rails', '~> 2.6.2' gem 'shakapacker', '~> 6.0' gem 'sprockets', '~> 4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 47018b863..5dee8872d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -835,7 +835,7 @@ DEPENDENCIES puma (= 6.4.3) pundit (~> 2.3) rack-attack - rails (~> 7.0.8.5) + rails (~> 7.0.8.7) rb-readline rdoc (~> 6.1.1) react-rails (~> 2.6.2) From f8ac18f741a9a83c219098c982001777b06c3686 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 17:38:02 +0200 Subject: [PATCH 17/20] removed unused method --- app/policies/stash_engine/admin_datasets_policy.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/policies/stash_engine/admin_datasets_policy.rb b/app/policies/stash_engine/admin_datasets_policy.rb index 73a505a55..c959ca8b4 100644 --- a/app/policies/stash_engine/admin_datasets_policy.rb +++ b/app/policies/stash_engine/admin_datasets_policy.rb @@ -28,9 +28,5 @@ def waiver_add? def change_delete_schedule? @user.superuser? end - - def delete_dataset? - @user.superuser? - end end end From caf25f55c04c6e977be3ea4187751afa87fe560f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 16:05:47 +0000 Subject: [PATCH 18/20] Bump nanoid from 3.3.7 to 3.3.8 Bumps [nanoid](https://github.com/ai/nanoid) from 3.3.7 to 3.3.8. - [Release notes](https://github.com/ai/nanoid/releases) - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md) - [Commits](https://github.com/ai/nanoid/compare/3.3.7...3.3.8) --- updated-dependencies: - dependency-name: nanoid dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index d9fd37baa..ad90e70cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7196,9 +7196,9 @@ nanoclone@^0.2.1: integrity sha512-wynEP02LmIbLpcYw8uBKpcfF6dmg2vcpKqxeH5UcoKEYdExslsdUA4ugFauuaeYdTB76ez6gJW8XAZ6CgkXYxA== nanoid@<4.0.0, nanoid@^3.3.7, nanoid@^5.0.0: - version "3.3.7" - resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.7.tgz#d0c301a691bc8d54efa0a2226ccf3fe2fd656bd8" - integrity sha512-eSRppjcPIatRIMC1U6UngP8XFcz8MQWGQdt1MTBQ7NaAmvXDfvNxbvWV3x2y6CdEUciCSsDHDQZbhYaB8QEo2g== + version "3.3.8" + resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.8.tgz#b1be3030bee36aaff18bacb375e5cce521684baf" + integrity sha512-WNLf5Sd8oZxOm+TzppcYk8gVOgP+l58xNy58D0nbUnOxOWRWvlcCV4kUF7ltmI6PsrLl/BgKEyS4mqsGChFN0w== natural-compare@^1.4.0: version "1.4.0" From fdb0e7c978b4325d85825eb7d0fef6427111494a Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 18:08:11 +0200 Subject: [PATCH 19/20] updated policy method --- app/policies/stash_engine/resource_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/stash_engine/resource_policy.rb b/app/policies/stash_engine/resource_policy.rb index f7aa29d3b..59a1d7ab2 100644 --- a/app/policies/stash_engine/resource_policy.rb +++ b/app/policies/stash_engine/resource_policy.rb @@ -24,7 +24,7 @@ def curator_edit? end def change_status? - @resource.curatable? || @user.superuser? + @record.curatable? || @user.superuser? end class VersionScope From bb23c42828b83f9fa096ce80aae95d98f161e205 Mon Sep 17 00:00:00 2001 From: Alin Vetian Date: Wed, 11 Dec 2024 18:27:05 +0200 Subject: [PATCH 20/20] updated node packages --- package.json | 3 +-- yarn.lock | 38 ++++++++++++++++---------------------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index af17010e2..b832aea23 100644 --- a/package.json +++ b/package.json @@ -150,8 +150,7 @@ "stylelint": "^16.0.0", "stylelint-config-property-sort-order-smacss": "^10.0.0", "stylelint-config-recommended-scss": "^14.0.0", - "webpack-dev-server": "^5.1.0", - "express": "^4.21.0", + "webpack-dev-server": "^5.2.0", "serve-static": "^1.16.1" }, "resolutions": { diff --git a/yarn.lock b/yarn.lock index d9fd37baa..ddc749316 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4698,10 +4698,10 @@ expect@^29.7.0: jest-message-util "^29.7.0" jest-util "^29.7.0" -express@^4.19.2, express@^4.21.0: - version "4.21.1" - resolved "https://registry.yarnpkg.com/express/-/express-4.21.1.tgz#9dae5dda832f16b4eec941a4e44aa89ec481b281" - integrity sha512-YSFlK1Ee0/GC8QaO91tHcDxJiE/X4FbpAyQWkxAvG6AXCuR65YzK8ua6D9hvi/TzUfZMpc+BwuM1IPw8fmQBiQ== +express@^4.21.2: + version "4.21.2" + resolved "https://registry.yarnpkg.com/express/-/express-4.21.2.tgz#cf250e48362174ead6cea4a566abef0162c1ec32" + integrity sha512-28HqgMZAmih1Czt9ny7qr6ek2qddF4FclbMzwhCREB6OFfH+rXAnuNCwo1/wFvrtbgsQDb4kSbX9de9lFbrXnA== dependencies: accepts "~1.3.8" array-flatten "1.1.1" @@ -4722,7 +4722,7 @@ express@^4.19.2, express@^4.21.0: methods "~1.1.2" on-finished "2.4.1" parseurl "~1.3.3" - path-to-regexp "0.1.10" + path-to-regexp "0.1.12" proxy-addr "~2.0.7" qs "6.13.0" range-parser "~1.2.1" @@ -5266,11 +5266,6 @@ html-encoding-sniffer@^3.0.0: dependencies: whatwg-encoding "^2.0.0" -html-entities@^2.4.0: - version "2.5.2" - resolved "https://registry.yarnpkg.com/html-entities/-/html-entities-2.5.2.tgz#201a3cf95d3a15be7099521620d19dfb4f65359f" - integrity sha512-K//PSRMQk4FZ78Kyau+mZurHn3FH0Vwr+H36eE0rPbeYkRRi9YxceYPhuN60UwWorxyKHhqoAJl2OFKa4BVtaA== - html-escaper@^2.0.0: version "2.0.2" resolved "https://registry.yarnpkg.com/html-escaper/-/html-escaper-2.0.2.tgz#dfd60027da36a36dfcbe236262c00a5822681453" @@ -5349,7 +5344,7 @@ http-proxy-agent@^7.0.0, http-proxy-agent@^7.0.1: agent-base "^7.1.0" debug "^4.3.4" -http-proxy-middleware@^2.0.3: +http-proxy-middleware@^2.0.7: version "2.0.7" resolved "https://registry.yarnpkg.com/http-proxy-middleware/-/http-proxy-middleware-2.0.7.tgz#915f236d92ae98ef48278a95dedf17e991936ec6" integrity sha512-fgVY8AV7qU7z/MmXJ/rxwbrtQH4jBQ9m7kp3llF0liB7glmFeVZFBepQb32T3y8n8k2+AEYuMPCpinYW+/CuRA== @@ -7558,10 +7553,10 @@ path-parse@^1.0.7: resolved "https://registry.yarnpkg.com/path-parse/-/path-parse-1.0.7.tgz#fbc114b60ca42b30d9daf5858e4bd68bbedb6735" integrity sha512-LDJzPVEEEPR+y48z93A0Ed0yXb8pAByGWo/k5YYdYgpY2/2EsOsksJrq7lOHxryrVOn1ejG6oAp8ahvOIQD8sw== -path-to-regexp@0.1.10: - version "0.1.10" - resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.10.tgz#67e9108c5c0551b9e5326064387de4763c4d5f8b" - integrity sha512-7lf7qcQidTku0Gu3YDPc8DJ1q7OOucfa/BSsIwjuh56VU7katFvuM8hULfkwB3Fns/rsVF7PwPKVw1sl5KQS9w== +path-to-regexp@0.1.12: + version "0.1.12" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.12.tgz#d5e1a12e478a976d432ef3c58d534b9923164bb7" + integrity sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ== path-type@^4.0.0: version "4.0.0" @@ -9850,10 +9845,10 @@ webpack-dev-middleware@^7.4.2: range-parser "^1.2.1" schema-utils "^4.0.0" -webpack-dev-server@^5.1.0: - version "5.1.0" - resolved "https://registry.yarnpkg.com/webpack-dev-server/-/webpack-dev-server-5.1.0.tgz#8f44147402b4d8ab99bfeb9b6880daa1411064e5" - integrity sha512-aQpaN81X6tXie1FoOB7xlMfCsN19pSvRAeYUHOdFWOlhpQ/LlbfTqYwwmEDFV0h8GGuqmCmKmT+pxcUV/Nt2gQ== +webpack-dev-server@^5.2.0: + version "5.2.0" + resolved "https://registry.yarnpkg.com/webpack-dev-server/-/webpack-dev-server-5.2.0.tgz#68043886edaa3fd875ad20e01589990a79612f9c" + integrity sha512-90SqqYXA2SK36KcT6o1bvwvZfJFcmoamqeJY7+boioffX9g9C0wjjJRGUrQIuh43pb0ttX7+ssavmj/WN2RHtA== dependencies: "@types/bonjour" "^3.5.13" "@types/connect-history-api-fallback" "^1.5.4" @@ -9868,10 +9863,9 @@ webpack-dev-server@^5.1.0: colorette "^2.0.10" compression "^1.7.4" connect-history-api-fallback "^2.0.0" - express "^4.19.2" + express "^4.21.2" graceful-fs "^4.2.6" - html-entities "^2.4.0" - http-proxy-middleware "^2.0.3" + http-proxy-middleware "^2.0.7" ipaddr.js "^2.1.0" launch-editor "^2.6.1" open "^10.0.3"