From 30f9585c9dc6fc06bff3c832b475afbdb446df46 Mon Sep 17 00:00:00 2001 From: Anna Headley Date: Fri, 17 Mar 2017 10:34:32 -0400 Subject: [PATCH] Add null constraint to permission_templates workflow_name, fix #3156 --- app/actors/sufia/default_admin_set_actor.rb | 9 +++++++-- app/models/concerns/sufia/admin_set_behavior.rb | 1 + app/services/sufia/admin_set_create_service.rb | 4 +--- ...21_permission_template_change_column_workflow_name.rb | 5 +++++ .../sufia/admin/admin_sets_controller_spec.rb | 7 ++++--- .../sufia/admin/permission_templates_controller_spec.rb | 2 +- spec/factories/permission_templates.rb | 1 + spec/factories/workflows.rb | 2 +- .../presenters/sufia/admin_set_options_presenter_spec.rb | 2 ++ spec/services/sufia/admin_set_create_service_spec.rb | 6 +++--- .../admin/admin_sets/_form_participants.html.erb_spec.rb | 2 +- .../admin/admin_sets/_form_visibility.html.erb_spec.rb | 4 +++- 12 files changed, 30 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20170317141521_permission_template_change_column_workflow_name.rb diff --git a/app/actors/sufia/default_admin_set_actor.rb b/app/actors/sufia/default_admin_set_actor.rb index 8405ea4b9b..9b06300bd5 100644 --- a/app/actors/sufia/default_admin_set_actor.rb +++ b/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 diff --git a/app/models/concerns/sufia/admin_set_behavior.rb b/app/models/concerns/sufia/admin_set_behavior.rb index 39d9779bb5..8d6d689a1b 100644 --- a/app/models/concerns/sufia/admin_set_behavior.rb +++ b/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 diff --git a/app/services/sufia/admin_set_create_service.rb b/app/services/sufia/admin_set_create_service.rb index 758965152d..b7aa39e5f6 100644 --- a/app/services/sufia/admin_set_create_service.rb +++ b/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 diff --git a/db/migrate/20170317141521_permission_template_change_column_workflow_name.rb b/db/migrate/20170317141521_permission_template_change_column_workflow_name.rb new file mode 100644 index 0000000000..34fa78f9f8 --- /dev/null +++ b/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 diff --git a/spec/controllers/sufia/admin/admin_sets_controller_spec.rb b/spec/controllers/sufia/admin/admin_sets_controller_spec.rb index 665003ef21..f5568b8eae 100644 --- a/spec/controllers/sufia/admin/admin_sets_controller_spec.rb +++ b/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 diff --git a/spec/controllers/sufia/admin/permission_templates_controller_spec.rb b/spec/controllers/sufia/admin/permission_templates_controller_spec.rb index 47ca38432e..a67e4e41db 100644 --- a/spec/controllers/sufia/admin/permission_templates_controller_spec.rb +++ b/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, diff --git a/spec/factories/permission_templates.rb b/spec/factories/permission_templates.rb index da9c5b432f..331e85e253 100644 --- a/spec/factories/permission_templates.rb +++ b/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 diff --git a/spec/factories/workflows.rb b/spec/factories/workflows.rb index 8749c876a5..1c08e9f6a4 100644 --- a/spec/factories/workflows.rb +++ b/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 diff --git a/spec/presenters/sufia/admin_set_options_presenter_spec.rb b/spec/presenters/sufia/admin_set_options_presenter_spec.rb index 1651bc50ab..6258d2aeb4 100644 --- a/spec/presenters/sufia/admin_set_options_presenter_spec.rb +++ b/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) diff --git a/spec/services/sufia/admin_set_create_service_spec.rb b/spec/services/sufia/admin_set_create_service_spec.rb index d7e83d2f62..b5a57e886d 100644 --- a/spec/services/sufia/admin_set_create_service_spec.rb +++ b/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 diff --git a/spec/views/sufia/admin/admin_sets/_form_participants.html.erb_spec.rb b/spec/views/sufia/admin/admin_sets/_form_participants.html.erb_spec.rb index fed22902f8..21bd2331ea 100644 --- a/spec/views/sufia/admin/admin_sets/_form_participants.html.erb_spec.rb +++ b/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 diff --git a/spec/views/sufia/admin/admin_sets/_form_visibility.html.erb_spec.rb b/spec/views/sufia/admin/admin_sets/_form_visibility.html.erb_spec.rb index 0775ca548c..c8c83cdf94 100644 --- a/spec/views/sufia/admin/admin_sets/_form_visibility.html.erb_spec.rb +++ b/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]')