[TAN-7137] Rework job into a sweep and move materialization to controllers

The ProcessScheduledPublicationTransitionsJob (plural) now sweeps through all due transitions by default, but you can also target a specific one by passing an admin_publication.

Moved synchronous materialization out of SideFx services and into controllers, right before assign_attributes. This sidesteps conflicts from unpersisted changes and avoids infinite recursion.

Also added the job to hourly_jobs.rake as a safety net.

Author: adessy

back/app/controllers/web_api/v1/folders_controller.rb

@@ -93,6 +93,7 @@ def create
   end
 
   def update
+    process_due_transition(@project_folder)
     @project_folder.assign_attributes project_folder_params
     authorize @project_folder
     remove_image_if_requested!(@project_folder, project_folder_params, :header_bg)
@@ -113,7 +114,9 @@ def destroy
     frozen_folder = nil
     frozen_projects = nil
 
+    process_due_transition(@project_folder)
     @project_folder.projects.each do |project|
+      process_due_transition(project)
       SideFxProjectService.new.before_destroy(project, current_user)
     end
 
@@ -135,6 +138,13 @@ def destroy
 
   private
 
+  def process_due_transition(publication)
+    admin_pub = publication.admin_publication
+    return unless admin_pub.scheduled_at&.<=(Time.current)
+
+    ProcessScheduledPublicationTransitionsJob.new.run(admin_pub)
+  end
+
   def set_project_folder
     @project_folder = ProjectFolders::Folder.find(params[:id])
     authorize @project_folder

back/app/controllers/web_api/v1/projects_controller.rb

@@ -276,6 +276,8 @@ def update
     params[:project][:area_ids] ||= [] if params[:project].key?(:area_ids)
     params[:project][:global_topic_ids] ||= [] if params[:project].key?(:global_topic_ids)
 
+    process_due_transition(@project)
+
     project_params = permitted_attributes(@project)
 
     @project.assign_attributes project_params
@@ -296,6 +298,7 @@ def update
   end
 
   def destroy
+    process_due_transition(@project)
     sidefx.before_destroy(@project, current_user)
     if @project.destroy
       sidefx.after_destroy(@project, current_user)
@@ -420,6 +423,13 @@ def set_project
     authorize @project
   end
 
+  def process_due_transition(publication)
+    admin_pub = publication.admin_publication
+    return unless admin_pub.scheduled_at&.<=(Time.current)
+
+    ProcessScheduledPublicationTransitionsJob.new.run(admin_pub)
+  end
+
   def base_render_mini_index
     render json: linked_json(
       @projects,

…_scheduled_publication_transition_job.rb …scheduled_publication_transitions_job.rbback/app/jobs/process_scheduled_publication_transition_job.rb renamed to back/app/jobs/process_scheduled_publication_transitions_job.rb back/app/jobs/process_scheduled_publication_transition_job.rb renamed to back/app/jobs/process_scheduled_publication_transitions_job.rb

@@ -1,22 +1,27 @@
 # frozen_string_literal: true
 
-class ProcessScheduledPublicationTransitionJob < ApplicationJob
+class ProcessScheduledPublicationTransitionsJob < ApplicationJob
   queue_as :default
 
-  def run(admin_publication_id)
-    admin_pub = AdminPublication.find_by(id: admin_publication_id)
-    return unless admin_pub
-    # We check if the transition is due twice. The important check happens inside the lock.
-    # This one is just an optimization to avoid locking unnecessarily.
-    return unless transition_due?(admin_pub)
+  def run(admin_publication = nil)
+    if admin_publication
+      process(admin_publication)
+    else
+      AdminPublication
+        .where(scheduled_at: ..Time.current)
+        .find_each { |admin_pub| process(admin_pub) }
+    end
+  end
+
+  private
 
+  def process(admin_pub)
     admin_pub.with_lock do
-      # Skip if the schedule was canceled, rescheduled to the future, or already processed.
-      next unless transition_due?(admin_pub)
+      return unless admin_pub.scheduled_at&.<=(Time.current)
 
       target = admin_pub.scheduled_status
       current = admin_pub.read_attribute(:publication_status)
-      next admin_pub.cancel_scheduled_transition! if current == target
+      return admin_pub.cancel_scheduled_transition! if current == target
 
       user = admin_pub.scheduled_by
 
@@ -37,12 +42,6 @@ def run(admin_publication_id)
     end
   end
 
-  private
-
-  def transition_due?(admin_pub)
-    admin_pub.scheduled_at.present? && admin_pub.scheduled_at <= Time.current
-  end
-
   def sidefx_service(publication)
     case publication
     when Project then SideFxProjectService.new

back/app/services/project_folders/side_fx_project_folder_service.rb

@@ -15,7 +15,6 @@ def after_create(folder, user)
     end
 
     def before_update(folder, user)
-      process_due_transition(folder.admin_publication)
       set_scheduled_by(folder.admin_publication, user)
     end
 
@@ -58,12 +57,6 @@ def after_destroy(frozen_folder, user)
 
     private
 
-    def process_due_transition(admin_pub)
-      return unless admin_pub.scheduled_at&.<=(Time.current)
-
-      ProcessScheduledPublicationTransitionJob.new.run(admin_pub.id)
-    end
-
     def set_scheduled_by(admin_pub, user)
       return unless admin_pub.will_save_change_to_scheduled_status?
 
@@ -74,9 +67,9 @@ def enqueue_scheduled_transition(admin_pub)
       return unless admin_pub.saved_change_to_scheduled_at?
       return unless admin_pub.scheduled_at.present?
 
-      ProcessScheduledPublicationTransitionJob
+      ProcessScheduledPublicationTransitionsJob
         .set(wait_until: admin_pub.scheduled_at)
-        .perform_later(admin_pub.id)
+        .perform_later
     end
   end
 end

back/app/services/side_fx_project_service.rb

@@ -49,7 +49,6 @@ def after_destroy_participation_data(project, user)
   end
 
   def before_update(project, user)
-    process_due_transition(project.admin_publication)
     @publication_status_was = project.admin_publication.publication_status_was
     @folder_id_was = project.admin_publication.parent_id_was
     set_scheduled_by(project.admin_publication, user)
@@ -79,9 +78,7 @@ def after_update(project, user)
     enqueue_scheduled_transition(project.admin_publication)
   end
 
-  def before_destroy(project, _user)
-    process_due_transition(project.admin_publication)
-  end
+  def before_destroy(project, user); end
 
   def after_destroy(frozen_project, user)
     ContentBuilder::LayoutService.new.clean_homepage_layout_when_publication_deleted(frozen_project)
@@ -133,12 +130,6 @@ def after_publish(project, user)
     LogActivityJob.perform_later project, 'published', user, project.updated_at.to_i
   end
 
-  def process_due_transition(admin_pub)
-    return unless admin_pub.scheduled_at&.<=(Time.current)
-
-    ProcessScheduledPublicationTransitionJob.new.run(admin_pub.id)
-  end
-
   def set_scheduled_by(admin_pub, user)
     return unless admin_pub.will_save_change_to_scheduled_status?
 
@@ -149,9 +140,9 @@ def enqueue_scheduled_transition(admin_pub)
     return unless admin_pub.saved_change_to_scheduled_at?
     return unless admin_pub.scheduled_at.present?
 
-    ProcessScheduledPublicationTransitionJob
+    ProcessScheduledPublicationTransitionsJob
       .set(wait_until: admin_pub.scheduled_at)
-      .perform_later(admin_pub.id)
+      .perform_later
   end
 
   def after_folder_changed(project, current_user)

back/engines/commercial/multi_tenancy/lib/tasks/core/hourly_jobs.rake

@@ -7,6 +7,7 @@ namespace :cl2back do
     Tenant.creation_finalized.each do |tenant|
       Apartment::Tenant.switch(tenant.schema_name) do
         AutomatedTransitionJob.perform_later
+        ProcessScheduledPublicationTransitionsJob.perform_later
         CreatePeriodicActivitiesJob.perform_later now.to_i
         CreateHeatmapGenerationJob.perform_later now.to_i
         IdeaFeed::TopicModelingSchedulerJob.perform_later

…duled_publication_transition_job_spec.rb …uled_publication_transitions_job_spec.rbback/spec/jobs/process_scheduled_publication_transition_job_spec.rb renamed to back/spec/jobs/process_scheduled_publication_transitions_job_spec.rb back/spec/jobs/process_scheduled_publication_transition_job_spec.rb renamed to back/spec/jobs/process_scheduled_publication_transitions_job_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe ProcessScheduledPublicationTransitionJob do
+RSpec.describe ProcessScheduledPublicationTransitionsJob do
   subject(:job) { described_class.new }
 
   let(:user) { create(:admin) }
@@ -18,7 +18,7 @@
       end
 
       it 'materializes the transition and clears scheduled fields' do
-        job.run(admin_publication.id)
+        job.run
         admin_publication.reload
 
         expect(admin_publication.read_attribute(:publication_status)).to eq('published')
@@ -29,14 +29,14 @@
 
       it 'sets first_published_at when publishing for the first time' do
         admin_publication.update_columns(first_published_at: nil)
-        job.run(admin_publication.id)
+        job.run
         admin_publication.reload
 
         expect(admin_publication.first_published_at).to be_present
       end
 
       it 'fires side effects via SideFxProjectService' do
-        expect { job.run(admin_publication.id) }
+        expect { job.run }
           .to have_enqueued_job(LogActivityJob)
           .with(project, 'changed', user, anything, anything)
       end
@@ -53,7 +53,7 @@
       end
 
       it 'fires the published activity' do
-        expect { job.run(admin_publication.id) }
+        expect { job.run }
           .to have_enqueued_job(LogActivityJob)
           .with(project, 'published', user, anything, anything)
       end
@@ -69,7 +69,7 @@
       end
 
       it 'does not fire the published activity' do
-        job.run(admin_publication.id)
+        job.run
         expect(LogActivityJob).not_to have_been_enqueued.with(project, 'published', anything, anything)
       end
     end
@@ -80,7 +80,7 @@
       end
 
       it 'does nothing' do
-        job.run(admin_publication.id)
+        job.run
         admin_publication.reload
 
         expect(admin_publication.scheduled_status).to eq('published')
@@ -98,7 +98,7 @@
       it 'clears scheduled fields without firing additional side effects' do
         ActiveJob::Base.queue_adapter.enqueued_jobs.clear
 
-        job.run(admin_publication.id)
+        job.run
         admin_publication.reload
 
         expect(admin_publication.scheduled_status).to be_nil
@@ -107,10 +107,19 @@
       end
     end
 
-    context 'missing record' do
-      it 'exits gracefully' do
-        expect { job.run('nonexistent-id') }.not_to raise_error
-      end
+    it 'processes multiple due transitions' do
+      project2 = create(:project, admin_publication_attributes: { publication_status: 'draft' })
+      admin_publication.update_columns(
+        scheduled_status: 'archived', scheduled_at: 1.hour.ago, scheduled_by_id: user.id
+      )
+      project2.admin_publication.update_columns(
+        scheduled_status: 'archived', scheduled_at: 30.minutes.ago, scheduled_by_id: user.id
+      )
+
+      job.run
+
+      expect(admin_publication.reload.read_attribute(:publication_status)).to eq('archived')
+      expect(project2.admin_publication.reload.read_attribute(:publication_status)).to eq('archived')
     end
   end
 end

back/spec/services/project_folders/side_fx_project_folder_service_spec.rb

@@ -50,15 +50,14 @@
       project_folder.save!
 
       expect { service.after_update(project_folder, user) }
-        .to have_enqueued_job(ProcessScheduledPublicationTransitionJob)
-        .with(project_folder.admin_publication.id)
+        .to have_enqueued_job(ProcessScheduledPublicationTransitionsJob)
     end
 
     it 'does not enqueue the transition job when no schedule is set' do
       project_folder.update!(title_multiloc: { en: 'changed' })
 
       expect { service.after_update(project_folder, user) }
-        .not_to have_enqueued_job(ProcessScheduledPublicationTransitionJob)
+        .not_to have_enqueued_job(ProcessScheduledPublicationTransitionsJob)
     end
   end
 

back/spec/services/side_fx_project_service_spec.rb

@@ -102,8 +102,7 @@
       project.save!
 
       expect { service.after_update(project, user) }
-        .to have_enqueued_job(ProcessScheduledPublicationTransitionJob)
-        .with(project.admin_publication.id)
+        .to have_enqueued_job(ProcessScheduledPublicationTransitionsJob)
     end
 
     it 'does not enqueue the transition job when no schedule is set' do
@@ -112,39 +111,9 @@
       project.save!
 
       expect { service.after_update(project, user) }
-        .not_to have_enqueued_job(ProcessScheduledPublicationTransitionJob)
+        .not_to have_enqueued_job(ProcessScheduledPublicationTransitionsJob)
     end
 
-    it 'materializes a due transition before the update without infinite loop' do
-      project.admin_publication.update_columns(
-        publication_status: 'draft', scheduled_status: 'published',
-        scheduled_at: 1.hour.ago, scheduled_by_id: user.id
-      )
-      project.admin_publication.reload
-      project.assign_attributes(title_multiloc: { en: 'changed' })
-
-      service.before_update(project, user)
-
-      project.admin_publication.reload
-      expect(project.admin_publication.read_attribute(:publication_status)).to eq('published')
-      expect(project.admin_publication.scheduled_status).to be_nil
-    end
-  end
-
-  describe 'before_destroy' do
-    it 'materializes a due transition before destroy' do
-      project.admin_publication.update_columns(
-        publication_status: 'draft', scheduled_status: 'published',
-        scheduled_at: 1.hour.ago, scheduled_by_id: user.id
-      )
-      project.admin_publication.reload
-
-      service.before_destroy(project, user)
-
-      project.admin_publication.reload
-      expect(project.admin_publication.read_attribute(:publication_status)).to eq('published')
-      expect(project.admin_publication.first_published_at).to be_present
-    end
   end
 
   describe 'after_destroy' do