From 82f28aa7fbd19c2c119b51f71229189023493c3f Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Wed, 15 Jul 2026 10:57:23 +0530 Subject: [PATCH] chore(automations): move specs to a follow-up branch --- .../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 deletions(-) delete mode 100644 spec/factories/automation_rule_pending_executions.rb delete mode 100644 spec/jobs/automation_rules/process_pending_execution_job_spec.rb delete mode 100644 spec/jobs/automation_rules/trigger_pending_executions_job_spec.rb delete 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 d9026be29..5b7c0112d 100644 --- a/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb @@ -451,61 +451,4 @@ 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 deleted file mode 100644 index 8298991ae..000000000 --- a/spec/factories/automation_rule_pending_executions.rb +++ /dev/null @@ -1,10 +0,0 @@ -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 deleted file mode 100644 index a7c88e9e0..000000000 --- a/spec/jobs/automation_rules/process_pending_execution_job_spec.rb +++ /dev/null @@ -1,139 +0,0 @@ -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 deleted file mode 100644 index d365f512d..000000000 --- a/spec/jobs/automation_rules/trigger_pending_executions_job_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -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 4b723d9fa..08085da7a 100644 --- a/spec/listeners/automation_rule_listener_spec.rb +++ b/spec/listeners/automation_rule_listener_spec.rb @@ -247,37 +247,4 @@ 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 deleted file mode 100644 index 46b16b3d4..000000000 --- a/spec/models/automation_rule_pending_execution_spec.rb +++ /dev/null @@ -1,171 +0,0 @@ -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 86d611a0f..7ae73496c 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -137,42 +137,4 @@ 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 d42c8200f..43bbab56f 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -1228,28 +1228,4 @@ 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