Refs #38856 - address rubocop concerns again.

ianballou · ianballou · commit 4489865fa3db · 2025-12-02T21:21:29.000Z
diff --git a/app/models/katello/content_view_version.rb b/app/models/katello/content_view_version.rb
@@ -385,6 +385,57 @@ def auto_publish_composites!(component_task_id)
       end
     end
 
+    class << self
+      # Trigger a composite publish with coordination for sibling tasks.
+      # Checks for running component CV publishes and chains if necessary.
+      def trigger_composite_publish_with_coordination(composite_cv, description, triggered_by_version_id, calling_task_id: nil)
+        # Find currently running component CV publish tasks
+        component_cv_ids = composite_cv.components.pluck(:content_view_id)
+        running_tasks = ForemanTasks::Task::DynflowTask
+          .for_action(::Actions::Katello::ContentView::Publish)
+          .where(state: ['planning', 'planned', 'running'])
+          .select do |task|
+            task_input = task.input
+            task_input && component_cv_ids.include?(task_input.dig('content_view', 'id'))
+          end
+
+        sibling_task_ids = running_tasks.map(&:external_id)
+        # Exclude the calling component task to avoid self-dependency
+        sibling_task_ids.reject! { |id| id == calling_task_id } if calling_task_id
+
+        trigger_publish_with_sibling_tasks(composite_cv, sibling_task_ids, description, triggered_by_version_id)
+      rescue ForemanTasks::Lock::LockConflict => e
+        Rails.logger.info("Composite CV #{composite_cv.name} publish lock conflict: #{e.class} - #{e.message}")
+        ::Katello::UINotifications::ContentView::AutoPublishFailure.deliver!(composite_cv)
+        raise
+      rescue StandardError => e
+        Rails.logger.error("Failed to auto-publish composite CV #{composite_cv.name}: #{e.class} - #{e.message}")
+        Rails.logger.debug(e.backtrace.join("\n")) if e.backtrace
+        ::Katello::UINotifications::ContentView::AutoPublishFailure.deliver!(composite_cv)
+        raise
+      end
+
+      # Trigger a composite publish, chaining to sibling tasks if any exist
+      def trigger_publish_with_sibling_tasks(composite_cv, sibling_task_ids, description, triggered_by_version_id)
+        if sibling_task_ids.any?
+          ForemanTasks.dynflow.world.chain(
+            sibling_task_ids,
+            ::Actions::Katello::ContentView::Publish,
+            composite_cv,
+            description,
+            triggered_by_id: triggered_by_version_id
+          )
+        else
+          ForemanTasks.async_task(
+            ::Actions::Katello::ContentView::Publish,
+            composite_cv,
+            description,
+            triggered_by_id: triggered_by_version_id
+          )
+        end
+      end
+    end
+
     private
 
     # Returns :scheduled, :running, or nil based on composite CV publish task status
@@ -420,61 +471,11 @@ def scheduled_task_for_composite?(task, composite_cv)
 
       args = delayed_plan.args
       args.first.is_a?(::Katello::ContentView) && args.first.id == composite_cv.id
-    rescue NoMethodError, TypeError, Dynflow::Error
+    rescue NoMethodError, TypeError, Dynflow::Error => e
       Rails.logger.error("Failed to check scheduled task for composite CV #{composite_cv.name}: #{e.message}")
       false
     end
 
-    # Trigger a composite publish with coordination for sibling tasks.
-    # Checks for running component CV publishes and chains if necessary.
-    def self.trigger_composite_publish_with_coordination(composite_cv, description, triggered_by_version_id, calling_task_id: nil)
-      # Find currently running component CV publish tasks
-      component_cv_ids = composite_cv.components.pluck(:content_view_id)
-      running_tasks = ForemanTasks::Task::DynflowTask
-        .for_action(::Actions::Katello::ContentView::Publish)
-        .where(state: ['planning', 'planned', 'running'])
-        .select do |task|
-          task_input = task.input
-          task_input && component_cv_ids.include?(task_input.dig('content_view', 'id'))
-        end
-
-      sibling_task_ids = running_tasks.map(&:external_id)
-      # Exclude the calling component task to avoid self-dependency
-      sibling_task_ids.reject! { |id| id == calling_task_id } if calling_task_id
-
-      trigger_publish_with_sibling_tasks(composite_cv, sibling_task_ids, description, triggered_by_version_id)
-    rescue ForemanTasks::Lock::LockConflict => e
-      Rails.logger.info("Composite CV #{composite_cv.name} publish lock conflict: #{e.class} - #{e.message}")
-      ::Katello::UINotifications::ContentView::AutoPublishFailure.deliver!(composite_cv)
-      raise
-    rescue StandardError => e
-      Rails.logger.error("Failed to auto-publish composite CV #{composite_cv.name}: #{e.class} - #{e.message}")
-      Rails.logger.debug(e.backtrace.join("\n")) if e.backtrace
-      ::Katello::UINotifications::ContentView::AutoPublishFailure.deliver!(composite_cv)
-      raise
-    end
-
-    # Trigger a composite publish, chaining to sibling tasks if any exist
-    def self.trigger_publish_with_sibling_tasks(composite_cv, sibling_task_ids, description, triggered_by_version_id)
-      if sibling_task_ids.any?
-        ForemanTasks.dynflow.world.chain(
-          sibling_task_ids,
-          ::Actions::Katello::ContentView::Publish,
-          composite_cv,
-          description,
-          triggered_by_id: triggered_by_version_id
-        )
-      else
-        ForemanTasks.async_task(
-          ::Actions::Katello::ContentView::Publish,
-          composite_cv,
-          description,
-          triggered_by_id: triggered_by_version_id
-        )
-      end
-    end
-
-
     # Find active (planning/planned/running) composite publish tasks (does NOT check scheduled tasks)
     def find_active_composite_publish_tasks(composite_cv)
       relevant_tasks = ForemanTasks::Task::DynflowTask
diff --git a/test/actions/katello/content_view_test.rb b/test/actions/katello/content_view_test.rb
@@ -249,7 +249,7 @@ class PublishTest < TestBase
     end
 
     context 'run phase' do
-      it 'triggers auto-publish for composite views' do
+      it 'schedules event for composite views' do
         composite_view = katello_content_views(:composite_view)
         action.stubs(:task).returns(success_task)
 
@@ -258,18 +258,15 @@ class PublishTest < TestBase
                           composite_content_view: composite_view,
                           content_view: content_view)
 
-        # Mock the task relations to simulate no scheduled, no running composite, no sibling tasks
+        # Mock the task relations to simulate no scheduled composite
         ForemanTasks::Task::DynflowTask.stubs(:for_action)
           .returns(stub(where: stub(any?: false))) # Scheduled check: no scheduled tasks
           .then.returns(stub(where: stub(select: []))) # Running composite check: none
-          .then.returns(stub(where: stub(select: []))) # Sibling check: none
-
-        # Expect async_task to be called (no siblings, so no chaining)
-        ForemanTasks.expects(:async_task).with(
-          ::Actions::Katello::ContentView::Publish,
-          composite_view,
-          anything,
-          triggered_by_id: anything
+
+        # Expect event to be scheduled
+        ::Katello::EventQueue.expects(:push_event).with(
+          ::Katello::Events::AutoPublishCompositeView::EVENT_TYPE,
+          composite_view.id
         )
 
         plan_action action, content_view
@@ -279,9 +276,8 @@ class PublishTest < TestBase
       it 'does nothing for non-composite view' do
         action.stubs(:task).returns(success_task)
 
-        # Should not trigger any auto-publish tasks
-        ForemanTasks.expects(:async_task).never
-        ForemanTasks.dynflow.world.expects(:chain).never
+        # Should not trigger any auto-publish events
+        ::Katello::EventQueue.expects(:push_event).never
 
         plan_action action, katello_content_views(:no_environment_view)
         run_action action
diff --git a/test/models/content_view_version_auto_publish_test.rb b/test/models/content_view_version_auto_publish_test.rb
@@ -45,7 +45,7 @@ def test_auto_publish_schedules_event_when_no_composite_activity
       # Stub to return no scheduled, no running composite
       ForemanTasks::Task::DynflowTask.stubs(:for_action)
         .returns(stub(where: stub(any?: false))) # Scheduled check: no scheduled tasks
-        .then.returns(stub(where: stub(select: [])))  # Running composite check: none
+        .then.returns(stub(where: stub(select: []))) # Running composite check: none
 
       ::Katello::EventQueue.expects(:push_event).with(
         ::Katello::Events::AutoPublishCompositeView::EVENT_TYPE,
@@ -99,5 +99,21 @@ def test_auto_publish_skips_when_composite_already_scheduled
       @component1_version.auto_publish_composites!(task_id)
     end
 
+    def test_scheduled_task_for_composite_handles_errors_gracefully
+      task_id = SecureRandom.uuid
+      composite_task = stub(external_id: task_id)
+
+      # Mock dynflow world to raise an error when loading delayed plan
+      world_stub = ForemanTasks.dynflow.world
+      persistence_stub = stub
+      persistence_stub.stubs(:load_delayed_plan).raises(Dynflow::Error, "Delayed plan not found")
+      world_stub.stubs(:persistence).returns(persistence_stub)
+
+      # Should log error and return false instead of raising
+      Rails.logger.expects(:error).with(regexp_matches(/Failed to check scheduled task/))
+
+      result = @component1_version.send(:scheduled_task_for_composite?, composite_task, @composite_cv)
+      refute result
+    end
   end
 end
