Merge branch 'develop' into feature/10945-search-results-page

app/helpers/reporting_event_helper.rb

@@ -18,12 +18,25 @@ module ReportingEventHelper
   end
 
   def last_non_human_activity(conversation)
-    # check if a handoff event already exists
-    handoff_event = ReportingEvent.where(conversation_id: conversation.id, name: 'conversation_bot_handoff').last
+    # Try to get either a handoff or reopened event first
+    # These will always take precedence over any other activity
+    # Also, any of these events can happen at any time in the course of a conversation lifecycle.
+    # So we pick the latest event
+    event = ReportingEvent.where(
+      conversation_id: conversation.id,
+      name: %w[conversation_bot_handoff conversation_opened]
+    ).order(event_end_time: :desc).first
 
-    # if a handoff exists, last non human activity is when the handoff ended,
-    # otherwise it's when the conversation was created
-    handoff_event&.event_end_time || conversation.created_at
+    return event.event_end_time if event&.event_end_time
+
+    # Fallback to bot resolved event
+    # Because this will be closest to the most accurate activity instead of conversation.created_at
+    bot_event = ReportingEvent.where(conversation_id: conversation.id, name: 'conversation_bot_resolved').last
+
+    return bot_event.event_end_time if bot_event&.event_end_time
+
+    # If no events found, return conversation creation time
+    conversation.created_at
   end
 
   private

app/listeners/reporting_event_listener.rb

@@ -90,8 +90,47 @@ class ReportingEventListener < BaseListener
     reporting_event.save!
   end
 
+  def conversation_opened(event)
+    conversation = extract_conversation_and_account(event)[0]
+
+    # Find the most recent resolved event for this conversation
+    last_resolved_event = ReportingEvent.where(
+      conversation_id: conversation.id,
+      name: 'conversation_resolved'
+    ).order(event_end_time: :desc).first
+
+    # For first-time openings, value is 0
+    # For reopenings, calculate time since resolution
+    if last_resolved_event
+      time_since_resolved = conversation.updated_at.to_i - last_resolved_event.event_end_time.to_i
+      business_hours_value = business_hours(conversation.inbox, last_resolved_event.event_end_time, conversation.updated_at)
+      start_time = last_resolved_event.event_end_time
+    else
+      time_since_resolved = 0
+      business_hours_value = 0
+      start_time = conversation.created_at
+    end
+
+    create_conversation_opened_event(conversation, time_since_resolved, business_hours_value, start_time)
+  end
+
   private
 
+  def create_conversation_opened_event(conversation, time_since_resolved, business_hours_value, start_time)
+    reporting_event = ReportingEvent.new(
+      name: 'conversation_opened',
+      value: time_since_resolved,
+      value_in_business_hours: business_hours_value,
+      account_id: conversation.account_id,
+      inbox_id: conversation.inbox_id,
+      user_id: conversation.assignee_id,
+      conversation_id: conversation.id,
+      event_start_time: start_time,
+      event_end_time: conversation.updated_at
+    )
+    reporting_event.save!
+  end
+
   def create_bot_resolved_event(conversation, reporting_event)
     return unless conversation.inbox.active_bot?
     # We don't want to create a bot_resolved event if there is user interaction on the conversation

app/models/channel/whatsapp.rb

@@ -32,7 +32,6 @@ class Channel::Whatsapp < ApplicationRecord
   validates :phone_number, presence: true, uniqueness: true
   validate :validate_provider_config
 
-  before_save :setup_webhooks
   after_create :sync_templates
   before_destroy :teardown_webhooks
 
@@ -60,6 +59,13 @@ class Channel::Whatsapp < ApplicationRecord
   delegate :media_url, to: :provider_service
   delegate :api_headers, to: :provider_service
 
+  def setup_webhooks
+    perform_webhook_setup
+  rescue StandardError => e
+    Rails.logger.error "[WHATSAPP] Webhook setup failed: #{e.message}"
+    prompt_reauthorization!
+  end
+
   private
 
   def ensure_webhook_verify_token
@@ -70,34 +76,6 @@ class Channel::Whatsapp < ApplicationRecord
     errors.add(:provider_config, 'Invalid Credentials') unless provider_service.validate_provider_config?
   end
 
-  def setup_webhooks
-    return unless should_setup_webhooks?
-
-    perform_webhook_setup
-  rescue StandardError => e
-    handle_webhook_setup_error(e)
-  end
-
-  def provider_config_changed?
-    will_save_change_to_provider_config?
-  end
-
-  def should_setup_webhooks?
-    whatsapp_cloud_provider? && embedded_signup_source? && webhook_config_present? && provider_config_changed?
-  end
-
-  def whatsapp_cloud_provider?
-    provider == 'whatsapp_cloud'
-  end
-
-  def embedded_signup_source?
-    provider_config['source'] == 'embedded_signup'
-  end
-
-  def webhook_config_present?
-    provider_config['business_account_id'].present? && provider_config['api_key'].present?
-  end
-
   def perform_webhook_setup
     business_account_id = provider_config['business_account_id']
     api_key = provider_config['api_key']
@@ -105,12 +83,6 @@ class Channel::Whatsapp < ApplicationRecord
     Whatsapp::WebhookSetupService.new(self, business_account_id, api_key).perform
   end
 
-  def handle_webhook_setup_error(error)
-    Rails.logger.error "[WHATSAPP] Webhook setup failed: #{error.message}"
-    # Don't raise the error to prevent channel creation from failing
-    # Webhooks can be retried later
-  end
-
   def teardown_webhooks
     Whatsapp::WebhookTeardownService.new(self).perform
   end

app/services/whatsapp/channel_creation_service.rb

@@ -33,15 +33,14 @@ class Whatsapp::ChannelCreationService
 
   def create_channel_with_inbox
     ActiveRecord::Base.transaction do
-      channel = create_channel
+      channel = build_channel
       create_inbox(channel)
-      channel.reload
       channel
     end
   end
 
-  def create_channel
-    Channel::Whatsapp.create!(
+  def build_channel
+    Channel::Whatsapp.build(
       account: @account,
       phone_number: @phone_info[:phone_number],
       provider: 'whatsapp_cloud',

app/services/whatsapp/embedded_signup_service.rb

@@ -11,16 +11,34 @@ class Whatsapp::EmbeddedSignupService
   def perform
     validate_parameters!
 
-    # Exchange code for user access token
-    access_token = Whatsapp::TokenExchangeService.new(@code).perform
+    access_token = exchange_code_for_token
+    phone_info = fetch_phone_info(access_token)
+    validate_token_access(access_token)
 
-    # Fetch phone information
-    phone_info = Whatsapp::PhoneInfoService.new(@waba_id, @phone_number_id, access_token).perform
+    channel = create_or_reauthorize_channel(access_token, phone_info)
+    channel.setup_webhooks
+    channel
 
-    # Validate token has access to the WABA
+  rescue StandardError => e
+    Rails.logger.error("[WHATSAPP] Embedded signup failed: #{e.message}")
+    raise e
+  end
+
+  private
+
+  def exchange_code_for_token
+    Whatsapp::TokenExchangeService.new(@code).perform
+  end
+
+  def fetch_phone_info(access_token)
+    Whatsapp::PhoneInfoService.new(@waba_id, @phone_number_id, access_token).perform
+  end
+
+  def validate_token_access(access_token)
     Whatsapp::TokenValidationService.new(access_token, @waba_id).perform
+  end
 
-    # Reauthorization flow if inbox_id is present
+  def create_or_reauthorize_channel(access_token, phone_info)
     if @inbox_id.present?
       Whatsapp::ReauthorizationService.new(
         account: @account,
@@ -29,17 +47,11 @@ class Whatsapp::EmbeddedSignupService
         business_id: @business_id
       ).perform(access_token, phone_info)
     else
-      # Create channel for new authorization
       waba_info = { waba_id: @waba_id, business_name: phone_info[:business_name] }
       Whatsapp::ChannelCreationService.new(@account, waba_info, phone_info, access_token).perform
     end
-  rescue StandardError => e
-    Rails.logger.error("[WHATSAPP] Embedded signup failed: #{e.message}")
-    raise e
   end
 
-  private
-
   def validate_parameters!
     missing_params = []
     missing_params << 'code' if @code.blank?

spec/helpers/reporting_event_helper_spec.rb

@@ -0,0 +1,177 @@
+require 'rails_helper'
+
+RSpec.describe ReportingEventHelper, type: :helper do
+  describe '#last_non_human_activity' do
+    let(:account) { create(:account) }
+    let(:inbox) { create(:inbox, account: account) }
+    let(:user) { create(:user, account: account) }
+    let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) }
+
+    context 'when conversation has no events' do
+      it 'returns conversation created_at' do
+        expect(helper.last_non_human_activity(conversation)).to eq(conversation.created_at)
+      end
+    end
+
+    context 'when conversation has bot handoff event' do
+      let!(:handoff_event) do
+        create(:reporting_event,
+               name: 'conversation_bot_handoff',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_end_time: 2.hours.ago)
+      end
+
+      it 'returns handoff event end time' do
+        expect(helper.last_non_human_activity(conversation).to_i).to eq(handoff_event.event_end_time.to_i)
+      end
+    end
+
+    context 'when conversation has bot resolved event' do
+      let!(:bot_resolved_event) do
+        create(:reporting_event,
+               name: 'conversation_bot_resolved',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_end_time: 3.hours.ago)
+      end
+
+      it 'returns bot resolved event end time' do
+        expect(helper.last_non_human_activity(conversation).to_i).to eq(bot_resolved_event.event_end_time.to_i)
+      end
+    end
+
+    context 'when conversation is reopened after bot resolution' do
+      let(:creation_time) { 5.days.ago }
+      let(:bot_resolution_time) { 5.days.ago + 5.minutes }
+      let(:reopening_time) { 1.hour.ago }
+
+      let!(:conversation) do
+        create(:conversation,
+               account: account,
+               inbox: inbox,
+               assignee: user,
+               created_at: creation_time)
+      end
+
+      before do
+        # First opened event
+        create(:reporting_event,
+               name: 'conversation_opened',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               value: 0,
+               event_start_time: creation_time,
+               event_end_time: creation_time)
+
+        # Bot resolved event
+        create(:reporting_event,
+               name: 'conversation_bot_resolved',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_start_time: creation_time,
+               event_end_time: bot_resolution_time)
+
+        # Resolved event
+        create(:reporting_event,
+               name: 'conversation_resolved',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_start_time: creation_time,
+               event_end_time: bot_resolution_time)
+
+        # Reopened event
+        create(:reporting_event,
+               name: 'conversation_opened',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               value: (reopening_time - bot_resolution_time).to_i,
+               event_start_time: bot_resolution_time,
+               event_end_time: reopening_time)
+      end
+
+      it 'returns the reopening event time, not the creation time' do
+        # This is the key test: last_non_human_activity should return the reopening time
+        # so that first response time is calculated from when the conversation was reopened,
+        # not from when it was originally created
+        expect(helper.last_non_human_activity(conversation).to_i).to eq(reopening_time.to_i)
+
+        # Verify it's not returning the creation time or bot resolution time
+        expect(helper.last_non_human_activity(conversation).to_i).not_to eq(creation_time.to_i)
+        expect(helper.last_non_human_activity(conversation).to_i).not_to eq(bot_resolution_time.to_i)
+      end
+    end
+
+    context 'when conversation has multiple types of events' do
+      let(:opened_event_time) { 1.hour.ago }
+
+      before do
+        create(:reporting_event,
+               name: 'conversation_bot_resolved',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_end_time: 4.hours.ago)
+
+        create(:reporting_event,
+               name: 'conversation_bot_handoff',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_end_time: 3.hours.ago)
+
+        create(:reporting_event,
+               name: 'conversation_opened',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               event_end_time: opened_event_time)
+      end
+
+      it 'returns the most recent handoff or opened event' do
+        # opened_event is more recent than handoff_event
+        expect(helper.last_non_human_activity(conversation).to_i).to eq(opened_event_time.to_i)
+      end
+    end
+
+    context 'when conversation has multiple reopenings' do
+      let(:third_opened_time) { 30.minutes.ago }
+
+      before do
+        create(:reporting_event,
+               name: 'conversation_opened',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               value: 0,
+               event_end_time: 5.days.ago)
+
+        create(:reporting_event,
+               name: 'conversation_opened',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               value: 3600,
+               event_end_time: 2.days.ago)
+
+        create(:reporting_event,
+               name: 'conversation_opened',
+               conversation_id: conversation.id,
+               account_id: account.id,
+               inbox_id: inbox.id,
+               value: 7200,
+               event_end_time: third_opened_time)
+      end
+
+      it 'returns the most recent opened event' do
+        expect(helper.last_non_human_activity(conversation).to_i).to eq(third_opened_time.to_i)
+      end
+    end
+  end
+end

spec/listeners/reporting_event_listener_spec.rb

@@ -267,4 +267,177 @@ describe ReportingEventListener do
       end
     end
   end
+
+  describe '#conversation_opened' do
+    context 'when conversation is opened for the first time' do
+      let(:new_conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) }
+
+      it 'creates conversation_opened event with value 0' do
+        expect(account.reporting_events.where(name: 'conversation_opened').count).to be 0
+        event = Events::Base.new('conversation.opened', Time.zone.now, conversation: new_conversation)
+        listener.conversation_opened(event)
+        expect(account.reporting_events.where(name: 'conversation_opened').count).to be 1
+
+        opened_event = account.reporting_events.where(name: 'conversation_opened').first
+        expect(opened_event.value).to eq 0
+        expect(opened_event.value_in_business_hours).to eq 0
+        expect(opened_event.event_start_time).to be_within(1.second).of(new_conversation.created_at)
+        expect(opened_event.event_end_time).to be_within(1.second).of(new_conversation.updated_at)
+      end
+    end
+
+    context 'when conversation is reopened after being resolved' do
+      let(:resolved_time) { 2.hours.ago }
+      let(:reopened_time) { 1.hour.ago }
+      let(:reopened_conversation) do
+        create(:conversation, account: account, inbox: inbox, assignee: user, updated_at: reopened_time)
+      end
+
+      before do
+        # Create a resolved event first
+        create(:reporting_event,
+               name: 'conversation_resolved',
+               account_id: account.id,
+               inbox_id: inbox.id,
+               conversation_id: reopened_conversation.id,
+               user_id: user.id,
+               value: 3600,
+               event_start_time: reopened_conversation.created_at,
+               event_end_time: resolved_time)
+      end
+
+      it 'creates conversation_opened event' do
+        expect(account.reporting_events.where(name: 'conversation_opened').count).to be 0
+        event = Events::Base.new('conversation.opened', reopened_time, conversation: reopened_conversation)
+        listener.conversation_opened(event)
+        expect(account.reporting_events.where(name: 'conversation_opened').count).to be 1
+      end
+
+      it 'calculates correct time since resolution' do
+        event = Events::Base.new('conversation.opened', reopened_time, conversation: reopened_conversation)
+        listener.conversation_opened(event)
+
+        reopened_event = account.reporting_events.where(name: 'conversation_opened').first
+        expect(reopened_event.value).to be_within(1).of(3600) # 1 hour = 3600 seconds
+        expect(reopened_event.event_start_time).to be_within(1.second).of(resolved_time)
+        expect(reopened_event.event_end_time).to be_within(1.second).of(reopened_time)
+      end
+
+      it 'sets correct attributes for conversation_opened event' do
+        event = Events::Base.new('conversation.opened', reopened_time, conversation: reopened_conversation)
+        listener.conversation_opened(event)
+
+        reopened_event = account.reporting_events.where(name: 'conversation_opened').first
+        expect(reopened_event.account_id).to eq(account.id)
+        expect(reopened_event.inbox_id).to eq(inbox.id)
+        expect(reopened_event.conversation_id).to eq(reopened_conversation.id)
+        expect(reopened_event.user_id).to eq(user.id)
+      end
+
+      context 'when business hours enabled for inbox' do
+        let(:resolved_time) { Time.zone.parse('March 20, 2022 12:00') }
+        let(:reopened_time) { Time.zone.parse('March 21, 2022 14:00') }
+        let!(:business_hours_inbox) { create(:inbox, working_hours_enabled: true, account: account) }
+        let!(:business_hours_conversation) do
+          create(:conversation, account: account, inbox: business_hours_inbox, assignee: user, updated_at: reopened_time)
+        end
+
+        before do
+          create(:reporting_event,
+                 name: 'conversation_resolved',
+                 account_id: account.id,
+                 inbox_id: business_hours_inbox.id,
+                 conversation_id: business_hours_conversation.id,
+                 user_id: user.id,
+                 value: 3600,
+                 event_start_time: business_hours_conversation.created_at,
+                 event_end_time: resolved_time)
+        end
+
+        it 'creates conversation_opened event with business hour value' do
+          event = Events::Base.new('conversation.opened', reopened_time, conversation: business_hours_conversation)
+          listener.conversation_opened(event)
+
+          reopened_event = account.reporting_events.where(name: 'conversation_opened').first
+          expect(reopened_event.value_in_business_hours).to be 18_000.0 # 5 business hours (26 hours total - 21 non-business hours)
+        end
+      end
+    end
+
+    context 'when conversation has multiple resolutions' do
+      let(:first_resolved_time) { 3.hours.ago }
+      let(:second_resolved_time) { 1.hour.ago }
+      let(:reopened_time) { 30.minutes.ago }
+      let(:multiple_resolution_conversation) do
+        create(:conversation, account: account, inbox: inbox, assignee: user, updated_at: reopened_time)
+      end
+
+      before do
+        # Create first resolved event
+        create(:reporting_event,
+               name: 'conversation_resolved',
+               account_id: account.id,
+               inbox_id: inbox.id,
+               conversation_id: multiple_resolution_conversation.id,
+               user_id: user.id,
+               value: 3600,
+               event_start_time: multiple_resolution_conversation.created_at,
+               event_end_time: first_resolved_time)
+
+        # Create second resolved event (more recent)
+        create(:reporting_event,
+               name: 'conversation_resolved',
+               account_id: account.id,
+               inbox_id: inbox.id,
+               conversation_id: multiple_resolution_conversation.id,
+               user_id: user.id,
+               value: 1800,
+               event_start_time: first_resolved_time,
+               event_end_time: second_resolved_time)
+      end
+
+      it 'uses the most recent resolved event for calculation' do
+        event = Events::Base.new('conversation.opened', reopened_time, conversation: multiple_resolution_conversation)
+        listener.conversation_opened(event)
+
+        reopened_event = account.reporting_events.where(name: 'conversation_opened').first
+        expect(reopened_event.value).to be_within(1).of(1800) # 30 minutes from second resolution
+        expect(reopened_event.event_start_time).to be_within(1.second).of(second_resolved_time)
+      end
+    end
+
+    context 'when agent bot resolves and conversation is reopened' do
+      # This implicitly tests that the first_response time is correctly calculated
+      # By checking that a conversation reopened event is created with the correct values
+      let(:agent_bot) { create(:agent_bot, account: account) }
+      let(:agent_bot_inbox) { create(:inbox, account: account) }
+      let(:bot_resolved_time) { 2.hours.ago }
+      let(:reopened_time) { 1.hour.ago }
+      let(:bot_conversation) do
+        create(:conversation, account: account, inbox: agent_bot_inbox, assignee: user, updated_at: reopened_time)
+      end
+
+      before do
+        create(:agent_bot_inbox, agent_bot: agent_bot, inbox: agent_bot_inbox)
+
+        create(:reporting_event,
+               name: 'conversation_resolved',
+               account_id: account.id,
+               inbox_id: agent_bot_inbox.id,
+               conversation_id: bot_conversation.id,
+               user_id: user.id,
+               event_end_time: bot_resolved_time)
+      end
+
+      it 'creates conversation_opened event for agent bot reopening' do
+        event = Events::Base.new('conversation.opened', reopened_time, conversation: bot_conversation)
+        listener.conversation_opened(event)
+
+        reopened_event = account.reporting_events.where(name: 'conversation_opened').first
+        expect(reopened_event.value).to be_within(1).of(3600) # 1 hour since resolution
+        expect(reopened_event.event_start_time).to be_within(1.second).of(bot_resolved_time)
+        expect(reopened_event.event_end_time).to be_within(1.second).of(reopened_time)
+      end
+    end
+  end
 end

spec/services/whatsapp/channel_creation_service_spec.rb

@@ -16,6 +16,11 @@ describe Whatsapp::ChannelCreationService do
 
   describe '#perform' do
     before do
+      # Stub the webhook teardown service to prevent HTTP calls during cleanup
+      teardown_service = instance_double(Whatsapp::WebhookTeardownService)
+      allow(Whatsapp::WebhookTeardownService).to receive(:new).and_return(teardown_service)
+      allow(teardown_service).to receive(:perform)
+
       # Clean up any existing channels to avoid phone number conflicts
       Channel::Whatsapp.destroy_all
 

spec/services/whatsapp/embedded_signup_service_spec.rb

@@ -10,121 +10,100 @@ describe Whatsapp::EmbeddedSignupService do
       phone_number_id: 'test_phone_number_id'
     }
   end
-  let(:service) do
-    described_class.new(
-      account: account,
-      params: params
-    )
+  let(:service) { described_class.new(account: account, params: params) }
+  let(:access_token) { 'test_access_token' }
+  let(:phone_info) do
+    {
+      phone_number_id: params[:phone_number_id],
+      phone_number: '+1234567890',
+      verified: true,
+      business_name: 'Test Business'
+    }
   end
+  let(:channel) { instance_double(Channel::Whatsapp) }
 
   describe '#perform' do
-    let(:access_token) { 'test_access_token' }
-    let(:phone_info) do
-      {
-        phone_number_id: params[:phone_number_id],
-        phone_number: '+1234567890',
-        verified: true,
-        business_name: 'Test Business'
-      }
-    end
-    let(:channel) { instance_double(Channel::Whatsapp) }
-    let(:service_doubles) do
-      {
-        token_exchange: instance_double(Whatsapp::TokenExchangeService),
-        phone_info: instance_double(Whatsapp::PhoneInfoService),
-        token_validation: instance_double(Whatsapp::TokenValidationService),
-        channel_creation: instance_double(Whatsapp::ChannelCreationService)
-      }
-    end
-
     before do
       allow(GlobalConfig).to receive(:clear_cache)
 
-      allow(Whatsapp::TokenExchangeService).to receive(:new).with(params[:code]).and_return(service_doubles[:token_exchange])
-      allow(service_doubles[:token_exchange]).to receive(:perform).and_return(access_token)
+      # Mock service dependencies
+      token_exchange = instance_double(Whatsapp::TokenExchangeService)
+      allow(Whatsapp::TokenExchangeService).to receive(:new).with(params[:code]).and_return(token_exchange)
+      allow(token_exchange).to receive(:perform).and_return(access_token)
 
+      phone_service = instance_double(Whatsapp::PhoneInfoService)
       allow(Whatsapp::PhoneInfoService).to receive(:new)
-        .with(params[:waba_id], params[:phone_number_id], access_token).and_return(service_doubles[:phone_info])
-      allow(service_doubles[:phone_info]).to receive(:perform).and_return(phone_info)
+        .with(params[:waba_id], params[:phone_number_id], access_token).and_return(phone_service)
+      allow(phone_service).to receive(:perform).and_return(phone_info)
 
+      validation_service = instance_double(Whatsapp::TokenValidationService)
       allow(Whatsapp::TokenValidationService).to receive(:new)
-        .with(access_token, params[:waba_id]).and_return(service_doubles[:token_validation])
-      allow(service_doubles[:token_validation]).to receive(:perform)
+        .with(access_token, params[:waba_id]).and_return(validation_service)
+      allow(validation_service).to receive(:perform)
 
+      channel_creation = instance_double(Whatsapp::ChannelCreationService)
       allow(Whatsapp::ChannelCreationService).to receive(:new)
         .with(account, { waba_id: params[:waba_id], business_name: 'Test Business' }, phone_info, access_token)
-        .and_return(service_doubles[:channel_creation])
-      allow(service_doubles[:channel_creation]).to receive(:perform).and_return(channel)
+        .and_return(channel_creation)
+      allow(channel_creation).to receive(:perform).and_return(channel)
 
-      # Webhook setup is now handled in the channel after_create callback
-      # So we stub it at the model level
-      webhook_service = instance_double(Whatsapp::WebhookSetupService)
-      allow(Whatsapp::WebhookSetupService).to receive(:new).and_return(webhook_service)
-      allow(webhook_service).to receive(:perform)
+      allow(channel).to receive(:setup_webhooks)
     end
 
-    it 'orchestrates all services in the correct order' do
-      expect(service_doubles[:token_exchange]).to receive(:perform).ordered
-      expect(service_doubles[:phone_info]).to receive(:perform).ordered
-      expect(service_doubles[:token_validation]).to receive(:perform).ordered
-      expect(service_doubles[:channel_creation]).to receive(:perform).ordered
+    it 'creates channel and sets up webhooks' do
+      expect(channel).to receive(:setup_webhooks)
 
       result = service.perform
       expect(result).to eq(channel)
     end
 
-    context 'when required parameters are missing' do
-      it 'raises error when code is blank' do
-        service = described_class.new(
-          account: account,
-          params: params.merge(code: '')
-        )
-        expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: code/)
-      end
-
-      it 'raises error when business_id is blank' do
-        service = described_class.new(
-          account: account,
-          params: params.merge(business_id: '')
-        )
-        expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: business_id/)
-      end
-
-      it 'raises error when waba_id is blank' do
-        service = described_class.new(
-          account: account,
-          params: params.merge(waba_id: '')
-        )
-        expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: waba_id/)
-      end
-
-      it 'raises error when multiple parameters are blank' do
-        service = described_class.new(
-          account: account,
-          params: params.merge(code: '', business_id: '')
-        )
-        expect { service.perform }.to raise_error(ArgumentError, /Required parameters are missing: code, business_id/)
+    context 'when parameters are invalid' do
+      it 'raises ArgumentError for missing parameters' do
+        invalid_service = described_class.new(account: account, params: { code: '', business_id: '', waba_id: '' })
+        expect { invalid_service.perform }.to raise_error(ArgumentError, /Required parameters are missing/)
       end
     end
 
-    context 'when any service fails' do
-      it 'logs and re-raises the error' do
-        allow(service_doubles[:token_exchange]).to receive(:perform).and_raise('Token error')
+    context 'when service fails' do
+      it 'logs and re-raises errors' do
+        token_exchange = instance_double(Whatsapp::TokenExchangeService)
+        allow(Whatsapp::TokenExchangeService).to receive(:new).and_return(token_exchange)
+        allow(token_exchange).to receive(:perform).and_raise('Token error')
 
         expect(Rails.logger).to receive(:error).with('[WHATSAPP] Embedded signup failed: Token error')
         expect { service.perform }.to raise_error('Token error')
       end
+
+      it 'prompts reauthorization when webhook setup fails' do
+        # Create a real channel to test the actual webhook failure behavior
+        real_channel = create(:channel_whatsapp, account: account, phone_number: '+1234567890',
+                                                 validate_provider_config: false, sync_templates: false)
+
+        # Mock the channel creation to return our real channel
+        channel_creation = instance_double(Whatsapp::ChannelCreationService)
+        allow(Whatsapp::ChannelCreationService).to receive(:new).and_return(channel_creation)
+        allow(channel_creation).to receive(:perform).and_return(real_channel)
+
+        # Mock webhook setup to fail
+        allow(real_channel).to receive(:perform_webhook_setup).and_raise('Webhook setup error')
+
+        # Verify channel is not marked for reauthorization initially
+        expect(real_channel.reauthorization_required?).to be false
+
+        # The service completes successfully even if webhook fails (webhook error is rescued in setup_webhooks)
+        result = service.perform
+        expect(result).to eq(real_channel)
+
+        # Verify the channel is now marked for reauthorization
+        expect(real_channel.reauthorization_required?).to be true
+      end
     end
 
-    context 'when inbox_id is provided (reauthorization flow)' do
+    context 'with reauthorization flow' do
       let(:inbox_id) { 123 }
       let(:reauth_service) { instance_double(Whatsapp::ReauthorizationService) }
       let(:service_with_inbox) do
-        described_class.new(
-          account: account,
-          params: params,
-          inbox_id: inbox_id
-        )
+        described_class.new(account: account, params: params, inbox_id: inbox_id)
       end
 
       before do
@@ -137,16 +116,45 @@ describe Whatsapp::EmbeddedSignupService do
         allow(reauth_service).to receive(:perform).with(access_token, phone_info).and_return(channel)
       end
 
-      it 'uses ReauthorizationService instead of ChannelCreationService' do
-        expect(service_doubles[:token_exchange]).to receive(:perform).ordered
-        expect(service_doubles[:phone_info]).to receive(:perform).ordered
-        expect(service_doubles[:token_validation]).to receive(:perform).ordered
-        expect(reauth_service).to receive(:perform).with(access_token, phone_info).ordered
-        expect(service_doubles[:channel_creation]).not_to receive(:perform)
+      it 'uses ReauthorizationService and sets up webhooks' do
+        expect(reauth_service).to receive(:perform)
+        expect(channel).to receive(:setup_webhooks)
 
         result = service_with_inbox.perform
         expect(result).to eq(channel)
       end
+
+      it 'clears reauthorization flag' do
+        inbox = create(:inbox, account: account)
+        whatsapp_channel = create(:channel_whatsapp, account: account, phone_number: '+1234567890',
+                                                     validate_provider_config: false, sync_templates: false)
+        inbox.update!(channel: whatsapp_channel)
+        whatsapp_channel.prompt_reauthorization!
+
+        service_with_real_inbox = described_class.new(account: account, params: params, inbox_id: inbox.id)
+
+        # Mock the ReauthorizationService to return our test channel
+        reauth_service = instance_double(Whatsapp::ReauthorizationService)
+        allow(Whatsapp::ReauthorizationService).to receive(:new).with(
+          account: account,
+          inbox_id: inbox.id,
+          phone_number_id: params[:phone_number_id],
+          business_id: params[:business_id]
+        ).and_return(reauth_service)
+
+        # Perform the reauthorization and clear the flag
+        allow(reauth_service).to receive(:perform) do
+          whatsapp_channel.reauthorized!
+          whatsapp_channel
+        end
+
+        allow(whatsapp_channel).to receive(:setup_webhooks).and_return(true)
+
+        expect(whatsapp_channel.reauthorization_required?).to be true
+        result = service_with_real_inbox.perform
+        expect(result).to eq(whatsapp_channel)
+        expect(whatsapp_channel.reauthorization_required?).to be false
+      end
     end
   end
 end

spec/services/whatsapp/webhook_setup_service_spec.rb

@@ -5,7 +5,7 @@ describe Whatsapp::WebhookSetupService do
     create(:channel_whatsapp,
            phone_number: '+1234567890',
            provider_config: {
-             'phone_number_id' => 'test_phone_id',
+             'phone_number_id' => '123456789',
              'webhook_verify_token' => 'test_verify_token'
            },
            provider: 'whatsapp_cloud',
@@ -18,9 +18,14 @@ describe Whatsapp::WebhookSetupService do
   let(:api_client) { instance_double(Whatsapp::FacebookApiClient) }
 
   before do
+    # Stub webhook teardown to prevent HTTP calls during cleanup
+    stub_request(:delete, /graph.facebook.com/).to_return(status: 200, body: '{}', headers: {})
+
     # Clean up any existing channels to avoid phone number conflicts
     Channel::Whatsapp.destroy_all
     allow(Whatsapp::FacebookApiClient).to receive(:new).and_return(api_client)
+    # Default stub for phone_number_verified? with any argument
+    allow(api_client).to receive(:phone_number_verified?).and_return(false)
   end
 
   describe '#perform' do
@@ -148,5 +153,87 @@ describe Whatsapp::WebhookSetupService do
         end
       end
     end
+
+    context 'when webhook setup fails and should trigger reauthorization' do
+      before do
+        allow(api_client).to receive(:phone_number_verified?).with('123456789').and_return(true)
+        allow(api_client).to receive(:subscribe_waba_webhook).and_raise('Invalid access token')
+      end
+
+      it 'raises error with webhook setup failure message' do
+        with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
+          expect { service.perform }.to raise_error(/Webhook setup failed: Invalid access token/)
+        end
+      end
+
+      it 'logs the webhook setup failure' do
+        with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
+          expect(Rails.logger).to receive(:error).with('[WHATSAPP] Webhook setup failed: Invalid access token')
+          expect { service.perform }.to raise_error(/Webhook setup failed/)
+        end
+      end
+    end
+
+    context 'when used during reauthorization flow' do
+      let(:existing_channel) do
+        create(:channel_whatsapp,
+               phone_number: '+1234567890',
+               provider_config: {
+                 'phone_number_id' => '123456789',
+                 'webhook_verify_token' => 'existing_verify_token',
+                 'business_id' => 'existing_business_id',
+                 'waba_id' => 'existing_waba_id'
+               },
+               provider: 'whatsapp_cloud',
+               sync_templates: false,
+               validate_provider_config: false)
+      end
+      let(:new_access_token) { 'new_access_token' }
+      let(:service_reauth) { described_class.new(existing_channel, waba_id, new_access_token) }
+
+      before do
+        allow(api_client).to receive(:phone_number_verified?).with('123456789').and_return(true)
+        allow(api_client).to receive(:subscribe_waba_webhook)
+          .with(waba_id, anything, 'existing_verify_token').and_return({ 'success' => true })
+      end
+
+      it 'successfully reauthorizes with new access token' do
+        with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
+          expect(api_client).not_to receive(:register_phone_number)
+          expect(api_client).to receive(:subscribe_waba_webhook)
+            .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'existing_verify_token')
+          service_reauth.perform
+        end
+      end
+
+      it 'uses the existing webhook verify token during reauthorization' do
+        with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
+          expect(api_client).to receive(:subscribe_waba_webhook)
+            .with(waba_id, anything, 'existing_verify_token')
+          service_reauth.perform
+        end
+      end
+    end
+
+    context 'when webhook setup is successful in creation flow' do
+      before do
+        allow(api_client).to receive(:phone_number_verified?).with('123456789').and_return(true)
+        allow(api_client).to receive(:subscribe_waba_webhook)
+          .with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true })
+      end
+
+      it 'completes successfully without errors' do
+        with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
+          expect { service.perform }.not_to raise_error
+        end
+      end
+
+      it 'does not log any errors' do
+        with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do
+          expect(Rails.logger).not_to receive(:error)
+          service.perform
+        end
+      end
+    end
   end
 end