Merge backport of #689: show the admin set name on the workflow role …

…widget

Fixes #908

app/controllers/hyrax/admin/workflow_roles_controller.rb

@@ -8,7 +8,7 @@ def index
         add_breadcrumb t(:'hyrax.controls.home'), root_path
         add_breadcrumb t(:'hyrax.toolbar.admin.menu'), hyrax.admin_path
         add_breadcrumb t(:'hyrax.admin.workflow_roles.header'), hyrax.admin_workflow_roles_path
-        @presenter = WorkflowRolePresenter.new
+        @presenter = WorkflowRolesPresenter.new
       end
 
       def destroy

app/forms/hyrax/forms/workflow_responsibility_form.rb

@@ -26,9 +26,12 @@ def user_options
         ::User.all
       end
 
-      # The select options for choosing a responsibility
+      # The select options for choosing a responsibility sorted by label
       def workflow_role_options
-        Sipity::WorkflowRole.all.map { |wf_role| [label(wf_role), wf_role.id] }
+        options = Sipity::WorkflowRole.all.map do |wf_role|
+          [Hyrax::Admin::WorkflowRolePresenter.new(wf_role).label, wf_role.id]
+        end
+        options.sort_by(&:first)
       end
 
       private

app/presenters/hyrax/admin/workflow_role_presenter.rb

@@ -1,44 +1,30 @@
 module Hyrax
   module Admin
+    # Displays a single workflow role
     class WorkflowRolePresenter
-      def users
-        ::User.registered
+      def initialize(workflow_role)
+        @workflow = workflow_role.workflow
+        @role = workflow_role.role
+        @admin_set_id = workflow.permission_template.admin_set_id
       end
 
-      def presenter_for(user)
-        agent = user.sipity_agent
-        return unless agent
-        AgentPresenter.new(agent)
+      # @todo This is a hack; I don't want to include reference to the admin set;
+      #       However based on the current UI, in which we list all workflows (spanning all admin sets) this is required.
+      # @return [String] A meaningful label for the given WorkflowRole
+      def label
+        "#{admin_set_label(admin_set_id)} - #{role.name} (#{workflow.name})"
       end
 
-      class AgentPresenter
-        def initialize(agent)
-          @agent = agent
-        end
-
-        def responsibilities_present?
-          @agent.workflow_responsibilities.any?
-        end
+      private
 
-        def responsibilities
-          @agent.workflow_responsibilities.each do |responsibility|
-            yield ResponsibilityPresenter.new(responsibility)
-          end
-        end
-      end
+        attr_accessor :workflow, :role, :admin_set_id
 
-      class ResponsibilityPresenter
-        def initialize(responsibility)
-          @responsibility = responsibility
-          @wf_role = responsibility.workflow_role
+        def admin_set_label(id)
+          result = ActiveFedora::Base.search_by_id(id, fl: 'title_tesim')
+          result['title_tesim'].first
+        rescue ActiveFedora::ObjectNotFoundError
+          "[AdminSet ID=#{id}]"
         end
-
-        attr_accessor :responsibility
-
-        def label
-          "#{@wf_role.workflow.name} - #{@wf_role.role.name}"
-        end
-      end
     end
   end
 end

app/presenters/hyrax/admin/workflow_roles_presenter.rb

@@ -0,0 +1,43 @@
+module Hyrax
+  module Admin
+    # Displays a list of users and their associated workflow roles
+    class WorkflowRolesPresenter
+      def users
+        ::User.registered
+      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_present?
+          @agent.workflow_responsibilities.any?
+        end
+
+        def responsibilities
+          @agent.workflow_responsibilities.each do |responsibility|
+            yield ResponsibilityPresenter.new(responsibility)
+          end
+        end
+      end
+
+      class ResponsibilityPresenter
+        def initialize(responsibility)
+          @responsibility = responsibility
+          @workflow_role_presenter = WorkflowRolePresenter.new(responsibility.workflow_role)
+        end
+
+        attr_accessor :responsibility
+
+        delegate :label, to: :@workflow_role_presenter
+      end
+    end
+  end
+end

spec/controllers/hyrax/admin/workflow_roles_controller_spec.rb

@@ -9,14 +9,14 @@
         allow(controller).to receive(:authorize!).with(:read, :admin_dashboard).and_return(true)
       end
 
-      it "draws breadcrumbs" do
+      it "is successful" do
         expect(controller).to receive(:add_breadcrumb).with('Home', root_path(locale: 'en'))
         expect(controller).to receive(:add_breadcrumb).with('Administration', Hyrax::Engine.routes.url_helpers.admin_path(locale: 'en'))
         expect(controller).to receive(:add_breadcrumb).with('Workflow Roles', Hyrax::Engine.routes.url_helpers.admin_workflow_roles_path(locale: 'en'))
         get :index
         expect(response).to be_success
-        expect(assigns[:presenter]).to be_kind_of Hyrax::Admin::WorkflowRolePresenter
-        expect(response).to render_template('admin')
+        expect(assigns[:presenter]).to be_kind_of Hyrax::Admin::WorkflowRolesPresenter
+        expect(response).to render_template('layouts/admin')
       end
     end
 

spec/features/workflow_roles_spec.rb

@@ -42,6 +42,6 @@
   it "shows the roles" do
     login_as(user, scope: :user)
     visit '/admin/workflow_roles'
-    expect(page).to have_content 'one_step - approving'
+    expect(page).to have_content 'approving (one_step)'
   end
 end

spec/forms/hyrax/forms/workflow_responsibility_form_spec.rb

@@ -15,19 +15,27 @@
 
   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) }
+    subject { instance.workflow_role_options }
+
+    let(:wf_role1) { instance_double(Sipity::WorkflowRole, id: 1) }
+    let(:wf_role2) { instance_double(Sipity::WorkflowRole, id: 2) }
+
     before do
       allow(Sipity::WorkflowRole).to receive(:all).and_return([wf_role1, wf_role2])
+      allow(Hyrax::Admin::WorkflowRolePresenter).to receive(:new)
+        .with(wf_role1)
+        .and_return(instance_double(Hyrax::Admin::WorkflowRolePresenter,
+                                    label: 'generic_work - foo'))
+      allow(Hyrax::Admin::WorkflowRolePresenter).to receive(:new)
+        .with(wf_role2)
+        .and_return(instance_double(Hyrax::Admin::WorkflowRolePresenter,
+                                    label: 'generic_work - bar'))
     end
-    subject { instance.workflow_role_options }
-    it { is_expected.to eq [['generic_work - foo', 1], ['generic_work - bar', 2]] }
+    it { is_expected.to eq [['generic_work - bar', 2], ['generic_work - foo', 1]] }
   end
 end

spec/models/sipity/workflow_role_spec.rb

@@ -1,6 +1,7 @@
 module Sipity
   RSpec.describe WorkflowRole, type: :model, no_clean: true do
     subject { described_class }
+
     its(:column_names) { is_expected.to include('workflow_id') }
     its(:column_names) { is_expected.to include('role_id') }
   end

spec/presenters/hyrax/admin/workflow_role_presenter_spec.rb

@@ -1,16 +1,14 @@
 require 'spec_helper'
 
 RSpec.describe Hyrax::Admin::WorkflowRolePresenter do
-  let(:presenter) { described_class.new }
+  let(:presenter) { described_class.new(workflow_role) }
+  let(:role) { Sipity::Role[:depositor] }
+  let(:workflow) { create(:workflow) }
+  let(:workflow_role) { Sipity::WorkflowRole.new(role: role, workflow: workflow) }
 
-  describe "#users" do
-    subject { presenter.users }
-    let!(:user) { create(:user) }
-    before do
-      create(:user, :guest)
-    end
-    it "doesn't include guests" do
-      expect(subject).to eq [user]
-    end
+  describe '#label' do
+    subject { presenter.label }
+
+    it { is_expected.to be_a(String) }
   end
 end

spec/presenters/hyrax/admin/workflow_roles_presenter_spec.rb

@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+RSpec.describe Hyrax::Admin::WorkflowRolesPresenter do
+  let(:presenter) { described_class.new }
+
+  describe "#users" do
+    subject { presenter.users }
+
+    let!(:user) { create(:user) }
+
+    before do
+      create(:user, :guest)
+    end
+    it "doesn't include guests" do
+      expect(subject).to eq [user]
+    end
+  end
+end

spec/views/hyrax/admin/workflow_roles/index.html.erb_spec.rb

@@ -1,10 +1,10 @@
 require 'spec_helper'
 
-describe 'hyrax/admin/workflow_roles/index.html.erb', type: :view do
+RSpec.describe 'hyrax/admin/workflow_roles/index.html.erb', type: :view do
   let!(:user1) { create(:user) }
   let!(:user2) { create(:user) }
   let(:presenter) do
-    Hyrax::Admin::WorkflowRolePresenter.new
+    Hyrax::Admin::WorkflowRolesPresenter.new
   end
 
   before do