diff --git a/app/controllers/sufia/admin/permission_templates_controller.rb b/app/controllers/sufia/admin/permission_templates_controller.rb index 027d8253d2..5c9d3df7c2 100644 --- a/app/controllers/sufia/admin/permission_templates_controller.rb +++ b/app/controllers/sufia/admin/permission_templates_controller.rb @@ -5,7 +5,7 @@ class PermissionTemplatesController < ApplicationController def update authorize! :update, @permission_template - @permission_template.update(update_params) + Forms::PermissionTemplateForm.new(@permission_template).update(update_params) # Ensure we redirect to active tab current_tab = params[:sufia_permission_template][:access_grants_attributes].present? ? 'participants' : 'visibility' redirect_to sufia.edit_admin_admin_set_path(params[:admin_set_id], anchor: current_tab), diff --git a/app/forms/sufia/forms/permission_template_form.rb b/app/forms/sufia/forms/permission_template_form.rb index baeb49387e..6e09f1f282 100644 --- a/app/forms/sufia/forms/permission_template_form.rb +++ b/app/forms/sufia/forms/permission_template_form.rb @@ -15,6 +15,21 @@ def visibility_options [Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED, I18n.t('.institution', scope: i18n_prefix)], [Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE, I18n.t('.restricted', scope: i18n_prefix)]] end + + def update(attributes) + manage_grants = attributes[:access_grants_attributes].select { |x| x[:access] == 'manage' } + grant_admin_set_access(manage_grants) if manage_grants.present? + model.update(attributes) + end + + private + + def grant_admin_set_access(manage_grants) + admin_set = AdminSet.find(model.admin_set_id) + admin_set.edit_users = manage_grants.select { |x| x[:agent_type] == 'user' }.map { |x| x[:agent_id] } + admin_set.edit_groups = manage_grants.select { |x| x[:agent_type] == 'group' }.map { |x| x[:agent_id] } + admin_set.save! + end end end end diff --git a/app/models/concerns/sufia/ability.rb b/app/models/concerns/sufia/ability.rb index 9beedf39ad..6a1cf71efa 100644 --- a/app/models/concerns/sufia/ability.rb +++ b/app/models/concerns/sufia/ability.rb @@ -63,8 +63,15 @@ def feature_abilities end def admin_set_abilities - return unless admin? - can :manage, [AdminSet, Sufia::PermissionTemplate, Sufia::PermissionTemplateAccess] + can :create, AdminSet if admin? + + can [:create, :edit, :update, :destroy], Sufia::PermissionTemplate do |template| + test_edit(template.admin_set_id) + end + + can [:create, :edit, :update, :destroy], Sufia::PermissionTemplateAccess do |access| + test_edit(access.permission_template.admin_set_id) + end end private diff --git a/spec/controllers/sufia/admin/permission_templates_controller_spec.rb b/spec/controllers/sufia/admin/permission_templates_controller_spec.rb index 8e47fd439e..47ca38432e 100644 --- a/spec/controllers/sufia/admin/permission_templates_controller_spec.rb +++ b/spec/controllers/sufia/admin/permission_templates_controller_spec.rb @@ -18,46 +18,29 @@ end end + let(:form) { instance_double(Sufia::Forms::PermissionTemplateForm) } + before do + allow(Sufia::Forms::PermissionTemplateForm).to receive(:new).with(permission_template).and_return(form) + end + context "when signed in as an admin" do describe "update participants" do let(:admin_set) { create(:admin_set) } let!(:permission_template) { Sufia::PermissionTemplate.create!(admin_set_id: admin_set.id) } + let(:grant_attributes) { [{ "agent_type" => "user", "agent_id" => "bob", "access" => "view" }] } let(:input_params) do { admin_set_id: admin_set.id, - sufia_permission_template: { - access_grants_attributes: [ - { "agent_type" => "Person", "agent_id" => "bob", "access" => "View" } - ] - } } + sufia_permission_template: form_attributes } end + let(:form_attributes) { { visibility: 'open', access_grants_attributes: grant_attributes } } it "is successful" do expect(controller).to receive(:authorize!).with(:update, permission_template) - expect do - put :update, params: input_params - end.to change { permission_template.access_grants.count }.by(1) + expect(form).to receive(:update).with(ActionController::Parameters.new(form_attributes).permit!) + put :update, params: input_params expect(response).to redirect_to(sufia.edit_admin_admin_set_path(admin_set, anchor: 'participants')) expect(flash[:notice]).to eq 'Permissions updated' end end - describe "update visibility" do - let(:admin_set) { create(:admin_set) } - let!(:permission_template) { Sufia::PermissionTemplate.create!(admin_set_id: admin_set.id) } - let(:input_params) do - { admin_set_id: admin_set.id, - sufia_permission_template: { - visibility: 'open' - } } - end - - it "is successful" do - expect(controller).to receive(:authorize!).with(:update, permission_template) - expect do - put :update, params: input_params - end.to change { permission_template.reload.visibility }.from(nil).to('open') - expect(response).to redirect_to(sufia.edit_admin_admin_set_path(admin_set, anchor: 'visibility')) - expect(flash[:notice]).to eq 'Permissions updated' - end - end end end diff --git a/spec/forms/sufia/forms/permission_template_form_spec.rb b/spec/forms/sufia/forms/permission_template_form_spec.rb new file mode 100644 index 0000000000..6b309e727e --- /dev/null +++ b/spec/forms/sufia/forms/permission_template_form_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +RSpec.describe Sufia::Forms::PermissionTemplateForm do + describe "#update" do + let(:input_params) do + ActionController::Parameters.new(access_grants_attributes: grant_attributes).permit! + end + let(:admin_set) { create(:admin_set) } + let(:permission_template) { create(:permission_template, admin_set_id: admin_set.id) } + let(:form) { described_class.new(permission_template) } + subject { form.update(input_params) } + + context "with a user manager" do + let(:grant_attributes) do + [ActionController::Parameters.new(agent_type: "user", + agent_id: "bob", + access: "manage").permit!] + end + it "also adds edit_access to the AdminSet itself" do + expect { subject }.to change { permission_template.access_grants.count }.by(1) + expect(admin_set.reload.edit_users).to include 'bob' + end + end + + context "with a group manager" do + let(:grant_attributes) do + [ActionController::Parameters.new(agent_type: "group", + agent_id: "bob", + access: "manage").permit!] + end + it "also adds edit_access to the AdminSet itself" do + expect { subject }.to change { permission_template.access_grants.count }.by(1) + expect(admin_set.reload.edit_groups).to include 'bob' + end + end + + context "without a manager" do + let(:grant_attributes) do + [ActionController::Parameters.new(agent_type: "user", + agent_id: "bob", + access: "view").permit!] + end + it "doesn't adds edit_access to the AdminSet itself" do + expect { subject }.to change { permission_template.access_grants.count }.by(1) + expect(admin_set.reload.edit_users).to be_empty + end + end + end +end diff --git a/spec/models/sufia/ability_spec.rb b/spec/models/sufia/ability_spec.rb index 6a492f734f..dd61a8f842 100644 --- a/spec/models/sufia/ability_spec.rb +++ b/spec/models/sufia/ability_spec.rb @@ -26,8 +26,6 @@ it { is_expected.not_to be_able_to(:read, Sufia::Statistics) } it { is_expected.not_to be_able_to(:read, :admin_dashboard) } it { is_expected.not_to be_able_to(:create, AdminSet) } - it { is_expected.not_to be_able_to(:update, Sufia::PermissionTemplate) } - it { is_expected.not_to be_able_to(:destroy, Sufia::PermissionTemplateAccess) } end describe "a user in the admin group" do @@ -40,9 +38,32 @@ it { is_expected.to be_able_to(:read, ContentBlock) } it { is_expected.to be_able_to(:read, Sufia::Statistics) } it { is_expected.to be_able_to(:read, :admin_dashboard) } - it { is_expected.to be_able_to(:manage, AdminSet) } - it { is_expected.to be_able_to(:manage, Sufia::PermissionTemplate) } - it { is_expected.to be_able_to(:manage, Sufia::PermissionTemplateAccess) } + it { is_expected.not_to be_able_to(:manage, AdminSet) } + it { is_expected.to be_able_to(:create, AdminSet) } + end + + describe "AdminSets and PermissionTemplates" do + let(:permission_template) { build(:permission_template, admin_set_id: admin_set.id) } + let(:permission_template_access) { build(:permission_template_access, permission_template: permission_template) } + describe "a user with edit access" do + let(:user) { create(:user) } + let(:admin_set) { create(:admin_set, edit_users: [user]) } + it { is_expected.to be_able_to(:edit, admin_set) } + it { is_expected.to be_able_to(:update, admin_set) } + it { is_expected.to be_able_to(:destroy, admin_set) } + it { is_expected.to be_able_to(:create, permission_template) } + it { is_expected.to be_able_to(:create, permission_template_access) } + end + + describe "a user without edit access" do + let(:user) { create(:user) } + let(:admin_set) { create(:admin_set) } + it { is_expected.not_to be_able_to(:edit, admin_set) } + it { is_expected.not_to be_able_to(:update, admin_set) } + it { is_expected.not_to be_able_to(:destroy, admin_set) } + it { is_expected.not_to be_able_to(:create, permission_template) } + it { is_expected.not_to be_able_to(:create, permission_template_access) } + end end describe "proxies and transfers" do