From 02ff71070985b6c15facb8074c1e502e66619a23 Mon Sep 17 00:00:00 2001 From: Muhsin <12408980+muhsin-k@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:57:57 +0400 Subject: [PATCH] fix(inboxes): address disabled inbox review feedback --- .../api/v1/widget/base_controller.rb | 1 - .../api/v1/widget/messages_controller.rb | 1 + app/jobs/webhooks/instagram_events_job.rb | 11 ++--- app/jobs/webhooks/tiktok_events_job.rb | 2 +- app/jobs/webhooks/whatsapp_events_job.rb | 11 +++-- app/mailboxes/reply_mailbox.rb | 1 + .../new_conversation_strategy.rb | 1 + .../whatsapp/incoming_message_base_service.rb | 4 +- .../v1/accounts/whatsapp_calls_controller.rb | 5 +++ .../services/voice/outbound_call_builder.rb | 1 + lib/custom_exceptions/inbox_disabled.rb | 4 ++ .../widget/conversations_controller_spec.rb | 25 +++++++++++ .../api/v1/widget/messages_controller_spec.rb | 41 +++++++++++++++++++ .../whatsapp_calls_controller_spec.rb | 12 ++++++ .../voice/outbound_call_builder_spec.rb | 15 +++++++ .../webhooks/instagram_events_job_spec.rb | 18 ++++++++ spec/jobs/webhooks/tiktok_events_job_spec.rb | 32 +++++++++++++++ .../jobs/webhooks/whatsapp_events_job_spec.rb | 28 +++++++++++++ spec/mailboxes/reply_mailbox_spec.rb | 32 +++++++++++++++ .../whatsapp/incoming_message_service_spec.rb | 12 ++++++ 20 files changed, 245 insertions(+), 12 deletions(-) diff --git a/app/controllers/api/v1/widget/base_controller.rb b/app/controllers/api/v1/widget/base_controller.rb index fb1e09805..94c96f421 100644 --- a/app/controllers/api/v1/widget/base_controller.rb +++ b/app/controllers/api/v1/widget/base_controller.rb @@ -3,7 +3,6 @@ class Api::V1::Widget::BaseController < ApplicationController include WebsiteTokenHelper before_action :set_web_widget - before_action :ensure_inbox_active before_action :set_contact private diff --git a/app/controllers/api/v1/widget/messages_controller.rb b/app/controllers/api/v1/widget/messages_controller.rb index 8b1e8ef48..1d45f4ce3 100644 --- a/app/controllers/api/v1/widget/messages_controller.rb +++ b/app/controllers/api/v1/widget/messages_controller.rb @@ -1,4 +1,5 @@ class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController + before_action :ensure_inbox_active, only: [:create, :update] before_action :set_conversation, only: [:create] before_action :set_message, only: [:update] diff --git a/app/jobs/webhooks/instagram_events_job.rb b/app/jobs/webhooks/instagram_events_job.rb index 576ba9efd..1295ec812 100644 --- a/app/jobs/webhooks/instagram_events_job.rb +++ b/app/jobs/webhooks/instagram_events_job.rb @@ -56,11 +56,12 @@ class Webhooks::InstagramEventsJob < MutexApplicationJob channel = find_channel(instagram_id) next if channel.blank? - next unless channel.inbox.active? - if (event_name = event_name(messaging)) - send(event_name, messaging, channel) - end + event_name = event_name(messaging) + next if event_name.blank? + next unless channel.inbox.active? || event_name == :read + + send(event_name, messaging, channel) end end @@ -127,7 +128,7 @@ class Webhooks::InstagramEventsJob < MutexApplicationJob end def event_name(messaging) - @event_name ||= SUPPORTED_EVENTS.find { |key| messaging.key?(key) } + SUPPORTED_EVENTS.find { |key| messaging.key?(key) } end def message(messaging, channel) diff --git a/app/jobs/webhooks/tiktok_events_job.rb b/app/jobs/webhooks/tiktok_events_job.rb index 21f058677..07e022f92 100644 --- a/app/jobs/webhooks/tiktok_events_job.rb +++ b/app/jobs/webhooks/tiktok_events_job.rb @@ -21,7 +21,7 @@ class Webhooks::TiktokEventsJob < MutexApplicationJob def channel_is_inactive? return true if channel.blank? return true unless channel.account.active? - return true unless channel.inbox.active? + return true unless channel.inbox.active? || event_name == 'im_mark_read_msg' false end diff --git a/app/jobs/webhooks/whatsapp_events_job.rb b/app/jobs/webhooks/whatsapp_events_job.rb index 7307c3775..f82562b5d 100644 --- a/app/jobs/webhooks/whatsapp_events_job.rb +++ b/app/jobs/webhooks/whatsapp_events_job.rb @@ -8,7 +8,7 @@ class Webhooks::WhatsappEventsJob < MutexApplicationJob def perform(params = {}) channel = find_channel_from_whatsapp_business_payload(params) - if channel_is_inactive?(channel) + if channel_is_inactive?(channel, params) Rails.logger.warn("Inactive WhatsApp channel: #{channel&.phone_number || "unknown - #{params[:phone_number]}"}") return end @@ -124,16 +124,21 @@ class Webhooks::WhatsappEventsJob < MutexApplicationJob ].compact_blank.first end - def channel_is_inactive?(channel) + def channel_is_inactive?(channel, params) return true if channel.blank? # Only skip for embedded signup when reauth is required; manual flow uses API keys and should still receive webhooks return true if channel.reauthorization_required? && embedded_signup_channel?(channel) return true unless channel.account.active? - return true unless channel.inbox.active? + return true unless channel.inbox.active? || status_update_event?(params) false end + def status_update_event?(params) + value = params.dig(:entry, 0, :changes, 0, :value) || params + value[:statuses].present? + end + def embedded_signup_channel?(channel) (channel.provider_config || {}).to_h['source'] == 'embedded_signup' end diff --git a/app/mailboxes/reply_mailbox.rb b/app/mailboxes/reply_mailbox.rb index 6e766f83e..f152d400a 100644 --- a/app/mailboxes/reply_mailbox.rb +++ b/app/mailboxes/reply_mailbox.rb @@ -6,6 +6,7 @@ class ReplyMailbox < ApplicationMailbox def process # Return early if no conversation was found (e.g., notification emails, suspended accounts) return unless @conversation + return unless @conversation.inbox.active? # Wrap everything in a transaction to ensure atomicity # This prevents orphan conversations if message/attachment creation fails diff --git a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb index f9f5c3cf8..4c91f9d71 100644 --- a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb @@ -22,6 +22,7 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: # The actual persistence happens in ReplyMailbox within a transaction that includes message creation. def find return nil unless @channel # No valid channel found + return nil unless @inbox.active? return nil unless incoming_email_from_valid_email? # Skip edge cases # Check if conversation already exists by in_reply_to diff --git a/app/services/whatsapp/incoming_message_base_service.rb b/app/services/whatsapp/incoming_message_base_service.rb index bf35d220d..0346394ab 100644 --- a/app/services/whatsapp/incoming_message_base_service.rb +++ b/app/services/whatsapp/incoming_message_base_service.rb @@ -8,13 +8,13 @@ class Whatsapp::IncomingMessageBaseService pattr_initialize [:inbox!, :params!, :outgoing_echo] def perform - return unless @inbox.active? - processed_params if processed_params.try(:[], :statuses).present? process_statuses elsif messages_data.present? + return unless @inbox.active? + process_messages end end diff --git a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb index 2f86e0de8..6c3bea135 100644 --- a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb @@ -1,6 +1,7 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseController before_action :set_call, only: %i[show accept reject terminate upload_recording] before_action :set_call_context, only: :initiate + before_action :ensure_inbox_active, only: :initiate before_action :ensure_calling_enabled, only: :initiate before_action :ensure_sdp_offer, only: :initiate before_action :ensure_contact_phone, only: :initiate @@ -96,6 +97,10 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro render_could_not_create_error(I18n.t('errors.whatsapp.calls.not_enabled')) end + def ensure_inbox_active + render_inbox_disabled_error unless @inbox.active? + end + def ensure_sdp_offer return if params[:sdp_offer].present? diff --git a/enterprise/app/services/voice/outbound_call_builder.rb b/enterprise/app/services/voice/outbound_call_builder.rb index c58407a3f..5480e777c 100644 --- a/enterprise/app/services/voice/outbound_call_builder.rb +++ b/enterprise/app/services/voice/outbound_call_builder.rb @@ -16,6 +16,7 @@ class Voice::OutboundCallBuilder def perform! raise ArgumentError, 'Contact phone number required' if contact.phone_number.blank? raise ArgumentError, 'Agent required' if user.blank? + raise CustomExceptions::InboxDisabled unless inbox.active? # Claim for the caller if a reused conversation is unassigned at trigger time; wins over auto-assignment. # New conversations set the assignee at creation instead (see create_conversation!). diff --git a/lib/custom_exceptions/inbox_disabled.rb b/lib/custom_exceptions/inbox_disabled.rb index e09ccd30e..52b120984 100644 --- a/lib/custom_exceptions/inbox_disabled.rb +++ b/lib/custom_exceptions/inbox_disabled.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class CustomExceptions::InboxDisabled < CustomExceptions::Base + def initialize(data = {}) + super + end + def message 'This inbox is currently disabled' end diff --git a/spec/controllers/api/v1/widget/conversations_controller_spec.rb b/spec/controllers/api/v1/widget/conversations_controller_spec.rb index 73e01ce30..954e5ab7c 100644 --- a/spec/controllers/api/v1/widget/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/widget/conversations_controller_spec.rb @@ -43,6 +43,18 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do expect(json_response['id']).to eq(conversation.display_id) expect(json_response['status']).to eq(conversation.status) end + + it 'returns the conversation when the inbox is disabled' do + web_widget.inbox.update!(active: false) + + get '/api/v1/widget/conversations', + headers: { 'X-Auth-Token' => token }, + params: { website_token: web_widget.website_token }, + as: :json + + expect(response).to have_http_status(:success) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end end context 'with a conversation but invalid source id' do @@ -62,6 +74,19 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do end describe 'POST /api/v1/widget/conversations' do + it 'does not create a conversation when the inbox is disabled' do + web_widget.inbox.update!(active: false) + + expect do + post '/api/v1/widget/conversations', + headers: { 'X-Auth-Token' => token }, + params: conversation_params, + as: :json + end.not_to change(Conversation, :count) + + expect(response).to have_http_status(:forbidden) + end + it 'creates a conversation with correct details' do post '/api/v1/widget/conversations', headers: { 'X-Auth-Token' => token }, diff --git a/spec/controllers/api/v1/widget/messages_controller_spec.rb b/spec/controllers/api/v1/widget/messages_controller_spec.rb index c4faf4245..06b628f81 100644 --- a/spec/controllers/api/v1/widget/messages_controller_spec.rb +++ b/spec/controllers/api/v1/widget/messages_controller_spec.rb @@ -28,6 +28,18 @@ RSpec.describe '/api/v1/widget/messages', type: :request do expect(json_response['meta']).not_to be_empty end + it 'returns messages when the inbox is disabled' do + web_widget.inbox.update!(active: false) + + get api_v1_widget_messages_url, + params: { website_token: web_widget.website_token }, + headers: { 'X-Auth-Token' => token }, + as: :json + + expect(response).to have_http_status(:success) + expect(response.parsed_body['payload'].length).to eq(4) + end + it 'returns empty messages', :skip_before do get api_v1_widget_messages_url, params: { website_token: web_widget.website_token }, @@ -43,6 +55,20 @@ RSpec.describe '/api/v1/widget/messages', type: :request do describe 'POST /api/v1/widget/messages' do context 'when post request is made' do + it 'does not create message in conversation when the inbox is disabled' do + web_widget.inbox.update!(active: false) + message_params = { content: 'hello world', timestamp: Time.current } + + expect do + post api_v1_widget_messages_url, + params: { website_token: web_widget.website_token, message: message_params }, + headers: { 'X-Auth-Token' => token }, + as: :json + end.not_to change(Message, :count) + + expect(response).to have_http_status(:forbidden) + end + it 'creates message in conversation' do conversation.destroy! # Test all params message_params = { content: 'hello world', timestamp: Time.current } @@ -213,6 +239,21 @@ RSpec.describe '/api/v1/widget/messages', type: :request do end describe 'PUT /api/v1/widget/messages' do + context 'when the inbox is disabled' do + it 'does not update the message' do + message = create(:message, content_type: 'input_email', account: account, inbox: web_widget.inbox, conversation: conversation) + web_widget.inbox.update!(active: false) + + put api_v1_widget_message_url(message.id), + params: { website_token: web_widget.website_token, contact: { email: Faker::Internet.email } }, + headers: { 'X-Auth-Token' => token }, + as: :json + + expect(response).to have_http_status(:forbidden) + expect(message.reload.submitted_email).to be_nil + end + end + context 'when put request targets a message from another visitor in the same inbox' do it 'does not update the foreign message' do other_contact = create(:contact, account: account, email: nil) diff --git a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb index a66249d5a..b45254e19 100644 --- a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb @@ -208,6 +208,18 @@ RSpec.describe 'WhatsApp Calls API', type: :request do expect(response).to have_http_status(:unprocessable_entity) expect(response.parsed_body['error']).to eq(I18n.t('errors.whatsapp.calls.not_enabled')) end + + it 'returns 403 before initiating the provider call when the inbox is disabled' do + inbox.update!(active: false) + allow(provider_service).to receive(:initiate_call) + + post "/api/v1/accounts/#{account.id}/whatsapp_calls/initiate", + params: { conversation_id: initiate_conversation.display_id, sdp_offer: 'sdp_offer' }, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:forbidden) + expect(provider_service).not_to have_received(:initiate_call) + end end describe 'POST /api/v1/accounts/:account_id/whatsapp_calls/:id/upload_recording' do diff --git a/spec/enterprise/services/voice/outbound_call_builder_spec.rb b/spec/enterprise/services/voice/outbound_call_builder_spec.rb index 0dc565eaf..0851ce204 100644 --- a/spec/enterprise/services/voice/outbound_call_builder_spec.rb +++ b/spec/enterprise/services/voice/outbound_call_builder_spec.rb @@ -127,5 +127,20 @@ RSpec.describe Voice::OutboundCallBuilder do ) end.to raise_error(ArgumentError, 'Agent required') end + + it 'raises before initiating the provider call when the inbox is disabled' do + inbox.update!(active: false) + + expect do + described_class.perform!( + account: account, + inbox: inbox, + user: user, + contact: contact + ) + end.to raise_error(CustomExceptions::InboxDisabled) + + expect(channel).not_to have_received(:initiate_call) + end end end diff --git a/spec/jobs/webhooks/instagram_events_job_spec.rb b/spec/jobs/webhooks/instagram_events_job_spec.rb index 4128385cf..27a725527 100644 --- a/spec/jobs/webhooks/instagram_events_job_spec.rb +++ b/spec/jobs/webhooks/instagram_events_job_spec.rb @@ -202,6 +202,24 @@ describe Webhooks::InstagramEventsJob do instagram_webhook.perform_now(messaging_seen_event[:entry]) end + it 'handles messaging_seen callback when the inbox is disabled' do + messaging_seen_event = build(:messaging_seen_event).with_indifferent_access + instagram_messenger_inbox.update!(active: false) + + expect(Instagram::ReadStatusService).to receive(:new).with(params: messaging_seen_event[:entry][0][:messaging][0], + channel: instagram_messenger_inbox.channel).and_call_original + instagram_webhook.perform_now(messaging_seen_event[:entry]) + end + + it 'does not create message callbacks when the inbox is disabled' do + dm_event = build(:instagram_message_create_event).with_indifferent_access + instagram_messenger_inbox.update!(active: false) + + instagram_webhook.perform_now(dm_event[:entry]) + + expect(instagram_messenger_inbox.messages.count).to be 0 + end + it 'handles unsupported message' do unsupported_event = build(:instagram_message_unsupported_event).with_indifferent_access sender_id = unsupported_event[:entry][0][:messaging][0][:sender][:id] diff --git a/spec/jobs/webhooks/tiktok_events_job_spec.rb b/spec/jobs/webhooks/tiktok_events_job_spec.rb index 297f16b64..75ceee217 100644 --- a/spec/jobs/webhooks/tiktok_events_job_spec.rb +++ b/spec/jobs/webhooks/tiktok_events_job_spec.rb @@ -42,6 +42,38 @@ RSpec.describe Webhooks::TiktokEventsJob do expect(read_status_service).to have_received(:perform) end + it 'processes im_mark_read_msg events when the inbox is disabled' do + channel.inbox.update!(active: false) + read_status_service = instance_double(Tiktok::ReadStatusService, perform: true) + allow(Tiktok::ReadStatusService).to receive(:new).and_return(read_status_service) + + event = { + event: 'im_mark_read_msg', + user_openid: 'biz-123', + content: { conversation_id: 'tt-conv-1', read: { last_read_timestamp: 1_700_000_000_000 }, from_user: { id: 'user-1' } }.to_json + } + + job.perform(event) + + expect(Tiktok::ReadStatusService).to have_received(:new).with(channel: channel, content: hash_including(conversation_id: 'tt-conv-1')) + expect(read_status_service).to have_received(:perform) + end + + it 'does not process im_receive_msg events when the inbox is disabled' do + channel.inbox.update!(active: false) + allow(Tiktok::MessageService).to receive(:new) + + event = { + event: 'im_receive_msg', + user_openid: 'biz-123', + content: { conversation_id: 'tt-conv-1' }.to_json + } + + job.perform(event) + + expect(Tiktok::MessageService).not_to have_received(:new) + end + it 'ignores unsupported event types' do allow(Tiktok::MessageService).to receive(:new) diff --git a/spec/jobs/webhooks/whatsapp_events_job_spec.rb b/spec/jobs/webhooks/whatsapp_events_job_spec.rb index 8310839da..a074dadc8 100644 --- a/spec/jobs/webhooks/whatsapp_events_job_spec.rb +++ b/spec/jobs/webhooks/whatsapp_events_job_spec.rb @@ -106,6 +106,34 @@ RSpec.describe Webhooks::WhatsappEventsJob do job.perform_now(phone_number: unknown_phone) end + it 'processes status callbacks when the inbox is disabled' do + wb_params = params.deep_dup + wb_params[:entry].first[:changes].first[:value][:statuses] = [ + { id: 'wamid-test', recipient_id: '919745786257', status: 'delivered' } + ] + channel.inbox.update!(active: false) + + allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).and_return(process_service) + + expect(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).with(inbox: channel.inbox, params: wb_params) + + job.perform_now(wb_params) + end + + it 'does not process message callbacks when the inbox is disabled' do + wb_params = params.deep_dup + wb_params[:entry].first[:changes].first[:value][:messages] = [ + { from: '919745786257', id: 'wamid-test', text: { body: 'Hello' }, type: 'text' } + ] + channel.inbox.update!(active: false) + + allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new) + + expect(Whatsapp::IncomingMessageWhatsappCloudService).not_to receive(:new) + + job.perform_now(wb_params) + end + it 'uses from_user_id as the mutex sender for BSUID-only inbound messages' do bsuid = 'IN.2081978709342942' wb_params = params.deep_dup diff --git a/spec/mailboxes/reply_mailbox_spec.rb b/spec/mailboxes/reply_mailbox_spec.rb index 20ce60dad..6c120b861 100644 --- a/spec/mailboxes/reply_mailbox_spec.rb +++ b/spec/mailboxes/reply_mailbox_spec.rb @@ -42,6 +42,17 @@ RSpec.describe ReplyMailbox do end end + context 'with reply uuid present for a disabled inbox' do + before do + conversation.update!(uuid: '6bdc3f4d-0bec-4515-a284-5d916fdde489') + conversation.inbox.update!(active: false) + end + + it 'does not add the mail content as a new message' do + expect { described_subject }.not_to change(Message, :count) + end + end + context 'with in reply to email' do let(:reply_mail_without_uuid) { create_inbound_email_from_fixture('reply_mail_without_uuid.eml') } let(:described_subject) { described_class.receive reply_mail_without_uuid } @@ -67,6 +78,27 @@ RSpec.describe ReplyMailbox do end end + context 'when forwarded email inbox is disabled' do + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:forwarded_mail) { create_inbound_email_from_mail(from: 'sender@example.com', to: email_channel.email, subject: 'Hello') } + + before do + email_channel.inbox.update!(active: false) + end + + it 'does not create contacts, conversations, or messages' do + contact_count = Contact.count + conversation_count = Conversation.count + message_count = Message.count + + described_class.receive forwarded_mail + + expect(Contact.count).to eq(contact_count) + expect(Conversation.count).to eq(conversation_count) + expect(Message.count).to eq(message_count) + end + end + context 'when new conversation email contains null bytes' do let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } let(:null_byte_mail) { create_inbound_email_from_mail(from: 'sender@example.com', to: email_channel.email, subject: 'Hello') } diff --git a/spec/services/whatsapp/incoming_message_service_spec.rb b/spec/services/whatsapp/incoming_message_service_spec.rb index 01b569cb9..d5b5aead5 100644 --- a/spec/services/whatsapp/incoming_message_service_spec.rb +++ b/spec/services/whatsapp/incoming_message_service_spec.rb @@ -252,6 +252,18 @@ describe Whatsapp::IncomingMessageService do expect(message.reload.status).to eq('read') end + it 'updates message status when the inbox is disabled' do + status_params = { + 'statuses' => [{ 'recipient_id' => from, 'id' => from, 'status' => 'read' }] + }.with_indifferent_access + message = Message.find_by!(source_id: from) + whatsapp_channel.inbox.update!(active: false) + + described_class.new(inbox: whatsapp_channel.inbox, params: status_params).perform + + expect(message.reload.status).to eq('read') + end + it 'stores BSUID source ids from status contacts' do bsuid = 'IN.2081978709342942' parent_bsuid = 'IN.ENT.9081726354'