fix(templates): silence inbound WARN + guard blank auto-replies (EVO-1720)

app/controllers/public/api/v1/inboxes/messages_controller.rb

@@ -1,16 +1,11 @@
 class Public::Api::V1::Inboxes::MessagesController < Public::Api::V1::InboxesController
-  include TemplateConsumerLogging
-
   before_action :set_message, only: [:update]
 
   def index
     @messages = @conversation.nil? ? [] : message_finder.perform
   end
 
   def create
-    # Coexistence (EVO-1235): flag external consumers still pushing inline body
-    # so they can be migrated to the templated send action.
-    warn_legacy_inline_content(action: 'messages#create', inbox_identifier: params[:inbox_id]) if permitted_params[:content].present?
     @message = @conversation.messages.new(message_params)
     build_attachment
     @message.save!

app/services/message_templates/template/greeting.rb

@@ -4,8 +4,17 @@ class MessageTemplates::Template::Greeting
   pattr_initialize [:conversation!]
 
   def perform
+    content = greeting_content
+    # Skip silently when neither the template nor the inline fallback yields
+    # content, so a blank auto-reply is never delivered (EVO-1720 [6.11]).
+    return if content.blank?
+
     ActiveRecord::Base.transaction do
-      conversation.messages.create!(greeting_message_params)
+      conversation.messages.create!(
+        inbox_id: @conversation.inbox_id,
+        message_type: :template,
+        content: content
+      )
     end
   rescue StandardError => e
     EvolutionExceptionTracker.new(e, account: nil).capture_exception
@@ -17,15 +26,10 @@ def perform
   delegate :contact, to: :conversation
   delegate :inbox, to: :message
 
-  def greeting_message_params
-    # Prefer a referenced MessageTemplate (EVO-1235); fall back to the inline string.
-    content = template_content_for(@conversation.inbox&.greeting_message_template_id) ||
-              @conversation.inbox&.greeting_message
-
-    {
-      inbox_id: @conversation.inbox_id,
-      message_type: :template,
-      content: content
-    }
+  # Prefer a referenced MessageTemplate (EVO-1235); fall back to the inline
+  # string. Resolved once so the blank guard adds no extra SendResolver lookup.
+  def greeting_content
+    template_content_for(@conversation.inbox&.greeting_message_template_id) ||
+      @conversation.inbox&.greeting_message
   end
 end

app/services/message_templates/template/out_of_office.rb

@@ -4,8 +4,17 @@ class MessageTemplates::Template::OutOfOffice
   pattr_initialize [:conversation!]
 
   def perform
+    content = out_of_office_content
+    # Skip silently when neither the template nor the inline fallback yields
+    # content, so a blank auto-reply is never delivered (EVO-1720 [6.11]).
+    return if content.blank?
+
     ActiveRecord::Base.transaction do
-      conversation.messages.create!(out_of_office_message_params)
+      conversation.messages.create!(
+        inbox_id: @conversation.inbox_id,
+        message_type: :template,
+        content: content
+      )
     end
   rescue StandardError => e
     EvolutionExceptionTracker.new(e, account: nil).capture_exception
@@ -17,15 +26,10 @@ def perform
   delegate :contact, to: :conversation
   delegate :inbox, to: :message
 
-  def out_of_office_message_params
-    # Prefer a referenced MessageTemplate (EVO-1235); fall back to the inline string.
-    content = template_content_for(@conversation.inbox&.out_of_office_message_template_id) ||
-              @conversation.inbox&.out_of_office_message
-
-    {
-      inbox_id: @conversation.inbox_id,
-      message_type: :template,
-      content: content
-    }
+  # Prefer a referenced MessageTemplate (EVO-1235); fall back to the inline
+  # string. Resolved once so the blank guard adds no extra SendResolver lookup.
+  def out_of_office_content
+    template_content_for(@conversation.inbox&.out_of_office_message_template_id) ||
+      @conversation.inbox&.out_of_office_message
   end
 end

spec/requests/public/api/v1/inboxes/messages_controller_spec.rb

@@ -14,13 +14,23 @@
       "/conversations/#{conversation.display_id}/messages"
   end
 
-  describe 'POST create with legacy inline content' do
-    it 'creates the incoming message and emits the legacy WARN with the consumer id' do
-      expect(Rails.logger).to receive(:warn).with(/deprecated inline content.*consumer=legacy-consumer/).at_least(:once)
+  describe 'POST create with inline content' do
+    # Inbound messages always carry content and have no template alternative, so
+    # the deprecated-inline-content WARN must NOT fire here — it would spam the
+    # signal meant to identify legacy OUTBOUND consumers (EVO-1720 [6.11]).
+    it 'creates the incoming message without the legacy deprecated-content WARN' do
+      allow(Rails.logger).to receive(:warn)
 
-      post path, params: { content: 'hello from a contact' }, headers: { 'X-Client-ID' => 'legacy-consumer' }, as: :json
+      expect do
+        post path, params: { content: 'hello from a contact' },
+                   headers: { 'X-Client-ID' => 'legacy-consumer' }, as: :json
+      end.to change(conversation.messages, :count).by(1)
 
       expect(response).to have_http_status(:success)
+      created = conversation.messages.last
+      expect(created.message_type).to eq('incoming')
+      expect(created.content).to eq('hello from a contact')
+      expect(Rails.logger).not_to have_received(:warn).with(/deprecated inline content/)
     end
   end
 end

spec/services/message_templates/template/greeting_spec.rb

@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe MessageTemplates::Template::Greeting do
+  # Plain ActiveRecord setup (this repo has no model factories beyond roles;
+  # mirrors spec/requests/.../messages_controller_spec.rb).
+  let(:api_channel) { Channel::Api.create! }
+  let(:inbox) { Inbox.create!(name: 'API Inbox', channel: api_channel) }
+  let(:contact) { Contact.create!(name: 'Ada Lovelace', email: "ada-#{SecureRandom.hex(4)}@test.com") }
+  let(:contact_inbox) { ContactInbox.create!(inbox: inbox, contact: contact, source_id: SecureRandom.hex(8)) }
+  let(:conversation) { Conversation.create!(inbox: inbox, contact: contact, contact_inbox: contact_inbox) }
+  let(:service) { described_class.new(conversation: conversation) }
+
+  # A global template with a required variable the auto-reply never supplies
+  # (the auto-reply only passes first_name/name/email), so rendering raises.
+  # NOTE: the token MUST appear in `content` or extract_variables_from_content
+  # strips the explicit variable on save (see EVO-1720 spec, F1).
+  let(:raising_template) do
+    MessageTemplate.create!(
+      name: "greeting-required-#{SecureRandom.hex(4)}",
+      content: 'Hi {{order_id}}',
+      variables: [{ 'name' => 'order_id', 'required' => true }],
+      channel: nil
+    )
+  end
+
+  it 'builds a template whose render actually raises (guards against a vacuous test)' do
+    expect { raising_template.render_with_variables('first_name' => 'Ada') }
+      .to raise_error(ArgumentError)
+  end
+
+  describe '#perform' do
+    context 'when the template render fails AND the inline fallback is blank (AC3)' do
+      before do
+        inbox.update!(
+          greeting_enabled: true,
+          greeting_message: '',
+          greeting_message_template_id: raising_template.id
+        )
+      end
+
+      it 'creates no message (no blank greeting is delivered)' do
+        expect { service.perform }.not_to change(conversation.messages, :count)
+      end
+    end
+
+    context 'when the template render fails but the inline fallback is present (AC4a)' do
+      before do
+        inbox.update!(
+          greeting_enabled: true,
+          greeting_message: 'Welcome!',
+          greeting_message_template_id: raising_template.id
+        )
+      end
+
+      it 'falls back to the inline content and creates one template message' do
+        expect { service.perform }.to change(conversation.messages, :count).by(1)
+
+        message = conversation.messages.last
+        expect(message.message_type).to eq('template')
+        expect(message.content_type).to eq('text')
+        expect(message.content).to eq('Welcome!')
+      end
+    end
+
+    context 'when there is no template id but inline content is present (AC4b)' do
+      before do
+        inbox.update!(
+          greeting_enabled: true,
+          greeting_message: 'Hello there',
+          greeting_message_template_id: nil
+        )
+      end
+
+      it 'creates one template message from the inline content' do
+        expect { service.perform }.to change(conversation.messages, :count).by(1)
+        expect(conversation.messages.last.content).to eq('Hello there')
+      end
+    end
+
+    context 'when a blank greeting was previously skipped (AC6 — no false "already greeted")' do
+      before do
+        inbox.update!(
+          greeting_enabled: true,
+          greeting_message: '',
+          greeting_message_template_id: raising_template.id
+        )
+      end
+
+      it 'leaves zero template messages so a later valid greeting can still fire' do
+        service.perform
+        # The blank skip created nothing — the messages.template count that
+        # HookExecutionService#first_message_from_contact? checks stays at zero.
+        expect(conversation.messages.template.count).to eq(0)
+
+        inbox.update!(greeting_message: 'Welcome!')
+        expect { described_class.new(conversation: conversation).perform }
+          .to change(conversation.messages.template, :count).by(1)
+      end
+    end
+  end
+end

spec/services/message_templates/template/out_of_office_spec.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe MessageTemplates::Template::OutOfOffice do
+  # Plain ActiveRecord setup (this repo has no model factories beyond roles;
+  # mirrors spec/requests/.../messages_controller_spec.rb).
+  let(:api_channel) { Channel::Api.create! }
+  let(:inbox) { Inbox.create!(name: 'API Inbox', channel: api_channel) }
+  let(:contact) { Contact.create!(name: 'Ada Lovelace', email: "ada-#{SecureRandom.hex(4)}@test.com") }
+  let(:contact_inbox) { ContactInbox.create!(inbox: inbox, contact: contact, source_id: SecureRandom.hex(8)) }
+  let(:conversation) { Conversation.create!(inbox: inbox, contact: contact, contact_inbox: contact_inbox) }
+  let(:service) { described_class.new(conversation: conversation) }
+
+  # Global template with a required variable the auto-reply never supplies, so
+  # rendering raises and TemplateContent falls back to the inline column. The
+  # token must appear in content or the explicit var is stripped on save (F1).
+  let(:raising_template) do
+    MessageTemplate.create!(
+      name: "ooo-required-#{SecureRandom.hex(4)}",
+      content: 'Away, ref {{order_id}}',
+      variables: [{ 'name' => 'order_id', 'required' => true }],
+      channel: nil
+    )
+  end
+
+  it 'builds a template whose render actually raises (guards against a vacuous test)' do
+    expect { raising_template.render_with_variables('first_name' => 'Ada') }
+      .to raise_error(ArgumentError)
+  end
+
+  describe '#perform' do
+    context 'when the template render fails AND the inline fallback is blank (AC5)' do
+      before do
+        inbox.update!(
+          out_of_office_message: '',
+          out_of_office_message_template_id: raising_template.id
+        )
+      end
+
+      it 'creates no message (no blank out-of-office is delivered)' do
+        expect { service.perform }.not_to change(conversation.messages, :count)
+      end
+    end
+
+    context 'when the template render fails but the inline fallback is present' do
+      before do
+        inbox.update!(
+          out_of_office_message: 'We are away',
+          out_of_office_message_template_id: raising_template.id
+        )
+      end
+
+      it 'falls back to the inline content and creates one template message' do
+        expect { service.perform }.to change(conversation.messages, :count).by(1)
+
+        message = conversation.messages.last
+        expect(message.message_type).to eq('template')
+        expect(message.content_type).to eq('text')
+        expect(message.content).to eq('We are away')
+      end
+    end
+
+    context 'when there is no template id but inline content is present' do
+      before do
+        inbox.update!(
+          out_of_office_message: 'Out of office',
+          out_of_office_message_template_id: nil
+        )
+      end
+
+      it 'creates one template message from the inline content' do
+        expect { service.perform }.to change(conversation.messages, :count).by(1)
+        expect(conversation.messages.last.content).to eq('Out of office')
+      end
+    end
+  end
+end