From ca956b114b5b704aae0a9e22b24687404c05be28 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 19 Oct 2016 17:34:56 -0700 Subject: [PATCH] UI for adding workflow roles. Fixes #1037 --- README.md | 9 +--- .../admin_controller_behavior.rb | 17 +++---- .../concerns/curation_concerns/admin_page.rb | 18 ++++++++ .../admin/workflow_roles_controller.rb | 36 +++++++++++++++ .../forms/workflow_responsibility_form.rb | 42 +++++++++++++++++ .../concerns/curation_concerns/ability.rb | 1 + .../admin/workflow_role_presenter.rb | 40 +++++++++++++++++ .../admin/workflow_roles/index.html.erb | 45 +++++++++++++++++++ config/locales/curation_concerns.en.yml | 15 +++++++ config/routes.rb | 1 + lib/curation_concerns/configuration.rb | 3 +- .../admin/workflow_roles_controller_spec.rb | 26 +++++++++++ spec/features/workflow_roles_spec.rb | 17 +++++++ .../workflow_responsibility_form_spec.rb | 33 ++++++++++++++ 14 files changed, 282 insertions(+), 21 deletions(-) create mode 100644 app/controllers/concerns/curation_concerns/admin_page.rb create mode 100644 app/controllers/curation_concerns/admin/workflow_roles_controller.rb create mode 100644 app/forms/curation_concerns/forms/workflow_responsibility_form.rb create mode 100644 app/presenters/curation_concerns/admin/workflow_role_presenter.rb create mode 100644 app/views/curation_concerns/admin/workflow_roles/index.html.erb create mode 100644 spec/controllers/curation_concerns/admin/workflow_roles_controller_spec.rb create mode 100644 spec/features/workflow_roles_spec.rb create mode 100644 spec/forms/curation_concerns/forms/workflow_responsibility_form_spec.rb diff --git a/README.md b/README.md index 910118913..72c6911d2 100644 --- a/README.md +++ b/README.md @@ -116,14 +116,7 @@ Load the workflows, workflow states, transitions and user roles: $ rails curation_concerns:workflow:load ``` -Now that the Roles are loaded, grant all the roles to all the users. This works for testing, but you probably don't want this in production: -```ruby -CurationConcerns::Workflow::PermissionGenerator.call(roles: Sipity::Role.all, - workflow: Sipity::Workflow.last, - agents: User.all) -``` - -In the future this functionality should be available in the user interface. +Now that the Roles are loaded, grant the appropriate roles to the users by visiting the "Workflow Roles" section of the admin dashboard [Further documentation](https://github.com/projecthydra/curation_concerns/wiki/Defining-a-Workflow) for defining and customizing workflows. diff --git a/app/controllers/concerns/curation_concerns/admin_controller_behavior.rb b/app/controllers/concerns/curation_concerns/admin_controller_behavior.rb index 04bd0f1c8..7eeb50910 100644 --- a/app/controllers/concerns/curation_concerns/admin_controller_behavior.rb +++ b/app/controllers/concerns/curation_concerns/admin_controller_behavior.rb @@ -3,16 +3,13 @@ module AdminControllerBehavior extend ActiveSupport::Concern included do - cattr_accessor :configuration - self.configuration = CurationConcerns.config.dashboard_configuration + include AdminPage before_action :require_permissions - before_action :load_configuration - layout "admin" + end - def index - @resource_statistics = @configuration.fetch(:data_sources).fetch(:resource_stats).new - render 'index' - end + def index + @resource_statistics = @configuration.fetch(:data_sources).fetch(:resource_stats).new + render 'index' end def search_builder @@ -33,10 +30,6 @@ def require_permissions authorize! :read, :admin_dashboard end - def load_configuration - @configuration = self.class.configuration.with_indifferent_access - end - # Loads the index action if it's only defined in the configuration. def action_missing(action) index if @configuration[:actions].include?(action) diff --git a/app/controllers/concerns/curation_concerns/admin_page.rb b/app/controllers/concerns/curation_concerns/admin_page.rb new file mode 100644 index 000000000..563eb4133 --- /dev/null +++ b/app/controllers/concerns/curation_concerns/admin_page.rb @@ -0,0 +1,18 @@ +module CurationConcerns + module AdminPage + extend ActiveSupport::Concern + + included do + cattr_accessor :configuration + self.configuration = CurationConcerns.config.dashboard_configuration + before_action :load_configuration + layout "admin" + end + + private + + def load_configuration + @configuration = self.class.configuration.with_indifferent_access + end + end +end diff --git a/app/controllers/curation_concerns/admin/workflow_roles_controller.rb b/app/controllers/curation_concerns/admin/workflow_roles_controller.rb new file mode 100644 index 000000000..332bbd091 --- /dev/null +++ b/app/controllers/curation_concerns/admin/workflow_roles_controller.rb @@ -0,0 +1,36 @@ +module CurationConcerns + module Admin + class WorkflowRolesController < ApplicationController + include AdminPage + before_action :require_permissions + + def index + @presenter = WorkflowRolePresenter.new + end + + def destroy + responsibility = Sipity::WorkflowResponsibility.find(params[:id]) + authorize! :destroy, responsibility + responsibility.destroy + redirect_to admin_workflow_roles_path + end + + def create + authorize! :create, Sipity::WorkflowResponsibility + form = Forms::WorkflowResponsibilityForm.new(params[:sipity_workflow_responsibility]) + begin + form.save! + rescue ActiveRecord::RecordNotUnique + logger.info "Not unique *****\n\n\n" + end + redirect_to admin_workflow_roles_path + end + + private + + def require_permissions + authorize! :read, :admin_dashboard + end + end + end +end diff --git a/app/forms/curation_concerns/forms/workflow_responsibility_form.rb b/app/forms/curation_concerns/forms/workflow_responsibility_form.rb new file mode 100644 index 000000000..48bcb1c2d --- /dev/null +++ b/app/forms/curation_concerns/forms/workflow_responsibility_form.rb @@ -0,0 +1,42 @@ +module CurationConcerns + module Forms + class WorkflowResponsibilityForm + def initialize(params = {}) + model_instance.workflow_role_id = params[:workflow_role_id] + if params[:user_id] + user = ::User.find(params[:user_id]) + model_instance.agent = user.to_sipity_agent + end + end + + def model_instance + @model ||= Sipity::WorkflowResponsibility.new + end + + def to_model + model_instance + end + + delegate :model_name, :to_key, :workflow_role_id, :persisted?, :save!, to: :model_instance + + def user_id + nil + end + + def user_options + ::User.all + end + + # The select options for choosing a responsibility + def workflow_role_options + Sipity::WorkflowRole.all.map { |wf_role| [label(wf_role), wf_role.id] } + end + + private + + def label(wf_role) + "#{wf_role.workflow.name} - #{wf_role.role.name}" + end + end + end +end diff --git a/app/models/concerns/curation_concerns/ability.rb b/app/models/concerns/curation_concerns/ability.rb index 5b04b5b9f..45a0c3d6b 100644 --- a/app/models/concerns/curation_concerns/ability.rb +++ b/app/models/concerns/curation_concerns/ability.rb @@ -34,6 +34,7 @@ def admin_permissions alias_action :discover, to: :read can :manage, curation_concerns_models + can :manage, Sipity::WorkflowResponsibility end # Override this method in your ability model if you use a different group diff --git a/app/presenters/curation_concerns/admin/workflow_role_presenter.rb b/app/presenters/curation_concerns/admin/workflow_role_presenter.rb new file mode 100644 index 000000000..4c2e75e28 --- /dev/null +++ b/app/presenters/curation_concerns/admin/workflow_role_presenter.rb @@ -0,0 +1,40 @@ +module CurationConcerns + module Admin + class WorkflowRolePresenter + def users + ::User.all + end + + def presenter_for(user) + agent = user.sipity_agent + return unless agent + AgentPresenter.new(agent) + end + + class AgentPresenter + def initialize(agent) + @agent = agent + end + + def responsibilities + @agent.workflow_responsibilities.each do |responsibility| + yield ResponsibilityPresenter.new(responsibility) + end + end + end + + class ResponsibilityPresenter + def initialize(responsibility) + @responsibility = responsibility + @wf_role = responsibility.workflow_role + end + + attr_accessor :responsibility + + def label + "#{@wf_role.workflow.name} - #{@wf_role.role.name}" + end + end + end + end +end diff --git a/app/views/curation_concerns/admin/workflow_roles/index.html.erb b/app/views/curation_concerns/admin/workflow_roles/index.html.erb new file mode 100644 index 000000000..04fbe0bf7 --- /dev/null +++ b/app/views/curation_concerns/admin/workflow_roles/index.html.erb @@ -0,0 +1,45 @@ +
"> +
+
+ + + + + + + <% @presenter.users.each do |user| %> + + + <% agent_presenter = @presenter.presenter_for(user) %> + <% if agent_presenter %> + + <% else %> + + <% end %> + + <% end %> + +
<%= t('.header.user') %><%= t('.header.roles') %>
<%= user.user_key %> +
    + <% agent_presenter.responsibilities do |responsibility_presenter| %> +
  • <%= responsibility_presenter.label %> + <%= link_to admin_workflow_role_path(responsibility_presenter.responsibility), + method: :delete, + data: { confirm: t('.delete.confirm') } do %> + × + <% end %> +
    • + <% end %> +
+ 		<%= t('.no_roles') %>
+ +

<%= t('.new_role') %>

+ <%= simple_form_for CurationConcerns::Forms::WorkflowResponsibilityForm.new, url: admin_workflow_roles_path do |f| %> + <%= f.input :user_id, as: :select, collection: f.object.user_options %> + <%= f.input :workflow_role_id, as: :select, collection: f.object.workflow_role_options %> + <%= f.submit %> + <% end %> +
+
+
+ diff --git a/config/locales/curation_concerns.en.yml b/config/locales/curation_concerns.en.yml index 50a405621..630fb8f56 100644 --- a/config/locales/curation_concerns.en.yml +++ b/config/locales/curation_concerns.en.yml @@ -1,5 +1,15 @@ en: curation_concerns: + admin: + workflow_roles: + index: + header: + user: User + roles: Roles + no_roles: No roles + new_role: "Grant a role to a user:" + delete: + confirm: Are you sure? api: accepted: default: "Your request has been accepted for processing, but processing is not complete. See job for more info." @@ -35,6 +45,7 @@ en: index: "Admin Dashboard" resource_details: "Resource Details" workflow: "Workflow" + workflow_roles: "Workflow Roles" division: name: "Your Division at Institution" homepage_url: "#" @@ -225,6 +236,10 @@ en: identifier_tesim: "Identifier" file_manager: link_text: 'File Manager' + helpers: + submit: + sipity_workflow_responsibility: + create: "Add" simple_form: required: html: 'required' diff --git a/config/routes.rb b/config/routes.rb index 83d4ceb9a..dad1c535c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -10,6 +10,7 @@ get '/', action: :index, as: :index get :resource_details get :workflow + resources :workflow_roles end # mount BrowseEverything::Engine => '/remote_files/browse' diff --git a/lib/curation_concerns/configuration.rb b/lib/curation_concerns/configuration.rb index 56afa5ca8..06f9ff651 100644 --- a/lib/curation_concerns/configuration.rb +++ b/lib/curation_concerns/configuration.rb @@ -172,7 +172,8 @@ def dashboard_configuration menu: { index: {}, resource_details: {}, - workflow: {} + workflow: {}, + workflow_roles: {} }, actions: { index: { diff --git a/spec/controllers/curation_concerns/admin/workflow_roles_controller_spec.rb b/spec/controllers/curation_concerns/admin/workflow_roles_controller_spec.rb new file mode 100644 index 000000000..e12569ad7 --- /dev/null +++ b/spec/controllers/curation_concerns/admin/workflow_roles_controller_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +RSpec.describe CurationConcerns::Admin::WorkflowRolesController, :no_clean do + routes { CurationConcerns::Engine.routes } + + describe "#get" do + context "when you have permission" do + before do + allow(controller).to receive(:authorize!).with(:read, :admin_dashboard).and_return(true) + end + + it "works" do + get :index + expect(response).to be_success + expect(assigns[:presenter]).to be_kind_of CurationConcerns::Admin::WorkflowRolePresenter + end + end + + context "when they don't have permission" do + it "throws a CanCan error" do + get :index + expect(response).to redirect_to new_user_session_path + end + end + end +end diff --git a/spec/features/workflow_roles_spec.rb b/spec/features/workflow_roles_spec.rb new file mode 100644 index 000000000..ab7eeb7de --- /dev/null +++ b/spec/features/workflow_roles_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +RSpec.describe "Manage workflow roles", type: :feature do + let(:user) { create(:user) } + before do + allow_any_instance_of(CurationConcerns::Admin::WorkflowRolesController).to receive(:authorize!).with(:read, :admin_dashboard).and_return(true) + CurationConcerns::Workflow::WorkflowImporter.generate_from_json_file(path: "#{EngineCart.destination}/config/workflows/generic_work_workflow.json") + CurationConcerns::Workflow::PermissionGenerator.call(roles: Sipity::Role.all, + workflow: Sipity::Workflow.last, + agents: user) + end + + it "shows the roles" do + visit '/admin/workflow_roles' + expect(page).to have_content 'generic_work - reviewing' + end +end diff --git a/spec/forms/curation_concerns/forms/workflow_responsibility_form_spec.rb b/spec/forms/curation_concerns/forms/workflow_responsibility_form_spec.rb new file mode 100644 index 000000000..95c482d39 --- /dev/null +++ b/spec/forms/curation_concerns/forms/workflow_responsibility_form_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe CurationConcerns::Forms::WorkflowResponsibilityForm, :no_clean do + let(:instance) { described_class.new } + + describe "#initialize" do + let(:user) { create(:user) } + let(:instance) { described_class.new(user_id: user.id, workflow_role_id: 7) } + subject { instance.model_instance } + it "creates an agent and sets the workflow_role_id" do + expect(subject.agent).to be_kind_of Sipity::Agent + expect(subject.workflow_role_id).to eq 7 + end + end + + describe "#user_options" do + subject { instance.user_options } + it { is_expected.to eq User.all } + end + + describe "#workflow_role_options" do + let(:workflow) { instance_double(Sipity::Workflow, name: 'generic_work') } + let(:role1) { instance_double(Sipity::Role, name: 'foo') } + let(:role2) { instance_double(Sipity::Role, name: 'bar') } + let(:wf_role1) { instance_double(Sipity::WorkflowRole, workflow: workflow, role: role1, id: 1) } + let(:wf_role2) { instance_double(Sipity::WorkflowRole, workflow: workflow, role: role2, id: 2) } + before do + allow(Sipity::WorkflowRole).to receive(:all).and_return([wf_role1, wf_role2]) + end + subject { instance.workflow_role_options } + it { is_expected.to eq [['generic_work - foo', 1], ['generic_work - bar', 2]] } + end +end