Default admin set grants the registered group deposit access

Fixes #602

app/forms/hyrax/forms/permission_template_form.rb

@@ -62,7 +62,7 @@ def update(attributes)
       # of the active workflow
       def update_management
         admin_set.update_access_controls!
-        update_workflow_responsibilities
+        update_workflow_approving_responsibilities
       end
 
       private
@@ -84,12 +84,11 @@ def update_participants_options(attributes)
 
         # Grant workflow approve roles for any admin set managers
         # and revoke the approving role for non-managers
-        def update_workflow_responsibilities
+        def update_workflow_approving_responsibilities
           return unless active_workflow
           approving_role = Sipity::Role.find_by_name('approving')
           return unless approving_role
-          add_workflow_responsibilities(approving_role, manager_agents)
-          remove_workflow_responsibilities(approving_role, manager_agents)
+          active_workflow.update_responsibilities(role: approving_role, agents: manager_agents)
         end
 
         # @return [Array<Sipity::Agent>] a list of sipity agents corresponding to the manager role of the permission_template
@@ -111,24 +110,6 @@ def manager_grants
           model.access_grants.where(access: 'manage'.freeze)
         end
 
-        # Find any workflow_responsibilities held by agents not in the authorized_agents
-        # and remove them
-        # @param [Sipity::Role] approving_role
-        # @param [Array<Sipity::Agent>] agents
-        def remove_workflow_responsibilities(approving_role, agents)
-          wf_role = Sipity::WorkflowRole.find_by(workflow: active_workflow, role_id: approving_role)
-          wf_role.workflow_responsibilities.where.not(agent: agents).destroy_all
-        end
-
-        # Give workflow responsibilites to the provided agents for the given role
-        # @param [Sipity::Role] approving_role
-        # @param [Array<Sipity::Agent>] authorized_agents
-        def add_workflow_responsibilities(approving_role, authorized_agents)
-          Workflow::PermissionGenerator.call(roles: approving_role,
-                                             workflow: active_workflow,
-                                             agents: authorized_agents)
-        end
-
         # @return [String, Nil] error_code if validation fails, nil otherwise
         def update_visibility_options(attributes)
           error_code = validate_visibility_combinations(attributes)

app/models/hyrax/permission_template_access.rb

@@ -5,16 +5,16 @@ class PermissionTemplateAccess < ActiveRecord::Base
 
     belongs_to :permission_template
 
-    def view?
-      access == 'view'
-    end
+    VIEW = 'view'.freeze
+    DEPOSIT = 'deposit'.freeze
+    MANAGE = 'manage'.freeze
 
-    def deposit?
-      access == 'deposit'
-    end
-
-    def manage?
-      access == 'manage'
-    end
+    enum(
+      access: {
+        VIEW => VIEW,
+        DEPOSIT => DEPOSIT,
+        MANAGE => MANAGE
+      }
+    )
   end
 end

app/models/sipity/workflow.rb

@@ -54,5 +54,35 @@ def self.activate!(permission_template:, workflow_id: nil, workflow_name: nil)
         workflow.update!(active: true)
       end
     end
+
+    # Grant a workflow responsibility to a set of agents and remove it from any
+    # agents who currently have the workflow responsibility, but are not in the
+    # provided list
+    # @param [Sipity::Role] role the role to grant
+    # @param [Array<Sipity::Agent>] agents the agents to grant it to
+    def update_responsibilities(role:, agents:)
+      add_workflow_responsibilities(role, agents)
+      remove_workflow_responsibilities(role, agents)
+    end
+
+    private
+
+      # Give workflow responsibilites to the provided agents for the given role
+      # @param [Sipity::Role] role
+      # @param [Array<Sipity::Agent>] agents
+      def add_workflow_responsibilities(role, agents)
+        Hyrax::Workflow::PermissionGenerator.call(roles: role,
+                                                  workflow: self,
+                                                  agents: agents)
+      end
+
+      # Find any workflow_responsibilities held by agents not in the allowed_agents
+      # and remove them
+      # @param [Sipity::Role] role
+      # @param [Array<Sipity::Agent>] allowed_agents
+      def remove_workflow_responsibilities(role, allowed_agents)
+        wf_role = Sipity::WorkflowRole.find_by(workflow: self, role_id: role)
+        wf_role.workflow_responsibilities.where.not(agent: allowed_agents).destroy_all
+      end
   end
 end

app/services/hyrax/admin_set_create_service.rb

@@ -58,8 +58,11 @@ def create
       admin_set.creator = [creating_user.user_key] if creating_user
       admin_set.save.tap do |result|
         if result
-          permission_template = create_permission_template
-          create_workflows_for(permission_template: permission_template)
+          ActiveRecord::Base.transaction do
+            permission_template = create_permission_template
+            workflow = create_workflows_for(permission_template: permission_template)
+            create_default_access_for(permission_template: permission_template, workflow: workflow) if admin_set.default_set?
+          end
         end
       end
     end
@@ -80,6 +83,13 @@ def create_workflows_for(permission_template:)
         Sipity::Workflow.activate!(permission_template: permission_template, workflow_name: Hyrax.config.default_active_workflow_name)
       end
 
+      # Gives deposit access to all registered users
+      def create_default_access_for(permission_template:, workflow:)
+        permission_template.access_grants.create(agent_type: 'group', agent_id: 'registered', access: 'deposit')
+        deposit = Sipity::Role.find_by_name!('depositing')
+        workflow.update_responsibilities(role: deposit, agents: Hyrax::Group.new('registered'))
+      end
+
       def default_workflow_importer
         Hyrax::Workflow::WorkflowImporter.method(:load_workflow_for)
       end

spec/models/hyrax/permission_template_spec.rb

@@ -43,7 +43,7 @@
     it 'queries the underlying access_grants' do
       template = create(:permission_template)
       to_find = template.access_grants.create!(agent_type: 'user', access: 'manage', agent_id: '123')
-      template.access_grants.create!(agent_type: 'user', access: 'read', agent_id: '456')
+      template.access_grants.create!(agent_type: 'user', access: 'view', agent_id: '456')
       template.access_grants.create!(agent_type: 'group', access: 'manage', agent_id: '789')
 
       expect(template.agent_ids_for(agent_type: 'user', access: 'manage')).to eq([to_find.agent_id])

spec/services/hyrax/admin_set_create_service_spec.rb

@@ -1,63 +1,81 @@
 require 'spec_helper'
 
 RSpec.describe Hyrax::AdminSetCreateService do
-  let(:admin_set) { AdminSet.new(title: ['test']) }
-  let(:workflow_importer) { double(call: true) }
-  let(:service) { described_class.new(admin_set: admin_set, creating_user: user, workflow_importer: workflow_importer) }
   let(:user) { instance_double(User, user_key: 'user-1234') }
 
-  subject { service }
-  its(:default_workflow_importer) { is_expected.to respond_to(:call) }
-
   describe '.create_default_admin_set' do
+    let(:admin_set) { AdminSet.find(AdminSet::DEFAULT_ID) }
+    let(:responsibilities) { Sipity::WorkflowResponsibility.where(workflow_role: admin_set.active_workflow.workflow_roles) }
     # It is important to test the side-effects as a default admin set is a fundamental assumption for Hyrax.
     it 'creates AdminSet, Hyrax::PermissionTemplate, Sipity::Workflow(s), and activates a Workflow', slow: true do
       described_class.create_default_admin_set(admin_set_id: AdminSet::DEFAULT_ID, title: AdminSet::DEFAULT_TITLE)
-      admin_set = AdminSet.find(AdminSet::DEFAULT_ID)
       expect(admin_set.permission_template).to be_persisted
       expect(admin_set.active_workflow).to be_persisted
+      expect(responsibilities.count).to eq 1
+      expect(responsibilities.first.agent.proxy_for_id).to eq "registered"
+      expect(responsibilities.first.agent.proxy_for_type).to eq "Hyrax::Group"
     end
   end
 
   describe ".call" do
-    it 'will raise ActiveFedora::IllegalOperation if you attempt to a default admin set' do
-      expect { described_class.call(admin_set: AdminSet.new(id: AdminSet::DEFAULT_ID), creating_user: user) }.to raise_error(RuntimeError)
+    subject { described_class.call(admin_set: admin_set, creating_user: user) }
+
+    let(:admin_set) { AdminSet.new(title: ['test']) }
+
+    context "when using the default admin set" do
+      let(:admin_set) { AdminSet.new(id: AdminSet::DEFAULT_ID) }
+
+      it 'will raise ActiveFedora::IllegalOperation if you attempt to a default admin set' do
+        expect { subject }.to raise_error(RuntimeError)
+      end
     end
 
     it "is a convenience method for .new#create" do
       service = instance_double(described_class)
       expect(described_class).to receive(:new).and_return(service)
       expect(service).to receive(:create)
-      described_class.call(admin_set: admin_set, creating_user: user)
+      subject
     end
   end
 
-  describe "#create" do
-    subject { service.create }
+  describe "an instance" do
+    subject { service }
+
+    let(:workflow_importer) { double(call: true) }
+    let(:admin_set) { AdminSet.new(title: ['test']) }
+    let(:service) { described_class.new(admin_set: admin_set, creating_user: user, workflow_importer: workflow_importer) }
 
-    context "when the admin_set is valid" do
-      let(:permission_template) { Hyrax::PermissionTemplate.find_by(admin_set_id: admin_set.id) }
-      let(:grant) { permission_template.access_grants.first }
-      it "creates an AdminSet, PermissionTemplate, Workflows, activates the default workflow, and sets access" do
-        expect(Sipity::Workflow).to receive(:activate!).with(permission_template: kind_of(Hyrax::PermissionTemplate), workflow_name: Hyrax.config.default_active_workflow_name)
-        expect do
-          expect(subject).to be true
-        end.to change { admin_set.persisted? }.from(false).to(true)
-        expect(admin_set.read_groups).to eq ['public']
-        expect(admin_set.edit_groups).to eq ['admin']
-        expect(grant.agent_id).to eq user.user_key
-        expect(grant.access).to eq 'manage'
-        expect(admin_set.creator).to eq [user.user_key]
-        expect(workflow_importer).to have_received(:call).with(permission_template: permission_template)
-        expect(permission_template).to be_persisted
+    its(:default_workflow_importer) { is_expected.to respond_to(:call) }
+
+    describe "#create" do
+      subject { service.create }
+
+      context "when the admin_set is valid" do
+        let(:permission_template) { Hyrax::PermissionTemplate.find_by(admin_set_id: admin_set.id) }
+        let(:grant) { permission_template.access_grants.first }
+
+        it "creates an AdminSet, PermissionTemplate, Workflows, activates the default workflow, and sets access" do
+          expect(Sipity::Workflow).to receive(:activate!).with(permission_template: kind_of(Hyrax::PermissionTemplate), workflow_name: Hyrax.config.default_active_workflow_name)
+          expect do
+            expect(subject).to be true
+          end.to change { admin_set.persisted? }.from(false).to(true)
+          expect(admin_set.read_groups).to eq ['public']
+          expect(admin_set.edit_groups).to eq ['admin']
+          expect(grant.agent_id).to eq user.user_key
+          expect(grant.access).to eq 'manage'
+          expect(admin_set.creator).to eq [user.user_key]
+          expect(workflow_importer).to have_received(:call).with(permission_template: permission_template)
+          expect(permission_template).to be_persisted
+        end
       end
-    end
 
-    context "when the admin_set is invalid" do
-      let(:admin_set) { AdminSet.new } # Missing title
-      it { is_expected.to be false }
-      it 'will not call the workflow_importer' do
-        expect(workflow_importer).not_to have_received(:call)
+      context "when the admin_set is invalid" do
+        let(:admin_set) { AdminSet.new } # Missing title
+
+        it { is_expected.to be false }
+        it 'will not call the workflow_importer' do
+          expect(workflow_importer).not_to have_received(:call)
+        end
       end
     end
   end