Address pr feedback from pr 29

MichaelCrawfordNava · web-flow · commit 0abbd735e127 · 2025-04-16T12:02:46.000-05:00
Addressed changes: - [EventsManager -> EventManager Rename](#29 (comment)) - [Using find_case callback](#29 (comment)) - [No-op loop rather than if block](#29 (comment)) - [Passing method reference instead of callback loop](#29 (comment)) - [Changing named parameters to normal parameters](#29 (comment)) - [Naming style change](#29 (comment))
diff --git a/template/{{app_name}}/engines/flex/app/helpers/flex/event_manager.rb b/template/{{app_name}}/engines/flex/app/helpers/flex/event_manager.rb
@@ -1,5 +1,5 @@
 module Flex
-  class EventsManager
+  class EventManager
     def self.subscribe(event_key, callback)
       subscription = ActiveSupport::Notifications.subscribe(event_key) do |name, _started, _finished, _unique_id, payload|
         callback.call({
diff --git a/template/{{app_name}}/engines/flex/app/models/concerns/flex/task_handler_service.rb b/template/{{app_name}}/engines/flex/app/models/concerns/flex/task_handler_service.rb
@@ -2,7 +2,7 @@ module Flex
   module TaskHandlerService
     extend ActiveSupport::Concern
 
-    def create_task(kase:)
+    def create_task(kase)
       raise NoMethodError, "#{self.class} must implement execute method"
     end
   end
diff --git a/template/{{app_name}}/engines/flex/app/models/flex/application_form.rb b/template/{{app_name}}/engines/flex/app/models/flex/application_form.rb
@@ -32,7 +32,7 @@ def prevent_changes_if_submitted
     end
 
     def publish_event
-      EventsManager.publish("application_submitted", self.event_payload)
+      EventManager.publish("application_submitted", self.event_payload)
     end
   end
 end
diff --git a/template/{{app_name}}/engines/flex/app/models/flex/business_process.rb b/template/{{app_name}}/engines/flex/app/models/flex/business_process.rb
@@ -4,10 +4,10 @@ class BusinessProcess
 
     attr_accessor :name, :description, :steps, :start, :transitions
 
-    def initialize(name:, type:, description: "", steps: {}, start: "", transitions: {})
+    def initialize(name:, find_case_callback:, description: "", steps: {}, start: "", transitions: {})
       @subscriptions = {}
       @name = name
-      @type = type # recognizing this is a code smell that we will want to address in the future
+      @find_case_callback = find_case_callback
       @description = description
       define_start(start)
       define_steps(steps)
@@ -28,18 +28,15 @@ def define_steps(steps)
     end
 
     def define_transitions(transitions)
-      if @subscriptions.any?
-        stop_listening_all_events
-      end
-
+      stop_listening_for_events
       @transitions = transitions
       start_listening_for_events
     end
 
     private
 
     def handle_event(event)
-      kase = @type.find(event[:payload][:case_id])
+      kase = @find_case_callback.call(event[:payload][:case_id])
       current_step = kase.business_process_current_step
       next_step = @transitions[current_step][event[:name]]
       kase.business_process_current_step = next_step
@@ -57,16 +54,13 @@ def get_event_names_from_transitions
 
     def start_listening_for_events
       get_event_names_from_transitions.each do |event_name|
-        new_subscription = EventsManager.subscribe(event_name, ->(event) {
-          handle_event(event)
-        })
-        @subscriptions[event_name] = new_subscription
+        @subscriptions[event_name] = EventManager.subscribe(event_name, method(:handle_event))
       end
     end
 
-    def stop_listening_all_events
+    def stop_listening_for_events
       @subscriptions.each do |event_name, subscription|
-        EventsManager.unsubscribe(subscription)
+        EventManager.unsubscribe(subscription)
       end
       @subscriptions.clear
     end
diff --git a/template/{{app_name}}/engines/flex/app/models/flex/system_process.rb b/template/{{app_name}}/engines/flex/app/models/flex/system_process.rb
@@ -5,7 +5,7 @@ class SystemProcess
     attr_accessor :name
     attr_accessor :callback
 
-    def initialize(name:, callback:)
+    def initialize(name, callback)
       @name = name
       @callback = callback
     end
diff --git a/template/{{app_name}}/engines/flex/app/models/flex/user_task.rb b/template/{{app_name}}/engines/flex/app/models/flex/user_task.rb
@@ -4,7 +4,7 @@ class UserTask
 
     attr_accessor :name, :task_management_service
 
-    def initialize(name:, task_management_service:)
+    def initialize(name, task_management_service)
       @name = name
       @task_management_service = task_management_service
     end
diff --git a/template/{{app_name}}/engines/flex/spec/acceptance/flex/passport_application_case_acceptance_spec.rb b/template/{{app_name}}/engines/flex/spec/acceptance/flex/passport_application_case_acceptance_spec.rb
@@ -24,17 +24,17 @@ module Flex
       expect(kase.business_process_current_step).to eq ("verify_identity")
 
       # verify identity (simulate action that an adjudicator takes)
-      EventsManager.publish("identity_verified", { case_id: kase.id })
+      EventManager.publish("identity_verified", { case_id: kase.id })
       kase.reload
       expect(kase.business_process_current_step).to eq ("review_passport_photo")
 
       # approve passport photo
-      EventsManager.publish("passport_photo_approved", { case_id: kase.id })
+      EventManager.publish("passport_photo_approved", { case_id: kase.id })
       kase.reload
       expect(kase.business_process_current_step).to eq ("notify_user_passport_approved")
 
       # notify user
-      EventsManager.publish("notification_completed", { case_id: kase.id })
+      EventManager.publish("notification_completed", { case_id: kase.id })
       kase.reload
       expect(kase.business_process_current_step).to eq ("end")
 
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/app/business_processes/flex/passport_application_business_process_manager.rb b/template/{{app_name}}/engines/flex/spec/dummy/app/business_processes/flex/passport_application_business_process_manager.rb
@@ -13,27 +13,27 @@ def initialize
     def create_passport_application_business_process
       business_process = BusinessProcess.new(
         name: 'Passport Application Process',
-        type: PassportCase,
+        find_case_callback: ->(case_id) { PassportCase.find(case_id) },
         description: 'Process for applying for a passport'
       )
       business_process.define_steps(
         {
           "collect_application_info" => UserTask.new(
-            name: "Collect App Info",
-            task_management_service: UserTaskCreatorService
+            "Collect App Info",
+            UserTaskCreationService
           ),
-          "verify_identity" => SystemProcess.new(name: "Verify Identity", callback: ->(kase) {
-            IdentityVerifierService.new(kase).verify_identity # IdentityVerifierService would publish an event when verify_identity completes
+          "verify_identity" => SystemProcess.new("Verify Identity", ->(kase) {
+            IdentityVerificationService.new(kase).verify_identity # IdentityVerificationService would publish an event when verify_identity completes
           }),
-          "manual_adjudicator_review" => UserTask.new(name: "Manual Adjudicator Review", task_management_service: AdjudicatorTaskCreatorService), # create an adjudicator task for manual review
-          "review_passport_photo" => SystemProcess.new(name: "Review Passport Photo", callback: ->(kase) {
-            PhotoVerifierService.new(kase).verify_photo # PhotoVerifierService would publish an event when verify_photo completes
+          "manual_adjudicator_review" => UserTask.new("Manual Adjudicator Review", AdjudicatorTaskCreationService), # create an adjudicator task for manual review
+          "review_passport_photo" => SystemProcess.new("Review Passport Photo", ->(kase) {
+            PhotoVerificationService.new(kase).verify_photo # PhotoVerificationService would publish an event when verify_photo completes
           }),
-          "notify_user_passport_approved" => SystemProcess.new(name: "Notify Passport Approval", callback: ->(kase) {
-            UserNotifierService.new(kase).send_notification("approval") # UserNotifierService would publish an event when send_notification completes
+          "notify_user_passport_approved" => SystemProcess.new("Notify Passport Approval", ->(kase) {
+            UserNotificationService.new(kase).send_notification("approval") # UserNotificationService would publish an event when send_notification completes
           }),
-          "notify_user_passport_rejected" => SystemProcess.new(name: "Notify Passport Rejection", callback: ->(kase) {
-            UserNotifierService.new(kase).send_notification("rejection") # UserNotifierService would publish an event when send_notification completes
+          "notify_user_passport_rejected" => SystemProcess.new("Notify Passport Rejection", ->(kase) {
+            UserNotificationService.new(kase).send_notification("rejection") # UserNotificationService would publish an event when send_notification completes
           })
         }
       )
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/adjudicator_task_creation_service.rb b/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/adjudicator_task_creation_service.rb
@@ -1,5 +1,5 @@
 module Flex
-  class AdjudicatorTaskCreatorService
+  class AdjudicatorTaskCreationService
     include TaskHandlerService
 
     def self.create_task(kase:)
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/identity_verification_service.rb b/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/identity_verification_service.rb
@@ -1,5 +1,5 @@
 module Flex
-  class IdentityVerifierService
+  class IdentityVerificationService
     def initialize(kase)
       @kase = kase
     end
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/photo_verification_service.rb b/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/photo_verification_service.rb
@@ -1,5 +1,5 @@
 module Flex
-  class PhotoVerifierService
+  class PhotoVerificationService
     def initialize(kase)
       @kase = kase
     end
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/user_notification_service.rb b/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/user_notification_service.rb
@@ -1,5 +1,5 @@
 module Flex
-  class UserNotifierService
+  class UserNotificationService
     def initialize(kase)
       @kase = kase
     end
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/user_task_creation_service.rb b/template/{{app_name}}/engines/flex/spec/dummy/app/services/flex/user_task_creation_service.rb
@@ -1,5 +1,5 @@
 module Flex
-  class UserTaskCreatorService
+  class UserTaskCreationService
     include TaskHandlerService
 
     def self.create_task(kase:)
diff --git a/template/{{app_name}}/engines/flex/spec/dummy/spec/models/flex/passport_application_form_spec.rb b/template/{{app_name}}/engines/flex/spec/dummy/spec/models/flex/passport_application_form_spec.rb
@@ -4,14 +4,14 @@ module Flex
   RSpec.describe PassportApplicationForm do
     describe "validations" do
       let(:passport_application_form) { described_class.new }
-      let(:mock_events_manager) { class_double(EventsManager) }
+      let(:mock_events_manager) { class_double(EventManager) }
 
       def generate_random_date_of_birth
         rand(100.years.ago..1.day.ago).to_date
       end
 
       before do
-        stub_const("Flex::EventsManager", mock_events_manager)
+        stub_const("Flex::EventManager", mock_events_manager)
         allow(PassportApplicationBusinessProcessManager.instance)
           .to receive(:business_process)
           .and_return(instance_double(BusinessProcess, execute: true))
diff --git a/template/{{app_name}}/engines/flex/spec/models/flex/application_form_spec.rb b/template/{{app_name}}/engines/flex/spec/models/flex/application_form_spec.rb
@@ -10,10 +10,10 @@ module Flex
   RSpec.describe TestApplicationForm do
     describe "validations" do
       let(:application_form) { described_class.new }
-      let(:mock_events_manager) { class_double(EventsManager) }
+      let(:mock_events_manager) { class_double(EventManager) }
 
       before do
-        stub_const("Flex::EventsManager", mock_events_manager)
+        stub_const("Flex::EventManager", mock_events_manager)
         application_form.save!
       end
 
diff --git a/template/{{app_name}}/engines/flex/spec/models/flex/business_process_spec.rb b/template/{{app_name}}/engines/flex/spec/models/flex/business_process_spec.rb
@@ -2,19 +2,20 @@
 
 module Flex
   RSpec.describe BusinessProcess do
-    let(:business_process) { described_class.new(name: "Test Business Process", type: PassportCase) }
-    let(:mock_events_manager) { class_double(EventsManager) }
+    let(:mock_case) { instance_double(PassportCase, business_process_current_step: 'step1') }
+    let(:find_case_callback) { ->(case_id) { mock_case } }
+    let(:business_process) { described_class.new(name: "Test Business Process", find_case_callback: find_case_callback) }
+    let(:mock_event_manager) { class_double(EventManager) }
     let(:mock_steps) { {
       "user_task" => instance_double(UserTask),
       "system_process" => instance_double(SystemProcess),
       "user_task_2" => instance_double(UserTask),
       "system_process_2" => instance_double(SystemProcess)
       }
     }
-    let(:mock_case) { instance_double(PassportCase, business_process_current_step: 'step1') }
 
     before do
-      stub_const("Flex::EventsManager", mock_events_manager)
+      stub_const("Flex::EventManager", mock_event_manager)
     end
 
     describe 'executing a business process' do
@@ -45,8 +46,8 @@ module Flex
 
     describe 'when defining transitions' do
       before do
-        allow(mock_events_manager).to receive(:subscribe)
-        allow(mock_events_manager).to receive(:unsubscribe)
+        allow(mock_event_manager).to receive(:subscribe)
+        allow(mock_event_manager).to receive(:unsubscribe)
       end
 
       it 'starts listening to events defined in transitions' do
@@ -55,8 +56,8 @@ module Flex
           "step2" => { "event2" => "end" }
         })
 
-        expect(mock_events_manager).to have_received(:subscribe).with("event1", anything)
-        expect(mock_events_manager).to have_received(:subscribe).with("event2", anything)
+        expect(mock_event_manager).to have_received(:subscribe).with("event1", anything)
+        expect(mock_event_manager).to have_received(:subscribe).with("event2", anything)
       end
 
       it 'stops listening to events when transitions are redefined and then subscribe to the new events' do
@@ -69,9 +70,9 @@ module Flex
           "step4" => { "event4" => "end" }
         })
 
-        expect(mock_events_manager).to have_received(:unsubscribe).twice
-        expect(mock_events_manager).to have_received(:subscribe).with("event3", anything)
-        expect(mock_events_manager).to have_received(:subscribe).with("event4", anything)
+        expect(mock_event_manager).to have_received(:unsubscribe).twice
+        expect(mock_event_manager).to have_received(:subscribe).with("event3", anything)
+        expect(mock_event_manager).to have_received(:subscribe).with("event4", anything)
       end
     end
   end
