Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2544 allow superusers to withdraw data submissions from any status #1939

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/controllers/stash_engine/admin_dashboard_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/stash_engine/admin_datasets_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 4 additions & 2 deletions app/models/stash_engine/curation_activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions app/policies/stash_engine/resource_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
ahamelers marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<p>Edit curation status of <em><%= @resource.title %></em></p>
<%= 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 policy(@resource).change_status? && filter_status_select(@resource.current_curation_status) %>
<%= form.fields_for :curation_activity, @curation_activity do |ca| %>
<div class="c-input">
<%= ca.label :status, 'Status', class: 'c-input__label' %>
Expand Down
1 change: 0 additions & 1 deletion spec/features/stash_engine/dataset_queuing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
include Mocks::RSolr
include Mocks::Stripe
include Mocks::Salesforce
include AjaxHelper

before(:each) do
FileUtils.rm_f(hold_submissions_path)
Expand Down
1 change: 0 additions & 1 deletion spec/features/stash_engine/frictionless_validation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
include Mocks::Salesforce
include Mocks::Stripe
include Mocks::Aws
include AjaxHelper

before(:each) do
mock_repository!
Expand Down
1 change: 0 additions & 1 deletion spec/features/stash_engine/review_and_submit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
include Mocks::Stripe
include Mocks::Aws
include Mocks::Salesforce
include AjaxHelper

before(:each) do
mock_repository!
Expand Down
27 changes: 21 additions & 6 deletions spec/models/stash_engine/curation_activity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.each_key do |status|
it "allows withdrawn for #{status} status" do
expect(CurationActivity.allowed_states(status, user)).to include('withdrawn')
end
end
end
end
end
end
4 changes: 2 additions & 2 deletions spec/support/capybara.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion spec/support/helpers/collection_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ 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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module AjaxHelper
module PageLoadHelper

def wait_for_ajax(seconds = Capybara.default_max_wait_time)
Timeout.timeout(seconds) do
Expand All @@ -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