Merge pull request #3158 from projecthydra/permission_template_null

Add null constraint to permission_templates workflow_name, fix #3156

app/actors/sufia/default_admin_set_actor.rb

@@ -25,7 +25,7 @@ def ensure_admin_set_attribute!(attributes)
       end
 
       def ensure_permission_template!(admin_set_id:)
-        Sufia::PermissionTemplate.find_or_create_by!(admin_set_id: admin_set_id)
+        Sufia::PermissionTemplate.find_by(admin_set_id: admin_set_id) || create_permission_template!(admin_set_id: admin_set_id)
       end
 
       def default_admin_set_id
@@ -40,8 +40,13 @@ def default_exists?
       # Creates the default AdminSet and an associated PermissionTemplate with workflow
       def create_default_admin_set
         AdminSet.create!(id: AdminSet::DEFAULT_ID, title: ['Default Admin Set']).tap do |_as|
-          PermissionTemplate.create!(admin_set_id: AdminSet::DEFAULT_ID, workflow_name: 'default')
+          create_permission_template!(admin_set_id: AdminSet::DEFAULT_ID)
         end
       end
+
+      # Creates a Sufia::PermissionTemplate for the given AdminSet
+      def create_permission_template!(admin_set_id:)
+        Sufia::PermissionTemplate.create!(admin_set_id: admin_set_id, workflow_name: AdminSet::DEFAULT_WORKFLOW_NAME)
+      end
   end
 end

app/models/concerns/sufia/admin_set_behavior.rb

@@ -4,6 +4,7 @@ module AdminSetBehavior
 
     included do
       DEFAULT_ID = 'admin_set/default'.freeze
+      DEFAULT_WORKFLOW_NAME = 'default'.freeze
 
       def self.default_set?(id)
         id == DEFAULT_ID

app/services/sufia/admin_set_create_service.rb

@@ -1,13 +1,11 @@
 module Sufia
   # Creates AdminSets
   class AdminSetCreateService
-    DEFAULT_WORKFLOW_NAME = 'default'.freeze
-
     def self.create_default!
       return if AdminSet.exists?(AdminSet::DEFAULT_ID)
       admin_set = AdminSet.new(id: AdminSet::DEFAULT_ID, title: ['Default Admin Set'])
       begin
-        new(admin_set, nil, DEFAULT_WORKFLOW_NAME).create
+        new(admin_set, nil, AdminSet::DEFAULT_WORKFLOW_NAME).create
       rescue ActiveFedora::IllegalOperation
         # It is possible that another thread created the AdminSet just before this method
         # was called, so ActiveFedora will raise IllegalOperation. In this case we can safely

db/migrate/20170317141521_permission_template_change_column_workflow_name.rb

@@ -0,0 +1,5 @@
+class PermissionTemplateChangeColumnWorkflowName < ActiveRecord::Migration
+  def change
+    change_column_null :permission_templates, :workflow_name, false
+  end
+end

spec/controllers/sufia/admin/admin_sets_controller_spec.rb

@@ -109,16 +109,17 @@
 
     describe "#update" do
       let(:admin_set) { create(:admin_set, edit_users: [user]) }
-      let(:permission_template) { Sufia::PermissionTemplate.find_or_create_by(admin_set_id: admin_set.id) }
+      let(:permission_template) { create(:permission_template, admin_set_id: admin_set.id, workflow_name: workflow_name) }
+      let(:workflow_name) { 'one_step_mediated_deposit' }
       it 'updates a record' do
         # Prevent a save which causes Fedora to complain it doesn't know the referenced node.
         expect_any_instance_of(AdminSet).to receive(:save).and_return(true)
         patch :update, params: { id: admin_set,
-                                 admin_set: { title: "Improved title", thumbnail_id: "mw22v559x", workflow_name: "one_step_mediated_deposit" } }
+                                 admin_set: { title: "Improved title", thumbnail_id: "mw22v559x", workflow_name: workflow_name } }
         expect(response).to be_redirect
         expect(assigns[:admin_set].title).to eq ['Improved title']
         expect(assigns[:admin_set].thumbnail_id).to eq 'mw22v559x'
-        expect(permission_template.workflow_name).to eq 'one_step_mediated_deposit'
+        expect(permission_template.workflow_name).to eq workflow_name
       end
     end
 

spec/controllers/sufia/admin/permission_templates_controller_spec.rb

@@ -26,7 +26,7 @@
   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!(:permission_template) { create(:permission_template, 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,

spec/factories/permission_templates.rb

@@ -1,5 +1,6 @@
 FactoryGirl.define do
   factory :permission_template, class: Sufia::PermissionTemplate do
     admin_set_id '88888'
+    workflow_name AdminSet::DEFAULT_WORKFLOW_NAME
   end
 end

spec/factories/workflows.rb

@@ -1,5 +1,5 @@
 FactoryGirl.define do
   factory :workflow, class: Sipity::Workflow do
-    name 'generic_work'
+    name 'one_step_mediated_deposit'
   end
 end

spec/presenters/sufia/admin_set_options_presenter_spec.rb

@@ -16,6 +16,7 @@
       let(:solr_doc1) { instance_double(SolrDocument, id: '123', to_s: 'Public Set') }
       let(:solr_doc2) { instance_double(SolrDocument, id: '345', to_s: 'Private Set') }
       before do
+        allow(presenter).to receive(:workflow) { nil }
         create(:permission_template, admin_set_id: '123', visibility: 'open')
         create(:permission_template, admin_set_id: '345', visibility: 'restricted')
       end
@@ -36,6 +37,7 @@
       let(:results) { [solr_doc1, solr_doc2, solr_doc3, solr_doc4] }
 
       before do
+        allow(presenter).to receive(:workflow) { nil }
         create(:permission_template, admin_set_id: '123', release_period: Sufia::PermissionTemplate::RELEASE_TEXT_VALUE_FIXED, release_date: today + 2.days)
         create(:permission_template, admin_set_id: '345', release_period: Sufia::PermissionTemplate::RELEASE_TEXT_VALUE_NO_DELAY)
         create(:permission_template, admin_set_id: '567', release_period: Sufia::PermissionTemplate::RELEASE_TEXT_VALUE_1_YEAR)

spec/services/sufia/admin_set_create_service_spec.rb

@@ -2,7 +2,7 @@
 
 RSpec.describe Sufia::AdminSetCreateService do
   let(:admin_set) { AdminSet.new(title: ['test']) }
-  let(:workflow_name) { nil }
+  let(:workflow_name) { AdminSet::DEFAULT_WORKFLOW_NAME }
   let(:service) { described_class.new(admin_set, user, workflow_name) }
   let(:user) { create(:user) }
 
@@ -31,7 +31,7 @@
   end
 
   describe '.create_default!' do
-    let(:default_admin_set_id) { 'admin_set/default' }
+    let(:default_admin_set_id) { AdminSet::DEFAULT_ID }
     let(:permission_template) { Sufia::PermissionTemplate.find_by!(admin_set_id: default_admin_set_id) }
     # It is important to test the side-effects as a default admin set is a fundamental assumption for Sufia >= 7.3
     it 'creates AdminSet, PermissionTemplate' do
@@ -40,7 +40,7 @@
       admin_set = AdminSet.find(default_admin_set_id)
       expect(admin_set).to be_persisted
       expect(permission_template).to be_persisted
-      expect(permission_template.workflow_name).to eq described_class::DEFAULT_WORKFLOW_NAME
+      expect(permission_template.workflow_name).to eq workflow_name
     end
   end
 end

spec/views/sufia/admin/admin_sets/_form_participants.html.erb_spec.rb

@@ -2,7 +2,7 @@
 
 RSpec.describe 'sufia/admin/admin_sets/_form_participants.html.erb', type: :view do
   let(:admin_set) { create(:admin_set) }
-  let(:permission_template) { Sufia::PermissionTemplate.find_or_create_by(admin_set_id: admin_set.id) }
+  let(:permission_template) { create(:permission_template, admin_set_id: admin_set.id) }
   before do
     @form = Sufia::Forms::AdminSetForm.new(admin_set, permission_template)
     render

spec/views/sufia/admin/admin_sets/_form_visibility.html.erb_spec.rb

@@ -2,11 +2,13 @@
 
 RSpec.describe 'sufia/admin/admin_sets/_form_visibility.html.erb', type: :view do
   let(:admin_set) { create(:admin_set) }
-  let(:permission_template) { Sufia::PermissionTemplate.find_or_create_by(admin_set_id: admin_set.id) }
+  let(:permission_template) { create(:permission_template, admin_set_id: admin_set.id) }
+
   before do
     @form = Sufia::Forms::AdminSetForm.new(admin_set, permission_template)
     render
   end
+
   it "has the release form" do
     expect(rendered).to have_selector('#visibility input[type=radio][name="sufia_permission_template[release_period]"][value=now]')
     expect(rendered).to have_selector('#visibility input[type=radio][name="sufia_permission_template[release_period]"][value=fixed]')