From ac9b055ada2621e15736228e3f836ca4203b9255 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Wed, 15 Jul 2026 10:57:41 +0530 Subject: [PATCH] test(automations): specs for delayed automation rules --- .../automation_rules_controller_spec.rb | 57 ++++++ .../automation_rule_pending_executions.rb | 10 + .../process_pending_execution_job_spec.rb | 139 ++++++++++++++ .../trigger_pending_executions_job_spec.rb | 48 +++++ .../automation_rule_listener_spec.rb | 33 ++++ .../automation_rule_pending_execution_spec.rb | 171 ++++++++++++++++++ spec/models/automation_rule_spec.rb | 38 ++++ spec/models/conversation_spec.rb | 24 +++ 8 files changed, 520 insertions(+) create mode 100644 spec/factories/automation_rule_pending_executions.rb create mode 100644 spec/jobs/automation_rules/process_pending_execution_job_spec.rb create mode 100644 spec/jobs/automation_rules/trigger_pending_executions_job_spec.rb create mode 100644 spec/models/automation_rule_pending_execution_spec.rb diff --git a/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb b/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb index 5b7c0112d..d9026be29 100644 --- a/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb @@ -451,4 +451,61 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do end end end + + describe 'execution_delay handling' do + let(:delayed_rule_params) do + { + name: 'Delayed rule', + event_name: 'conversation_updated', + execution_delay: 240, + conditions: [{ attribute_key: 'status', filter_operator: 'equal_to', values: ['pending'], query_operator: nil }], + actions: [{ action_name: 'add_label', action_params: ['stale'] }] + } + end + + context 'when the delayed_automations feature is enabled' do + before { account.enable_features!('delayed_automations') } + + it 'persists and serializes execution_delay' do + post "/api/v1/accounts/#{account.id}/automation_rules", + headers: administrator.create_new_auth_token, + params: delayed_rule_params + + expect(response).to have_http_status(:success) + body = JSON.parse(response.body, symbolize_names: true) + expect(body[:execution_delay]).to eq(240) + expect(account.automation_rules.last.execution_delay).to eq(240) + end + + it 'copies execution_delay on clone' do + automation_rule = create(:automation_rule, account: account, execution_delay: 240) + + post "/api/v1/accounts/#{account.id}/automation_rules/#{automation_rule.id}/clone", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + expect(account.automation_rules.last.execution_delay).to eq(240) + end + end + + context 'when the delayed_automations feature is disabled' do + it 'rejects a payload carrying execution_delay with 422' do + post "/api/v1/accounts/#{account.id}/automation_rules", + headers: administrator.create_new_auth_token, + params: delayed_rule_params + + expect(response).to have_http_status(:unprocessable_entity) + expect(account.automation_rules.count).to eq(0) + end + + it 'still accepts payloads without execution_delay' do + post "/api/v1/accounts/#{account.id}/automation_rules", + headers: administrator.create_new_auth_token, + params: delayed_rule_params.except(:execution_delay) + + expect(response).to have_http_status(:success) + expect(account.automation_rules.last.execution_delay).to be_nil + end + end + end end diff --git a/spec/factories/automation_rule_pending_executions.rb b/spec/factories/automation_rule_pending_executions.rb new file mode 100644 index 000000000..8298991ae --- /dev/null +++ b/spec/factories/automation_rule_pending_executions.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :automation_rule_pending_execution do + account + automation_rule { association :automation_rule, account: account } + conversation { association :conversation, account: account } + episode_key { "status:#{Time.current.to_i}" } + due_at { 1.hour.from_now } + status { :pending } + end +end diff --git a/spec/jobs/automation_rules/process_pending_execution_job_spec.rb b/spec/jobs/automation_rules/process_pending_execution_job_spec.rb new file mode 100644 index 000000000..a7c88e9e0 --- /dev/null +++ b/spec/jobs/automation_rules/process_pending_execution_job_spec.rb @@ -0,0 +1,139 @@ +require 'rails_helper' + +RSpec.describe AutomationRules::ProcessPendingExecutionJob do + subject(:job) { described_class.new } + + let(:account) { create(:account) } + let(:conversation) { create(:conversation, account: account, status: :pending) } + let(:rule) do + create(:automation_rule, account: account, event_name: 'conversation_updated', execution_delay: 60, + conditions: [{ 'values' => ['pending'], 'attribute_key' => 'status', 'query_operator' => nil, + 'filter_operator' => 'equal_to' }], + actions: [{ 'action_name' => 'add_label', 'action_params' => ['stale'] }]) + end + let(:pending_execution) do + AutomationRulePendingExecution.schedule(rule: rule, conversation: conversation) + AutomationRulePendingExecution.last + end + + before do + GlobalConfig.clear_cache + account.enable_features!('delayed_automations') + end + + it 'runs the actions and marks the row executed when every guard passes' do + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_executed + expect(conversation.reload.label_list).to include('stale') + end + + it 'skips with rule_inactive when the rule was disabled' do + rule.update!(active: false) + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_skipped + expect(pending_execution.skip_reason).to eq('rule_inactive') + expect(conversation.reload.label_list).to be_empty + end + + it 'skips with flag_disabled when the account flag was turned off' do + account.disable_features!('delayed_automations') + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_skipped + expect(pending_execution.skip_reason).to eq('flag_disabled') + end + + it 'skips with episode_moved when the conversation left the armed status' do + pending_execution + conversation.update!(status: :resolved) + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_skipped + expect(pending_execution.skip_reason).to eq('episode_moved') + expect(conversation.reload.label_list).to be_empty + end + + it 'skips with conditions_changed when the re-check no longer matches the edited rule' do + pending_execution + # Rule edited while pending: the re-check enforces the current conditions (by design). + rule.update!(conditions: [{ 'values' => ['open'], 'attribute_key' => 'status', 'query_operator' => nil, + 'filter_operator' => 'equal_to' }]) + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_skipped + expect(pending_execution.skip_reason).to eq('conditions_changed') + end + + it 'skips with expired when the row is past the due window' do + pending_execution.update!(due_at: 4.days.ago) + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_skipped + expect(pending_execution.skip_reason).to eq('expired') + expect(conversation.reload.label_list).to be_empty + end + + it 'leaves the row untouched without executing when the kill switch is set' do + create(:installation_config, name: 'DISABLE_DELAYED_AUTOMATIONS', serialized_value: { value: true }.with_indifferent_access) + GlobalConfig.clear_cache + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_pending + expect(conversation.reload.label_list).to be_empty + end + + it 'runs the actions once when the same row is processed twice concurrently' do + allow(AutomationRules::ActionService).to receive(:new).and_call_original + duplicate = AutomationRulePendingExecution.find(pending_execution.id) + + job.perform(pending_execution.reload) + described_class.new.perform(duplicate) + + expect(AutomationRules::ActionService).to have_received(:new).once + expect(pending_execution.reload).to be_executed + end + + it 'leaves the row processing and reports the error when an action blows up' do + allow(AutomationRules::ActionService).to receive(:new).and_raise(StandardError, 'boom') + allow(ChatwootExceptionTracker).to receive(:new).and_call_original + + job.perform(pending_execution.reload) + + expect(pending_execution.reload).to be_processing + expect(ChatwootExceptionTracker).to have_received(:new) + end + + it 'sends the follow-up exactly once for the reply-chase story' do + message_rule = create(:automation_rule, account: account, event_name: 'message_created', execution_delay: 60, + conditions: [{ 'values' => ['outgoing'], 'attribute_key' => 'message_type', + 'query_operator' => nil, 'filter_operator' => 'equal_to' }], + actions: [{ 'action_name' => 'send_message', 'action_params' => ['Just checking in'] }]) + agent_reply = create(:message, conversation: conversation, account: account, message_type: :outgoing) + AutomationRulePendingExecution.schedule(rule: message_rule, conversation: conversation, message: agent_reply) + row = AutomationRulePendingExecution.last + + job.perform(row.reload) + + expect(row.reload).to be_executed + expect(conversation.messages.outgoing.pluck(:content)).to include('Just checking in') + end + + it 'cancels the follow-up when the customer replied before it was due' do + message_rule = create(:automation_rule, account: account, event_name: 'message_created', execution_delay: 60, + conditions: [{ 'values' => ['outgoing'], 'attribute_key' => 'message_type', + 'query_operator' => nil, 'filter_operator' => 'equal_to' }], + actions: [{ 'action_name' => 'send_message', 'action_params' => ['Just checking in'] }]) + agent_reply = create(:message, conversation: conversation, account: account, message_type: :outgoing) + AutomationRulePendingExecution.schedule(rule: message_rule, conversation: conversation, message: agent_reply) + row = AutomationRulePendingExecution.last + + create(:message, conversation: conversation, account: account, message_type: :incoming) + job.perform(row.reload) + + expect(row.reload).to be_skipped + expect(row.skip_reason).to eq('episode_moved') + expect(conversation.messages.outgoing.pluck(:content)).not_to include('Just checking in') + end +end diff --git a/spec/jobs/automation_rules/trigger_pending_executions_job_spec.rb b/spec/jobs/automation_rules/trigger_pending_executions_job_spec.rb new file mode 100644 index 000000000..d365f512d --- /dev/null +++ b/spec/jobs/automation_rules/trigger_pending_executions_job_spec.rb @@ -0,0 +1,48 @@ +require 'rails_helper' + +RSpec.describe AutomationRules::TriggerPendingExecutionsJob do + subject(:job) { described_class.new } + + let(:account) { create(:account) } + let(:conversation) { create(:conversation, account: account) } + + before { GlobalConfig.clear_cache } + + it 'enqueues a per-row job for due pending rows but not future ones' do + due_row = create(:automation_rule_pending_execution, account: account, conversation: conversation, due_at: 1.minute.ago) + create(:automation_rule_pending_execution, account: account, due_at: 1.hour.from_now) + + expect { job.perform }.to have_enqueued_job(AutomationRules::ProcessPendingExecutionJob).exactly(:once).with(due_row) + end + + it 're-enqueues stale processing rows so they get retried' do + stale_row = travel_to(20.minutes.ago) do + create(:automation_rule_pending_execution, account: account, conversation: conversation, status: :processing, due_at: 19.minutes.from_now) + end + + expect { job.perform }.to have_enqueued_job(AutomationRules::ProcessPendingExecutionJob).with(stale_row) + end + + it 'caps enqueues at the configured sweep limit' do + create(:installation_config, name: 'AUTOMATION_PENDING_EXECUTIONS_SWEEP_LIMIT', serialized_value: { value: 1 }.with_indifferent_access) + create_list(:automation_rule_pending_execution, 2, account: account, due_at: 1.minute.ago) + + expect { job.perform }.to have_enqueued_job(AutomationRules::ProcessPendingExecutionJob).exactly(:once) + end + + it 'purges terminal rows past the retention window' do + old_row = travel_to(31.days.ago) { create(:automation_rule_pending_execution, account: account, status: :executed) } + + job.perform + + expect { old_row.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'does nothing when the kill switch is set' do + create(:installation_config, name: 'DISABLE_DELAYED_AUTOMATIONS', serialized_value: { value: true }.with_indifferent_access) + GlobalConfig.clear_cache + create(:automation_rule_pending_execution, account: account, conversation: conversation, due_at: 1.minute.ago) + + expect { job.perform }.not_to have_enqueued_job(AutomationRules::ProcessPendingExecutionJob) + end +end diff --git a/spec/listeners/automation_rule_listener_spec.rb b/spec/listeners/automation_rule_listener_spec.rb index 08085da7a..4b723d9fa 100644 --- a/spec/listeners/automation_rule_listener_spec.rb +++ b/spec/listeners/automation_rule_listener_spec.rb @@ -247,4 +247,37 @@ describe AutomationRuleListener do end end end + + describe 'delayed rules' do + let!(:automation_rule) { create(:automation_rule, event_name: 'conversation_updated', account: account, execution_delay: 60) } + let(:event) do + Events::Base.new('conversation_updated', Time.zone.now, { conversation: conversation, changed_attributes: {} }) + end + + before { allow(condition_match).to receive(:present?).and_return(true) } + + context 'when the delayed_automations feature is enabled' do + before { account.enable_features!('delayed_automations') } + + it 'records a pending execution instead of running actions' do + expect { listener.conversation_updated(event) }.to change(AutomationRulePendingExecution, :count).by(1) + expect(AutomationRules::ActionService).not_to have_received(:new) + expect(AutomationRulePendingExecution.last.due_at).to be_within(5.seconds).of(60.minutes.from_now) + end + + it 'still runs rules without a delay immediately' do + automation_rule.update!(execution_delay: nil) + + expect { listener.conversation_updated(event) }.not_to change(AutomationRulePendingExecution, :count) + expect(AutomationRules::ActionService).to have_received(:new).with(automation_rule, account, conversation) + end + end + + context 'when the delayed_automations feature is disabled' do + it 'neither arms a pending execution nor falls back to immediate execution' do + expect { listener.conversation_updated(event) }.not_to change(AutomationRulePendingExecution, :count) + expect(AutomationRules::ActionService).not_to have_received(:new) + end + end + end end diff --git a/spec/models/automation_rule_pending_execution_spec.rb b/spec/models/automation_rule_pending_execution_spec.rb new file mode 100644 index 000000000..46b16b3d4 --- /dev/null +++ b/spec/models/automation_rule_pending_execution_spec.rb @@ -0,0 +1,171 @@ +require 'rails_helper' + +RSpec.describe AutomationRulePendingExecution do + let(:account) { create(:account) } + let(:conversation) { create(:conversation, account: account) } + let(:rule) do + create(:automation_rule, account: account, event_name: 'conversation_updated', execution_delay: 60, + actions: [{ 'action_name' => 'add_label', 'action_params' => ['stale'] }]) + end + + describe '.episode_key_for' do + it 'derives status episodes from status_changed_at' do + expect(described_class.episode_key_for(conversation, nil)).to eq("status:#{conversation.status_changed_at.to_f}") + end + + it 'falls back to created_at when status_changed_at is blank' do + conversation.update!(status_changed_at: nil) + expect(described_class.episode_key_for(conversation.reload, nil)).to eq("status:#{conversation.created_at.to_f}") + end + + it 'derives awaiting_agent episodes from waiting_since for incoming messages' do + message = create(:message, conversation: conversation, account: account, message_type: :incoming) + expect(described_class.episode_key_for(conversation.reload, message)).to eq("awaiting_agent:#{conversation.waiting_since.to_i}") + end + + it 'derives reply_chase episodes from the max incoming message id for outgoing messages' do + incoming = create(:message, conversation: conversation, account: account, message_type: :incoming) + outgoing = create(:message, conversation: conversation, account: account, message_type: :outgoing) + expect(described_class.episode_key_for(conversation.reload, outgoing)).to eq("reply_chase:#{incoming.id}") + end + + it 'uses 0 for reply_chase when there is no incoming message' do + outgoing = create(:message, conversation: conversation, account: account, message_type: :outgoing) + expect(described_class.episode_key_for(conversation.reload, outgoing)).to eq('reply_chase:0') + end + end + + describe '.schedule' do + it 'creates a pending row due after the rule delay' do + described_class.schedule(rule: rule, conversation: conversation) + + row = described_class.last + expect(row).to have_attributes(account_id: account.id, conversation_id: conversation.id, status: 'pending') + expect(row.due_at).to be_within(5.seconds).of(60.minutes.from_now) + end + + it 'does not reset the clock for a repeated status episode' do + described_class.schedule(rule: rule, conversation: conversation) + original_due_at = described_class.last.due_at + + travel_to(30.minutes.from_now) { described_class.schedule(rule: rule, conversation: conversation) } + + expect(described_class.count).to eq(1) + expect(described_class.last.due_at).to be_within(1.second).of(original_due_at) + end + + it 'moves the clock and anchor for a repeated reply_chase episode' do + first_reply = create(:message, conversation: conversation, account: account, message_type: :outgoing) + described_class.schedule(rule: rule, conversation: conversation, message: first_reply) + + second_reply = create(:message, conversation: conversation, account: account, message_type: :outgoing) + travel_to(30.minutes.from_now) do + described_class.schedule(rule: rule, conversation: conversation, message: second_reply) + + expect(described_class.count).to eq(1) + expect(described_class.last.message_id).to eq(second_reply.id) + expect(described_class.last.due_at).to be_within(5.seconds).of(60.minutes.from_now) + end + end + + it 'does not re-arm an executed reply_chase episode' do + reply = create(:message, conversation: conversation, account: account, message_type: :outgoing) + described_class.schedule(rule: rule, conversation: conversation, message: reply) + described_class.last.update!(status: :executed) + + described_class.schedule(rule: rule, conversation: conversation, message: reply) + + expect(described_class.count).to eq(1) + expect(described_class.last).to be_executed + end + + it 'does not reset the clock for a repeated awaiting_agent episode' do + first_message = create(:message, conversation: conversation, account: account, message_type: :incoming) + described_class.schedule(rule: rule, conversation: conversation, message: first_message) + original_due_at = described_class.last.due_at + + second_message = create(:message, conversation: conversation, account: account, message_type: :incoming) + travel_to(30.minutes.from_now) { described_class.schedule(rule: rule, conversation: conversation, message: second_message) } + + expect(described_class.count).to eq(1) + expect(described_class.last.due_at).to be_within(1.second).of(original_due_at) + end + end + + describe '#episode_current?' do + it 'is true while the conversation stays in the armed status' do + described_class.schedule(rule: rule, conversation: conversation) + expect(described_class.last.episode_current?).to be(true) + end + + it 'is false after a status transition' do + described_class.schedule(rule: rule, conversation: conversation) + conversation.update!(status: :resolved) + expect(described_class.last.reload.episode_current?).to be(false) + end + + it 'is false for awaiting_agent episodes once the agent replies (waiting_since cleared)' do + message = create(:message, conversation: conversation, account: account, message_type: :incoming) + described_class.schedule(rule: rule, conversation: conversation, message: message) + + conversation.update!(waiting_since: nil) + expect(described_class.last.episode_current?).to be(false) + end + + it 'is false for reply_chase episodes once the customer replies' do + reply = create(:message, conversation: conversation, account: account, message_type: :outgoing) + described_class.schedule(rule: rule, conversation: conversation, message: reply) + + create(:message, conversation: conversation, account: account, message_type: :incoming) + expect(described_class.last.episode_current?).to be(false) + end + end + + describe '#claim!' do + let(:row) { create(:automation_rule_pending_execution, account: account, conversation: conversation) } + + it 'claims a pending row exactly once so a duplicate enqueue cannot double-fire' do + expect(row.claim!).to be(true) + expect(row.reload).to be_processing + expect(described_class.find(row.id).claim!).to be(false) + end + + it 'does not claim terminal rows' do + row.update!(status: :executed) + expect(row.claim!).to be(false) + end + + it 'reclaims a processing row only after its lock goes stale' do + row.update!(status: :processing) + expect(row.claim!).to be(false) + + travel_to(20.minutes.from_now) { expect(row.claim!).to be(true) } + end + end + + describe '.sweepable' do + it 'selects due pending rows and stale processing rows, but not future or fresh ones' do + due = create(:automation_rule_pending_execution, account: account, conversation: conversation, due_at: 1.minute.ago) + create(:automation_rule_pending_execution, account: account, due_at: 1.hour.from_now) + create(:automation_rule_pending_execution, account: account, status: :processing) + stale = travel_to(20.minutes.ago) do + create(:automation_rule_pending_execution, account: account, conversation: conversation, status: :processing) + end + + expect(described_class.sweepable).to contain_exactly(due, stale) + end + end + + describe '.purge_terminal!' do + it 'deletes terminal rows past the retention window and keeps everything else' do + old_executed = travel_to(31.days.ago) { create(:automation_rule_pending_execution, account: account, status: :executed) } + recent_skipped = create(:automation_rule_pending_execution, account: account, status: :skipped) + pending = create(:automation_rule_pending_execution, account: account, conversation: conversation) + + described_class.purge_terminal! + + expect(described_class.pluck(:id)).to contain_exactly(recent_skipped.id, pending.id) + expect { old_executed.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/spec/models/automation_rule_spec.rb b/spec/models/automation_rule_spec.rb index 7ae73496c..86d611a0f 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -137,4 +137,42 @@ RSpec.describe AutomationRule do end end end + + describe 'execution_delay validations' do + let(:rule) { build(:automation_rule, account: create(:account)) } + + it 'allows nil (immediate execution)' do + rule.execution_delay = nil + expect(rule).to be_valid + end + + it 'allows delays between 10 minutes and 30 days' do + rule.execution_delay = 240 + expect(rule).to be_valid + end + + it 'rejects delays below 10 minutes' do + rule.execution_delay = 5 + expect(rule).not_to be_valid + expect(rule.errors[:execution_delay]).to be_present + end + + it 'rejects delays above 30 days' do + rule.execution_delay = 43_201 + expect(rule).not_to be_valid + end + + it 'rejects non-integer delays' do + rule.execution_delay = 10.5 + expect(rule).not_to be_valid + end + + it 'rejects a delay combined with an attribute_changed condition' do + rule.execution_delay = 60 + rule.conditions = [{ 'attribute_key' => 'status', 'filter_operator' => 'attribute_changed', + 'values' => { 'from' => ['open'], 'to' => ['pending'] }, 'query_operator' => nil }] + expect(rule).not_to be_valid + expect(rule.errors[:execution_delay]).to include('cannot be used with attribute_changed conditions.') + end + end end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 43bbab56f..d42c8200f 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -1228,4 +1228,28 @@ RSpec.describe Conversation do end end end + + describe '#status_changed_at' do + let(:conversation) { create(:conversation) } + + it 'is set on create' do + expect(conversation.status_changed_at).to be_present + end + + it 'is updated on every status transition' do + original = conversation.status_changed_at + + travel_to(1.hour.from_now) { conversation.update!(status: :resolved) } + + expect(conversation.reload.status_changed_at).to be > original + end + + it 'is untouched by non-status saves' do + original = conversation.status_changed_at + + travel_to(1.hour.from_now) { conversation.update!(priority: :high) } + + expect(conversation.reload.status_changed_at).to be_within(1.second).of(original) + end + end end