From 3f71bef23b699458e93b15a740386b10cb9ad374 Mon Sep 17 00:00:00 2001 From: Bess Sadler Date: Tue, 6 Jun 2017 14:30:45 -0400 Subject: [PATCH] Superusers need to be in the approving role in order to participate in the approval workflow --- .rubocop.yml | 3 + lib/workflow_setup.rb | 30 ++++++++- spec/features/laney_workflow_etd_spec.rb | 14 ++-- .../config/emory/laney_graduate_school.yml | 1 - spec/lib/workflow_setup_spec.rb | 64 +++++++++++++------ 5 files changed, 82 insertions(+), 30 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 681ed4f21..5fe95472c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,6 +1,9 @@ inherit_gem: bixby: bixby_default.yml +Metrics/AbcSize: + Enabled: false + Metrics/BlockLength: Enabled: false diff --git a/lib/workflow_setup.rb b/lib/workflow_setup.rb index cfa851c3b..90a56096c 100644 --- a/lib/workflow_setup.rb +++ b/lib/workflow_setup.rb @@ -53,6 +53,7 @@ def make_mediated_deposit_admin_set(admin_set_title, workflow_name = "one_step_m # currently active workflow # @param [AdminSet] admin_set # @param [String|Sipity::Role] role e.g., "approving" "depositing" "managing" + # @return [Array] An array of Sipity::Agent objects def users_in_role(admin_set, role) return [] unless admin_set.permission_template.available_workflows.where(active: true).count > 0 users_in_role = [] @@ -138,10 +139,20 @@ def everyone_can_deposit_everywhere end end - # Give all superusers the managing role all workflows + # Give superusers the managing role in all AdminSets + # Also give them all workflow roles for all AdminSets def give_superusers_superpowers @logger.info "Giving superuser powers to #{superusers.pluck(:email)}" - superusers_as_sipity_agents = superusers.map(&:to_sipity_agent) + give_superusers_managing_role + give_superusers_workflow_roles + end + + def superusers_as_sipity_agents + superusers.map(&:to_sipity_agent) + end + + # Give all superusers the managing role all workflows + def give_superusers_managing_role AdminSet.all.each do |admin_set| admin_set.permission_template.available_workflows.each do |workflow| # .where(active: true) ? workflow.update_responsibilities(role: Sipity::Role.where(name: "managing").first, agents: superusers_as_sipity_agents) @@ -149,6 +160,21 @@ def give_superusers_superpowers end end + def give_superusers_workflow_roles + AdminSet.all.each do |admin_set| + admin_set.permission_template.available_workflows.where(active: true).each do |workflow| + workflow_roles = Sipity::WorkflowRole.where(workflow_id: workflow.id) + workflow_roles.each do |workflow_role| + workflow_role_name = Sipity::Role.where(id: workflow_role.role_id).first.name + next if workflow_role_name == "depositing" || workflow_role_name == "managing" + union_of_users = superusers_as_sipity_agents.concat(users_in_role(admin_set, workflow_role_name)).uniq + @logger.debug "Granting #{workflow_role_name} to #{union_of_users.map { |u| User.where(id: u.proxy_for_id).first.email }}" + workflow.update_responsibilities(role: Sipity::Role.where(id: workflow_role.role_id), agents: union_of_users) + end + end + end + end + # Check to see if there is an uberadmin defined. If there isn't, throw an # exception. If there is, return that user. The uberadmin is the first superuser. # @return [User] the uberadmin user diff --git a/spec/features/laney_workflow_etd_spec.rb b/spec/features/laney_workflow_etd_spec.rb index 5d40ef80d..932f4bb78 100644 --- a/spec/features/laney_workflow_etd_spec.rb +++ b/spec/features/laney_workflow_etd_spec.rb @@ -81,13 +81,13 @@ # puts roles.map { |r| Sipity::Role.where(id: r).first.name } # end # Check workflow permissions for superuser - # available_workflow_actions = Hyrax::Workflow::PermissionQuery.scope_permitted_workflow_actions_available_for_current_state(user: w.superusers.last, entity: etd.to_sipity_entity).pluck(:name) - # puts w.superusers.last - # puts available_workflow_actions.inspect - # expect(available_workflow_actions.include?("mark_as_reviewed")).to eq true - # expect(available_workflow_actions.include?("approve")).to eq false # it can't be approved until after it has been marked as reviewed - # expect(available_workflow_actions.include?("request_changes")).to eq true - # expect(available_workflow_actions.include?("comment_only")).to eq true + available_workflow_actions = Hyrax::Workflow::PermissionQuery.scope_permitted_workflow_actions_available_for_current_state(user: w.superusers.last, entity: etd.to_sipity_entity).pluck(:name) + puts w.superusers.last + puts available_workflow_actions.inspect + expect(available_workflow_actions.include?("mark_as_reviewed")).to eq true + expect(available_workflow_actions.include?("approve")).to eq false # it can't be approved until after it has been marked as reviewed + expect(available_workflow_actions.include?("request_changes")).to eq true + expect(available_workflow_actions.include?("comment_only")).to eq true # The approving user marks the etd as reviewed subject = Hyrax::WorkflowActionInfo.new(etd, approving_user) diff --git a/spec/fixtures/config/emory/laney_graduate_school.yml b/spec/fixtures/config/emory/laney_graduate_school.yml index 151ab9058..ed32c18c3 100644 --- a/spec/fixtures/config/emory/laney_graduate_school.yml +++ b/spec/fixtures/config/emory/laney_graduate_school.yml @@ -3,4 +3,3 @@ workflow: approving: - laneyadmin@emory.edu - laneyadmin2@emory.edu - diff --git a/spec/lib/workflow_setup_spec.rb b/spec/lib/workflow_setup_spec.rb index 3be57b39f..93c35ccbc 100644 --- a/spec/lib/workflow_setup_spec.rb +++ b/spec/lib/workflow_setup_spec.rb @@ -100,32 +100,56 @@ end end end - - context "already existing participants" do + context "gives superusers superpowers" do + it "gives superusers the managing role in all newly created admin sets" do + ActiveFedora::Cleaner.clean! + w.load_superusers + expect(w.superusers.count).to be > 1 # This test won't be meaningful if there is only one superuser + admin_set = w.make_mediated_deposit_admin_set("River School") + workflow = admin_set.permission_template.available_workflows.where(active: true).first + expect(workflow).to be_instance_of Sipity::Workflow + w.give_superusers_superpowers + w.superusers.each do |su| + roles = Hyrax::Workflow::PermissionQuery.scope_processing_workflow_roles_for_user_and_workflow(user: su, workflow: workflow).pluck(:role_id) + su_role_names = roles.map { |r| Sipity::Role.where(id: r).first.name } + expect(su_role_names.include?("managing")).to eq true + end + end it "knows what users are enrolled in a given role for a given admin_set" do ActiveFedora::Cleaner.clean! w.load_superusers - admin_set = w.make_mediated_deposit_admin_set("Yellow Submarine") - yellow_submarine_approvers = w.users_in_role(admin_set, "approving") - expect(yellow_submarine_approvers.count).to eq 1 - expect(yellow_submarine_approvers).to be_instance_of Array - expect(yellow_submarine_approvers.first).to be_instance_of Sipity::Agent + laney_admin_set = w.make_admin_set_from_config("Laney Graduate School") + expect(laney_admin_set.permission_template.available_workflows.where(active: true).first.name).to eq "laney_graduate_school" + laney_approvers = w.users_in_role(laney_admin_set, "approving") + expect(laney_approvers.count).to eq 3 + expect(laney_approvers).to be_instance_of Array + expect(laney_approvers.first).to be_instance_of Sipity::Agent + laney_approvers = laney_approvers.map { |u| User.find(u.proxy_for_id).email } + expect(laney_approvers.include?("laneyadmin@emory.edu")).to eq true + expect(laney_approvers.include?("laneyadmin2@emory.edu")).to eq true end - end - it "gives superusers superpowers" do - ActiveFedora::Cleaner.clean! - w.load_superusers - expect(w.superusers.count).to be > 1 # This test won't be meaningful if there is only one superuser - admin_set = w.make_mediated_deposit_admin_set("River School") - workflow = admin_set.permission_template.available_workflows.where(active: true).first - expect(workflow).to be_instance_of Sipity::Workflow - w.give_superusers_superpowers - w.superusers.each do |su| - roles = Hyrax::Workflow::PermissionQuery.scope_processing_workflow_roles_for_user_and_workflow(user: su, workflow: workflow).pluck(:role_id) - su_role_names = roles.map { |r| Sipity::Role.where(id: r).first.name } - expect(su_role_names.include?("managing")).to eq true + it "gives superusers reviewing and approving roles in the Laney workflow without removing existing laney admins" do + ActiveFedora::Cleaner.clean! + w.load_superusers + expect(w.superusers.count).to be > 1 # This test won't be meaningful if there is only one superuser + admin_set = w.make_admin_set_from_config("Laney Graduate School") + workflow = admin_set.permission_template.available_workflows.where(active: true).first + expect(workflow).to be_instance_of Sipity::Workflow + w.give_superusers_superpowers + w.superusers.each do |su| + roles = Hyrax::Workflow::PermissionQuery.scope_processing_workflow_roles_for_user_and_workflow(user: su, workflow: workflow).pluck(:role_id) + su_role_names = roles.map { |r| Sipity::Role.where(id: r).first.name } + expect(su_role_names.include?("reviewing")).to eq true + expect(su_role_names.include?("approving")).to eq true + end + laney_admin_user = User.where(email: "laneyadmin@emory.edu").first + roles = Hyrax::Workflow::PermissionQuery.scope_processing_workflow_roles_for_user_and_workflow(user: laney_admin_user, workflow: workflow).pluck(:role_id) + la_role_names = roles.map { |r| Sipity::Role.where(id: r).first.name } + expect(la_role_names.include?("reviewing")).to eq false + expect(la_role_names.include?("approving")).to eq true end end + it "allows any registered user to deposit anywhere" do ActiveFedora::Cleaner.clean! w.load_superusers