Only superusers update embargos

app/controllers/hyrax/embargoes_controller.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+module Hyrax
+  class EmbargoesController < ApplicationController
+    include Hyrax::EmbargoesControllerBehavior
+    # Override edit method to check authorization
+    def edit
+      add_breadcrumb t(:'hyrax.controls.home'), root_path
+      add_breadcrumb t(:'hyrax.dashboard.breadcrumbs.admin'), hyrax.dashboard_path
+      add_breadcrumb t(:'hyrax.embargoes.index.manage_embargoes'), hyrax.embargoes_path
+      add_breadcrumb t(:'hyrax.embargoes.edit.embargo_update'), '#'
+      authorize! :edit, Hydra::AccessControls::Embargo
+    end
+    with_themed_layout 'dashboard'
+  end
+end

app/models/ability.rb

@@ -14,9 +14,7 @@ def custom_permissions
         approver_for?(admin_set: obj.admin_set)
       end
     end
-
     return unless admin?
-
     can [:create, :show, :add_user, :remove_user, :index, :edit, :update, :destroy], Role
     can [:destroy], ActiveFedora::Base
     can [:read], Schools::School
@@ -30,9 +28,8 @@ def curation_concerns_permissions
     alias_action :versions, to: :update
     alias_action :file_manager, to: :update
 
-    return if admin? || can_review_submissions?
-
-    cannot :index, Hydra::AccessControls::Embargo
+    return if admin?
+    cannot [:update, :edit, :manage, :index], Hydra::AccessControls::Embargo
     cannot :index, Hydra::AccessControls::Lease
   end
 

app/views/hyrax/dashboard/sidebar/_tasks.html.erb

@@ -12,7 +12,7 @@
     <% end %>
   <% end %>
 
-  <% if can?(:index, Hydra::AccessControls::Embargo) %>
+  <% if can?(:update, Hydra::AccessControls::Embargo) %>
     <%= menu.nav_link(hyrax.embargoes_path) do %>
       <span class="fa fa-flag"></span> <span class="sidebar-action-text"><%= t('hyrax.embargoes.index.manage_embargoes') %></span>
     <% end %>

spec/system/admin_dashboard_spec.rb

@@ -43,8 +43,8 @@
     end
   end
 
-  context 'as an approving user' do
-    let(:user)     { User.find_by(ppid: 'candleradmin') }
+  context 'as a superuser' do
+    let(:superuser) { User.find_by(uid: 'superman001') }
     let(:etd)      { FactoryBot.build(:sample_data_with_everything_embargoed) }
     let(:file_set) { FactoryBot.create(:public_pdf) }
 
@@ -53,12 +53,17 @@
       etd.representative = file_set
       etd.save
 
-      login_as user
+      login_as superuser
     end
 
     scenario 'editing an embargo' do
       visit '/dashboard'
       click_link 'Manage Embargoes'
+      expect(page).to have_content('All Active Embargoes')
+      # check that breadcrumbs are present
+      expect(page).to have_link('Home')
+      expect(page).to have_link('Dashboard')
+      expect(page).to have_link('Manage Embargoes')
       # test that tabs are working correctly
       click_link 'Expired Active Embargoes'
       expect(page).to have_content('There are no expired embargoes')
@@ -82,4 +87,23 @@
         .to eq Date.parse('2199-01-01')
     end
   end
+
+  context 'as an approving user' do
+    let(:approving_user) { User.find_by(ppid: 'candleradmin') }
+    let(:etd)      { FactoryBot.build(:sample_data_with_everything_embargoed) }
+    let(:file_set) { FactoryBot.create(:public_pdf) }
+
+    before do
+      etd.ordered_members << file_set
+      etd.representative = file_set
+      etd.save
+
+      login_as approving_user
+    end
+
+    scenario 'editing an embargo' do
+      visit '/dashboard'
+      expect(page).not_to have_link 'Manage Embargoes'
+    end
+  end
 end

spec/system/embargo_edit_spec.rb

@@ -47,7 +47,9 @@
     FactoryBot.create :primary_uploaded_file, user_id: user.id
   end
   let(:six_years_from_today) { Time.zone.today + 6.years }
+  let(:eight_years_from_today) { Time.zone.today + 8.years }
   let(:approving_user) { User.find_by(uid: "candleradmin") }
+  let(:superuser) { User.find_by(uid: 'superman001') }
 
   before do
     allow(CharacterizeJob).to receive(:perform_later) # There is no fits installed on travis-ci
@@ -65,18 +67,35 @@
       .to have_attributes visibility: restricted
   end
 
-  scenario "Approver can change the embargo settings" do
+  scenario "A regular user cannot change the embargo settings after saving" do
+    login_as user
+    visit("/embargoes/#{etd.id}/edit")
+    expect(page).to have_content("You are not authorized to access this page.")
+  end
+
+  scenario "Approver cannot change the embargo settings" do
     login_as approving_user
+    expect(approving_user.reload.admin?).to eq false
+    expect(approving_user.reload.ability.can?(:manage, Hydra::AccessControls::Embargo)).to eq false
+    expect(approving_user.reload.ability.can?(:edit, Hydra::AccessControls::Embargo)).to eq false
+    expect(approving_user.reload.ability.can?(:update, Hydra::AccessControls::Embargo)).to eq false
+    visit("/embargoes/#{etd.id}/edit")
+    expect(page).to have_content("You are not authorized to access this page.")
+  end
+
+  scenario "Superuser can change the embargo settings" do
+    login_as superuser
     visit("/embargoes/#{etd.id}/edit")
     expect(find('#etd_visibility_during_embargo').find(:xpath, 'option[1]').text).to eq 'All Restricted'
     find('#etd_embargo_release_date')
-    fill_in 'etd_embargo_release_date', with: (Time.zone.today + 8.years).to_s, fill_options: { clear: :backspace }
+    fill_in 'etd_embargo_release_date', with: eight_years_from_today.to_s, fill_options: { clear: :backspace }
     execute_script('$("form").submit()')
     expect(page).to have_current_path("/concern/etds/#{etd.id}?locale=en")
     expect(page).to have_content etd.title.first
     expect(page).to have_content "successfully updated"
     expect(page).to have_content etd.abstract.first
     expect(page).to have_content etd.table_of_contents.first
+    expect(etd.reload.embargo_release_date).to eq eight_years_from_today
     expect(etd.reload.file_sets.first.embargo.embargo_release_date).to eq etd.reload.embargo_release_date
   end
 end