Merge pull request #1079 from projecthydra/prefer_notification_names_…

…as_ruby_constants_in_workflow_schema

Workflow schemas should use Ruby constants as notification names

.travis.yml

@@ -11,11 +11,5 @@ env:
       RDF_VERSION=1.99.1
     - RAILS_VERSION=5.0.0.1
       RDF_VERSION=2.1.0
-notifications:
-  irc:
-    channels:
-      - "irc.freenode.org#projecthydra"
-    template:
-      - "%{repository}//%{branch}@%{commit} by %{author}: %{message} - %{build_url}"
 before_script:
   - jdk_switcher use oraclejdk8

app/services/curation_concerns/workflow/workflow_schema.rb

@@ -15,6 +15,21 @@ module Workflow
     # @see Sipity::Method
     # @see ./lib/generators/curation_concerns/work/templates/workflow.json.erb
     WorkflowSchema = Dry::Validation.Schema do
+      configure do
+        def self.messages
+          Dry::Validation::Messages.default.merge(
+            en: { errors: { constant_name?: 'must be an initialized Ruby constant' } }
+          )
+        end
+
+        def constant_name?(value)
+          value.constantize
+          true
+        rescue NameError
+          false
+        end
+      end
+
       required(:workflows).each do
         required(:name).filled(:str?) # Sipity::Workflow#name
         optional(:label).filled(:str?) # Sipity::Workflow#label
@@ -27,7 +42,7 @@ module Workflow
           end
           optional(:transition_to).filled(:str?) # Sipity::WorkflowState#name
           optional(:notifications).each do
-            required(:name).value(format?: /\A[a-z|_]+\Z/i) # Sipity::Notification#name
+            required(:name).value(:constant_name?) # Sipity::Notification#name
             required(:notification_type).value(included_in?: Sipity::Notification.valid_notification_types)
             required(:to) { array? { each(:str?) } }
             optional(:cc) { array? { each(:str?) } }

lib/generators/curation_concerns/templates/workflow.json.erb

@@ -26,14 +26,7 @@
            "attributes": {
              "presentation_sequence": 3
            },
-           "notifications": [
-             {
-               "notification_type": "email",
-               "name": "confirmation_of_submitted_to_ulra_committee",
-               "to": ["creating_user"],
-               "cc": ["advising"]
-             }
-           ],
+           "notifications": [],
            "methods": [
              "CurationConcerns::Workflow::ActivateObject"
            ]

spec/services/curation_concerns/workflow/state_machine_generator_spec.rb

@@ -4,6 +4,11 @@ module CurationConcerns
   RSpec.describe Workflow::StateMachineGenerator, :no_clean do
     let(:workflow) { Sipity::Workflow.create!(name: 'hello') }
     let(:action_name) { 'do_it' }
+    before do
+      class ConfirmSubmission
+      end
+    end
+    after { CurationConcerns.send(:remove_const, :ConfirmSubmission) }
     it 'exposes .generate_from_schema as a convenience method' do
       expect_any_instance_of(described_class).to receive(:call)
       described_class.generate_from_schema(workflow: workflow, name: action_name, config: {})
@@ -17,7 +22,7 @@ module CurationConcerns
         ],
         transition_to: :under_review,
         notifications: [
-          { notification_type: 'email', name: 'confirmation_of_submitted_to_ulra_committee', to: 'creating_user', cc: 'advising' }
+          { notification_type: 'email', name: 'CurationConcerns::ConfirmSubmission', to: 'creating_user', cc: 'advising' }
         ]
       }
     end

spec/services/curation_concerns/workflow/workflow_schema_spec.rb

@@ -3,36 +3,82 @@
 module CurationConcerns
   module Workflow
     RSpec.describe 'WorkflowSchema', no_clean: true do
-      it 'will validate valid data by returning an empty message' do
-        valid_data = {
-          workflows: [{
-            name: "valid",
-            actions: [{
-              name: "finalize_digitization",
-              from_states: [{ names: ["pending"], roles: ['finalizing_digitation_review'] }],
-              transition_to: "metadata_review"
-            }, {
-              name: "finalize_metadata",
-              from_states: [{ names: ["metadata_review"], roles: ['finalizing_metadata_review'] }],
-              transition_to: "final_review",
-              notifications: [{
-                name: 'thank_you', notification_type: Sipity::Notification::NOTIFICATION_TYPE_EMAIL, to: ['finalizing_metadata_review']
-              }]
-            }]
-          }]
+      let(:valid_data) do
+        {
+          workflows: [
+            {
+              name: "valid",
+              actions: [
+                {
+                  name: "finalize_digitization",
+                  from_states: [{ names: ["pending"], roles: ['finalizing_digitation_review'] }],
+                  transition_to: "metadata_review"
+                }, {
+                  name: "finalize_metadata",
+                  from_states: [{ names: ["metadata_review"], roles: ['finalizing_metadata_review'] }],
+                  transition_to: "final_review",
+                  notifications: [
+                    {
+                      name: notification_name, notification_type: Sipity::Notification::NOTIFICATION_TYPE_EMAIL, to: ['finalizing_metadata_review']
+                    }
+                  ]
+                }
+              ]
+            }
+          ]
         }
-        expect(WorkflowSchema.call(valid_data).messages).to be_empty
       end
 
-      it 'will report invalid data via the returned messages' do
-        invalid_data = {
-          workflows: [{
-            actions: [
-              { name: "finalize_digitization", from_states: [{ names: ["pending"], roles: [] }], transition_to: "metadata_review" }
-            ]
-          }]
+      let(:notification_name) { 'DoTheThing' }
+
+      let(:invalid_data) do
+        {
+          workflows: [
+            {
+              actions: [
+                { name: "finalize_digitization", from_states: [{ names: ["pending"], roles: [] }], transition_to: "metadata_review" }
+              ]
+            }
+          ]
         }
-        expect(WorkflowSchema.call(invalid_data).messages).to_not be_empty
+      end
+
+      before do
+        class ::DoTheThing
+        end
+      end
+
+      after do
+        Object.send(:remove_const, :DoTheThing)
+      end
+
+      it 'validates valid data by returning an empty message' do
+        expect(WorkflowSchema.call(valid_data).messages).to be_empty
+      end
+
+      it 'reports invalid data via the returned messages' do
+        expect(WorkflowSchema.call(invalid_data).messages).not_to be_empty
+      end
+
+      describe 'notification names' do
+        context 'with an uninitialized constant' do
+          let(:notification_name) { 'FooBar' }
+          it 'is invalid' do
+            expect(WorkflowSchema.call(valid_data).messages).not_to be_empty
+          end
+        end
+
+        context 'within a namespace' do
+          before do
+            class DoTheThing
+            end
+          end
+          after { CurationConcerns::Workflow.send(:remove_const, :DoTheThing) }
+          let(:notification_name) { 'CurationConcerns::Workflow::DoTheThing' }
+          it 'returns an empty message because valid' do
+            expect(WorkflowSchema.call(valid_data).messages).to be_empty
+          end
+        end
       end
     end
   end